fix(trace-stats): read OTel HTTP names for the status and method dimensions - #2323
Draft
link04 wants to merge 3 commits into
Draft
fix(trace-stats): read OTel HTTP names for the status and method dimensions#2323link04 wants to merge 3 commits into
link04 wants to merge 3 commits into
Conversation
…nsions The stats aggregation key reads http.status_code and http.method only. A tracer running with DD_TRACE_OTEL_SEMANTICS_ENABLED emits http.response.status_code and http.request.method instead, so both dimensions silently collapse: HTTPStatusCode becomes 0 and HTTPMethod becomes empty, for every HTTP span, with no error anywhere. The span still reports its status correctly, so the span and the stats disagree and nothing at span level can see it. Both names are now read, Datadog first so a span using Datadog naming still costs a single lookup and a tracer emitting both during a migration keeps the dimension it already reports. The status code is looked up in metrics as well as meta under the OTel name, because OTel types it as an int and the v04 encoder routes int attributes to metrics. http.endpoint and http.route need no equivalent. http.route is already the OTel name, and http.endpoint is Datadog-only and deliberately retained in that mode. Not gated on the otel_trace_semantics_enabled flag in libdd-data-pipeline: that flag is not plumbed into libdd-trace-stats, and threading it in would mean a breaking change to SpanConcentrator::new. The two names cannot legitimately disagree, so reading both unconditionally is safe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BenchmarksComparisonBenchmark execution time: 2026-08-06 01:11:31 Comparing candidate commit a142769 in PR branch Found 9 performance improvements and 33 performance regressions! Performance is the same for 98 metrics, 0 unstable metrics.
|
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
Review follow-up on the two lookups added in the previous commit. An empty or unparseable value under the Datadog name terminated the search at the default instead of falling through, so meta["http.status_code"] = "" next to a valid http.response.status_code produced a bucket with status 0, which is the same span/stats disagreement this branch set out to fix. Same for an empty http.method shadowing http.request.method, because or_else only fires on None. Both lookups now skip a value that is empty or does not parse, matching what get_grpc_status_code in the same file already does. Also drops the claim that the v04 encoder is what routes the OTel status code into metrics. The concentrator runs before serialization, so the split is whatever the tracer's own setter did. Three cases added: Datadog name in meta against OTel name in metrics, an unparseable Datadog status with a valid OTel one, and an empty Datadog method. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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?
Makes the client-side stats aggregation key read the OpenTelemetry HTTP attribute names in addition to the Datadog ones, for the two dimensions where the names differ:
HTTPStatusCodehttp.status_codehttp.response.status_codeHTTPMethodhttp.methodhttp.request.methodOne function,
BorrowedAggregationKey::from_obfuscated_spaninlibdd-trace-stats/src/span_concentrator/aggregation.rs. No struct, signature, or wire changes, so the/v0.6/statspayload stays byte-compatible.Motivation
The tracers are adding an OTel semantics mode behind
DD_TRACE_OTEL_SEMANTICS_ENABLEDwhich renames HTTP span attributes to the OTel semantic-convention names. Under that flag the aggregation key finds neitherhttp.status_codenorhttp.method, soHTTPStatusCodebecomes0andHTTPMethodbecomes empty for every HTTP span, with no error anywhere.That failure mode is invisible from the span side. The span reports the right status, the stats bucket reports
0, and no span-level assertion can detect the disagreement. Measured on a Django app with the flag on: the resource, hits and errors were all correct andHTTPStatusCodewas0.http.endpointandhttp.routeneed no equivalent.http.routeis already the OTel name, andhttp.endpointis Datadog-only and deliberately retained under the flag.Design notes
metricsas well asmetaunder the OTel name. OTel typeshttp.response.status_codeas an int, andlibdd-trace-utils/src/msgpack_encoder/v04/span_v1.rsroutes int attributes tometrics. A meta-only lookup would miss the common case.otel_trace_semantics_enabled, deliberately rather than for cost reasons. Gating is cheap:SpanConcentrator::newhas two production call sites, both inlibdd-data-pipelinewhere that flag already lives. The reason not to is that this function already reads two naming conventions for a dimension and does it unconditionally:grpc_status_codesweepsrpc.grpc.status_code(OTel) alongsidegrpc.codeandgrpc.status.code(Datadog), added on purpose in feat(trace-stats): add grpc status code in the stats bucket key #1701, andhttp_endpointfalls back tohttp.route, which is the same key in both conventions. Keying on span content rather than on a tracer-side flag is the established pattern here, and it is also the more robust one for a library that several tracers feed.One thing to decide before this merges
The
http.methodfallback changes stats for spans that already carry the OTel name without the flag, and that case is not hypothetical. Both OTel API bridges mirrorhttp.response.status_codeontohttp.status_codeand leave the method alone:dd-trace-py/ddtrace/internal/opentelemetry/span.pymaps four keys and the method is not one of them, anddd-trace-js/packages/dd-trace/src/opentelemetry/span-helpers.jsmirrors only the status code.So an app on
ddtrace.opentelemetry.TracerProviderwith OTel HTTP instrumentation and the flag unset aggregates into one bucket with an emptyHTTPMethodtoday, and would split into per-method buckets after this change. I think populating the dimension is the correct behavior and the split is a fix rather than a regression, but it is a visible change to existing series and it can add key multiplicity againstDEFAULT_MAX_ENTRIES_PER_BUCKET, so it should be an explicit call by someone who owns this data rather than a side effect of an OTel-semantics PR.The status-code fallback does not have this exposure, precisely because the bridges already mirror it. If the method change is unwanted, dropping it leaves
HTTPMethodempty under the flag and the rest of the PR stands.Adjacent gaps, not addressed here
Flagging rather than fixing, since each is a separate decision:
libdd-trace-normalization/src/normalizer.rsvalidates and strips only the Datadog status key, so a span carrying onlyhttp.response.status_codeskips status-code normalization.libdd-sampling/src/v04_span.rsreads only the Datadog status key and returnsNonefromget_alternate_key, with the comment "v04 spans use Datadog naming conventions natively", which the flag makes false. Concretely: with the flag on,DD_TRACE_SAMPLING_RULES=[{"tags":{"http.status_code":"5??"}}]stops matching on the value lookup and{"tags":{"http.method":"GET"}}stops matching on the key aliasing, so the same flag gives correct stats and silently broken sampling.Testing
Four cases added to the table-driven
test_aggregation_key_from_span: OTel status inmetrics, OTel status inmeta, both names present asserting Datadog precedence, and OTel method withhttp.route.cargo test -p libdd-trace-statspasses 34 tests.cargo clippy -p libdd-trace-stats --all-targetsis clean.Measured against dd-trace-py running a Django app with
DD_TRACE_OTEL_SEMANTICS_ENABLED=true, reading the tracer's own/v0.6/statspayload rather than the agent's forwarded one:Resource, hits, errors, span kind and trace-root are all correct; the two dimensions this PR touches are the only ones lost, which is why the method fallback is here alongside the status code.
With this branch compiled into that same tracer, the same request pattern:
The two system-tests cases that compare the span's error decision against the stats bucket (
Test_OtelSemantics_Stats_Consistency) go from failing to passing on that change alone.🤖 Generated with Claude Code