fix(ddtrace/tracer): decouple trace protocol v1.0 from client-side stats - #5161
Conversation
5e7ee42 to
e6d89d5
Compare
Config Audit |
![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: 5e7ee42630
ℹ️ 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".
darccio
added
the
AI Generated
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: ea71066 | Docs | Datadog PR Page | Give us feedback! |
e6d89d5 to
4e26e73
Compare
…tale v1 protocol startTestTracer's v1-capability override ran after newTracer had already built the writer's initial payload. On a developer machine where a real Agent at the default address advertises v1, that payload had already latched onto v1 before the override ran, and overriding config alone does not retroactively change an already-built payload — only an empty-payload flush re-reads the effective protocol. Without one, the first real trace in such an environment would still encode as v1 despite the override's intent to force v0.4, exactly the kind of dev-machine-dependent flake this override exists to prevent (see BenchmarkPartialFlushing's prior allocated_mem regression from the same root cause). Extract the override into pinTestTracerToV04, which now flushes the writer immediately after applying the override so an idle (still-empty) payload rotates. Flagged by Codex review on #5161.
BenchmarksBenchmark execution time: 2026-08-10 08:30:13 Comparing candidate commit ea71066 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 326 metrics, 0 unstable metrics, 1 flaky benchmarks without significant changes.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 669febce01
ℹ️ 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".
1112811 to
eae4d5c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eae4d5c09a
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71d891f185
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dd92c82fc
ℹ️ 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".
2dd92c8 to
aa0fdae
Compare
aa0fdae to
3a4417a
Compare
The v1.0 protocol was gated on client-side stats being available
(!canComputeStats(), commented "v1 requires CSS"), but that isn't true:
- The Datadog Agent has accepted v1.0 payloads without CSS since the
format's first release; its handler computes stats server-side when
ClientComputedStats is false, exactly as it does for v0.4.
- CSS is negotiated out-of-band, via the Datadog-Client-Computed-Stats
header and the separate /v0.6/stats endpoint. The trace-send path is
otherwise protocol-blind.
- The v1 payload encoder reads no config and no agent capability state.
- The original ETP RFC never mentions stats or CSS; its only gating rule
is agent endpoint availability.
No commit, PR or doc states a rationale for the coupling — it arrived as
an unexplained one-line addition to a test-enablement PR. The silent
consequence: DD_TRACE_STATS_COMPUTATION_ENABLED=false (or disabling the
Agent's client_drop_p0s, e.g. via its probabilistic sampler) downgraded
the wire protocol with no log, even though 1.0 is the tracer's default.
Replace the startup gate with (*config).effectiveTraceProtocol, a pure
function of (requested protocol, agent v1 availability) evaluated at point
of use. Nothing is downgraded in config and nothing is baked into a
component, so the requested value stays inspectable and the effective one
can't drift from it. Config telemetry now reports the effective protocol
via ReportEffectiveTraceProtocol, which dedupes so repeated evaluation
cannot inflate seqIDs.
TestNewConfigKeepsV1WhenCSSDisabled pins this end-to-end at the wire
level: with CSS off and the agent advertising /v1.0/traces, the request
must land on /v1.0/traces with a msgpack map body. Before this change it
went to /v0.4/traces with an array body (0x91).
3a4417a to
ea71066
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: removed
and removed
mergequeue-status: in_progress
labels
2 participants
What does this PR do?
This is the decoupling layer of the restored review stack:
Only this PR is required for the decoupling release. It retains the established startup capability decision: if the Agent advertises
/v1.0/traceswhen the tracer starts, disabling client-side stats no longer forces the tracer down to v0.4. If the Agent does not advertise v1 at startup, the tracer still uses v0.4.The runtime refresh is deliberately separate. A running Agent does not dynamically toggle its trace intake endpoints; a changed capability answer requires reaching a different Agent process, such as during an upgrade, rollback, or load-balanced routing transition. That hardening is useful, but it is not a functional prerequisite for decoupling CSS from trace encoding.
Why this change
Client-side stats are negotiated separately through the
Datadog-Client-Computed-Statsheader and/v0.6/stats. The Agent accepts v1 payloads whether or not the client computed stats, so CSS must not determine the trace wire format.Validation
go test ./ddtrace/tracer -run "^(TestNewConfigKeepsV1WhenCSSDisabled|TestTraceProtocolDecoupling|TestWithStatsComputation)$" -count=1git diff --checkThe former combined branch head is retained at
dario.castane/decouple-stack-pre-split-2dd92c82for recovery.