fix(trace-stats): read OTel HTTP names for the status and method dimensions - #2323
fix(trace-stats): read OTel HTTP names for the status and method dimensions#2323link04 wants to merge 1 commit into
Conversation
BenchmarksComparisonBenchmark execution time: 2026-08-25 02:32:48 Comparing candidate commit 6219f24 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 73 metrics, 10 unstable metrics.
|
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
|
a142769 to
a8e29fd
Compare
📚 Documentation Check Results📦
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 6219f24 | Docs | View more details | Give us feedback! |
🔒 Cargo Deny Results📦
|
ichinaski
left a comment
There was a problem hiding this comment.
Changes regarding the OTel tags for method and status_code extraction look good.
Read the OTel HTTP names when computing trace-metric dimensions, so stats keep working once the Datadog names are replaced, and fall through to the OTel name rather than dropping the dimension when a value cannot be parsed. Emit status.code on every trace-metric data point, STATUS_CODE_OK on success and STATUS_CODE_ERROR otherwise. It is a dimension of the metric rather than a span status, so omitting it on success gives successful and error cells different attribute sets and loses the OK series for anything grouping by it. Spans are untouched, so a successful span still exports with status UNSET. Propagate the chunk's sampling decision to the OTLP sampled flag, matching the policy the Datadog path already applies, so a child without its own sampling priority still carries the chunk-level decision. Use Agent-compatible HTTP aliases and reject malformed status values so OTLP trace metrics keep valid dimensions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9758e56 to
6219f24
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6219f24678
ℹ️ 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".
| } | ||
|
|
||
| fn float_to_u32(f: f64) -> Option<u32> { | ||
| if !f.is_finite() || f.floor() != f || f < 0.0 || (u32::MAX as f64) < f { |
There was a problem hiding this comment.
Reject out-of-range HTTP status aliases
When the preferred Datadog alias contains an integral but invalid HTTP code such as 0 or 600, while http.response.status_code contains a valid fallback such as 200, this check accepts the invalid value and get_http_status_code stops searching. The resulting trace metric therefore records an invalid dimension instead of the valid OTel value; apply the same 100..600 validity range used by libdd-trace-normalization/src/normalizer.rs::is_valid_http_status_code to both metric and metadata inputs.
Useful? React with 👍 / 👎.
| if !f.is_finite() || f.floor() != f || f < 0.0 || (u32::MAX as f64) < f { | ||
| return None; | ||
| } | ||
| Some(f as u32) |
There was a problem hiding this comment.
Justify the guarded float-to-integer cast
This new helper uses an as u32 numeric conversion without documenting why the preceding finite, integral, and range checks make it lossless. Add the required short justification beside the cast (or use a non-as conversion approach) so future changes do not accidentally weaken those safety conditions.
AGENTS.md reference: AGENTS.md:L74-L74
Useful? React with 👍 / 👎.
| let priority = chunk | ||
| .iter() | ||
| .find_map(|span| span.metrics.get("_sampling_priority_v1")); | ||
| priority.is_none_or(|priority| *priority > 0.0) as u32 |
There was a problem hiding this comment.
Preserve rejected priority for single-span exports
When a rejected trace stores _sampling_priority_v1 <= 0 only on a root span but a child is retained by single-span sampling or the analyzed-span mechanism, drop_chunks removes the root before this mapper runs. The remaining child has no priority, so this default marks it sampled even though the trace-level decision was rejection; preserve the original chunk decision through filtering rather than interpreting the removed metric as an absent decision.
Useful? React with 👍 / 👎.
What does this PR do?
Preserves OpenTelemetry HTTP semantics in libdatadog's in-process consumers and OTLP output.
Trace stats and OTLP trace metrics
http.request.methodandhttp.response.status_codestatus.codeunset and prevents additional tags from overriding the computed statusHTTP tag handling
libdatadog does not rename tracer span tags in this PR. It accepts the Datadog and OpenTelemetry names below as equivalent inputs to trace-stat aggregation and emits the canonical OpenTelemetry name in OTLP trace metrics.
http.methodhttp.request.methodhttp.request.methodhttp.status_codehttp.response.status_codehttp.response.status_codeas an integerstatus.code=STATUS_CODE_ERRORfor errors; omitted for successful spansWhen both naming variants are present, the Datadog name wins. Empty or malformed values fall through to the alternate name instead of hiding a valid value.
OTLP sampled flags
Sampling priority is a trace-level decision, but v0.4/v0.5 inputs may attach
_sampling_priority_v1to only one span. The mapper now uses the same first-priority policy asdrop_chunks, treats a missing priority as sampled, and applies one decision to every span in the chunk.This prevents sampled traces from exporting child spans with
flags=0and keeps OTLP flags consistent with the existing chunk-retention policy.No public signature, wire-format, setting, or environment-variable change is introduced.
Motivation
Under
DD_TRACE_OTEL_SEMANTICS_ENABLED, spans use OTel HTTP names before client-side stats are computed. Stats and OTLP metrics must preserve those dimensions, and all spans in an exported trace must carry the same sampled decision.Companion changes:
Testing
cargo fmt --check -p libdd-trace-utilsEnd-to-end with the companion tracer and system-test changes:
OTEL_SEMANTICS_OTLP_TRACE_METRICSOTEL_SEMANTICS_OTLP_SAMPLING_RULES