chore: introduce classifications to dynamic configs #7616
+44
−0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,55 +29,99 @@ import ( | |
| "github.com/uber/cadence/common/definition" | ||
| ) | ||
|
|
||
| // Lifecycle indicates if a config is temporary or permanent | ||
| type Lifecycle int | ||
|
|
||
| const ( | ||
| // LifecycleUnknown is the default for untagged configs | ||
| LifecycleUnknown Lifecycle = iota | ||
| // LifecyclePermanent indicates a config expected to exist indefinitely | ||
| LifecyclePermanent | ||
| // LifecycleTemporary indicates a config that should be removed eventually | ||
| LifecycleTemporary | ||
| ) | ||
|
|
||
| // Category indicates the purpose/nature of a config | ||
| type Category int | ||
|
Member
There was a problem hiding this comment. What do you think about making this a struct with some info available that may be passed in (such as a string for which release it's intended, or when it's being introduced or when it should be removed?) |
||
|
|
||
| const ( | ||
|
Contributor
There was a problem hiding this comment. These are solid categories. Statsig has similar ideas with:
I think just adding experiment would be helpful here. |
||
| // CategoryUnknown is the default for untagged configs | ||
| CategoryUnknown Category = iota | ||
| // CategoryOperational indicates tuning params: batch sizes, timeouts, limits, kill switches | ||
| CategoryOperational | ||
| // CategoryFeatureFlag indicates enable/disable features during rollout | ||
| CategoryFeatureFlag | ||
| // CategoryMigration indicates data migration or gradual rollout flags | ||
| CategoryMigration | ||
| // CategoryDebug indicates debugging/troubleshooting tools | ||
| CategoryDebug | ||
| // CategoryDeprecated indicates configs marked for removal (implies Temporary) | ||
|
Comment on lines
+53
to
+58
Contributor
There was a problem hiding this comment. It would be nice to be able to enforce that these are temporary flags and not permanent (at least for migration/debug/experiment - feature flag I guess could conceivable be permanent, but at that point is it just an operatoinal flag?). |
||
| CategoryDeprecated | ||
| ) | ||
|
|
||
| type ( | ||
| // DynamicInt defines the properties for a dynamic config with int value type | ||
| DynamicInt struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue int | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicBool struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue bool | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicFloat struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue float64 | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicString struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue string | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicDuration struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue time.Duration | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicMap struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue map[string]interface{} | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| DynamicList struct { | ||
| KeyName string | ||
| Description string | ||
| DefaultValue []interface{} | ||
| Filters []Filter | ||
| Lifecycle Lifecycle | ||
| Category Category | ||
| } | ||
|
|
||
| IntKey int | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we'd be able to enforce an 'end date' for temporary flags? That would allow us to log whenever a flag is used (maybe debug normally, but an info or a warn after a flag has gone 'stale'). That would emulate something like this statsig feature: https://statsig.com/blog/removing-old-stale-feature-gates
There are some cons to this - it'll be hard to figure out up front how long you want the feature to exist (especially for some of our core feature flags), and we might have a bit of extra overhead with querying for stale flags often (maybe as part of on call).
But it would give us a good way to at least enforce that we should review stale flags.