Skip to content

test(ffe): repair context bounds and isolate overload accounting - #8093

Open
vjfridge wants to merge 20 commits into
mainfrom
vickie/FFL-3407/ffe-harness-repair
Open

vjfridge wants to merge 20 commits into
mainfrom
vickie/FFL-3407/ffe-harness-repair

Conversation

@vjfridge

@vjfridge vjfridge commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Why

FFL-3407 makes the shared flag-evaluation tests check SDK behavior reliably, without depending on flush timing or which context fields an SDK keeps when it reaches a limit.

What changed

  • Test context limits separately. With consent enabled, verify that all 256 valid fields survive at the limit. Above the limit, allow SDKs to keep a valid subset or omit context, while preserving the flag result, user identity, and evaluation count. Check oversized strings separately: omit the oversized value, retain valid neighboring fields, and reject truncation.
  • Account for the full stress workload. Run 12,000 unique evaluations in a separate scenario and require delivered evaluations plus explicitly reported capacity drops to match that total. Reject unexplained loss, invalid events, and complete loss of the workload. Isolation matters because drop counters cover the whole process.
  • Remove the flush-timing assumption. A slow workload can span several flushes without triggering degradation. This stress test does not require degradation or drops to occur; it validates accounting and the shape of any full or degraded events received. Short targeting keys keep requests small. Normal aggregation tests still require lossless delivery.
  • Preserve numeric context in the Java weblog. Jackson represents large JSON integers as Long; pass them through OpenFeature's numeric API instead of replacing them with null. The existing full-consent test verifies that the numeric org_id survives unchanged.
  • Preserve JSON context types in the .NET weblog. Convert JSON numbers, booleans, arrays, and objects into their OpenFeature equivalents rather than stringifying them. This fixes the full-consent test input, including numeric org_id.
  • Wire the isolated stress scenario into CI. Follow the scenario-module layout on main and reuse the shared FFE settings. Wait at least 15 seconds for library telemetry before stopping the weblog, preserving longer language defaults. Gate Go accounting coverage at 2.11.0, which supplies drop telemetry.

No production SDK timing changes or test-only SDK APIs are needed. The PHP and Python manifest edits preserve the existing disabled status when renaming the stress test; enabling supported SDK versions remains a separate follow-up.

PHP-specific oversized-input and same-request recovery coverage belongs in the PHP API-to-wire suite and is proposed in dd-trace-php #4364. This PR focuses on shared SDK contracts. PHP activation remains separate work in FFL-3020.

How to test

Run the contract tests and repository checks:

./run.sh TEST_THE_TEST tests/test_the_test/test_ffe_tests.py
./run.sh TEST_THE_TEST tests/test_the_test/test_minimal_number_of_scenarios.py
./format.sh --check

With the system-tests runner and Docker available, build a Go weblog and run the SDK-backed checks:

./build.sh golang -w net-http
./run.sh FEATURE_FLAGGING_AND_EXPERIMENTATION --library golang tests/ffe/test_flag_eval_evp.py
./run.sh FEATURE_FLAGGING_EVP_STRESS --library golang

For Node.js, build with ./build.sh nodejs -w express5 and repeat the two runs with --library nodejs.

After merging main at d454d0d07, all 242 focused framework tests passed. Following review fixes, 157 focused checks and full formatting/static checks passed. Scratch checks confirmed Go version gates and the 15-second minimum without shortening longer waits; mutations of a live value-bounds capture were rejected for incorrect counts, identity, outcomes, and errors. No synthetic self-tests were added to the repository. See the PR checks for current CI results.

Fresh review validation on Go 2.11.0 and Node.js 6.20.0, through Agent 7.84.2: all three context-boundary cases and the isolated stress scenario (accounting plus two schema checks) passed for each SDK. Node was forced past its manifest gate; Go ran with its normal gate. Both stress runs used the 15-second library wait. These runs do not force drops or prove every possible flush schedule.

Earlier forced SDK runs covered the 13 evaluation tests and isolated stress test, including cases disabled by manifests:

  • Go 2.11.2, Node.js 6.20.0 and 5.131.0, Python 4.16.0, and Ruby 2.42.0 passed all 14 selected tests per target.
  • Java 1.66.0 passed all 14 after the numeric-context harness fix.
  • The .NET merged-support development build passed 12 of 13 evaluation tests and stress after the typed-context harness fix. Its remaining 256-field boundary failure is addressed separately in dd-trace-dotnet #9511.
  • The PHP merged-support development build passed nine evaluation tests and stress; four tests fail because flag-evaluation batches lack application identity. That SDK/sidecar work is tracked in FFL-3438 and FFL-3439.

