Repository navigation
[SVLS-10112] feat(traces): relay feature flag events through the EVP proxy - #1507
clifordshelton wants to merge 7 commits into
Conversation
Tracers probe /info early in their own init. The trace agent used to bind its socket inside the spawned task, so that probe could be refused if it ran before the task did, and the tracer then skipped the Extension for the life of the execution environment. Bind synchronously in start_trace_agent, before returning to the caller that calls /next. Based on Rey Abolofia's change on rey.abolofia/llmobs-info-endpoint. SVLS-10110 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 3 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: 436084d | Docs | View more details | Give us feedback! |
Bind through tokio::net::TcpSocket with SO_REUSEADDR and an explicit backlog instead of std::net::TcpListener, so the early bind keeps the socket options the trace agent had before and needs no std-to-tokio conversion. SVLS-10110 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3336b93 to
a66d419
Compare
Lambda starts the runtime, and with it the tracer's init and its early /info probe, once every extension has registered. The extension's first /next only marks its own init as done. Binding inside start_trace_agent was after register and most of the extension's setup, so that probe could still be refused; main already listened by the first /next. Bind 8126 in main() before registering and pass the listener through extension_loop_active to start_trace_agent. A bind failure is logged as before and no trace agent is served. The fake Extensions API now probes 8126 once while it handles register, and new lifecycle tests assert the trace agent is listening by then in On-Demand and Managed Instance modes. They fail on the previous code. Suggested by Aaron Silverman in review. SVLS-10110 Co-Authored-By: Aaron Silverman <aaron.silverman@datadoghq.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a66d419 to
dc606c7
Compare
Codex Review Summary: ✅ CompletedThis 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. |
![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: dc606c7d4f
ℹ️ 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".
| LLM_OBS_EVAL_METRIC_ENDPOINT_PATH, | ||
| LLM_OBS_EVAL_METRIC_ENDPOINT_PATH_V2, | ||
| LLM_OBS_SPANS_ENDPOINT_PATH, | ||
| EVP_PROXY_V2_BASE_PATH, |
There was a problem hiding this comment.
🤖 Comment from Claude working with aarsilv:
🟡 [P2] Listing /evp_proxy/v2/ moves dd-trace-go LLM Obs to agent mode, where its dataset and experiment calls get 404
dd-trace-go turns on its EVP proxy support when /info lists exactly "/evp_proxy/v2/" (option.go#L666). Unless agentless mode is set, through DD_LLMOBS_AGENTLESS_ENABLED or the WithLLMObsAgentlessEnabled option, ResolveAgentlessEnabled then picks agent mode (llmobs.go#L185-L195). In agent mode every LLM Obs call goes to the agent URL plus /evp_proxy/v2 (transport.go#L211). That includes the dataset and experiment APIs under /api/unstable/llm-obs/v1 and /api/v2/llm-obs/v1 (transport.go#L41-L42). The Extension does not route those paths, so they get 404, and dd-trace-go treats the failure as permanent (transport.go#L385). The span and evaluation-metric routes still return 200 in the router test below.
This affects Go functions that use LLM Obs datasets or experiments without agentless mode set. Before this PR they ran agentless.
Why the Extension cannot relay these calls
In agent mode those calls send X-Datadog-NeedsAppKey: true (transport.go#L304-L310), and the Extension has no application key. The pinned Datadog Agent does not add one for EVP requests either: "ApplicationKey is the top-level application key (app_key). Only the AI Guard proxy uses it." (config.go#L330).
A unit test against make_router() with this PR's commit gives:
| POST path | Status |
|---|---|
/evp_proxy/v2/api/unstable/llm-obs/v1/datasets |
404 |
/evp_proxy/v2/api/v2/llm-obs/v1/experiments |
404 |
/evp_proxy/v2/api/v2/llmobs |
200 |
/evp_proxy/v2/api/intake/llm-obs/v2/eval-metric |
200 |
Suggested fix
- In the PR description and release notes: "dd-trace-go LLM Observability datasets and experiments: set
DD_LLMOBS_AGENTLESS_ENABLED=true, withDD_API_KEYand an application key." - Add a Go dataset or experiment call to the pending LLM Obs smoke test.
- Optional: ask dd-trace-go to fall back to direct intake when a local EVP call gets 404.
- Worth noting too: with
DD_LLMOBS_AGENTLESS_ENABLED=false, Go LLM Obs used to fail to start because the agent did not support it (llmobs.go#L189-L191). That startup error is now avoided; dataset and experiment calls still get 404.
Was this finding useful? React with 👍 or 👎.
There was a problem hiding this comment.
Thanks, confirmed. dd-trace-go switches to agent mode when /info lists /evp_proxy/v2/ and DD_LLMOBS_AGENTLESS_ENABLED is unset. After that, dataset and experiment calls go through the Extension, which doesn't route them.
What we'll do in this PR: document it. The PR description and release notes will say:
Go functions that use LLM Observability datasets or experiments: set
DD_LLMOBS_AGENTLESS_ENABLED=trueand keepDD_API_KEYandDD_APP_KEYset, so those calls go directly to Datadog. LLM Observability spans and evaluations work through the Extension with no change.
The setups affected are Go functions that have DD_API_KEY and DD_APP_KEY set in plain text, use datasets or experiments, and leave agentless mode unset. Functions that use DD_API_KEY_SECRET_ARN couldn't use datasets before this PR either, and they now get spans and evaluations through the Extension.
Why the Extension won't pass DD_APP_KEY: dd-trace-go reads the app key only from DD_APP_KEY, so the Extension could read the same variable. We decided against it for two reasons:
- The Datadog Agent removed this on purpose. trace: remove evp proxy appkey injection datadog-agent#51580 (June 2026) dropped app key injection and
X-Datadog-NeedsAppKeyfrom its EVP proxy because "it's not used by anyone and could be misused." Adding it to the Extension would bring back a local endpoint that attaches an app key to requests, which the Agent team removed. That decision belongs to the Agent, LLM Observability and security teams, not to a flag PR. - It's more than a header. Dataset and experiment calls are request and response (
GETrecords,POSTandPATCHexperiments, multipart uploads). The store-and-forward proxy answers 200 right away and sends later, so these calls would need a synchronous pass-through with wildcard paths.
Likely a dd-trace-go issue: in agent mode, dd-trace-go sends these calls with X-Datadog-NeedsAppKey: true and no app key (transport.go#L303-L310). The current Agent no longer adds one. So dataset and experiment calls in agent mode look broken against the Datadog Agent too, not only in Lambda. A fix in dd-trace-go would cover both, for example sending these calls directly whenever DD_APP_KEY is set, or falling back to direct on a local 404. I'll raise it with the dd-trace-go team.
| state.config, | ||
| state.proxy_aggregator, | ||
| request, | ||
| "event-platform-intake", |
There was a problem hiding this comment.
🤖 Comment from Claude working with aarsilv:
Heads up: the end-of-invocation flush now depends on reaching event-platform-intake.<DD_SITE>
The Extension answers the tracer at once and sends flag events during its flush. The blocking flush waits for the proxy requests (flushing/service.rs:369-374, awaited at main.rs:775). If the intake host is unreachable or slow, for example with restricted VPC egress, the next /next waits as follows:
- Each request is tried up to
FLUSH_RETRY_COUNT(3) times (lib.rs:45), withflush_timeoutper attempt (proxy_flusher.rs:133). - The HTTP client keeps no idle connections (
pool_max_idle_per_host(0),http.rs:34), so each flush opens a new TLS connection.
This mechanism already exists for the LLM Obs routes, but flag customers now reach it by default. A release note could say: "Allow egress to event-platform-intake.<DD_SITE>."
There was a problem hiding this comment.
Thanks. I added "Allow egress to event-platform-intake.<DD_SITE>" to the release notes in the PR description.
|
🤖 Comment from Claude working with aarsilv: 🔵 [P3] Some statements in the PR description do not match the tracers
Was this finding useful? React with 👍 or 👎. |
|
🤖 Comment from Claude working with aarsilv: Reminder: this PR ships v2 only and leaves |
Thanks. I updated the description with your corrections: the 404 fallback applies only to flag writers in agentless mode, dd-trace-rb uses fixed paths, Java switches unless agentless is set to |
Good catch: SVLS-10112 still asked for v4. I updated its scope and "Done when" to v2 only, with the reason. |
|
🤖 Comment from Claude working with aarsilv: Heads up: the downstream e2e job |
…bind test Update three doc comments left over from the bind-before-register change (start_with_listener, the tracer retry claim, and a link to the removed TraceAgent::bind), and assert no panic before killing the process in the trace-agent-listens-before-register tests. Co-Authored-By: Aaron Silverman <aaron.silverman@datadoghq.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
013d6ee to
6140981
Compare
Thanks, looked at the log: both failures are |
The socket accepts connections from bind_listener, before the router is built, so log there. The start timing log now covers building the router. Co-Authored-By: Aaron Silverman <aaron.silverman@datadoghq.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tracers send feature flag evaluations and exposures as Event Platform
events. They use a local EVP proxy only when /info advertises the bare
/evp_proxy/v2/ path and evp_proxy_allowed_headers with DD-EVP-ORIGIN and
DD-EVP-ORIGIN-VERSION; otherwise they send directly, which needs
DD_API_KEY. With only DD_API_KEY_SECRET_ARN, those events were dropped.
- Advertise /evp_proxy/v2/ and the two allowed headers in /info.
- Route /evp_proxy/v2/api/v2/{flagevaluation,exposures} to
event-platform-intake through the existing store-and-forward proxy,
which adds the API key at flush.
- v4 is not advertised, so tracers use v2.
- Unrouted EVP paths keep returning 404, so tracers that match the base
path by prefix send that data directly.
Based on Aaron Silverman's patch from datadog-lambda-python#872.
SVLS-10112
Co-Authored-By: Aaron Silverman <aaron.silverman@datadoghq.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tracers send EVP batches of up to 5 MiB, and the shared 2 MiB proxy limit rejected them with 413, which no tracer retries. Move the LLM Obs and feature flag EVP routes to their own router with the Datadog Agent's 10 MiB limit. Also assert the exact /info and route strings in tests, add a flush test for the API key and origin headers, and correct comments about tracer behavior. Co-Authored-By: Aaron Silverman <aaron.silverman@datadoghq.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6140981 to
436084d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 436084dc55
ℹ️ 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".
| ], | ||
| // The proxy flusher keeps these two headers. Agentless flag discovery needs | ||
| // both listed before it uses the local route. | ||
| "evp_proxy_allowed_headers": ["DD-EVP-ORIGIN", "DD-EVP-ORIGIN-VERSION"], |
There was a problem hiding this comment.
When a request to either newly advertised EVP route contains additional headers, handle_proxy queues every incoming header and proxy_flusher::create_request removes only host and content-length, so headers such as Authorization, cookies, or other credentials are forwarded to the Datadog intake even though /info declares that only these two headers are allowed. The EVP contract treats this field as a forwarding allowlist; use an Agent-compatible allowlist—including required payload headers such as Content-Type and Content-Encoding—and enforce it on EVP requests before queueing them.
Useful? React with 👍 / 👎.
| post(Self::ffe_flag_evaluation_proxy), | ||
| ) | ||
| .route(FFE_EXPOSURES_ENDPOINT_PATH, post(Self::ffe_exposures_proxy)) | ||
| .layer(RequestBodyLimitLayer::new(EVP_PROXY_REQUEST_BODY_LIMIT)) |
There was a problem hiding this comment.
For a long-running or high-volume invocation using an end-of-invocation flush strategy, several valid roughly 5 MiB LLM/EVP batches can arrive before any flush occurs. This layer now accepts up to 10 MiB per request, but proxy_aggregator::Aggregator stores every body in an unbounded Vec, and handle_proxy always acknowledges it with 200, so a small-memory Lambda can exhaust its memory and terminate before reaching the flush. Add a byte/count bound with backpressure or rejection before increasing and advertising this capacity.
AGENTS.md reference: AGENTS.md:L48-L53
Useful? React with 👍 / 👎.
…ering (#1506) **Jira:** [SVLS-10110](https://datadoghq.atlassian.net/browse/SVLS-10110), a sub-task of [SVLS-10109](https://datadoghq.atlassian.net/browse/SVLS-10109) ## Overview Tracers call `/info` early in their own startup. Lambda starts the runtime, and with it the tracer's init, as soon as every extension has **registered**; the extension's first `/next` only marks its own init as done. The trace agent used to bind its socket much later, after register and most of the extension's setup, so that first `/info` call could be refused. The tracer then skips the Extension for the life of the execution environment. For example, dd-trace-js 6.20 either sends feature flag events directly (if it has an API key) or turns delivery off and retries after 60 seconds. This binds the trace agent's port in `main()` **before the extension registers**. Connections that arrive before the server starts are queued by the kernel and served once it starts. - `TraceAgent::bind_listener(port)` binds the socket through `tokio::net::TcpSocket` (`127.0.0.1`, `SO_REUSEADDR`). - `main()` calls it before register and passes the listener through `extension_loop_active` to `startup::start_trace_agent`, which serves it with `TraceAgent::start_with_listener()`. - Bind failures still log `Error starting trace agent: ...`, and no trace agent is served, as before. `start()` still binds and serves, so the test-mode binary is unchanged. Based on Rey's change on `rey.abolofia/llmobs-info-endpoint` (`33483fd8`), and on Aaron's review finding and patch that moved the bind before register. The `/evp_proxy/v2/` advertisement from Rey's branch ships with the EVP proxy work in #1507 (SVLS-10112). Part of SVLS-10109 (feature flags through the Lambda Extension). ## Likely concerns **Does this slow down cold starts?** No measurable change. 100 cold starts per layer, alternating `main` and this branch: p50 814 vs 812 ms, p90 1065 vs 1041 ms. Details in the comment below. That run bound inside `start_trace_agent`; moving the bind earlier does the same work at a different point. **Does it block the async runtime?** No. Binding a socket on `127.0.0.1` is a local syscall with no network wait. Serving still happens in the spawned task. **What if port 8126 is already taken?** Same as before: the error is logged as `Error starting trace agent: ...`, nothing panics, and the rest of the Extension keeps running. `port_conflict_test` (`trace_agent_port_8126_held`) still passes. **What happens to a request that arrives before the server is ready?** It waits in the kernel queue (backlog 1024, capped by the kernel) until the spawned task starts serving, then gets a normal reply. Before this change, it was refused. **What if the extension falls back to idle mode?** The listener is moved into `extension_loop_active`, so it's dropped if that loop fails, and nothing keeps 8126 open without serving it. **Does it affect Managed Instance mode or the test-mode binary?** Both modes go through the same `main()` path, so both get the fix. The test-mode binary calls `start()`, which binds and serves as before. ## Testing - New lifecycle tests: the fake Extensions API makes one non-retrying connection to 8126 while it handles register, and `trace_agent_listens_before_register` (plus a Managed Instance variant) asserts it was accepted. Both fail on the previous code and on `main`, and pass now. - Unit test: a client can connect right after `bind_listener()`, before the agent serves. - Full `cargo test` (lib and all integration test binaries). - Cold start before/after with custom layers built from `main` and this branch (serverless-simulation-lab): no measurable change (see comment). 🤖 Generated with [Claude Code](https://claude.com/claude-code) [SVLS-10110]: https://datadoghq.atlassian.net/browse/SVLS-10110?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [SVLS-10109]: https://datadoghq.atlassian.net/browse/SVLS-10109?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Aaron Silverman <aaron.silverman@datadoghq.com>
lucaspimentel
left a comment
lucaspimentel
left a comment
There was a problem hiding this comment.
I looked at bottlecap/src/traces/trace_agent.rs only for apm-serverless 😅
Jira: SVLS-10112, a sub-task of SVLS-10109
Overview
Tracers send feature flag evaluations and exposures as Event Platform (EVP) events. They use a local EVP proxy only when
/infoadvertises it. Otherwise they send directly to the intake, which needsDD_API_KEY. The Extension doesn't advertise one today, so with onlyDD_API_KEY_SECRET_ARNthese events are dropped.This change:
/evp_proxy/v2/path in/infoendpoints, plusevp_proxy_allowed_headerswithDD-EVP-ORIGINandDD-EVP-ORIGIN-VERSION. In agentless mode, the flag writers need both before they use the local route. dd-trace-rb uses the fixed v2 paths./evp_proxy/v2/api/v2/flagevaluationand/evp_proxy/v2/api/v2/exposurestoevent-platform-intakethrough the existing store-and-forward proxy (like the LLM Observability routes). The proxy flusher adds the API key.413, which no tracer retries. DSM, profiling, debugger and instrumentation stay at 2 MiB./evp_proxy/v4/unadvertised. Every tracer's flag writer accepts v2, and dd-trace-java's EVP discovery prefers v4 when it's listed, which would move its LLM Obs spans to v4 paths the Extension doesn't serve.DD_API_KEYset). LLM Observability writers don't resend it directly.Based on Aaron Silverman's tested patch from DataDog/datadog-lambda-python#872 (co-author on the commit).
LLM Observability side effect
With
/evp_proxy/v2/advertised, dd-trace-java and dd-trace-go send LLM Observability through the Extension unlessDD_LLMOBS_AGENTLESS_ENABLED=true. Spans and evaluation metrics are relayed (smoke test below). That also fixes Go functions with onlyDD_API_KEY_SECRET_ARN, where LLM Obs couldn't start before.Go functions that use LLM Observability datasets or experiments: set
DD_LLMOBS_AGENTLESS_ENABLED=trueand keepDD_API_KEYandDD_APP_KEYset, so those calls go directly to Datadog. In agent mode, dd-trace-go expects the proxy to add an application key, which neither the Extension nor the current Datadog Agent does (datadog-agent#51580). Tracked as a likely dd-trace-go issue in SVLS-10127.Release notes
DD_API_KEY_SECRET_ARN(or KMS/SSM).event-platform-intake.<DD_SITE>. The end-of-invocation flush now sends flag events there.DD_LLMOBS_AGENTLESS_ENABLED=true, withDD_API_KEYandDD_APP_KEY.Testing
/infotest checks the exact strings:/evp_proxy/v2/with the trailing slash, both flag routes, both allowed headers, and no v4.event-platform-intakepaths with the origin headers intact./evp_proxy/v2/...path returns 404 and queues nothing.evp_proxy_flush_test: the proxy flusher sends a queued flag batch with the API key and the tracer's origin headers.cargo fmt --check,cargo clippy --workspace --all-targets,cargo test --lib, plusapm_integration_test,lifecycle_contract_testandport_conflict_test.DD_API_KEY_SECRET_ARN, dd-trace-js 6.20.1): the tracer chose the local/evp_proxy/v2route (FFE Writer enabled with local EVP route /evp_proxy/v2), the Extension loggedProxied request for ffe_flag_evaluationand thenPROXY_FLUSHER | Successfully sent request ... /api/v2/flagevaluationwith no failures, and the evaluations showed up on the flag's page in Datadog.DD_API_KEYorDD_API_KEY_SECRET_ARN, andDD_LLMOBS_AGENTLESS_ENABLEDunset orfalse:DD_API_KEYwith agentless unset works (directly to the intake). The other three setups never start LLM Obs.Follow-up: SVLS-10128 caps the proxy queue in bytes, since bodies now wait in memory until the next flush and can be up to 10 MiB.
🤖 Generated with Claude Code