feat(openfeature): add agentless feature flag configuration source - #19331
Conversation
Codeowners resolved as |
Circular import analysis
|
🎉 All green!🧪 All tests passed 🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: 774fa3a | Docs | Datadog PR Page | Give us feedback! |
pavlokhrebto
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
BenchmarksBenchmark execution time: 2026-08-07 09:45:51 Comparing candidate commit 774fa3a in PR branch Found 0 performance improvements and 4 performance regressions! Performance is the same for 606 metrics, 10 unstable metrics. scenario:iastaspects-swapcase_aspect
scenario:iastaspectsospath-ospathbasename_aspect
scenario:span-start
scenario:tracer-small
|
![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: 128172f5a9
ℹ️ 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".
…19425) ## Description Adds an optional `sleep_func` parameter to `ddtrace/internal/utils/retry.py`, used for the initial wait and for each backoff between attempts. It defaults to `time.sleep`, so every existing caller behaves identically — the change is purely additive. The motivation is background-thread callers: `time.sleep` cannot be interrupted, so a service shutting down mid-backoff has to wait out the remaining delay. Passing an interruptible wait such as `threading.Event.wait` lets the caller abandon the backoff immediately. Split out of #19331 at review request. The Feature Flagging agentless poller is the first consumer and uses it to make shutdown prompt instead of waiting out up to ~30s of backoff. ## Testing Four tests added to `tests/internal/test_utils_retry.py`: - `sleep_func` receives the initial wait and every backoff delay, in order. - No backoff wait happens when the first attempt is accepted. - The default remains bound to `time.sleep` for callers that do not pass it. - An event-style wait can stop the remaining attempts (the interruptible-shutdown case). `tests/internal/test_utils_retry.py` passes in full (9 tests). ## Risks None expected. The parameter is optional and defaults to the previous behavior, and no existing caller passes it. ## Additional Notes No release note: this is an internal utility with no customer-visible behavior change, so `changelog/no-changelog` is applied. Co-authored-by: pavlo.khrebto <pavlo.khrebto@datadoghq.com>
…19425) ## Description Adds an optional `sleep_func` parameter to `ddtrace/internal/utils/retry.py`, used for the initial wait and for each backoff between attempts. It defaults to `time.sleep`, so every existing caller behaves identically — the change is purely additive. The motivation is background-thread callers: `time.sleep` cannot be interrupted, so a service shutting down mid-backoff has to wait out the remaining delay. Passing an interruptible wait such as `threading.Event.wait` lets the caller abandon the backoff immediately. Split out of #19331 at review request. The Feature Flagging agentless poller is the first consumer and uses it to make shutdown prompt instead of waiting out up to ~30s of backoff. ## Testing Four tests added to `tests/internal/test_utils_retry.py`: - `sleep_func` receives the initial wait and every backoff delay, in order. - No backoff wait happens when the first attempt is accepted. - The default remains bound to `time.sleep` for callers that do not pass it. - An event-style wait can stop the remaining attempts (the interruptible-shutdown case). `tests/internal/test_utils_retry.py` passes in full (9 tests). ## Risks None expected. The parameter is optional and defaults to the previous behavior, and no existing caller passes it. ## Additional Notes No release note: this is an internal utility with no customer-visible behavior change, so `changelog/no-changelog` is applied. Co-authored-by: pavlo.khrebto <pavlo.khrebto@datadoghq.com>
|
@pavlokhrebto can you fix failing linting checks? |
…com:DataDog/dd-trace-py into pavlo.khrebto/FFL-2699/agentless-ff-configs
@brettlangdon sure, done |
brettlangdon
left a comment
brettlangdon
left a comment
There was a problem hiding this comment.
one nit on the docs, otherwise config, docs, and release note lgtm
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
Two things to review or augment please.
Blocker: provider readiness is not canonical.
Python returns from initialize() before the first UFC configuration arrives. Its own test confirms that the SDK can emit PROVIDER_READY while the provider remains NOT_READY. The first evaluation can therefore return the default with PROVIDER_NOT_READY.
Node.js waits for configuration through setProviderAndWait(). Java completes an initial poll and waits for configuration during provider initialization.
References: Python provider
(https://github.com/DataDog/dd-trace-py/blob/b5cb1213cf2f8fa18294d3ecf7d56444885606c0/ddtrace/internal/openfeature/_provider.py#L222-L286), Python
readiness test (https://github.com/DataDog/dd-trace-py/blob/b5cb1213cf2f8fa18294d3ecf7d56444885606c0/tests/openfeature/test_provider_status.py#L143-L165),
Node.js contract
(https://github.com/DataDog/dd-trace-js/blob/d678882c997c1c0f5b7f6f779cff9995bc84f806/integration-tests/openfeature/app/configuration-source-evaluation.js#L34-L41),
Java provider
(https://github.com/DataDog/dd-trace-java/blob/f54355526dabf515d0c8884f74a6e01ced61c1ff/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java#L109-L133).
Should fix: shutdown does not cancel an active HTTP request.
[Python ](https://github.com/DataDog/dd-trace-py/blob/b5cb1213cf2f8fa18294d3ecf7d56444885606c0/tests/openfeature/test_provider_status.py#L143-L165)interrupts retry waits, but shutdown() joins the poller without cancelling the current connection. Shutdown can block until the configured request
timeout. Node.js aborts the request. Java cancels the active call.
Blocker: final L3 validation is stale.
The documented 29 passed result is useful, but it predates the current head and later retry changes. The current head is also 19 commits behind Python main. Latest main includes a periodic-thread crash fix (#19445), which directly affects this new poller.
Merge current main, then rerun all 29 tests with [system-tests #7411](https://github.com/DataDog/system-tests/pull/7411) (https://github.com/DataDog/system-tests/pull/7411) against the exact final head.
|
@leoromanovsky made the fixes:
Two details worth calling out, because they're deliberate: The wait does not raise on expiry. A previous revision (#18613 reverted it) waited and then raised The default stays 10s rather than Java/Node's 30s. This is a Python-specific constraint, not drift. Gunicorn's default worker timeout is also 30s, so a 30s blocking initialize() at import time sits right on the boundary and gets the worker SIGKILLed with Side effects of the change: the initialization_timeout constructor argument was accepted and silently discarded, and the initialization_timeout_ms setting was dead — both are wired up now, with the constructor argument taking precedence. The release note for #18613 still claimed "the provider now waits for configuration before returning," which was false on main; it's accurate again and now documents the timeout behavior. New tests cover: config arriving mid-wait → READY; timeout → NOT_READY and no raise; late delivery still promotes; timeout read from the env var; constructor override. If you'd rather standardize on a single cross-SDK number, I'm happy to — but I'd argue it should be 10s for everyone rather than 30s for Python, and that's a guild decision rather than something to settle in this PR.
836b9d7. _stop_service now sets the stop flag first, then half-closes the socket of the poll in flight (socket.shutdown(SHUT_RDWR)) before joining. close() alone wouldn't do it — the worker is already parked inside a syscall on that descriptor — so the half-close is what makes the pending recv return immediately. The worker still owns cleanup: _request closes the connection in its finally block and reports the resulting error as a failed poll, which preserves last-known-good. One window is genuinely uncancellable and documented in the code: http.client connects lazily inside request(), so a stop landing before the socket exists has nothing to half-close and the worker can block in connect() for up to the request timeout. Everything past connect — the long pole, since a poll spends its time reading the UFC body — cancels at once. Claiming the connection slot and re-reading the stop flag happen under one lock, so a poll can't start after shutdown has already looked for something to cancel. Seven new tests cover the cancel path, the no-op paths, and the ordering guarantee. |
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
Functional result:
- Startup timeout now raises the SDK’s canonical ProviderNotReadyError.
- OpenFeature 0.8 reports provider status ERROR.
- OpenFeature 0.10 set_provider_and_wait() raises the error.
- OpenFeature 0.10 set_provider() returns immediately. Background initialization then reports ERROR.
- A later valid configuration changes the provider to READY.
- The implementation emits one recovery event.
Validation:
- OpenFeature 0.8 full suite: 625 passed, 2 skipped.
- OpenFeature 0.10 focused suite: 18 passed.
- L3 at system-tests commit ef1cb447c9: 29 passed in 168.20s.
|
/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
Tests failed on this commit 23f1a14: What to do next?
|
|
/code blockers |
|
View all feedbacks in Devflow UI.
Checking merge blockers for #19331...
No merge blockers detected. |
5 participants
Description
Adds agentless delivery for the Feature Flagging & Experimentation (FFE) OpenFeature
provider: Universal Flag Configuration (UFC) is loaded directly from Datadog over HTTPS
with no Datadog Agent. Agentless becomes the default source; the existing Agent Remote
Configuration path remains available as explicit opt-in. Ports dd-trace-js #9397 and the
follow-ups #9481 (custom endpoints) / #9482 (grandfathering, fail-closed), adapted to
Python provider-lifecycle conventions.
Key pieces:
DD_FEATURE_FLAGS_ENABLED(stable kill switch, defaulttrue),DD_FEATURE_FLAGS_CONFIGURATION_SOURCE(agentlessdefault |remote_config;offlinereserved), and agentless tunables
..._AGENTLESS_BASE_URL,..._AGENTLESS_POLL_INTERVAL_SECONDS(30, capped at 1h),..._AGENTLESS_REQUEST_TIMEOUT_SECONDS(5).offline→ legacy grandfathering (
DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED) → default agentless.AgentlessConfigurationSource(PeriodicService): fixed-delay polling, in-tickretry/backoff (408/429/5xx/network, jittered), per-request timeout, gzip, strict JSON:API
validation, ETag/
304, last-known-good preserved on every failure, self-tracing suppressed.(only when the OpenFeature provider is registered), so there is no default-on CDN polling.
RC is subscribed only when
remote_configis the resolved source. Both sources feed thesame apply path.
hit the CDN in lockstep.
JIRA: FFL-2699
Testing
dd_env), JSON:API accept/reject,gzip, poller behavior (200/304/401/malformed/retry/last-known-good/ETag/headers/no-trace),
source-resolution matrix (kill switch, grandfathering, fail-closed), agentless factory
(API-key handling), fork jitter.
initialize()/shutdown()start/stop the poller; kill switch starts no source.tests/openfeaturesuite passes (613). style/typing/spelling/registry/suitespec pass.System tests
Ran the shared configuration-source contract against this branch, with the suite activated for
Python via DataDog/system-tests#7411 and
binaries/python-load-from-localpointing at it:Covers default lazy agentless delivery, explicit and grandfathered Remote Configuration, the kill
switch, invalid and reserved-
offlinefail-closed behavior, custom-endpoint authentication,recovery and ETag handling, timeout/retry, and non-overlapping polling.
No regressions in the rest of the FFE parametric directory:
30 passed, 12 xfailed, 29 xpassed(
test_dynamic_evaluation.py24 passed,test_span_enrichment.py6 passed + 12 pre-existingxfails).
The suite is run sequentially; with parallel workers two cases intermittently error with test-agent
port-routing noise unrelated to the library.
Risks
DD_FEATURE_FLAGS_ENABLEDdefaultstrue, but the agentless poller only activates whenthe OpenFeature provider is registered — users who don't use FFE are unaffected and there is
no default-on CDN traffic.
DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLEDnow only feeds source grandfathering. Existing userswho set it keep Remote Configuration delivery and reporting; agentless users get reporting too.
planned follow-up.
Additional Notes