Skip to content

Move FFE exposure waits into setup#7229

Draft
leoromanovsky wants to merge 19 commits into
mainfrom
leo.romanovsky/ffe-exposure-setup-waits
Draft

Move FFE exposure waits into setup#7229
leoromanovsky wants to merge 19 commits into
mainfrom
leo.romanovsky/ffe-exposure-setup-waits

Conversation

@leoromanovsky

@leoromanovsky leoromanovsky commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Motivation

Waiting inside a test_* method is too late for this scenario. By the time tests assert, setup has finished, teardown has run, and the weblog/agent path that can produce new /api/v2/exposures payloads is no longer the right place to wait. The test method should assert facts that setup already observed, not wait for new EVP data after the system under test has been shut down.

The original reason we added the wait was PHP exposure delivery. PHP evaluates the flag synchronously, but exposure EVP delivery goes through a sidecar-backed sender, so the /ffe response can return before the exposure payload is flushed out to the agent. The fix keeps that PHP-specific flush at the PHP endpoint boundary and keeps the Python tests universal.

Removing the late exposure wait also exposed a separate race in eval metric tests: some feature_flag.evaluations metric payloads were produced but not yet ingested by the agent interface when assertion began. That is the same lifecycle problem: setup should make captured data available before assertions, while the existing metric assertions remain the source of truth.

Go and Java confirmed the remote-config behavior used by the multiple-config test: last received UFC configuration wins. Multiple FFE remote-config files are not merged into one flag set.

flowchart TD
    Setup["setup_* phase: weblog and agent are alive"]
    Eval["direct weblog.post('/ffe')"]
    SDK["SDK evaluates flag"]
    EVP["SDK emits exposure EVP"]
    Metric["SDK emits feature_flag.evaluations metric"]
    PHPFlush["PHP endpoint flushes sidecar-backed exposures"]
    Agent["agent interface captures outbound payloads"]
    Store["setup stores exposure wait results"]
    Drain["setup best-effort drains late eval metrics"]
    Teardown["teardown shuts down producers"]
    Test["test_* phase asserts responses and captured payloads"]

    Eval --> SDK
    SDK --> EVP
    SDK --> Metric
    EVP --> PHPFlush
    PHPFlush --> Agent
    Metric --> Agent
    Setup --> Eval
    Agent --> Store
    Agent --> Drain
    Store --> Teardown --> Test
    Drain --> Teardown
Loading

Changes

The exposure tests now keep the normal weblog contract: setup methods call weblog.post("/ffe", json=...) directly. There is no post_ffe wrapper, no Python-side request rewriting, and no Python-side context.library branch for PHP.

The setup phase waits for outbound exposure events from the agent interface and stores those wait results for the test method to assert later. That keeps the wait at the point where new exposure data can still be produced, while the assertion still happens in the pytest test method. Before counting, the helper attempts the common /flush endpoint and treats 404/405 as unsupported, so this is capability-based rather than SDK-name-based.

The PHP /ffe endpoint keeps only the small PHP-specific responsibility: after evaluating a flag once, it calls DDTrace\Testing\flush_ffe_exposures() when available. It does not retry evaluation, wait for a flag to become visible, or expose request fields such as waitForFlag; the proof source is outbound exposure EVP captured by the agent.

The exact-count exposure cache tests now also store setup-time wait results before asserting final counts for allocation-cycle and variant-cycle behavior. That preserves the previous wait-before-count protection without moving the wait back into test_*.

The eval metric tests now perform one bounded, best-effort setup-phase drain for the metric keys that were late in CI: config-exists/missing-flag, nested-attributes, lowercase reason, and lowercase error type. This is not a wait in the assertion phase and it is not PHP-specific. It gives the agent interface a chance to ingest already-produced feature_flag.evaluations payloads, but the existing metric payload assertions remain authoritative.

The multiple-remote-config test follows last-received-config-wins semantics for every SDK: apply config 1, evaluate flag 1, wait for exposure 1, then replace it with config 2, evaluate flag 2, and wait for exposure 2.

sequenceDiagram
    participant Setup as setup phase
    participant RC as Remote Config state
    participant Weblog as weblog /ffe
    participant PHP as PHP flush hook
    participant Agent as agent interface
    participant Test as test phase

    Setup->>RC: apply current UFC config
    Setup->>Weblog: direct POST /ffe
    Weblog-->>Agent: exposure EVP or eval metric payload
    Weblog->>PHP: flush_ffe_exposures when PHP exposes it
    PHP-->>Agent: sidecar exposure EVP flushed
    Setup->>Agent: wait for exposure payloads
    Setup->>Agent: best-effort drain late eval metrics once
    Setup->>Setup: store exposure wait result
    Setup->>RC: replace UFC config when test needs next config
    Test->>Test: assert HTTP responses
    Test->>Test: assert stored exposure setup results
    Test->>Agent: inspect captured exposure and metric payloads
