refactor(ddtrace/tracer): derive the trace-intake URL from the payload's protocol - #5147
Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 1 commit intoAug 6, 2026
Conversation
4 tasks
Contributor
Config Audit |
This was referenced Aug 5, 2026
darccio
added
the
AI Generated
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 6078606 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-06 13:41:00 Comparing candidate commit 6078606 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.
|
kakkoyun
approved these changes
Aug 6, 2026
…d's protocol httpTransport held a single mutable traceURL that had to be kept in step with the configured protocol. Keeping them in step was the caller's job: newConfig patched traceURL by string-comparing it against the v1 path whenever it downgraded the protocol, which only worked as long as every future protocol decision remembered to do the same. Hold one immutable URL per protocol instead, computed once in newHTTPTransport, and have send() select between them from the payload's own protocol(). A payload is encoded for exactly one protocol and carries that with it, so wire format and destination can no longer diverge — not because a lock protects them, but because there is no shared mutable state left to desynchronize. newConfig's URL patch-up goes away, and resolveTraceURL with it. This also fixes reportAPIErrorsMetric, which passed a hardcoded tracesAPIPath: a failed v1.0 send was reported to datadog.tracer.api.errors tagged endpoint:/v0.4/traces. endpoint() gains a protocol argument for the startup diagnostics probe. It is documented as diagnostics-only — an actual send never consults it, so it cannot reintroduce the mismatch. No behaviour change beyond the metric tag: the protocol downgrade still happens in config, and the writer still reads it when creating a payload.
darccio
force-pushed
the
dario.castane/tp-2-transport-payload-url
branch
from
August 6, 2026 13:05
8ec0d60 to
6078606
Compare
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-mergequeue-cf854d
Bot
merged commit Aug 6, 2026
2ebdd37
into
main
234 of 237 checks passed
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
dario.castane/tp-2-transport-payload-url
branch
August 6, 2026 14:00
darccio
added a commit
that referenced
this pull request
Aug 6, 2026
…merge blockingTransport's endpoint() lost sync with the ddTransport interface change in #5147 (endpoint now takes the trace protocol), which only surfaced once GitHub's PR merge-ref checks combined this branch with current main.
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
that referenced
this pull request
Aug 6, 2026
…ocol (#5148) ## What does this PR do? Third in a split of [#5122](#5122) into a reviewable stack (see that PR for the full sequence). Stacked on [#5147](#5147) — this PR's diff is scoped to just the change below; merge #5147 first. The name `TraceProtocol` read like "the protocol in use", but the value is only what was asked for — it says nothing about whether the trace-agent can actually serve it. Callers have to combine it with agent capability to get the protocol on the wire, and the old name gave no hint of that, which is how the gate in `newConfig` ended up being the only place that knew the difference. Renames to `RequestedTraceProtocol` and documents the distinction. No behaviour change: the OTLP-span-metrics override stays exactly as it was (removed later in this stack, in a sibling PR). Also adds `TraceProtocolVersionString`, the inverse of `resolveTraceProtocol`, and uses it in `SetTraceProtocol`. The env-var path reports `DD_TRACE_AGENT_PROTOCOL_VERSION` through the provider as the raw string (`"1.0"`), while the setter reported a `float64` — the same telemetry key arrived with two different types depending on which source set it last. ### Reviewer's Checklist - [x] Changed code has unit tests for its functionality at or near 100% coverage. - [x] New code doesn't break existing tests. You can check this by running `make test` locally. - [x] New code is free of linting errors. You can check this by running `make lint` locally. - [x] All generated files are up to date. You can check this by running `make generate` locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: dario.castane <dario.castane@datadoghq.com>
This was referenced Aug 7, 2026
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
2 participants
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?
Second in a split of #5122 into a reviewable stack (see that PR for the full sequence). Stacked on #5146 — this PR's diff is scoped to just the change below; merge #5146 first.
httpTransportheld a single mutabletraceURLthat had to be kept in step with the configured protocol. Keeping them in step was the caller's job:newConfigpatchedtraceURLby string-comparing it against the v1 path whenever it downgraded the protocol, which only worked as long as every future protocol decision remembered to do the same.Holds one immutable URL per protocol instead, computed once in
newHTTPTransport, and hassend()select between them from the payload's ownprotocol(). A payload is encoded for exactly one protocol and carries that with it, so wire format and destination can no longer diverge — not because a lock protects them, but because there is no shared mutable state left to desynchronize.newConfig's URL patch-up goes away, andresolveTraceURLwith it.This also fixes
reportAPIErrorsMetric, which passed a hardcodedtracesAPIPath: a failed v1.0 send was reported todatadog.tracer.api.errorstaggedendpoint:/v0.4/traces.endpoint()gains a protocol argument for the startup diagnostics probe. It is documented as diagnostics-only — an actual send never consults it, so it cannot reintroduce the mismatch.No behaviour change beyond the metric tag: the protocol downgrade still happens in config, and the writer still reads it when creating a payload.
Reviewer's Checklist
make testlocally.make lintlocally.make generatelocally.🤖 Generated with Claude Code