fix: Feature flag package

[nityanandagohain]

Fixes https://github.com/SigNoz/platform-pod/issues/418

---

Important

Introduces a feature flag system with management, storage, and configuration, including SQL migration and provider integration.

• Feature Flags:
  • Introduces feature flag system with basePlanFeatures, proPlanFeatures, and enterprisePlanFeatures in ee/featureflag/zeus/flags.go.
  • Adds Provider in ee/featureflag/zeus/provider.go to manage feature flags.
  • Implements FeatureFlagManager in pkg/featureflag/manager.go for managing feature flags across organizations.
• Configuration and Providers:
  • Adds featureflag.Config in pkg/featureflag/config.go.
  • Updates ProviderConfig in pkg/signoz/provider.go to include feature flag providers.
  • Modifies pkg/signoz/signoz.go to initialize feature flag manager (commented out).
• Database and Storage:
  • Adds SQL migration 010_add_features.go for feature flag table.
  • Implements FeatureStorage in pkg/featureflag/storage.go for database operations related to feature flags.
• Miscellaneous:
  • Removes DurationSort and TimestampSort from frontend/src/constants/features.ts and pkg/query-service/constants/constants.go.
  • Adds NewAddFeatureFlagFactory in pkg/sqlmigration/010_add_features.go for SQL migration.

