fix(ddtrace/tracer): re-evaluate agent v1.0 support at runtime - #5167
Draft
darccio wants to merge 6 commits into
Draft
fix(ddtrace/tracer): re-evaluate agent v1.0 support at runtime#5167darccio wants to merge 6 commits into
darccio wants to merge 6 commits into
Conversation
Contributor
Config Audit |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: d196c45 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-10 08:32:16 Comparing candidate commit d196c45 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.
|
darccio
force-pushed
the
dario.castane/tp-7-runtime-reeval-rebased
branch
from
August 10, 2026 07:55
9055f8b to
dbaf763
Compare
darccio
force-pushed
the
dario.castane/nostalgic-fermat-150b6e
branch
from
August 10, 2026 07:55
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).
…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.
add() pushed straight into h.payload without checking whether an idle payload still matched the agent's currently effective protocol. flush() already rotated an empty stale payload, but nothing calls flush() between an agent-info downgrade and the next trace, so that trace landed in the old-protocol payload and was posted to an endpoint the agent no longer serves. Extract the rotation into rotateStalePayload and call it from add() too.
…hing rotateStalePayload only rotates a payload for free when it is empty. When the agent withdraws /v1.0/traces while the writer already holds buffered traces, those traces were unavoidably lost, but every trace accepted afterwards - until the next scheduled flush - was also encoded into the same stale payload and rejected alongside them. add() now seals a non-empty stale payload with a real flush() before pushing, bounding the loss to what was already buffered.
…seal The previous fix checked for a stale non-empty payload and pushed the new trace under two separate h.mu acquisitions (calling flush() in between). A protocol change landing in that window still let the push reach the old payload despite the check having just passed. Merges the mismatch check, the payload swap, and the push into one critical section; the async send is now handed off via the new sendAsync helper (extracted from flush) after the lock is released, preserving the existing non-blocking-climit behavior.
darccio
force-pushed
the
dario.castane/nostalgic-fermat-150b6e
branch
from
August 10, 2026 08:01
3a4417a to
ea71066
Compare
darccio
force-pushed
the
dario.castane/tp-7-runtime-reeval-rebased
branch
from
August 10, 2026 08:01
dbaf763 to
d196c45
Compare
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.
What does this PR do?
This restores the runtime Agent-capability work as a focused follow-up to #5161. It is intentionally not required to release the CSS/trace-protocol decoupling in #5161.
The Datadog Agent's
/v1.0/tracesendpoint is a startup-time capability, but a long-lived tracer can be routed to a different Agent process after an upgrade, rollback, or load-balancer transition. This PR refreshes that capability from/info:The payload-insertion and capability-update synchronization is deliberately contained here for independent review.
Validation
go test ./ddtrace/tracer -run 'TraceProtocol|PayloadRotates|UsesAgentTraceWriter|PinTestTracerToV04' -count=1go test -race ./ddtrace/tracer -run '^TestConcurrentProtocolChangeDuringFlush$' -count=3