feat(openfeature): add agentless feature flag configuration source - #19331
feat(openfeature): add agentless feature flag configuration source#19331pavlokhrebto wants to merge 12 commits into
Conversation
Codeowners resolved as |
Circular import analysis
|
|
pavlokhrebto
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
BenchmarksBenchmark execution time: 2026-07-29 10:55:08 Comparing candidate commit 898dbc7 in PR branch Found 0 performance improvements and 4 performance regressions! Performance is the same for 617 metrics, 10 unstable metrics. scenario:iastaspects-lstrip_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".
| ) | ||
| poll_interval = MAX_POLL_INTERVAL_SECONDS | ||
|
|
||
| super().__init__(interval=poll_interval, no_wait_at_start=True) |
There was a problem hiding this comment.
When DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS is set to 0 or a negative value, that value reaches PeriodicService unchanged here. PeriodicThread schedules its next run as now + interval, so 0 turns successful 200/304 polls into a tight loop that repeatedly hits the CDN and burns CPU; negative values can also make the fork-jitter path call time.sleep() with a negative duration. Please reject or clamp non-positive intervals before starting the poller.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, fixed. Non-positive poll intervals and request timeouts now fall back to the documented defaults (30s / 5s) with a warning, so a 0 cannot busy-loop the CDN and a negative cannot reach time.sleep(). Added tests.
| # Legacy experimental flag: an input to source grandfathering (handled in | ||
| # resolution) and the gate for the existing exposure/evaluation telemetry | ||
| # writers below, kept unchanged to preserve current telemetry behavior. | ||
| self._enabled = ffe_config.experimental_flagging_provider_enabled |
There was a problem hiding this comment.
When DD_FEATURE_FLAGS_ENABLED=false is used as the stable kill switch but the legacy DD_EXPERIMENTAL_FLAGGING_PROVIDER_ENABLED=true remains set, _active is false but _enabled stays true here, so the constructor still allocates the OTel/EVP/span-enrichment hooks and get_provider_hooks() returns them even though initialization and resolution report the provider as disabled. The OpenFeature SDK can still run those hooks for evaluations, causing disabled FFE to emit or enqueue evaluation telemetry instead of being fully off; include _active in these hook/writer gates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Telemetry is now gated on the resolved source (_active) instead of the legacy flag, so a disabled provider allocates and returns no hooks.
| # If provider is not enabled, return default value | ||
| if not self._enabled: | ||
| # If provider is not active, return default value | ||
| if not self._active: |
There was a problem hiding this comment.
When users adopt the new default or explicit agentless/remote_config source without setting the legacy experimental flag, _active is true while _enabled is false. Because this guard now lets evaluation continue, any flag with do_log=true reaches _report_exposure() below, creating and enqueueing an ExposureWriter even though initialize() intentionally did not start exposure telemetry under the legacy gate; if a writer is already running, those exposures can be sent. Gate the exposure calls on _enabled or start/disable the writer consistently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed by the same change: self._enabled is gone and everything keys off _active, so evaluation and exposure reporting can no longer disagree. This also matches dd-trace-js, where the same switch gates the provider and its writers.
| over HTTPS without requiring a Datadog Agent. The delivery source is selected with | ||
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE``: ``agentless`` (default) loads from Datadog and | ||
| requires ``DD_API_KEY``, while ``remote_config`` keeps the existing Agent Remote | ||
| Configuration path. Agentless polling is tuned with | ||
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS`` (default ``30``, | ||
| capped at one hour) and | ||
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS`` (default ``5``), | ||
| and can target a custom endpoint with | ||
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL``. The provider can be turned off | ||
| with ``DD_FEATURE_FLAGS_ENABLED=false``. |
There was a problem hiding this comment.
| over HTTPS without requiring a Datadog Agent. The delivery source is selected with | |
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE``: ``agentless`` (default) loads from Datadog and | |
| requires ``DD_API_KEY``, while ``remote_config`` keeps the existing Agent Remote | |
| Configuration path. Agentless polling is tuned with | |
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS`` (default ``30``, | |
| capped at one hour) and | |
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS`` (default ``5``), | |
| and can target a custom endpoint with | |
| ``DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL``. The provider can be turned off | |
| with ``DD_FEATURE_FLAGS_ENABLED=false``. | |
| over HTTPS without requiring a Datadog Agent. The provider can be turned off | |
| with ``DD_FEATURE_FLAGS_ENABLED=false``. |
Is all that part really needed? I feel like it's duplicating the documentation and not necessarily something relevant as a release note.
There was a problem hiding this comment.
Agreed, applied — trimmed to the feature and the kill switch. The env vars are covered in docs/configuration.rst.
leoromanovsky
left a comment
leoromanovsky
left a comment
There was a problem hiding this comment.
Please show proof in the PR body that this branch ran against system tests, something like:
TEST_LIBRARY=python ./run.sh PARAMETRIC \
tests/parametric/test_ffe/test_configuration_sources.py
After activating them.
| if self._configuration_source is not None: | ||
| return | ||
|
|
||
| from ddtrace.internal.openfeature._native import process_ffe_configuration |
There was a problem hiding this comment.
This catches a value error; would that mean that the etag can be advanced with a bad configuration? Please add a test to prove/disprove.
There was a problem hiding this comment.
Confirmed reachable, and fixed. process_ffe_configuration swallowed the ValueError and returned None, so the poller read every apply as success and advanced the ETag — the next poll would then get a 304 and keep the stale config indefinitely. It now returns a bool, and the agentless path raises on rejection, so the ETag stays put and the payload is retried next poll.
One thing the tests turned up: the evaluator tolerates malformed individual flags (they're skipped) but rejects an unparseable createdAt — which passes our JSON:API validation since we only check that it's a string. That's the payload the two regression tests use.
| result = provider.resolve_boolean_details("my-flag", False) | ||
| assert result.value is True | ||
| finally: | ||
| provider.shutdown() |
There was a problem hiding this comment.
does shutdown abort active work?
There was a problem hiding this comment.
It didn't, and now it does. stop() ended the poll loop, but an in-tick retry backoff ran to completion on a blocking sleep, so shutdown could wait up to ~30s. There's now a shutdown Event: backoffs wait on it (via a new optional sleep_func on the shared retry() util), the retry predicate stops retrying once it's set, and a response arriving after shutdown no longer applies config or advances the ETag.
Remaining bound is one in-flight request (..._REQUEST_TIMEOUT_SECONDS, default 5s) — Python can't abort a blocking socket read from another thread the way AbortController can.
| "DD-Client-Library-Version": _pep440_to_semver(), | ||
| } | ||
| if self._api_key: | ||
| headers["DD-API-KEY"] = self._api_key |
There was a problem hiding this comment.
omit from custom backends
There was a problem hiding this comment.
Already the case — create_agentless_source passes api_key=None whenever a custom base URL is set, so the header is never added (covered by test_create_custom_endpoint_omits_api_key). Matches merged dd-trace-js #9481 (apiKey: hasCustomEndpoint ? undefined : ...). Let me know if you also meant the DD-Client-Library-* headers — those are currently sent to custom endpoints too.
|
Added the proof to the PR body. Activated the suite for Python in DataDog/system-tests#7411 and ran it against this branch via Getting there needed two fixes in the Python parametric controller, both in #7411 and mirroring what #7355 did for Node.js:
Progression as those landed: 21 passed/6 failed → 27 passed → 29 passed. Also ran the full Only the parametric suite is activated; |
| after: t.Union[int, float, t.Iterable[t.Union[int, float]]], | ||
| until: t.Callable[[t.Any], bool] = lambda result: result is None, | ||
| initial_wait: float = 0, | ||
| sleep_func: t.Callable[[float], t.Any] = sleep, |
There was a problem hiding this comment.
can we make this as it's own PR and add tests for the change in tests/internal/test_utils_retry.py ?
| return status == 408 or status == 429 or (500 <= status <= 599) | ||
|
|
||
|
|
||
| class AgentlessConfigurationSource(PeriodicService): |
There was a problem hiding this comment.
would this make sense as a component in libdatadog built on the shared runtime?
I know there are some existing pieces of FFE in libdatadog, would it save on some duplication to add this there as well?
| openfeature: Adds agentless delivery for the Feature Flagging and Experimentation | ||
| (FFE) OpenFeature provider, loading Universal Flag Configuration directly from Datadog | ||
| over HTTPS without requiring a Datadog Agent. The provider can be turned off | ||
| with ``DD_FEATURE_FLAGS_ENABLED=false``. |
There was a problem hiding this comment.
do you want to guide towards DD_FEATURE_FLAGS_ENABLED=false? or the setting for changing the source from agentless to remote config ?
this is wording as "added support for agentless", but doesn't claim that it is now the default.
you should also consider adding the new feature, not on by default, so you can ship and validate internally or with select customers, then turn to making it on by default. it helps derisk this PR. (not a requirement for you to do it just calling it out)
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