Use canonical FFE fixtures - #11355
gh-worker-dd-mergequeue-cf854d[bot] merged 14 commits into
Conversation
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 057cdefcb8
ℹ️ 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".
|
🎯 Code Coverage (details) 🔗 Commit SHA: f27c243 | Docs | Datadog PR Page | Give us feedback! |
dougqh
left a comment
There was a problem hiding this comment.
Claude detected a couple behavioral changes that I'm uncertain are intentional.
I'll leave those to your discretion.
Otherwise, looks good to me.
@dougqh Thanks for the review; agreed with the assessment to take a closer look and remediate. I'll merge after someone on FFE reviews. |
There was a problem hiding this comment.
Maybe one documentation thing, not strictly required
Line 182 in fbf651e
could mention both submodules: This will keep the submodules in `dd-java-agent/agent-jmxfetch/integrations-core` and `dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-data` up-to-date.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
This PR is rejected because it was updated |
|
/merge |
|
View all feedbacks in Devflow UI.
PR already in the queue with status in_progress |
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
devflow unqueued this merge request: It did not become mergeable within the expected time |
🟢 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. |
…canonical-fixtures-20260512 # Conflicts: # products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
## Motivation Use the shared FFE fixture corpus. This prevents copied Go fixtures from drifting from other SDKs. The same migration is merged in [Java](DataDog/dd-trace-java#11355) and [libdatadog](DataDog/libdatadog#1979). Related migrations are open for [Python](DataDog/dd-trace-py#19390), [JavaScript](DataDog/dd-trace-js#8441), [Ruby](DataDog/dd-trace-rb#5742), and [.NET](DataDog/dd-trace-dotnet#8616). ## Changes and Decisions - Add `DataDog/ffe-system-test-data` as an OpenFeature test submodule. - Read all canonical configuration and evaluation cases from the submodule. - Parse and validate flags independently so one invalid flag does not reject valid neighbors. - Assert canonical values and reasons, including temporal defaults and missing-flag errors. - Initialize submodules in Go test workflows and configure weekly Dependabot updates. Co-authored-by: leo.romanovsky <[email protected]>
## Motivation Use the shared FFE fixture corpus so the .NET evaluator is checked against the same behavior as the other tracer implementations. This reduces fixture drift and gives us a repeatable way to expose and correct evaluator bugs when new canonical cases are added. The same fixture corpus is used by [Java](DataDog/dd-trace-java#11355), [libdatadog](DataDog/libdatadog#1979), [Go](DataDog/dd-trace-go#4753), [Python](DataDog/dd-trace-py#19390), [JavaScript](DataDog/dd-trace-js#8441), and [Ruby](DataDog/dd-trace-rb#5742). ## Changes - Replace the legacy copied fixtures with a generated, checked-in snapshot from `DataDog/ffe-system-test-data`. - Record the exact upstream commit in `SOURCE.md`. - Add a script that fetches, validates, and copies the canonical configuration and evaluation cases. - Add a weekly and manually dispatchable workflow that opens a signed draft dependency PR only when fixture contents have changed. - Parse flags independently so malformed flags do not reject valid neighbors. - Return `FLAG_NOT_FOUND` for missing flags and classify temporal, static, and split allocations. - Assert canonical values and reasons through the existing .NET unit-test suite. ## Fixture update flow When we add or change shared evaluator behavior, I imagine the flow working like this: 1. Add the new configuration and evaluation cases to [`DataDog/ffe-system-test-data`](https://github.com/DataDog/ffe-system-test-data) and review the expected behavior there. 2. The weekly updater, or a manually dispatched run for a specific ref, fetches the canonical repository and compares its fixture contents with the checked-in .NET snapshot. 3. If nothing changed, the workflow exits without opening or updating a PR. 4. If fixtures changed, the workflow copies them into this repository, records the source commit, and opens a signed draft PR with the normal dependency labels. 5. The .NET unit tests run against the updated cases. New tests may intentionally fail when they catch an evaluator bug or unsupported behavior. 6. Address those evaluator failures in the same dependency PR, keeping the fixture expectations unchanged unless the shared expectation itself is incorrect. 7. Merge the update once the .NET evaluator satisfies the new canonical cases. This keeps new behavior explicit and reviewable: fixture changes land in the canonical repository first, and each tracer then gets a visible compatibility PR rather than silently changing at build time. ## Decisions - `DataDog/ffe-system-test-data` remains the canonical source of shared evaluator behavior. - Keep the generated snapshot checked in so local and CI unit tests do not require network access or submodule initialization. - Use a scheduled dependency-update workflow instead of a git submodule. - Treat failures introduced by new canonical fixtures as useful regression signals and fix the evaluator as part of accepting the update. - Do not create a PR when the canonical fixture contents are unchanged, even if the upstream repository has unrelated commits.
Motivation
Make
DataDog/ffe-system-test-datathe canonical source for FFE/OpenFeature JSON fixtures in dd-trace-java. This removes copied smoke-test fixtures and moves evaluator behavior coverage to the shared corpus. Depends on DataDog/ffe-system-test-data#9.Changes
dd-smoke-tests/openfeature/src/test/resources/ffe-system-test-dataas a git submodule pinned to the canonical fixture update.ufc-config.jsonand all sortedevaluation-cases/*.jsonfiles.gitsubmoduleupdates.📖 https://datadoghq.atlassian.net/wiki/spaces/PANA/pages/6713639211/FFE+SDK+Fixture+Contribution+Guide
Decisions
ffe-system-test-data, not programmatic or copied fixture sets.Validation
JAVA_HOME=/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home PATH=/opt/homebrew/opt/openjdk@17/bin:$PATH ./gradlew :dd-smoke-tests:openfeature:codenarcTestJAVA_HOME=/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home PATH=/opt/homebrew/opt/openjdk@17/bin:$PATH ./gradlew :products:feature-flagging:feature-flagging-api:test --tests datadog.trace.api.openfeature.DDEvaluatorTest