These SDK runs had no skips and preceded the latest main merge. The .NET and PHP results are development-build results, not validation of published 3.57.0/1.27.0 releases. Separate agentless scenarios were outside this local SDK validation matrix. No all-SDK green result is claimed.

Review and follow-up

Please review the shared context rules, workload accounting, and isolated scenario/CI integration.

  • After merge, remove the task-owned worktree once it is clean and all work is preserved. Leave the original checkout and its unrelated changes untouched; cleanup details are in FFL-3407.

FFE reviewer selection

Reviewers: @leoromanovsky, @sameerank

Why: Flag evaluation on EVP — The PR changes the shared system tests for flag-evaluation telemetry sent over EVP. Reviewers come from the matched pod.

🤖 Automated selection.

Selection details

Initiative: project / flag-evaluation-evp

Why this match: The PR changes the shared system tests for flag-evaluation telemetry sent over EVP. It adds context-bound checks for fully consented evaluation data. These verify that 256 fields are kept at the limit and that oversized values are omitted without truncation, while identity and the evaluation count stay intact. It also replaces the degradation test with an isolated high-cardinality stress scenario. That scenario checks that delivered evaluations plus reported capacity drops add up to the full workload. Finally, it fixes the Java and .NET weblogs so numeric and typed JSON context reaches OpenFeature with its original types, which the full-consent payload tests depend on. All of this is about EVP evaluation payloads, consent and delivery, and none of it is onboarding diagnostics or bandits. The catalog has no entries with supplemental review enabled, so no additional areas apply.

  • @leoromanovsky — primary (flag-evaluation-evp): Matched pod member. Existing review coverage preserved.
  • @sameerank — floating (flag-evaluation-evp): Coverage and knowledge sharing. Existing review coverage preserved.

The PR author was excluded. Existing reviewers and teams were preserved.

Debug or fix a problem

Workflow run · Bits Code session

  • Wrong project or pod? Clarify this PR’s purpose, or correct the reviewer registry and publish the change. Sheet edits take effect after publication.
  • Need another reviewer now? Request them in GitHub. This workflow only adds requests and keeps existing reviewers.
  • Stuck beyond five minutes or failed? Inspect the run’s failed step and available Bits session. After fixing the cause, rerun the workflow with this PR URL and dry_run=false. Unchanged completed runs are skipped. If the receipt says needs_reconciliation or a write outcome is uncertain, ask the workflow owner to reconcile GitHub and the receipt before retrying; do not delete the lock blindly.
  • Report a problem: Comment on the reviewer-routing skill PR with this PR URL, the workflow run link, and the expected initiative or reviewer correction.
Evidence and run details

✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head.

Generated asynchronously by the FFE reviewer-routing workflow.

Changed paths: tests/ffe/test_flag_eval_evp.py, tests/ffe/utils/evp.py, tests/ffe/README.md, utils/_context/_scenarios/feature_flagging.py, utils/_context/_scenarios/__init__.py, .github/workflows/run-end-to-end.yml, utils/build/docker/java/spring-boot/src/main/java/com/datadoghq/system_tests/springboot/featureflag/FeatureFlagEvaluatorController.java, utils/build/docker/dotnet/weblog/Controllers/FeatureFlagEvaluatorController.cs, manifests/golang.yml

Evaluated head: 9b6ae1cbd822fcba0a2cf10da318188dbfcd11be

Registry revision: revision:1946aa2a-530e-487a-84a7-83aff44a1089

Pinned skill source: d214cfc65ae154b97d39af5cf32caf48aaec9bf0

Instructions: workflow-injected snapshot (no plugin loaded)

Instruction snapshot SHA-256: f23b2a88c13c0da90e2dd58246a14f257af91e2195c555039bd555b52e6f3516

vjfridge and others added 2 commits October 19, 2026 19:19
Port FFL-3407 coverage onto current main and exclude the PHP native IPC regression from other SDKs.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>

Co-authored-by: Vickie Boettcher <vickie.fridge@datadoghq.com>
Separate context count and value bounds, validate reported capacity drops in an isolated scenario, and add contract regression coverage.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

