Repository navigation
Add dd-openfeature SDK flag configuration and evaluation core - #12840
Conversation
PerfectSlayer
added
type: feature
🟢 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. |
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: e88985265a
ℹ️ 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".
![datadog-datadog-us1-prod[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Malformed scheduling fields can silently remove allocation bounds, and explicit null enablement is reported as disabled instead of a parse error. Focused execution confirmed both regressions and verified that the inline replacements reject affected flags while retaining valid siblings.
🤖 Bits Code Review · Commit e889852
acdca1e to
bbd00b2
Compare
|
@DataDog review |
There was a problem hiding this comment.
The parser accepts 1e400 as infinity, allowing a NUMERIC flag to resolve successfully with a non-finite value instead of rejecting the configuration.
🤖 Bits Code Review · Commit bbd00b2
|
@DataDog review |
sarahchen6
left a comment
sarahchen6
left a comment
There was a problem hiding this comment.
Nice! IIUC most of these changes look straightforward - just duplicating files over from products/feature-flagging/ - which are then removed in a later stacked PR. The "new" jackson parser looks reasonable. I ran the changes through Codex again, and it mentioned one potential StackOverflowError that I think could be addressed. Otherwise LGTM!
5995b85 to
7005e85
Compare
7005e85 to
dfc6594
Compare
There was a problem hiding this comment.
More details
Allocation timing, shard matching, and evaluator type conversion remain consistent with the existing implementation. The completed static review identified no reportable defect; tests were not run.
🤖 Bits Code Review · Commit 7005e85
75a7cf0 to
73f361a
Compare
Introduce the standalone :dd-openfeature module, not published yet, with the Universal Flag Configuration model and parser and the local flag evaluator. It depends on no dd-trace-java internal module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The evaluator built its own metadata type that the provider copied into an OpenFeature ImmutableMetadata on every evaluation. The SDK already depends on the OpenFeature SDK, so build ImmutableMetadata directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the EvaluationContext and EvaluationResult mirrors of the OpenFeature types: the evaluator takes the OpenFeature context and returns a ProviderEvaluation, removing the per-evaluation context adapter and result conversion. Exposures are reported through an ExposureListener at resolution time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An explicit "enabled": null was read as false, keeping the malformed flag as disabled instead of reporting it invalid. Only an absent field now defaults to false, as with the previous Moshi parsing. Also convert the serial id coercion cases to @TableTest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A boolean, object or array startAt / endAt was read as null, silently removing the allocation bound. Reject the flag as invalid instead, as with the previous Moshi parsing, while still ignoring numbers and unparsable date strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A number overflowing double, like 1e400, was read as infinity and could resolve as a valid NUMERIC flag value. Reject the configuration instead, as the previous strict Moshi reader did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the UFC split features parsing from #12772 to the dd-openfeature SDK parser. Features only feed exposure hooks, so a malformed list or entry is dropped instead of rejecting the flag, and a feature without destinations is dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
73f361a to
9996190
Compare
There was a problem hiding this comment.
More details
Confidence rests on source comparison with the existing parser and evaluator, plus inspection of parsing tests. Tests were not executed, so runtime behavior remains unverified.
🤖 Bits Code Review · Commit 73f361a
|
/merge |
|
View all feedbacks in Devflow UI.
It will be processed automatically as soon as GitHub reports it as mergeable. View in MergeQueue UI.
The expected merge time in
|
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #12840...
Detected 1 merge blocker(s) to address: 🔴 Blocking
|
The UFC parser reads the document recursively, including ignored fields, so a deeply nested document could overflow the stack with an error the configuration sources do not catch. Reject documents nested deeper than 32 levels, as Moshi capped them at 255 before. The cap is enforced by the reader rather than Jackson StreamReadConstraints, which jackson-core older than 2.15 (e.g. Spring Boot 2.7) does not provide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9996190 to
de691e4
Compare
52f31b8
into
master
2 participants
What Does This Do
Adds the new
:dd-openfeaturemodule (not published yet) with:internal/ufc), including splitfeatures(port of Parse features on UFC splits #12772)internal/evaluation)Motivation
dd-openfeaturebecomes a standalone Java 11 SDK depending on no dd-trace-java internal module (APMLP-1512). The new module is built alongside the existingfeature-flagging-apimodule, which keeps owning thedd-openfeatureartifact until the switch PR of the stack.Additional Notes
Most files are moved from master. Only the parser and the evaluator need a full review: everything else is a package move or a small model change. The parser rejects documents nested deeper than 32 levels.
New files (build only)
build.gradle.kts,gradle.lockfileMoved, package rename only (no logic change)
From
datadog.trace.api.featureflag.ufc.v1(feature-flagging-bootstrap) tocom.datadog.openfeature.internal.ufc, some classes madefinal:ConditionConfiguration,ConditionOperator,Environment,Flag,ParsedSemver,Rule,ValueType,VariantServerConfiguration,Split: Moshi-specific comments reduced to JavadocParsedSemverTest: package onlyremote-config/*.jsonfixtures (fromfeature-flagging-libtest resources): identicalMoved with significant changes
ufc/Allocationufc/v1/AllocationDate→Instant; drops the dualDate/ preciseInstantconstructors and accessorsufc/Shard,ufc/ShardRangeufc/v1/Shard,ShardRangeint→longfor unsigned 32-bit values, no bootstrap binary compatibility to preserveufc/UniversalFlagConfigParserUniversalFlagConfigParser+JsonApiUfcResponseParserparse/parseJsonApi), Moshi reflective adapters replaced byjackson-corestreaming + explicit mapping with strict / lenient coercion; rejects duplicate keys and non-object documents. Same per-flag rejection rules as master. Review as new codeevaluation/FlagEvaluatorDDEvaluator(~43% similar)DDEvaluator(#12843)UniversalFlagConfigParserJsonApiTestJsonApiUfcResponseParserTestUniversalFlagConfigParserTestRemoteConfigServiceImplTest(~52% similar)FlagEvaluatorchanges against the extractedDDEvaluatorlogic (~25 of ~340 lines):EvaluationContext,ProviderEvaluation,ImmutableMetadata,Value)ExposureListenercallback instead of dispatched to the gatewaySPLIT_SERIAL_ID_SUPPORTEDprobe removed (no version skew inside the SDK)AllocationInstantfields directly, no unsigned int conversionsevaluate()entry point nearly unchangedShared files
.github/CODEOWNERS: adds/dd-openfeature/settings.gradle.kts: includes:dd-openfeaturegradle/libs.versions.toml: addsjackson-corealiasThe evaluator is covered by
DDEvaluatorTestandCanonicalFixtureTest, which come with the provider PR (#12843).Stack
The full change is in #12709. It was split into this stack to ease the review.
Jira ticket: APMLP-1512
🤖 Generated with Claude Code