[Feature Flags] Add agentless EVP fallback transport - #9235
Open
leoromanovsky wants to merge 1 commit into
Open
leoromanovsky wants to merge 1 commit into
leoromanovsky wants to merge 1 commit into
Conversation
BenchmarksBenchmark execution time: 2026-10-01 23:22:37 Comparing candidate commit 8d55124 in PR branch Found 1 performance improvements and 11 performance regressions! Performance is the same for 60 metrics, 0 unstable metrics, 78 known flaky benchmarks, 48 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9235) and master. ✅ No regressions detected |
leoromanovsky
marked this pull request as ready for review
September 21, 2026 14:40
leoromanovsky
requested review from
anna-git,
danyal002 and
vjfridge
and removed request for
a team
September 21, 2026 14:40
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
leoromanovsky
requested review from
pavlokhrebto
and removed request for
danyal002
September 21, 2026 14:40
![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: 7eba078530
ℹ️ 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 on lines
+162
to
+166
| [ | ||
| new(TelemetryConstants.ApiKeyHeader, apiKey), | ||
| new(FeatureFlagsEvpHeaderHelper.EvpOriginHeader, FeatureFlagsEvpHeaderHelper.EvpOrigin), | ||
| new(FeatureFlagsEvpHeaderHelper.EvpOriginVersionHeader, TracerConstants.ThreePartVersion), | ||
| ]; |
There was a problem hiding this comment.
When agentless delivery falls back to direct intake, these are the only default headers installed on the new HttpClient/HttpWebRequest, so the request omits x-datadog-tracing-enabled: false. The HTTP integrations explicitly use that header to suppress instrumentation, and the existing agentless configuration client adds it for the same reason; without it, every direct exposure upload can create an internal HTTP span, potentially attached to the customer trace whose execution context started the send loop. Add the tracing-suppression header to the direct header set as well.
Useful? React with 👍 / 👎.
Comment on lines
+516
to
+518
| if (response.StatusCode is < 200 or >= 300) | ||
| { | ||
| Log.Warning<int>("Feature Flags direct EVP request failed with HTTP status code {StatusCode}", response.StatusCode); |
There was a problem hiding this comment.
When direct intake returns a non-retryable response such as HTTP 400, this method performs no retry and the exposure batch has already been removed from the queue, yet the terminal failure is logged only as a warning. This hides payload bugs and permanent event loss from error-level diagnostics; log the final response as an error (with the failed endpoint and attempt context), especially for 400 responses.
AGENTS.md reference: AGENTS.md:L192-L197
Useful? React with 👍 / 👎.
Comment on lines
+102
to
+107
| _settingsSubscription = settings.Manager.SubscribeToChanges(changes => | ||
| { | ||
| if (changes.UpdatedExporter is { } exporter) | ||
| { | ||
| Interlocked.Exchange(ref _localRequestFactory!, CreateLocalRequestFactory(exporter)); | ||
| } |
There was a problem hiding this comment.
When runtime configuration changes the agent URL or transport, this callback swaps _localRequestFactory but retains _localProxyEndpoint and _discoveryKnown from the previous agent. Until the shared discovery loop polls again, exposures are therefore sent to the new relay using capabilities established for the old one; if the new agent exposes the route but does not forward the two origin headers, it silently strips the producer identity that this transport otherwise requires before selecting a local route. Invalidate the cached route/discovery state when replacing the factory and wait for discovery of the new endpoint.
Useful? React with 👍 / 👎.
Comment on lines
+381
to
+385
| ? configuration.EventPlatformProxyEndpoint switch | ||
| { | ||
| EventPlatformProxyV4 => EventPlatformProxyV4, | ||
| EventPlatformProxyV2 => EventPlatformProxyV2, | ||
| _ => null, |
There was a problem hiding this comment.
If /info advertises a supported route with different casing, DiscoveryService accepts it using OrdinalIgnoreCase and preserves the advertised spelling in EventPlatformProxyEndpoint, but this case-sensitive switch then rejects it and unnecessarily falls back to direct intake or drops events when credentials are unavailable. Classify v2/v4 with the same case-insensitive comparison used during discovery.
Useful? React with 👍 / 👎.
Comment on lines
+87
to
+91
| // The shared discovery service owns its own bounded retry/backoff loop. Event flushes | ||
| // wait for its first result once and then consume later callbacks; they never start an | ||
| // independent /info request on every flush. | ||
| _discoveryService.SubscribeToChanges(_discoveryCallback); | ||
| _discoverySubscribed = true; |
There was a problem hiding this comment.
When DD_AGENT_FEATURE_POLLING_ENABLED=false, TracerManagerFactory supplies NullDiscoveryService, whose subscription is a no-op, but this constructor still leaves _discoveryKnown unset. The first direct exposure consequently waits the full five-second discovery timeout even though no callback can ever arrive; in short-lived or abruptly frozen processes this delays or loses the first batch unnecessarily. Detect the null discovery implementation and mark discovery complete so direct delivery can start immediately.
Useful? React with 👍 / 👎.
2 of 6 tasks
Prefer identity-capable local relays and keep a credential-isolated direct route with conservative replay and sticky selection. Environment: Datadog workspace
This was referenced Oct 1, 2026
leoromanovsky
force-pushed
the
leo.romanovsky/fflsdk-189-evp-fallback-dotnet-master
branch
from
October 1, 2026 22:29
f439192 to
8d55124
Compare
leoromanovsky
changed the base branch from
master
to
leo.romanovsky/fflsdk-189-dotnet-exposure-lifecycle
October 1, 2026 22:31
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Agentless configuration lets customers evaluate flags without running an Agent, but exposure delivery still requires its local EVP proxy. Evaluations can succeed while their experiment exposures are lost.
flowchart LR C[Agentless configuration] --> E[Flag evaluation] E --> X[Exposure batch] X --> A[Local Agent required] X -. missing route .-> D[Direct EVP intake]Changes and Decisions
This is the single-commit final activation after endpoint-bound discovery #9352 and bounded exposure lifecycle #9353. It keeps Remote Config on its historical fixed EVP v2 behavior and does not add flag-evaluation emission.
Agentless delivery prefers local EVP v4, then v2, only when discovery confirms forwarding of both SDK identity headers. Configured Agent path prefixes are preserved. Otherwise it uses canonical HTTPS intake; API keys remain direct-only, relay headers local-only, and redirects are disabled for Agentless delivery.
Direct selection is sticky. An unavailable route has bounded recovery; each batch has bounded discovery waiting. The current batch can move to direct only after local 404/405 or a proven pre-send connection failure. Ambiguous failures never replay that batch, preventing duplicate exposures. Identity, credentials and this replay policy ship together.
flowchart LR E[Agentless exposure batch] --> R{Selected route} R -->|Identity-capable v4 or v2| A[Local relay] R -->|No eligible relay| D[Direct HTTPS intake] A -->|404 / 405 or proven pre-send failure| D A -->|Ambiguous failure| N[No current-batch replay] D --> S[Direct remains selected]