.github/workflows/run-end-to-end.yml                                    @DataDog/system-tests-core
docs/understand/weblogs/end-to-end_weblog.md                            @DataDog/system-tests-core
manifests/golang.yml                                                    @DataDog/system-tests-reviewers
manifests/php.yml                                                       @DataDog/system-tests-reviewers
manifests/python.yml                                                    @DataDog/system-tests-reviewers
tests/ffe/README.md                                                     @DataDog/feature-flagging-and-experimentation-sdk
tests/ffe/test_flag_eval_evp.py                                         @DataDog/feature-flagging-and-experimentation-sdk
tests/ffe/utils/evp.py                                                  @DataDog/feature-flagging-and-experimentation-sdk
tests/test_the_test/test_minimal_number_of_scenarios.py                 @DataDog/system-tests-reviewers
utils/_context/_scenarios/__init__.py                                   @DataDog/system-tests-core
utils/_context/_scenarios/feature_flagging.py                           @DataDog/system-tests-core
utils/build/docker/dotnet/weblog/Controllers/FeatureFlagEvaluatorController.cs  @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
utils/build/docker/java/spring-boot/src/main/java/com/datadoghq/system_tests/springboot/featureflag/FeatureFlagEvaluatorController.java  @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core

@datadog-official

datadog-official Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Pipelines  Tests

✨ Unblock PR with BitsAI

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 12 Pipeline jobs failed

Testing the test | System Tests (php, dev) / End-to-end #1 / apache-mod-7.0-zts 1 — ❌ 1 test failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

❌ tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language.test_expression_language_access_variables[apache-mod-7.0-zts] from system_tests_suite
AssertionError: assert &#39;Probes did not reach INSTALLED status&#39; is None
 &#43;  where &#39;Probes did not reach INSTALLED status&#39; = &lt;built-in method join of str object at 0x7f09ba42b720&gt;([&#39;Probes did not reach INSTALLED status&#39;])
 &#43;    where &lt;built-in method join of str object at 0x7f09ba42b720&gt; = &#39;\n&#39;.join
 &#43;    and   [&#39;Probes did not reach INSTALLED status&#39;] = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f09a3a2ee40&gt;.setup_failures

self = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f09a3a2ee40&gt;

    def test_expression_language_access_variables(self):
&gt;       self._assert(expected_response=200)

...
Testing the test | System Tests (php, dev) / End-to-end #1 / apache-mod-7.2-zts 1 — ❌ 2 tests failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

❌ tests.debugger.test_debugger_capture_expressions.Test_Debugger_Method_Capture_Expressions.test_method_parameter_capture_expressions[apache-mod-7.2-zts] from system_tests_suite
AssertionError: assert &#39;Probes did not reach INSTALLED status&#39; is None
 &#43;  where &#39;Probes did not reach INSTALLED status&#39; = &lt;built-in method join of str object at 0x7f7a74e2b720&gt;([&#39;Probes did not reach INSTALLED status&#39;])
 &#43;    where &lt;built-in method join of str object at 0x7f7a74e2b720&gt; = &#39;\n&#39;.join
 &#43;    and   [&#39;Probes did not reach INSTALLED status&#39;] = &lt;tests.debugger.test_debugger_capture_expressions.Test_Debugger_Method_Capture_Expressions object at 0x7f7a66e485f0&gt;.setup_failures

self = &lt;tests.debugger.test_debugger_capture_expressions.Test_Debugger_Method_Capture_Expressions object at 0x7f7a66e485f0&gt;

    def test_method_parameter_capture_expressions(self):
&gt;       self._assert()

...
❌ tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language.test_expression_language_access_variables[apache-mod-7.2-zts] from system_tests_suite
AssertionError: assert &#39;Probes did not reach INSTALLED status&#39; is None
 &#43;  where &#39;Probes did not reach INSTALLED status&#39; = &lt;built-in method join of str object at 0x7f551122b720&gt;([&#39;Probes did not reach INSTALLED status&#39;])
 &#43;    where &lt;built-in method join of str object at 0x7f551122b720&gt; = &#39;\n&#39;.join
 &#43;    and   [&#39;Probes did not reach INSTALLED status&#39;] = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f55084a0890&gt;.setup_failures

self = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f55084a0890&gt;

    def test_expression_language_access_variables(self):
&gt;       self._assert(expected_response=200)

...
Testing the test | System Tests (php, dev) / End-to-end #1 / apache-mod-7.3-zts 1 — ❌ 1 test failed · 🔧 Needs a code fix, caused by this PR

