diff --git a/tracer/src/Datadog.Trace/OtelTraceState.cs b/tracer/src/Datadog.Trace/OtelTraceState.cs new file mode 100644 index 000000000000..461d2a60aa01 --- /dev/null +++ b/tracer/src/Datadog.Trace/OtelTraceState.cs @@ -0,0 +1,107 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; +using Datadog.Trace.Propagators; + +namespace Datadog.Trace; + +internal sealed class OtelTraceState +{ + internal OtelTraceState(string? headerString) + { + CachedHeaderString = headerString; + } + + /// + /// Initializes a new instance of the class holding the same values as + /// . A single extracted can start more than one + /// trace, and mutates this object in place, so each + /// must take its own copy rather than alias the extracted one. + /// The copy is inherited state by definition, so + /// deliberately starts out false. + /// + internal OtelTraceState(OtelTraceState other) + { + CachedHeaderString = other.CachedHeaderString; + RandomValue = other.RandomValue; + Threshold = other.Threshold; + IsModified = other.IsModified; + } + + public ulong? Threshold { get; set; } + + public ulong? RandomValue { get; set; } + + public string? CachedHeaderString { get; } + + public bool IsModified { get; set; } + + public bool LocallyGeneratedOtelRandomValue { get; set; } + + /// + /// Converts the original header into a TraceState object and stores valid "rv" and "th" items + /// into their first-class properties. + /// Unknown items remain present in the cached string. + /// Returns null when is null or empty, to avoid allocating for the common + /// case where the "ot" tracestate member is absent. + /// + internal static OtelTraceState? Parse(string? raw) + { + if (StringUtil.IsNullOrEmpty(raw)) + { + return null; + } + + var traceState = new OtelTraceState(raw); + var remaining = raw!.AsSpan(); + + while (true) + { + var separatorIndex = remaining.IndexOf(';'); + var item = separatorIndex < 0 ? remaining : remaining.Slice(0, separatorIndex); + var colonIndex = item.IndexOf(':'); + var key = colonIndex > 0 ? item.Slice(0, colonIndex) : item; + var value = colonIndex > 0 ? item.Slice(colonIndex + 1) : default; + + if (key.Equals("rv".AsSpan(), StringComparison.Ordinal)) + { + if (value.Length != OtelTraceStateHelpers.MaxHexDigits || !OtelTraceStateHelpers.TryParseLowercaseHex(value, out ulong randomValue)) + { + // Do not store the randomValue + // Instead, keep it null and report that the object is modified from the original header + traceState.IsModified = true; + } + else + { + traceState.RandomValue = randomValue; + } + } + + if (key.Equals("th".AsSpan(), StringComparison.Ordinal)) + { + if (value.Length is < 1 or > OtelTraceStateHelpers.MaxHexDigits || !OtelTraceStateHelpers.TryParseLowercaseHex(value, out ulong threshold)) + { + // Do not store the threshold + // Instead, keep it null and report that the object is modified from the original header + traceState.IsModified = true; + } + else + { + traceState.Threshold = threshold; + } + } + + if (separatorIndex < 0) + { + return traceState; + } + + remaining = remaining.Slice(separatorIndex + 1); + } + } +} diff --git a/tracer/src/Datadog.Trace/Propagators/OtelTraceStateHelpers.cs b/tracer/src/Datadog.Trace/Propagators/OtelTraceStateHelpers.cs index 5a9822aac01a..25fa95228415 100644 --- a/tracer/src/Datadog.Trace/Propagators/OtelTraceStateHelpers.cs +++ b/tracer/src/Datadog.Trace/Propagators/OtelTraceStateHelpers.cs @@ -20,8 +20,8 @@ namespace Datadog.Trace.Propagators /// internal static class OtelTraceStateHelpers { - private const int MaxHexDigits = 14; - private const ulong MaxOtelTraceStateValue = (1UL << (MaxHexDigits * 4)) - 1; + internal const int MaxHexDigits = 14; + internal const ulong MaxOtelTraceStateValue = (1UL << (MaxHexDigits * 4)) - 1; /// /// Finds the "rv" item in the raw "ot=" value (items separated by ';', key/value by ':') @@ -60,38 +60,6 @@ internal static class OtelTraceStateHelpers return null; } - /// - /// Removes malformed "rv" and "th" items while preserving valid and unknown items. - /// Returns the original string when no rewrite is needed, and null when nothing remains. - /// - internal static string? Normalize(string? raw) - { - if (StringUtil.IsNullOrEmpty(raw)) - { - return null; - } - - var remaining = raw!.AsSpan(); - - while (true) - { - var separatorIndex = remaining.IndexOf(';'); - var item = separatorIndex < 0 ? remaining : remaining.Slice(0, separatorIndex); - - if (IsInvalidRvOrTh(item)) - { - return RemoveInvalidRvTh(raw); - } - - if (separatorIndex < 0) - { - return raw; - } - - remaining = remaining.Slice(separatorIndex + 1); - } - } - /// /// Drops any existing "rv"/"th" items from (whether well-formed /// or not), then emits "rv:<14-hex-digits>" (if is non-null) @@ -99,71 +67,64 @@ internal static class OtelTraceStateHelpers /// is non-null), followed by every other item from in its original /// order. Returns null when nothing is left to emit. /// - internal static string? SetRvTh(string? raw, ulong? rv, ulong? th) + internal static void SetRvTh(StringBuilder sb, string? raw, ulong? rv, ulong? th) { if (rv is > MaxOtelTraceStateValue) { throw new ArgumentOutOfRangeException(nameof(rv)); } - var sb = StringBuilderCache.Acquire(); + // "sb" already holds the preceding tracestate members (e.g. "dd=...,ot="), so item + // separators must be relative to where this member's content starts, not to the whole builder. + var startLength = sb.Length; + + if (rv is { } rvValue) + { + AppendRandomValueHex(sb, rvValue); + } - try + if (th is { } thValue) { - if (rv is { } rvValue) + if (sb.Length > startLength) { - AppendRandomValueHex(sb, rvValue); + sb.Append(';'); } - if (th is { } thValue) - { - if (sb.Length > 0) - { - sb.Append(';'); - } + AppendThresholdHex(sb, thValue); + } - AppendThresholdHex(sb, thValue); - } + if (!StringUtil.IsNullOrEmpty(raw)) + { + var remaining = raw!.AsSpan(); - if (!StringUtil.IsNullOrEmpty(raw)) + while (true) { - var remaining = raw!.AsSpan(); + var separatorIndex = remaining.IndexOf(';'); + var item = separatorIndex < 0 ? remaining : remaining.Slice(0, separatorIndex); + var colonIndex = item.IndexOf(':'); + var key = colonIndex > 0 ? item.Slice(0, colonIndex) : item; - while (true) + if (!key.Equals("rv".AsSpan(), StringComparison.Ordinal) && !key.Equals("th".AsSpan(), StringComparison.Ordinal)) { - var separatorIndex = remaining.IndexOf(';'); - var item = separatorIndex < 0 ? remaining : remaining.Slice(0, separatorIndex); - var colonIndex = item.IndexOf(':'); - var key = colonIndex > 0 ? item.Slice(0, colonIndex) : item; - - if (!key.Equals("rv".AsSpan(), StringComparison.Ordinal) && !key.Equals("th".AsSpan(), StringComparison.Ordinal)) + if (sb.Length > startLength) { - if (sb.Length > 0) - { - sb.Append(';'); - } - - sb.Append(item); + sb.Append(';'); } - if (separatorIndex < 0) - { - break; - } + sb.Append(item); + } - remaining = remaining.Slice(separatorIndex + 1); + if (separatorIndex < 0) + { + break; } - } - return sb.Length == 0 ? null : sb.ToString(); - } - finally - { - StringBuilderCache.Release(sb); + remaining = remaining.Slice(separatorIndex + 1); + } } } - private static void AppendRandomValueHex(StringBuilder sb, ulong rv) + internal static void AppendRandomValueHex(StringBuilder sb, ulong rv) { sb.Append("rv:"); #if NETCOREAPP3_1_OR_GREATER @@ -175,7 +136,7 @@ private static void AppendRandomValueHex(StringBuilder sb, ulong rv) #endif } - private static void AppendThresholdHex(StringBuilder sb, ulong th) + internal static void AppendThresholdHex(StringBuilder sb, ulong th) { // Format as 14 hex digits, then trim trailing zero nibbles. // A fully-zero threshold trims to the empty string; represent it as a single "0". @@ -200,66 +161,7 @@ private static void AppendThresholdHex(StringBuilder sb, ulong th) #endif } - private static string? RemoveInvalidRvTh(string raw) - { - var sb = StringBuilderCache.Acquire(); - - try - { - var remaining = raw.AsSpan(); - - while (true) - { - var separatorIndex = remaining.IndexOf(';'); - var item = separatorIndex < 0 ? remaining : remaining.Slice(0, separatorIndex); - - if (!IsInvalidRvOrTh(item)) - { - if (sb.Length > 0) - { - sb.Append(';'); - } - - sb.Append(item); - } - - if (separatorIndex < 0) - { - break; - } - - remaining = remaining.Slice(separatorIndex + 1); - } - - return sb.Length == 0 ? null : sb.ToString(); - } - finally - { - StringBuilderCache.Release(sb); - } - } - - private static bool IsInvalidRvOrTh(ReadOnlySpan item) - { - var colonIndex = item.IndexOf(':'); - var key = colonIndex > 0 ? item.Slice(0, colonIndex) : item; - - if (key.Equals("rv".AsSpan(), StringComparison.Ordinal)) - { - var value = colonIndex > 0 ? item.Slice(colonIndex + 1) : default; - return value.Length != MaxHexDigits || !TryParseLowercaseHex(value, out _); - } - - if (key.Equals("th".AsSpan(), StringComparison.Ordinal)) - { - var value = colonIndex > 0 ? item.Slice(colonIndex + 1) : default; - return value.Length is < 1 or > MaxHexDigits || !TryParseLowercaseHex(value, out _); - } - - return false; - } - - private static bool TryParseLowercaseHex(ReadOnlySpan value, out ulong result) + internal static bool TryParseLowercaseHex(ReadOnlySpan value, out ulong result) { result = 0; foreach (var character in value) diff --git a/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs b/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs index 51a46d737e00..a3c47ee009f6 100644 --- a/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs +++ b/tracer/src/Datadog.Trace/Propagators/W3CTraceContextPropagator.cs @@ -201,38 +201,38 @@ internal static string CreateTraceStateHeader(SpanContext context) var otelTraceState = context.OtelTraceState; var additionalTraceState = context.AdditionalW3CTraceState; - ExtractMember( - additionalTraceState.AsSpan(), - "ot=", - out var originalOtelTraceState, - out var precedingMembers, - out var succeedingMembers, - out var hasOriginalOtelTraceState); - - var hasOtelTraceState = !string.IsNullOrWhiteSpace(otelTraceState); - var preserveOriginalOtelTraceState = hasOriginalOtelTraceState && - hasOtelTraceState && - originalOtelTraceState.Equals(otelTraceState.AsSpan(), StringComparison.Ordinal); - - if (preserveOriginalOtelTraceState) - { - AppendTraceStateMembers(sb, additionalTraceState.AsSpan()); - } - else + + if (otelTraceState is { IsModified: true }) { - if (hasOtelTraceState) + ExtractMember( + additionalTraceState.AsSpan(), + "ot=", + out _, + out var precedingMembers, + out var succeedingMembers, + out _); + + // Save the position prior to emitting the "ot" so we can "rewind" the length + // in case the actual contents are empty. + var rewindPosition = sb.Length; + + sb.Append(TraceStateHeaderValuesSeparator); + sb.Append("ot="); + + var otPosition = sb.Length; + OtelTraceStateHelpers.SetRvTh(sb, otelTraceState.CachedHeaderString, otelTraceState.RandomValue, otelTraceState.Threshold); + if (sb.Length == otPosition) { - if (sb.Length > 0) - { - sb.Append(TraceStateHeaderValuesSeparator); - } - - sb.Append("ot=").Append(otelTraceState); + sb.Length = rewindPosition; } AppendTraceStateMembers(sb, precedingMembers); AppendTraceStateMembers(sb, succeedingMembers); } + else + { + AppendTraceStateMembers(sb, additionalTraceState.AsSpan()); + } return StringBuilderCache.GetStringAndRelease(sb); } @@ -604,7 +604,7 @@ public bool TryExtract( spanContext.PropagatedTags = traceTags; spanContext.AdditionalW3CTraceState = traceState.AdditionalValues; - spanContext.OtelTraceState = OtelTraceStateHelpers.Normalize(traceState.OtTraceState); + spanContext.OtelTraceState = OtelTraceState.Parse(traceState.OtTraceState); spanContext.LastParentId = traceState.LastParent; context = new PropagationContext(spanContext, baggage: null); diff --git a/tracer/src/Datadog.Trace/SpanContext.cs b/tracer/src/Datadog.Trace/SpanContext.cs index 00d3c2a41e06..7b2396b59dca 100644 --- a/tracer/src/Datadog.Trace/SpanContext.cs +++ b/tracer/src/Datadog.Trace/SpanContext.cs @@ -296,7 +296,7 @@ internal string AdditionalW3CTraceState /// [MaybeNull] [AllowNull] - internal string OtelTraceState + internal OtelTraceState OtelTraceState { get => TraceContext?.OtelTraceState ?? _remoteW3CTraceState?.OtelTraceState; set @@ -547,7 +547,7 @@ private sealed class RemoteW3CTraceState { public string AdditionalW3CTraceState { get; set; } - public string OtelTraceState { get; set; } + public OtelTraceState OtelTraceState { get; set; } } internal static class Keys diff --git a/tracer/src/Datadog.Trace/TraceContext.cs b/tracer/src/Datadog.Trace/TraceContext.cs index d594b001e1ac..edfdd9dad36d 100644 --- a/tracer/src/Datadog.Trace/TraceContext.cs +++ b/tracer/src/Datadog.Trace/TraceContext.cs @@ -41,8 +41,7 @@ internal sealed class TraceContext private IastRequestContext? _iastRequestContext; private AppSecRequestContext? _appSecRequestContext; - private string? _otelTraceState; - private bool _containsLocallyGeneratedOtelRandomValue; + private OtelTraceState? _otelTraceState; // Lazily created on the first feature-flag evaluation for this trace; null until then, so // traces that never evaluate a flag pay nothing. State dies with the TraceContext. @@ -125,14 +124,16 @@ public Span? RootSpan /// for the only code that inspects /// or rewrites its "rv"/"th" sub-keys. /// - internal string? OtelTraceState + internal OtelTraceState? OtelTraceState { get => _otelTraceState; - set - { - _otelTraceState = value; - _containsLocallyGeneratedOtelRandomValue = false; - } + + // Store a copy, never the caller's instance: the value normally comes from an extracted + // SpanContext that may start several traces, and the sampling decision below mutates this + // object in place. Aliasing it would let one trace's override rewrite a sibling trace's + // "ot=" member. The copy constructor also resets LocallyGeneratedOtelRandomValue, since + // anything assigned through here arrived from outside this trace. + set => _otelTraceState = value is null ? null : new OtelTraceState(value); } /// Gets the IAST context @@ -432,24 +433,36 @@ or Sampling.SamplingMechanism.RemoteUserSamplingRule rv = th > 0 ? th - 1 : 0; } + _otelTraceState ??= new(headerString: null); + _otelTraceState.IsModified = true; + var rateLimiterRejected = didSample && SamplingPriorityValues.IsDrop(p); if (rateLimiterRejected) { - var inheritedRv = _containsLocallyGeneratedOtelRandomValue ? null : OtelTraceStateHelpers.ExtractRv(_otelTraceState); - _otelTraceState = OtelTraceStateHelpers.SetRvTh(_otelTraceState, inheritedRv ?? rv, th: null); - _containsLocallyGeneratedOtelRandomValue = inheritedRv is null; + var inheritedRv = _otelTraceState.LocallyGeneratedOtelRandomValue ? null : OtelTraceStateHelpers.ExtractRv(_otelTraceState.CachedHeaderString); + + _otelTraceState.RandomValue = inheritedRv ?? rv; + _otelTraceState.Threshold = null; + _otelTraceState.LocallyGeneratedOtelRandomValue = inheritedRv is null; } else { - _otelTraceState = OtelTraceStateHelpers.SetRvTh(_otelTraceState, rv, th); - _containsLocallyGeneratedOtelRandomValue = true; + _otelTraceState.RandomValue = rv; + _otelTraceState.Threshold = th; + _otelTraceState.LocallyGeneratedOtelRandomValue = true; } } } else if (mechanism is Sampling.SamplingMechanism.Manual or Sampling.SamplingMechanism.Asm) { - var inheritedRv = _containsLocallyGeneratedOtelRandomValue ? null : OtelTraceStateHelpers.ExtractRv(_otelTraceState); - OtelTraceState = OtelTraceStateHelpers.SetRvTh(_otelTraceState, inheritedRv, th: null); + // Only rewrite an "ot=" state that already exists + // If none exists, then there's no need to allocate a new object only to set its properties to null + if (_otelTraceState is { } otelTraceState) + { + otelTraceState.IsModified = true; + otelTraceState.RandomValue = otelTraceState.LocallyGeneratedOtelRandomValue ? null : OtelTraceStateHelpers.ExtractRv(otelTraceState.CachedHeaderString); + otelTraceState.Threshold = null; + } } if (notifyDistributedTracer) diff --git a/tracer/test/Datadog.Trace.Tests/Propagators/MultiSpanContextPropagatorTests.cs b/tracer/test/Datadog.Trace.Tests/Propagators/MultiSpanContextPropagatorTests.cs index 8c17db09692a..a26a24bbf2f2 100644 --- a/tracer/test/Datadog.Trace.Tests/Propagators/MultiSpanContextPropagatorTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Propagators/MultiSpanContextPropagatorTests.cs @@ -482,7 +482,7 @@ public void Extract_Behavior_Restart_DoesNotCarryOverOtelTraceState() var result = restartPropagator.Extract(headers.Object); result.SpanContext.Should().BeNull(); - result.Links.Should().ContainSingle().Which.Context.OtelTraceState.Should().Be("rv:ef284ace7a91e1;th:e6666666666668"); + result.Links.Should().ContainSingle().Which.Context.OtelTraceState.CachedHeaderString.Should().Be("rv:ef284ace7a91e1;th:e6666666666668"); } [Fact] @@ -809,7 +809,7 @@ public void TraceContextPrecedence_Respected_WhenHavingMatchingTraceIds(bool ext }, opts => opts.ExcludingMissingMembers()); - result.SpanContext!.OtelTraceState.Should().Be(!extractFirst || w3CHeaderFirst ? "rv:ef284ace7a91e1;th:e6666666666668" : null); + result.SpanContext!.OtelTraceState?.CachedHeaderString.Should().Be(!extractFirst || w3CHeaderFirst ? "rv:ef284ace7a91e1;th:e6666666666668" : null); result.Baggage.Should().BeNull(); result.Links.Should().BeNullOrEmpty(); } diff --git a/tracer/test/Datadog.Trace.Tests/Propagators/OtelTraceStateHelpersTests.cs b/tracer/test/Datadog.Trace.Tests/Propagators/OtelTraceStateHelpersTests.cs index 9e0b4c12b48e..f5c92d7fa2bd 100644 --- a/tracer/test/Datadog.Trace.Tests/Propagators/OtelTraceStateHelpersTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Propagators/OtelTraceStateHelpersTests.cs @@ -6,6 +6,7 @@ #nullable enable using System; +using System.Text; using Datadog.Trace.Propagators; using FluentAssertions; using Xunit; @@ -33,37 +34,38 @@ public void ExtractRv_ReturnsValueOrNull(string? raw, ulong? expected) } [Theory] - [InlineData(null, null)] - [InlineData("", null)] - [InlineData("rv:ef284ace7a91e1", "rv:ef284ace7a91e1")] - [InlineData("th:0", "th:0")] - [InlineData("th:e6666666666668", "th:e6666666666668")] - [InlineData("unknownkey:whatever", "unknownkey:whatever")] - [InlineData("rv:zz;th:zz", null)] - [InlineData("rv:ef284ace7a91e1;th:zz", "rv:ef284ace7a91e1")] - [InlineData("rv:zz;th:e6666666666668", "th:e6666666666668")] - [InlineData("foo:bar;rv:zz;th:e6666666666668;baz:qux", "foo:bar;th:e6666666666668;baz:qux")] + [InlineData(null, null, null)] + [InlineData("", null, null)] + [InlineData("rv:ef284ace7a91e1", 0xef284ace7a91e1UL, null)] + [InlineData("th:0", null, 0UL)] + [InlineData("th:e6666666666668", null, 0xe6666666666668UL)] + [InlineData("unknownkey:whatever", null, null)] + [InlineData("rv:zz;th:zz", null, null)] + [InlineData("rv:ef284ace7a91e1;th:zz", 0xef284ace7a91e1UL, null)] + [InlineData("rv:zz;th:e6666666666668", null, 0xe6666666666668UL)] + [InlineData("foo:bar;rv:zz;th:e6666666666668;baz:qux", null, 0xe6666666666668UL)] // rv values use lowercase hexadecimal digits. - [InlineData("rv:EF284ACE7A91E1;foo:bar", "foo:bar")] + [InlineData("rv:EF284ACE7A91E1;foo:bar", null, null)] // th must contain no more than 14 lowercase hexadecimal digits. - [InlineData("th:123456789abcdef;foo:bar", "foo:bar")] - [InlineData("rv:;th;foo:bar", "foo:bar")] - public void Normalize_RemovesOnlyMalformedRvAndTh(string? raw, string? expected) + [InlineData("th:123456789abcdef;foo:bar", null, null)] + [InlineData("rv:;th;foo:bar", null, null)] + public void Parse_RemovesOnlyMalformedRvAndTh(string? raw, ulong? expectedRv, ulong? expectedTh) { - OtelTraceStateHelpers.Normalize(raw).Should().Be(expected); - } + var otelTraceState = OtelTraceState.Parse(raw); - [Theory] - [InlineData("rv:ef284ace7a91e1;th:e6666666666668")] - [InlineData("unknownkey:whatever")] - public void Normalize_ValidContentReturnsOriginalInstance(string raw) - { - OtelTraceStateHelpers.Normalize(raw).Should().BeSameAs(raw); + if (string.IsNullOrEmpty(raw)) + { + otelTraceState.Should().BeNull(); + return; + } + + otelTraceState!.RandomValue.Should().Be(expectedRv); + otelTraceState.Threshold.Should().Be(expectedTh); } [Theory] - [InlineData(null, null, null, null)] - [InlineData("", null, null, null)] + [InlineData(null, null, null, "")] + [InlineData("", null, null, "")] [InlineData(null, 0x1UL, null, "rv:00000000000001")] [InlineData(null, 0xef284ace7a91e1UL, null, "rv:ef284ace7a91e1")] [InlineData(null, null, 0xe6666666666668UL, "th:e6666666666668")] @@ -75,13 +77,35 @@ public void Normalize_ValidContentReturnsOriginalInstance(string raw) [InlineData("foo:bar", null, null, "foo:bar")] public void SetRvTh_RewritesRvAndThInPlace(string? raw, ulong? rv, ulong? th, string? expected) { - OtelTraceStateHelpers.SetRvTh(raw, rv, th).Should().Be(expected); + var sb = new StringBuilder(); + OtelTraceStateHelpers.SetRvTh(sb, raw, rv, th); + sb.ToString().Should().Be(expected); + } + + // SetRvTh writes into the same StringBuilder that already holds "dd=...,ot=", so its + // item separators must be relative to where it started appending, not to the whole builder. + [Theory] + [InlineData(null, null, null, "")] + [InlineData("foo:bar", null, null, "foo:bar")] + [InlineData(null, null, 0xe6666666666668UL, "th:e6666666666668")] + [InlineData("rv:zzzz;foo:bar", null, null, "foo:bar")] + [InlineData("rv:zzzz;th:2;foo:bar", null, 0xe6666666666668UL, "th:e6666666666668;foo:bar")] + [InlineData("foo:bar;rv:1;th:2", 0xef284ace7a91e1UL, 0xe6666666666668UL, "rv:ef284ace7a91e1;th:e6666666666668;foo:bar")] + public void SetRvTh_AppendingToNonEmptyBuilder_DoesNotEmitLeadingSeparator(string? raw, ulong? rv, ulong? th, string expected) + { + const string prefix = "dd=s:1;p:0000000000000002,ot="; + + var sb = new StringBuilder(prefix); + OtelTraceStateHelpers.SetRvTh(sb, raw, rv, th); + + sb.ToString().Should().Be(prefix + expected); } [Fact] public void SetRvTh_ThrowsWhenRvExceeds56Bits() { - FluentActions.Invoking(() => OtelTraceStateHelpers.SetRvTh(null, 1UL << 56, null)) + var sb = new StringBuilder(); + FluentActions.Invoking(() => OtelTraceStateHelpers.SetRvTh(sb, null, 1UL << 56, null)) .Should().Throw(); } } diff --git a/tracer/test/Datadog.Trace.Tests/Propagators/W3CTraceContextPropagatorTests.cs b/tracer/test/Datadog.Trace.Tests/Propagators/W3CTraceContextPropagatorTests.cs index 7edefecea4b0..b719c85bb47f 100644 --- a/tracer/test/Datadog.Trace.Tests/Propagators/W3CTraceContextPropagatorTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Propagators/W3CTraceContextPropagatorTests.cs @@ -233,18 +233,63 @@ public void CreateTraceStateHeader_With128Bit_TraceId() } [Fact] - public void CreateTraceStateHeader_EmitsOtRightAfterDd_WhenOtelTraceStateIsSet() + public void CreateTraceStateHeader_EmitsOtInOriginalPosition_WhenOtelTraceStateIsUnchanged() { var traceContext = new TraceContext(new StubDatadogTracer()); var spanContext = new SpanContext(parent: SpanContext.None, traceContext, serviceName: null, traceId: (TraceId)1, spanId: 2) { - OtelTraceState = "rv:ef284ace7a91e1;th:e6666666666668", - AdditionalW3CTraceState = "congo=t61rcWkgMzE" + OtelTraceState = OtelTraceState.Parse("rv:ef284ace7a91e1;th:e6666666666668"), + AdditionalW3CTraceState = "congo=t61rcWkgMzE,ot=rv:ef284ace7a91e1;th:e6666666666668" }; var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(spanContext); - tracestate.Should().Be("dd=s:1;p:0000000000000002,ot=rv:ef284ace7a91e1;th:e6666666666668,congo=t61rcWkgMzE"); + tracestate.Should().Be("dd=s:1;p:0000000000000002,congo=t61rcWkgMzE,ot=rv:ef284ace7a91e1;th:e6666666666668"); + } + + [Fact] + public void CreateTraceStateHeader_EmitsOtRightAfterDd_WhenOtelTraceStateIsChanged() + { + var traceContext = new TraceContext(new StubDatadogTracer()); + var spanContext = new SpanContext(parent: SpanContext.None, traceContext, serviceName: null, traceId: (TraceId)1, spanId: 2) + { + OtelTraceState = OtelTraceState.Parse("rv:ef284ace7a91e1;th:zz"), + AdditionalW3CTraceState = "ot=rv:ef284ace7a91e1;th:zz,congo=t61rcWkgMzE" + }; + + var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(spanContext); + + tracestate.Should().Be("dd=s:1;p:0000000000000002,ot=rv:ef284ace7a91e1,congo=t61rcWkgMzE"); + } + + [Fact] + public void CreateTraceStateHeader_DoesNotEmitEmptySubKey_WhenOnlyUnknownOtItemsRemain() + { + var traceContext = new TraceContext(new StubDatadogTracer()); + var spanContext = new SpanContext(parent: SpanContext.None, traceContext, serviceName: null, traceId: (TraceId)1, spanId: 2) + { + OtelTraceState = OtelTraceState.Parse("rv:zz;vendor:x"), + AdditionalW3CTraceState = "ot=rv:zz;vendor:x,congo=t61rcWkgMzE" + }; + + var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(spanContext); + + tracestate.Should().Be("dd=s:1;p:0000000000000002,ot=vendor:x,congo=t61rcWkgMzE"); + } + + [Fact] + public void CreateTraceStateHeader_DoesNotEmitEmptySubKey_WhenRvIsDroppedButThRemains() + { + var traceContext = new TraceContext(new StubDatadogTracer()); + var spanContext = new SpanContext(parent: SpanContext.None, traceContext, serviceName: null, traceId: (TraceId)1, spanId: 2) + { + OtelTraceState = OtelTraceState.Parse("rv:zz;th:e6666666666668"), + AdditionalW3CTraceState = "ot=rv:zz;th:e6666666666668,congo=t61rcWkgMzE" + }; + + var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(spanContext); + + tracestate.Should().Be("dd=s:1;p:0000000000000002,ot=th:e6666666666668,congo=t61rcWkgMzE"); } [Fact] @@ -859,7 +904,14 @@ public void Continuation_RoundTripsOtelTraceState(string inboundOtelTraceState) var result = W3CPropagator.Extract(headers.Object); - result.SpanContext!.OtelTraceState.Should().Be(inboundOtelTraceState); + if (inboundOtelTraceState is null) + { + result.SpanContext!.OtelTraceState.Should().BeNull(); + } + else + { + result.SpanContext!.OtelTraceState.CachedHeaderString.Should().Be(inboundOtelTraceState); + } var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(result.SpanContext); if (inboundOtelTraceState is null) @@ -888,7 +940,7 @@ public void Continuation_RemovesMalformedKnownOtSubkeys() var result = W3CPropagator.Extract(headers.Object); - result.SpanContext!.OtelTraceState.Should().Be(ExpectedOtelTraceState); + result.SpanContext!.OtelTraceState.CachedHeaderString.Should().Be(InboundOtelTraceState); result.SpanContext.AdditionalW3CTraceState.Should().Be($"foo=bar,ot={InboundOtelTraceState},something=else"); var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(result.SpanContext!); @@ -909,7 +961,7 @@ public void Continuation_UnknownOtContent_RoundTripsByteForByte() var result = W3CPropagator.Extract(headers.Object); - result.SpanContext!.OtelTraceState.Should().Be(UnknownOtelTraceState); + result.SpanContext!.OtelTraceState.CachedHeaderString.Should().Be(UnknownOtelTraceState); var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(result.SpanContext); tracestate.Should().Contain($"ot={UnknownOtelTraceState}"); @@ -950,7 +1002,7 @@ public void Continuation_MultiVendorTracestate_RoundTripsFully() var result = W3CPropagator.Extract(headers.Object); - result.SpanContext!.OtelTraceState.Should().Be("rv:ef284ace7a91e1;th:e6666666666668;unknownsubkey:x"); + result.SpanContext!.OtelTraceState.CachedHeaderString.Should().Be("rv:ef284ace7a91e1;th:e6666666666668;unknownsubkey:x"); result.SpanContext.AdditionalW3CTraceState.Should().Be("foo1=bar1,ot=rv:ef284ace7a91e1;th:e6666666666668;unknownsubkey:x,congo=t61rcWkgMzE"); var tracestate = W3CTraceContextPropagator.CreateTraceStateHeader(result.SpanContext); diff --git a/tracer/test/Datadog.Trace.Tests/TraceContextTests.cs b/tracer/test/Datadog.Trace.Tests/TraceContextTests.cs index d25741c3071e..63a83a4e6069 100644 --- a/tracer/test/Datadog.Trace.Tests/TraceContextTests.cs +++ b/tracer/test/Datadog.Trace.Tests/TraceContextTests.cs @@ -4,9 +4,11 @@ // using System; +using System.Text; using System.Threading.Tasks; using Datadog.Trace.Agent; using Datadog.Trace.Configuration; +using Datadog.Trace.Headers; using Datadog.Trace.Propagators; using Datadog.Trace.Sampling; using Datadog.Trace.TestHelpers; @@ -251,7 +253,7 @@ public void SetSamplingPriority_RootProbabilityKeep_DerivesRvTh_MatchesRfcWorked rate: OtelTraceStateExampleSamplingRate, sample: true); - traceContext.OtelTraceState.Should().Be(OtelTraceStateExample); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(OtelTraceStateExample); } [Fact] @@ -265,7 +267,7 @@ public void SetSamplingPriority_RootProbabilityDrop_StillEmitsTh() rate: OtelTraceStateExampleSamplingRate, sample: false); - traceContext.OtelTraceState.Should().Contain("th:" + OtelTraceStateExampleThreshold); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Contain("th:" + OtelTraceStateExampleThreshold); } [Fact] @@ -278,7 +280,7 @@ public void SetSamplingPriority_WithoutW3CInjection_DoesNotDeriveOtelTraceState( traceContext.SetSamplingPriority(SamplingPriorityValues.UserKeep, SamplingMechanism.LocalTraceSamplingRule, rate: OtelTraceStateExampleSamplingRate, sample: true); - traceContext.OtelTraceState.Should().BeNull(); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(string.Empty); } [Fact] @@ -295,8 +297,8 @@ public void SetSamplingPriority_ImprecisionClamp_ForcesAgreementWithDdDecision() rate: (float)OtelTraceStateImprecisionClampSamplingRate, sample: sample); - var rv = OtelTraceStateHelpers.ExtractRv(traceContext.OtelTraceState)!.Value; - var th = ParseThForTest(traceContext.OtelTraceState); + var rv = traceContext.OtelTraceState!.RandomValue; + var th = traceContext.OtelTraceState.Threshold; (rv >= th).Should().Be(sample); } @@ -314,7 +316,7 @@ public void SetSamplingPriority_NonProbabilityOverride_RemovesLocallyGeneratedRv traceContext.SetSamplingPriority(SamplingPriorityValues.UserKeep, mechanism); - traceContext.OtelTraceState.Should().BeNull(); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(string.Empty); } [Fact] @@ -329,7 +331,7 @@ public void SetSamplingPriority_RateLimiterDemotesKeep_StripsThButKeepsLocallyGe limiterRate: OtelTraceStateRateLimiterRate, sample: true); - traceContext.OtelTraceState.Should().Be(OtelTraceStateExampleWithoutThreshold); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(OtelTraceStateExampleWithoutThreshold); } [Fact] @@ -346,14 +348,14 @@ public void TraceSampler_LimiterDemotesKeep_KeepsRv_ViaGetOrMakeSamplingDecision traceContext.GetOrMakeSamplingDecision(); - traceContext.OtelTraceState.Should().Be(OtelTraceStateExampleWithoutThreshold); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(OtelTraceStateExampleWithoutThreshold); } [Fact] public void SetSamplingPriority_RateLimiterDemotesKeep_StripsInheritedThButKeepsRvAndUnknownItems() { var traceContext = TraceContextTestHelpers.CreateTraceContextWithRootSpan(traceIdLower: OtelTraceStateExampleTraceIdLower); - traceContext.OtelTraceState = OtelTraceStateExampleWithUnrelatedValue; + traceContext.OtelTraceState = OtelTraceState.Parse(OtelTraceStateExampleWithUnrelatedValue); traceContext.SetSamplingPriority( priority: SamplingPriorityValues.UserReject, @@ -362,7 +364,7 @@ public void SetSamplingPriority_RateLimiterDemotesKeep_StripsInheritedThButKeeps limiterRate: OtelTraceStateRateLimiterRate, sample: true); - traceContext.OtelTraceState.Should().Be(OtelTraceStateExampleWithoutThresholdWithUnrelatedValue); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(OtelTraceStateExampleWithoutThresholdWithUnrelatedValue); } [Theory] @@ -382,18 +384,55 @@ public void SetSamplingPriority_BoundaryRate_ProducesValidOtelTraceState(double rate: rate, sample: sample); - traceContext.OtelTraceState.Should().Be($"rv:{expectedRandomValue};th:{expectedThreshold}"); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be($"rv:{expectedRandomValue};th:{expectedThreshold}"); } [Fact] public void SetSamplingPriority_ManualOverride_StripsInheritedThButKeepsRv() { var traceContext = TraceContextTestHelpers.CreateTraceContextWithRootSpan(traceIdLower: OtelTraceStateMinimumTraceIdLower); - traceContext.OtelTraceState = OtelTraceStateExample; + traceContext.OtelTraceState = OtelTraceState.Parse(OtelTraceStateExample); traceContext.SetSamplingPriority(SamplingPriorityValues.UserKeep, SamplingMechanism.Manual); - traceContext.OtelTraceState.Should().Be(OtelTraceStateExampleWithoutThreshold); + WriteOtelTraceStateHeader(traceContext.OtelTraceState).Should().Be(OtelTraceStateExampleWithoutThreshold); + } + + /// + /// Two traces continued from the same extracted must each own their + /// . Otherwise a sampling override on one trace mutates the other. + /// + [Fact] + public async Task OtelTraceState_IsNotSharedBetweenTracesContinuedFromTheSameExtractedContext() + { + const string inboundTraceState = "foo=1,ot=rv:aaaaaaaaaaaaaa;th:8,congo=2"; + + var propagator = SpanContextPropagatorFactory.GetSpanContextPropagator( + [ContextPropagationHeaderStyle.W3CTraceContext], + [ContextPropagationHeaderStyle.W3CTraceContext], + propagationExtractFirst: false); + + var headers = new Mock(MockBehavior.Strict); + headers.Setup(h => h.GetValues("traceparent")).Returns(new[] { "00-11111111111111111111111111111111-1111111111111111-01" }); + headers.Setup(h => h.GetValues("tracestate")).Returns(new[] { inboundTraceState }); + + var extracted = propagator.Extract(headers.Object).SpanContext; + + await using var tracer = TracerHelper.Create(); + + var spanA = tracer.StartSpan("a", parent: extracted); + var spanB = tracer.StartSpan("b", parent: extracted); + + // a manual override applies to trace A only, and must not disturb trace B + spanA.Context.TraceContext.SetSamplingPriority(SamplingPriorityValues.UserKeep, SamplingMechanism.Manual); + + // trace A drops "th" and moves "ot" to the front, as a rewritten member should + W3CTraceContextPropagator.CreateTraceStateHeader(spanA.Context) + .Should().Contain("ot=rv:aaaaaaaaaaaaaa,").And.NotContain("th:8"); + + // trace B keeps its inherited "th" and its original member ordering. + W3CTraceContextPropagator.CreateTraceStateHeader(spanB.Context) + .Should().Contain("foo=1,ot=rv:aaaaaaaaaaaaaa;th:8,congo=2"); } private static ulong ParseThForTest(string otelTraceState) @@ -408,5 +447,12 @@ private static ulong ParseThForTest(string otelTraceState) throw new InvalidOperationException("no th found"); } + + private static string WriteOtelTraceStateHeader(OtelTraceState traceState) + { + var sb = new StringBuilder(); + OtelTraceStateHelpers.SetRvTh(sb, traceState?.CachedHeaderString, traceState?.RandomValue, traceState?.Threshold); + return sb.ToString(); + } } }