Loading

Decisions

The proof source is the agent interface payloads, not PHP response readiness. That is why php_ffe_response_ready, expected-value response polling, PHP native RC version comparisons, and PHP evaluation retry helpers were removed.

The Python test does not create a new EVP collector or abstract /ffe behind a custom request helper. It keeps direct weblog calls and uses the existing agent interface as the collector.

Metric readiness is handled as an agent-interface ingestion drain in setup, not as evaluation retry logic, not as a PHP endpoint contract, and not as a new assertion result. The tests still assert the same metric tags and values after setup completes.

The test does not model multiple UFC files as a merged flag set. Last received configuration wins, matching Go and Java RC client behavior.

Validation

  • ./build.sh --library php --weblog-variant apache-mod-8.1 --images weblog --docker-platform linux/amd64
  • ./run.sh +l php FEATURE_FLAGGING_AND_EXPERIMENTATION: 37 passed, 8 skipped, 2602 deselected in 163.08s
  • ./run.sh +l php FEATURE_FLAGGING_AND_EXPERIMENTATION tests/ffe/test_flag_eval_metrics.py: 17 passed in 83.44s
  • ./build.sh --library ruby --weblog-variant rails72 --images weblog
  • ./run.sh +l ruby FEATURE_FLAGGING_AND_EXPERIMENTATION tests/ffe/test_exposures.py::Test_FFE_Exposure_Events::test_ffe_exposure_event_generation tests/ffe/test_flag_eval_metrics.py::Test_FFE_Eval_Lowercase_Consistency: 3 passed in 34.10s
  • ./run.sh +l ruby FEATURE_FLAGGING_AND_EXPERIMENTATION: 37 passed, 8 skipped, 2602 deselected in 163.58s
  • ./build.sh --library java --weblog-variant spring-boot --images weblog --docker-platform linux/amd64
  • ./run.sh +l java FEATURE_FLAGGING_AND_EXPERIMENTATION: 32 passed, 8 skipped, 2602 deselected, 2 xfailed, 3 xpassed in 318.30s
  • venv/bin/mypy --config pyproject.toml tests/ffe/test_flag_eval_metrics.py
  • venv/bin/ruff format --check tests/ffe/test_flag_eval_metrics.py
  • venv/bin/ruff check tests/ffe/test_flag_eval_metrics.py
  • venv/bin/python -m py_compile tests/ffe/test_flag_eval_metrics.py
  • git diff --check
  • Existing exposure validation before the metric follow-ups: PHP full FFE 37 passed, 8 skipped, 2602 deselected in 161.15s; Java full FFE 32 passed, 8 skipped, 2602 deselected, 2 xfailed, 3 xpassed in 314.98s; Java multiple-RC exposure test 1 passed in 49.25s; targeted PHP allocation/variant exposure tests 2 passed in 49.97s.

@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

CODEOWNERS have been resolved as:

tests/ffe/test_exposures.py                                             @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
tests/ffe/test_flag_eval_metrics.py                                     @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
utils/build/docker/php/common/ffe.php                                   @DataDog/apm-php @DataDog/system-tests-core

@datadog-prod-us1-5

datadog-prod-us1-5 Bot commented Jun 29, 2026

Copy link
Copy Markdown

Pipelines  Tests

Fix all issues with BitsAI

⚠️ Warnings

🚦 4 Pipeline jobs failed

Testing the test | System Tests (java, prod) / End-to-end #2 / akka-http 2   View in Datadog   GitHub Actions

🧪 1 Test failed

tests.appsec.test_blocking_addresses.Test_Blocking_request_body_filenames.test_blocking[akka-http] from system_tests_suite   View in Datadog
ValueError: No appsec event validate this condition

self = <tests.appsec.test_blocking_addresses.Test_Blocking_request_body_filenames object at 0x7efe29d84e60>

    def test_blocking(self):
        """Can block on server.request.body.filenames"""
>       interfaces.library.assert_waf_attack(self.rbf_req, rule="tst-037-014")

tests/appsec/test_blocking_addresses.py:606: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
...

Testing the test | System Tests (python, prod) / End-to-end #2 / django-poc 2   View in Datadog   GitHub Actions

🧪 1 Test failed