View more details · View in GitHub Actions

❌ tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language.test_expression_language_access_variables[apache-mod-7.3-zts] from system_tests_suite
AssertionError: assert &#39;Probes did not reach INSTALLED status&#39; is None
 &#43;  where &#39;Probes did not reach INSTALLED status&#39; = &lt;built-in method join of str object at 0x7f805a22b720&gt;([&#39;Probes did not reach INSTALLED status&#39;])
 &#43;    where &lt;built-in method join of str object at 0x7f805a22b720&gt; = &#39;\n&#39;.join
 &#43;    and   [&#39;Probes did not reach INSTALLED status&#39;] = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f80439325d0&gt;.setup_failures

self = &lt;tests.debugger.test_debugger_expression_language.Test_Debugger_Expression_Language object at 0x7f80439325d0&gt;

    def test_expression_language_access_variables(self):
&gt;       self._assert(expected_response=200)

...

View all 12 failed jobs.

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 2f0b9f6 | Docs | View more details | Give us feedback!

vjfridge and others added 5 commits October 20, 2026 16:46
Require complete context at the field limit and preserve results, identity, and counts when oversized contexts are pruned or omitted.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Merge main at f8d7b76, including the gqlgen fixture upgrade; preserve the feature-flag evaluation harness changes.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Retain compact targeting keys from #8133 while preserving isolated 12000-evaluation accounting and native cap-boundary coverage. Add compact-payload and multi-window contract regressions; document why doubling the workload cannot guarantee degradation across arbitrary flushes.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Merge main at 3f9f99b and retain all FFE regression coverage under the upstream TEST_THE_TEST relocation. Resolve README instructions and migrate added tests off the removed contract-only scenario.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Keep the shared suite focused on context bounds and overload accounting; cover native IPC degradation through the PHP API-to-wire tests.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
vjfridge and others added 6 commits October 22, 2026 22:10
Describe flush timing without claiming exact-cap coverage in SDK tests.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Keep the shared contract, run command, and isolation requirement; link to implementation details.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Check workload properties and cover each malformed capture once while retaining valid flush layouts.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Remove the redundant class-level missing-feature declaration.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Pass Jackson Long attributes through OpenFeature numeric context instead of replacing them with null.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Build typed OpenFeature values instead of stringifying numeric, boolean, and structured evaluation attributes.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Bring in current main and rerun CI after investigating the Node shutdown timing failure and Payara startup failure.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
@vjfridge
vjfridge marked this pull request as ready for review October 23, 2026 02:03
@vjfridge
vjfridge requested review from a team as code owners October 23, 2026 02:03
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T02:11:17.524450Z a6be98e Draft marked ready
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6be98ef02

ℹ️ 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".

Comment thread tests/ffe/test_flag_eval_evp.py
Comment thread tests/ffe/test_flag_eval_evp.py Outdated
Comment thread tests/test_the_test/test_ffe_tests.py Outdated
Remove the FFE self-tests added by this PR while preserving the pre-existing coverage.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
@vjfridge
vjfridge requested a review from cbeauchesne October 23, 2026 17:55

@sameerank sameerank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple non-blocking suggestions!

Comment thread utils/_context/_scenarios/__init__.py Outdated
Comment thread tests/ffe/test_flag_eval_evp.py

@aarsilv aarsilv left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Approving with some comments you can take or leave as you see fit.

Comment thread utils/_context/_scenarios/__init__.py Outdated
Comment thread tests/ffe/test_flag_eval_evp.py
Comment thread tests/ffe/test_flag_eval_evp.py
Comment thread tests/ffe/README.md Outdated
Comment thread docs/understand/weblogs/end-to-end_weblog.md Outdated
Comment thread utils/_context/_scenarios/__init__.py Outdated
vjfridge and others added 2 commits October 26, 2026 17:18
Preserve the isolated stress scenario in the refactored FFE module and retain typed .NET context conversion alongside the opt-out scenarios.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Preserve longer capture waits while allowing telemetry to flush, gate Go drop accounting at 2.11.0, and apply shared outcome checks to oversized values. Clarify counter units and batch semantics.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
vjfridge and others added 3 commits October 27, 2026 14:20
Include the matching Java development provider download from #8188 and current main updates.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Include the latest main changes while preserving the FFE repair scope.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>
Preserve the FFE harness repairs alongside the merged SDK identity tests from PR 8153.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-6 <noreply@localhost>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants