test(ffe): enable Python agentless configuration - #7411
Conversation
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef1cb447c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if os.environ.get("DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED") == "true" and not any( | ||
| name in os.environ for name in _FFE_CONFIGURATION_ENVVARS | ||
| ): |
There was a problem hiding this comment.
Preserve eager provider startup for RC tests
When DD_FEATURE_FLAGS_CONFIGURATION_SOURCE=remote_config is present, this new guard leaves openfeature_client unset until /ffe/start, but several existing parametric FFE paths set Remote Config and call wait_for_rc_apply_state() before they call ffe_start() (for example test_dynamic_evaluation.py::test_ffe_flag_evaluation and test_configuration_sources.py::test_remote_config_positive_ignores_agentless_env). Those tests rely on the eager DataDogProvider() construction to subscribe to FFE_FLAGS and ACK the config; with this condition they never advertise/apply that product and the wait times out for Python versions newly enabled by the manifest.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Checked this — it doesn't reproduce, because in dd-trace-py the FFE_FLAGS subscription isn't owned by the provider. ddtrace/internal/openfeature/product.py calls enable_featureflags_rc() at tracer startup whenever the resolved source is remote_config, independent of whether DataDogProvider() has been constructed. So skipping the eager construction still advertises the product and ACKs the config; /ffe/start only wires up the OpenFeature client.
You're right that the guard applies to test_dynamic_evaluation.py — its DEFAULT_ENVVARS do set DD_FEATURE_FLAGS_CONFIGURATION_SOURCE=remote_config. I ran the whole directory sequentially against dd-trace-py #19331 to confirm:
PYTEST_XDIST_AUTO_NUM_WORKERS=1 TEST_LIBRARY=python ./run.sh PARAMETRIC tests/parametric/test_ffe/
30 passed, 12 xfailed, 29 xpassed in 197.18s
Per file: test_dynamic_evaluation.py 24 passed / 0 failed, test_span_enrichment.py 6 passed + 12 xfailed (pre-existing manifest declarations), test_configuration_sources.py 29 xpassed. No wait_for_rc_apply_state() timeouts.
(The configuration-source cases show as xpassed only locally: with binaries/python-load-from-local, ddtrace.__version__ reports the pip-installed release rather than the branch, so the v4.14.0-dev gate can't match. They pass 29/29 when the manifest declares a version the local build satisfies.)
|
Motivation
DataDog/dd-trace-py#19331 implements the Python agentless Feature Flagging configuration source (FFL-2699). This enables the shared configuration-source contract for Python, following the Node.js (#7355) and Java (#7300) activations.
Two Python parametric-controller gaps had to be fixed for the contract to be observable; both mirror what #7355 fixed for Node.js.
Changes
tests/parametric/test_ffe/test_configuration_sources.pyfor Python fromv4.14.0-dev.reasonanderrorCodefrom/ffe/evaluate. It usedget_*_value(), which discards both, so the controller always reportedreason=DEFAULTwith no error code. The cold-state assertions requirePROVIDER_NOT_READY/GENERAL/PARSE_ERRORorreason=ERROR; the library already returnedPROVIDER_NOT_READY, but it never reached the test.DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED=truecaused the provider to be created at import, polling the CDN before/ffe/startand breaking the lazy-activation assertions.Decisions
The eager initialization is gated, not removed. Tests that predate this contract still rely on it:
test_dynamic_evaluation.pyhas 2 tests but 1ffe_startcall, andtest_span_enrichment.pyhas 18 tests with 13. Gating on the new configuration variables keeps those paths byte-identical.Only the parametric suite is activated.
tests/ffe/test_agentless_configuration.py(end-to-end, weblog) staysmissing_feature— it was not exercised here.Validation
Run against dd-trace-py #19331 using
binaries/python-load-from-local:Progression while fixing the controller: 21 passed / 6 failed → 27 passed → 29 passed. The suite was run sequentially; with parallel workers, two cases intermittently error with test-agent port-routing noise (
Client sent an HTTP request to an HTTPS server), unrelated to the library.Note the local run declared
v4.12.0in the manifest rather thanv4.14.0-dev: withpython-load-from-local,ddtrace.__version__reports the pip-installed release (4.12.0) whilePYTHONPATHsupplies the branch under test, so the version gate cannot see the branch locally. CI installs the real artifact, sov4.14.0-devis the correct declaration there.Workflow
🚀 Once your PR is reviewed and the CI green, you can merge it!
🛟 #apm-shared-testing 🛟
Reviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present