tests.remote_config.test_remote_configuration.Test_RemoteConfigurationExtraServices.test_tracer_extra_services[django-poc] from system_tests_suite   View in Datadog
assert None == 200
 +  where None = HttpResponse(status_code:None, headers:{}, text:None).status_code
 +    where HttpResponse(status_code:None, headers:{}, text:None) = <tests.remote_config.test_remote_configuration.Test_RemoteConfigurationExtraServices object at 0x7fae9ffceb40>.r_outgoing

self = <tests.remote_config.test_remote_configuration.Test_RemoteConfigurationExtraServices object at 0x7fae9ffceb40>

    def test_tracer_extra_services(self):
        """Test extra services field"""
    
        # filter extra services
...

Testing the test | System Tests (ruby, prod) / End-to-end #1 / rails72 1   View in Datadog   GitHub Actions

🧪 16 Tests failed

tests.ffe.test_flag_eval_metrics.Test_FFE_Eval_Config_Exists_Flag_Missing.test_ffe_eval_config_exists_flag_missing[rails72] from system_tests_suite   View in Datadog
AssertionError: Expected metric for non-existent flag 'non-existent-eval-metric-flag', found none. All: [{'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:operator-grease-flag', 'feature_flag.result.variant:n/a', 'feature_flag.result.reason:error', 'error.type:parse_error', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849483'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:valid-flag', 'feature_flag.result.variant:expected', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849486'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:malformed-flag', 'feature_flag.result.variant:n/a', 'feature_flag.result.reason:error', 'error.type:parse_error', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849489'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:valid-flag', 'feature_flag.result.variant:expected', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849492'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag', 'feature_flag.result.variant:on', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849495'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag', 'feature_flag.result.variant:on', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849498'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag', 'feature_flag.result.variant:on', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849499'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag-not-delivered', 'feature_flag.result.variant:n/a', 'feature_flag.result.reason:error', 'error.type:flag_not_found', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849499'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag-never-delivered', 'feature_flag.result.variant:n/a', 'feature_flag.result.reason:error', 'error.type:flag_not_found', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddtrace.openfeature', 'instrumentation_scope_version:n/a'], 'points': [{'value': 1.0, 'timestamp': '1782849499'}], 'type': 'COUNT', 'metadata': {'origin': {'originProduct': 19}}}, {'resources': [{'type': 'host', 'name': 'test'}], 'metric': 'feature_flag.evaluations', 'tags': ['feature_flag.key:test-flag', 'feature_flag.result.variant:on', 'feature_flag.result.reason:static', 'feature_flag.result.allocation_key:default-allocation', 'env:system-tests', 'version:1.0.0', 'service:weblog', 'instrumentation_scope:ddt
tests.ffe.test_flag_eval_metrics.Test_FFE_Eval_Lowercase_Consistency.test_ffe_lowercase_error_type[rails72] from system_tests_suite   View in Datadog
AssertionError: Expected metric for flag 'lowercase-error-flag', found none.
assert 0 > 0
 +  where 0 = len([])

self = <tests.ffe.test_flag_eval_metrics.Test_FFE_Eval_Lowercase_Consistency object at 0x7f2219b61520>

    def test_ffe_lowercase_error_type(self):
        """Test that error.type values are lowercase."""
        assert self.r_error.status_code == 200, f"Flag evaluation request failed: {self.r_error.text}"
    
...
View all 16 test failures

View all 4 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: 97d0ca4 | Docs | Datadog PR Page | Give us feedback!

@leoromanovsky

Copy link
Copy Markdown
Contributor Author

@codex Review this to make sure it meets the stated goals of collecting data during the test asserts, works universally for all languages (no special cases in test code), and is canonical with other system tests like it. Verify that existing assertions are preserved.

@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: 9be2205fcc

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

# Store setup-time wait results; tests run after teardown and cannot produce new payloads.
self.exposure_ready_1 = wait_for_exposure_event({self.flag_1}, self.targeting_key)

rc.tracer_rc_state.reset().set_config(f"{RC_PATH}/{config_id_2}/config", rc_config_2).apply()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve simultaneous RC config coverage

This reset removes config 1 before applying config 2, so test_ffe_multiple_remote_config_files no longer sends two FFE_FLAGS files in one RC state as the pre-change test did; it evaluates each flag under an isolated config. A tracer can now pass while failing the multi-file case the test name/assertions are meant to cover, so the existing assertion coverage is not preserved. Keep both configs active and just store the per-evaluation exposure wait results in setup.

Useful? React with 👍 / 👎.

Comment thread tests/ffe/test_exposures.py
Comment thread tests/ffe/test_exposures.py
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.

1 participant