^{This description was created by for 30db0b2. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to e73b41d in 1 minute and 53 seconds

More details

• Looked at 653 lines of code in 16 files
• Skipped 0 files when reviewing.
• Skipped posting 9 drafted comments based on config settings.

1. ee/featureflag/zeus/provider.go:21

• Draft comment:
  GetFeatures always returns basePlanFeatures. Consider handling proPlan and enterprisePlan features based on configuration.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The code already has a TODO comment indicating this is temporary and will be replaced with Zeus features. The comment is suggesting something that the author is already aware of and planning to implement. The comment doesn't add any new information beyond what's already indicated by the TODO.
  The comment could be highlighting an important architectural consideration about different plan types that needs to be addressed in the final implementation.
  While plan types are important, the TODO comment shows the author is aware this is temporary. The comment doesn't provide actionable feedback beyond what's already acknowledged.
  Delete the comment as it's redundant with the existing TODO and doesn't provide additional actionable feedback.

2. pkg/signoz/signoz.go:90

• Draft comment:
  There is a TODO regarding starting the feature flag manager. Clarify if it should be started here or elsewhere.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

3. ee/featureflag/zeus/flags.go:5

• Draft comment:
  Add comments to clarify the differences between basePlanFeatures, proPlanFeatures, and enterprisePlanFeatures.
• Reason this comment was not posted:
  Marked as duplicate.

4. ee/featureflag/zeus/provider.go:21

• Draft comment:
  GetFeatures always returns basePlanFeatures; consider using configuration to select the appropriate feature set (e.g. pro or enterprise).
• Reason this comment was not posted:
  Marked as duplicate.

5. ee/query-service/main.go:156

• Draft comment:
  Both base and Zeus feature flag factories are provided; add a mechanism (e.g., a config flag) to disable Zeus for OSS if needed.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  This is enterprise edition code, so having Zeus enabled here makes sense. The concern about OSS would be handled in the OSS version of the codebase, not here. The comment is speculative about a potential OSS issue rather than addressing a concrete problem in this enterprise code. We don't have enough context about the OSS implementation to know if this is even an issue.
  I could be wrong about the separation between OSS and EE code - maybe there are cases where this EE code needs to work in an OSS context. The comment might be raising a valid architectural concern.
  Even if there is some validity to the architectural concern, this comment is speculative ("if needed") and doesn't point to a concrete issue that needs fixing in this PR. The proper place to handle OSS vs EE feature flag providers would be in the OSS codebase.
  Delete the comment because it's speculative and suggests handling OSS concerns in EE code, without clear evidence that this is actually needed.

6. frontend/src/constants/features.ts:1

• Draft comment:
  Ensure that the enum keys here remain fully synchronized with the backend feature flag names.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. pkg/featureflag/manager.go:32

• Draft comment:
  The asynchronous call to SaveAllFeatures() ignores potential errors; consider logging any errors from the background save task.
• Reason this comment was not posted:
  Marked as duplicate.

8. pkg/signoz/provider.go:69

• Draft comment:
  The comment about disabling Zeus for OSS remains; implement a configuration or build flag to control inclusion of the Zeus provider.
• Reason this comment was not posted:
  Marked as duplicate.

9. pkg/signoz/signoz.go:92

• Draft comment:
  Review if featureFlagManager.Start() should be awaited and its error handled to ensure complete initialization.
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_H9qni2whoSLlu8xo

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on be989bb in 1 minute and 15 seconds

More details

• Looked at 669 lines of code in 12 files
• Skipped 0 files when reviewing.
• Skipped posting 9 drafted comments based on config settings.

1. ee/featureflag/zeus/flags.go:8

• Draft comment:
  The change setting SSO active in proPlanFeatures seems intentional, but verify it aligns with the feature rollout plans.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. ee/featureflag/zeus/provider.go:21

• Draft comment:
  Updated GetFeatures method signature to accept orgId; ensure the TODO is implemented later to fetch org-specific flags.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

3. pkg/featureflag/base/flags.go:9

• Draft comment:
  Addition of IsChangeable property is clear; ensure that feature flags in constants are always non-changeable to prevent unexpected user updates.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

4. pkg/signoz/signoz.go:107

• Draft comment:
  FeatureFlagManager is started immediately; consider if any explicit control (or error handling) is needed for startup sequencing.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

5. ee/featureflag/zeus/flags.go:34

• Draft comment:
  Verify that setting FeatureSSO active (true) for the pro plan is intentional.
• Reason this comment was not posted:
  Comment did not seem useful.

6. ee/featureflag/zeus/provider.go:21

• Draft comment:
  The 'orgId' parameter is currently unused in GetFeatures; consider adding a comment or using it as intended.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The TODO comment suggests this is intentionally incomplete. The orgId parameter was just added and will likely be used when implementing the TODO. Commenting on unused parameters during an obviously incomplete implementation isn't helpful.
  The comment could be valid if this was meant to be a complete implementation. The unused parameter might indicate a mistake.
  The explicit TODO comment makes it clear this is an interim state. The orgId parameter was deliberately added and will be used when implementing the zeus feature retrieval.
  Delete the comment as it's pointing out something that's clearly temporary due to the TODO comment.

7. pkg/featureflag/featureflag.go:32

• Draft comment:
  The Equals method does not compare the 'IsChanged' field. Confirm that this is the intended behavior for update logic.
• Reason this comment was not posted:
  Comment did not seem useful.

8. pkg/featureflag/storage.go:55

• Draft comment:
  Avoid using context.Background() here; use the provided 'ctx' to ensure proper context propagation.
• Reason this comment was not posted:
  Marked as duplicate.

9. pkg/sqlmigration/010_add_features.go:39

• Draft comment:
  A missing comma between the UNIQUE constraint and the FOREIGN KEY clause in the CREATE TABLE statement may cause SQL syntax errors.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_CvREZucx02tmdZSi

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on de7dc96 in 56 seconds

More details

• Looked at 279 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 13 drafted comments based on config settings.

1. ee/featureflag/zeus/flags.go:7

• Draft comment:
  Added IsChangeable field to feature flags. Ensure this aligns with overall flag behavior.
• Reason this comment was not posted:
  Comment did not seem useful.

2. ee/query-service/main.go:152

• Draft comment:
  Commented-out feature flag provider initialization; verify that this is intended for future use.
• Reason this comment was not posted:
  Comment did not seem useful.

3. pkg/featureflag/manager.go:79

• Draft comment:
  Merged features by org in one goroutine. Confirm that reduced parallelism across providers is acceptable performance-wise.
• Reason this comment was not posted:
  Comment did not seem useful.

4. pkg/query-service/utils/testutils.go:49

• Draft comment:
  Renamed migration factory from NewAddFeaturesFactory to NewAddFeatureFlagFactory. Ensure migration naming consistency.
• Reason this comment was not posted:
  Comment did not seem useful.

5. pkg/signoz/provider.go:66

• Draft comment:
  Including NewAddFeatureFlagFactory in SQLMigrationProviderFactories; verify naming consistency with other factories.
• Reason this comment was not posted:
  Marked as duplicate.

6. pkg/signoz/signoz.go:103

• Draft comment:
  FeatureFlagManager initialization is commented out. Ensure removal or activation is handled before production use.
• Reason this comment was not posted:
  Marked as duplicate.

7. pkg/sqlmigration/010_add_features.go:29

• Draft comment:
  SQL migration: Check that the FOREIGN KEY constraint is correctly separated (comma may be missing) and note the default for is_changeable (TRUE) differs from provider settings.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. ee/featureflag/zeus/flags.go:7

• Draft comment:
  Ensure the newly added 'IsChangeable' field is intentionally set to false for all features. Verify this matches your intended behavior.
• Reason this comment was not posted:
  Marked as duplicate.

9. ee/query-service/main.go:152

• Draft comment:
  The zeus feature flag provider initialization is commented out. Confirm that this temporary disablement is intended for the current release.
• Reason this comment was not posted:
  Marked as duplicate.

10. pkg/featureflag/manager.go:77

• Draft comment:
  Consider using a passed context instead of context.Background() inside RefreshFeatureFlags, and evaluate if running provider calls concurrently per org would improve performance.
• Reason this comment was not posted:
  Marked as duplicate.

11. pkg/sqlmigration/010_add_features.go:30

• Draft comment:
  Possible SQL syntax issue: Missing comma between the UNIQUE constraint and the FOREIGN KEY clause. Also, the migration sets the default for 'is_changeable' to TRUE, which contrasts with the code where IsChangeable is set to false. Verify if this discrepancy is intended.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. pkg/query-service/utils/testutils.go:49

• Draft comment:
  Migration factory name updated to NewAddFeatureFlagFactory. Ensure this renaming aligns with your feature flag migration changes.
• Reason this comment was not posted:
  Marked as duplicate.

13. pkg/signoz/signoz.go:103

• Draft comment:
  The feature flag initialization code is commented out. Confirm that disabling it is deliberate and update it when feature flag support should be enabled.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_GdN81PVeV8k13noH

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on ccfc9e4 in 21 seconds

More details

• Looked at 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. pkg/featureflag/storage.go:55

• Draft comment:
  Good change: using the passed context instead of context.Background() ensures proper handling of cancellation and deadlines.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. pkg/featureflag/storage.go:55

• Draft comment:
  Good fix: using the passed-in ctx ensures proper context propagation and cancellation instead of defaulting to context.Background().
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_rbuuFVYAOfMyOLkR

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 2a14f57 in 24 seconds

More details

• Looked at 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. pkg/sqlmigration/010_add_features.go:39

• Draft comment:
  Good catch: added the missing comma after UNIQUE(org_id, name) to correctly separate SQL constraints.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. pkg/sqlmigration/010_add_features.go:39

• Draft comment:
  Corrected SQL syntax by adding a comma between UNIQUE constraint and foreign key. This ensures the table creation statement is valid.
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_SE4ePR0C62WCwKSR

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on e6cc5b5 in 57 seconds

More details

• Looked at 230 lines of code in 5 files
• Skipped 0 files when reviewing.
• Skipped posting 8 drafted comments based on config settings.

1. pkg/featureflag/manager.go:69

• Draft comment:
  Consider passing a cancellable or timeout context to ListOrgIDs instead of context.Background.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. pkg/featureflag/manager.go:96

• Draft comment:
  Consider using a more controlled context (with cancellation/timeout) for SaveFeatureFlags instead of context.Background().
• Reason this comment was not posted:
  Comment was on unchanged code.

3. pkg/featureflag/storage.go:83

• Draft comment:
  Review the deferred transaction commit/rollback logic to ensure error handling is robust; consider explicitly capturing errors to avoid accidental commits on error.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

4. ee/featureflag/zeus/provider.go:21

• Draft comment:
  Good rename to orgID for consistency. Make sure basePlanFeatures is defined.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

5. pkg/featureflag/base/provider.go:21

• Draft comment:
  Consistent use of orgID; this improves readability.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

6. pkg/featureflag/featureflag.go:33

• Draft comment:
  Renamed OrgId field to OrgID and updated Equals method; ensures consistent naming across the project.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

7. pkg/featureflag/manager.go:69

• Draft comment:
  Using context.Background() here may bypass cancellation; consider passing a derived context if cancellation is needed.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

8. pkg/featureflag/storage.go:110

• Draft comment:
  Good consistency in using orgID for queries and updates; ensures clarity in DB operations.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

Workflow ID: wflow_2Ah81hYNeNw0sKeb

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on ee6fd55 in 3 minutes and 12 seconds

More details

• Looked at 4926 lines of code in 113 files
• Skipped 0 files when reviewing.
• Skipped posting 8 drafted comments based on config settings.

1. pkg/sqlmigration/009_add_pats.go:29

• Draft comment:
  Consider adding a Down migration implementation if rollback is needed, currently Down returns nil.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. pkg/sqlmigration/009_add_pats.go:42

• Draft comment:
  In the CREATE TABLE for personal_access_tokens, column types vary (e.g. TEXT, VARCHAR(50)). Consider standardizing column types if possible.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. pkg/sqlmigration/009_add_pats.go:59

• Draft comment:
  Using exec for schema changes is fine; ensure that WrapIfNotExists correctly handles existing columns to avoid errors.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

4. pkg/query-service/app/traces/v4/query_builder.go:241

• Draft comment:
  The buildTracesQuery function is quite long and complex. Consider refactoring it by extracting helper functions to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. pkg/query-service/metrics/transition.go:3

• Draft comment:
  Consider adding inline documentation explaining the rationale behind the metrics transition mapping for clarity and future maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

6. pkg/query-service/tests/integration/signoz_cloud_integrations_test.go:62

• Draft comment:
  Using time.Now() for capturing the pre-check-in timestamp may lead to flaky tests. Consider using fixed or mocked time values to ensure test consistency.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. pkg/query-service/utils/format.go:341

• Draft comment:
  In GetEpochNanoSecs, the loop-based exponent calculation can be simplified using math.Log10 for clarity and slight performance improvement.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. pkg/sqlmigration/009_add_pats.go:59

• Draft comment:
  The migration adds columns (like 'role') even though the CREATE TABLE statement already includes them. Please add a comment clarifying that this migration is intended to update legacy schemas where defaults may be missing.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_VtPvGyE1ZItsSmPH

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on 30db0b2 in 1 minute and 50 seconds

More details

• Looked at 351 lines of code in 5 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. pkg/featureflag/manager.go:33

• Draft comment:
  Ticker duration is hardcoded; prefer sourcing from configuration.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

2. pkg/featureflag/manager.go:97

• Draft comment:
  Avoid using context.Background() in goroutines; consider propagating a parent or derived context.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

3. pkg/featureflag/featureflag.go:7

• Draft comment:
  Suggestion: Consider adding a context parameter to GetFeatures for cancellation and timeout support.
• Reason this comment was not posted:
  Comment was on unchanged code.

4. pkg/featureflag/manager.go:33

• Draft comment:
  Suggestion: Make the 24-hour ticker interval configurable instead of hardcoding it.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

5. pkg/featureflag/manager.go:97

• Draft comment:
  Suggestion: Use a proper context (e.g. the parent context) instead of context.Background() when calling SaveFeatureFlags to allow for cancellation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. pkg/featureflag/storage.go:84

• Draft comment:
  Issue: The deferred transaction commit pattern may not update the returned error because the function doesn't use a named return. Consider using a named error return variable so tx.Commit() errors are properly captured.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_jqS0saqiKBKf647G

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.