Fix OTLP trace metrics attribute gaps in span stats serializer - #8992
Conversation
- status.code: fix protobuf path to emit the OTel string literal (STATUS_CODE_OK/STATUS_CODE_ERROR) instead of an int enum, and make it unconditional in both JSON and protobuf paths (was previously omitted on non-error data points). - is_trace_root: emit datadog.is_trace_root next to datadog.span.top_level, gated behind OTel-semantics suppression like other datadog.-prefixed attributes. - additional_metric_tags: emit each configured key as its own unprefixed data-point attribute, never gated by OTel-semantics. - span.kind: canonicalize to the uppercase OTel Span Metrics Connector convention (SPAN_KIND_SERVER, etc.) via a lookup table. - process_tags/peer_tags: leave a TODO comment; not wired into OTLP output this round. Companion system-tests PR: DataDog/system-tests#7363
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (8992) and master. ✅ No regressions detected |
BenchmarksBenchmark execution time: 2026-08-12 18:22:21 Comparing candidate commit d0cd8d9 in PR branch Found 0 performance improvements and 2 performance regressions! Performance is the same for 70 metrics, 0 unstable metrics, 67 known flaky benchmarks, 59 flaky benchmarks without significant changes.
|
Previously omitted when it matched the resource-level default, inconsistent with status.code and other data-point attributes which are always present.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9586cb676b
ℹ️ 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".
This reverts commit ba86285.
Co-authored-by: Munir Abdinur <munir.abdinur@datadoghq.com>
|
|
||
| private static bool TrySplitEncodedTag(byte[] encodedTag, out string key, out string value) | ||
| { | ||
| var decoded = EncodingHelpers.Utf8NoBom.GetString(encodedTag); |
There was a problem hiding this comment.
Man we lose a lot of performance by having to do a UTF8 decode on the bytes we worked so hard to optimize 😞
@andrewlock do you think we could store StatsBucket.AdditionalMetricTags as a List<Tuple<byte[],byte[]>> so that we can store the key and value separately, each as UTF-8 bytes? That would allow us to write additional metrics in our stats payload as <key_bytes><colon_bytes><value_bytes> and in OTLP as KeyValue: key=<key_bytes>, value=StringValue: string_value=<value_bytes>.
Since this OTLP trace metrics feature is opt-in, perhaps we can do this in a follow-up, but the performance profile of additional metrics tags will be bad until we make further changes
There was a problem hiding this comment.
moved this to draft PR #9002 so we can optimize it separately
There was a problem hiding this comment.
Urgh, yeah, we definitely shouldn't be doing the decode, as that completely defeats the purpose of the optimisation 😅
I'm also not sure if splitting by Tuple<byte[],byte[]> quite works, as it depends on the APIs - we might end up needing to allocate the full array 🤔
Another potential approach would be something like this (not saying exactly this, depends on the APIs, I haven't checked 😅)
public class Utf8EncodedTag(byte[] fullBytes, int keyLength, int valueLength)
{
private readonly byte[] _fullBytes = fullBytes;
private readonly int _keyLength = keyLength;
private readonly int _valueLength = valueLength;
public ReadOnlySpan<byte> KeyAsSpan() => _fullBytes.AsSpan(0, _keyLength);
public ReadOnlySpan<byte> ValueAsSpan() => _fullBytes.AsSpan(_fullBytes.Length - _valueLength - 1, _valueLength);
}Anyway, we can explore in the other PR given this is removed here 🙂
zacharycmontoya
left a comment
There was a problem hiding this comment.
LGTM. Please get a 2nd approval (ideally from the .NET language platform team) before merging
b2caa13 to
d0cd8d9
Compare
Summary
Aligns .NET OTLP trace metrics with the cross-tracer contract in DataDog/system-tests#7466.
DD_TRACE_OTEL_SEMANTICS_ENABLEDwhile retaining that setting for its other consumers.service.name,status.code,span.kind, andspan.name; unknown span kinds default toSPAN_KIND_INTERNAL.datadog.operation.name, emits known trace-root and top-level values as booleans, and emitsdatadog.svc_srcwhen present.Process and peer-tag arrays remain intentionally deferred to #9002.
Validation
OtlpSpanStatsSerializerTests: 59 passeddotnet format --verify-no-changes: passedDatadog.Trace.csproj -f netstandard2.0 --no-restore: passed with no warnings or errorsgit diff --check: passed