Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add feature flags on start #314

Merged
merged 12 commits into from
Aug 18, 2022

Conversation

puckpuck
Copy link
Contributor

@puckpuck puckpuck commented Aug 17, 2022

Fixes #301

Changes

  • Inserts Feature Flag records for a productcatalog and shipping service failure modes

Inserting the records into the database using Ecto migrations with a default of false for enabled. The user can enable the flag by going to the Feature Flag service web UI http://localhost:8081. As of the creation of this PR only the productCatalogFailure flag is leveraged in service code. Using migrations means feature flags state will be preserved if you stop/restart the service/demo. The migrations only reset if the postgres_ffs service container is recreated with docker compose (using docker compose down or the --force-recreate flag)

We should merge #315 before this

  • Appropriate CHANGELOG.md updated for non-trivial changes

@puckpuck puckpuck requested a review from tsloughter August 17, 2022 18:02
@puckpuck
Copy link
Contributor Author

@tsloughter Can you offer guidance on how to get Ecto Migrations to wait for a database connection before proceeding instead of doing the Process.sleep(5000)?

@tsloughter
Copy link
Member

I can't but I can find someone who can.

Would be nicer if the docker depends_on just did what one would expect. Actually, yea, I bet just a different postgres image may fix the issue (or a custom one). I'll also look into that, a postgres container that will not be considered started until its accessible.

@tsloughter
Copy link
Member

@puckpuck can you try out #315

@puckpuck puckpuck marked this pull request as ready for review August 17, 2022 20:57
@puckpuck puckpuck requested a review from a team August 17, 2022 20:57
@puckpuck
Copy link
Contributor Author

ooof... I got caught mid-docs restructuring on this one. Sorry for the commits reverting back and forth there.

Copy link
Contributor

@mic-max mic-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

docs/feature_flags.md Outdated Show resolved Hide resolved
Copy link
Member

@mviitane mviitane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@puckpuck puckpuck merged commit eca1ae1 into open-telemetry:main Aug 18, 2022
@puckpuck puckpuck deleted the add-feature-flag-records branch August 18, 2022 19:11
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* add feature flags on start

* revert sleep change

* document feature flags

* feature flag definitions

* Revert "document feature flags"

This reverts commit c975f5d.

* document feature flags

* document feature flags

* add reference to feature flags

* fix nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create migration for feature flags introduced by #245
5 participants