Repository navigation
Conversation
Add strict dependent-flag evaluation, ordered telemetry and transactional exposures, and the agentless capability header. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
Publish detached prerequisite evaluations only after graph completion while keeping dependency exposures transactional. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
Scope returned dependency exposures to their root hook lifecycle and isolate their delivery from ordinary root telemetry. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
Integrate current evaluator opt-out behavior and align dependency telemetry with the existing hook lifecycle. Environment: Datadog workspace Co-Authored-By: Codex GPT-5 <noreply@localhost>
Advertise bit 50 with FFE_FLAGS on the agent-based configuration path. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
hhan2024
added
tag: ai generated
ChatGPT Codex Connector
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9122ac3d9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.configurationPoller.removeCapabilities( | ||
| CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | ||
| | CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1); |
There was a problem hiding this comment.
When ConfigurationPoller.start() throws, FeatureFlaggingSystem.start() closes the backend and reaches this call with both capability bits, but FeatureFlaggingSystemTest.failedStartRollsBackTheRemoteConfigRegistration still verifies an exact removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES) invocation at line 68. Mockito will therefore fail that system test whenever this rollback path is run; update the assertion to expect the combined mask as well.
Useful? React with 👍 / 👎.
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
The startup-rollback test still expects only bit 46, so the new cleanup mask containing bits 46 and 50 makes its Mockito verification fail.
🤖 Bits Code Review · Commit 9122ac3
| this.configurationPoller.removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); | ||
| this.configurationPoller.removeCapabilities( | ||
| CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | ||
| | CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1); |
There was a problem hiding this comment.
The existing FeatureFlaggingSystemTest.failedStartRollsBackTheRemoteConfigRegistration test simulates poller.start() throwing, which invokes this cleanup through FeatureFlagsBackend.close(). Its assertion at line 68 still expects removeCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES), so Mockito's exact-argument verification fails against the new combined mask. Update that agent test to expect bits 46 and 50, alongside the library test updates.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| this.configurationPoller.addCapabilities(CAPABILITY_FFE_FLAG_CONFIGURATION_RULES); | ||
| this.configurationPoller.addCapabilities( | ||
| CAPABILITY_FFE_FLAG_CONFIGURATION_RULES | ||
| | CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1); |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P1] When this Remote Config service starts, it advertises bit 50, whatever the dd-openfeature version of the app. In Java, the agent and the dd-openfeature jar ship separately, so a new agent can run with an older provider that cannot evaluate prerequisites. With ddoghq/dd-source#151678, Remote Config then sends that provider the dependency-bearing UFC.
I tested the old provider in the ffe-dogfooding bench, with agent and dd-openfeature 1.66.0. The emulator sent the dependency-bearing UFC although the client advertised only bit 46. The provider returned wrong variants with success reasons and no error, consistent with the tested flagEvaluation conditions not matching. For example, depflags-synth-root returned control (reason STATIC) instead of treatment, and ffe-prereq-demo-pricing-exp-a returned default-control instead of treatment.
#13167 gates bit 50 on the capabilities that each subscriber reports. If both PRs merge unchanged, the gate leaves bit 50 on when an older provider subscribes first: start() adds it, dependentFlagsAdvertised starts as false, and an older subscriber makes supported false, so updateDependentFlagCapability returns early and the bit stays on.
Suggested change: register only CAPABILITY_FFE_FLAG_CONFIGURATION_RULES in start(), and let the #13167 gate add CAPABILITY_FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1. With that change and the evaluator report turned on (EvaluatorCapabilities.supported()), the bench advertised bits 46 and 50, received the dependent UFC over Remote Config, and returned the expected variants in the prerequisite scenarios.
d948973 to
edf686b
Compare
What Does This Do
Advertise remote-config capability bit 50 (
FFE_FLAG_DEPENDENT_FLAG_VARIANT_KEY_V1) alongside bit 46 when the feature flag remote-config service starts. Remove both capabilities when it closes.Motivation
Stacked on #13166, which implements dependent flag evaluation and telemetry. Advertising bit 50 allows agent-based remote config to deliver dependent flag configurations, matching the Python reference.
Additional Notes
Validation: 35 targeted tests passed across
RemoteConfigServiceImplTest(including diagnostics) andFeatureFlagsBackendTest. Regression assertions check bit 50 at startup, subscription, and shutdown. The changed files pass google-java-format. The Gradle Spotless task could not provision its P2 dependencies; targeted tests ran successfully separately. Performance and technical-debt self-review found no actionable issues.