Skip to content

Revert TraceContextPropagator performance refactor from 5749 #6161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 3, 2025
Merged

Revert TraceContextPropagator performance refactor from 5749 #6161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
756e55b
Revert TraceContextPropagator performance refactor from 5749
stevejgordon Mar 3, 2025
a8f226e
Update changelog with PR number
stevejgordon Mar 3, 2025
066c2c6
Refine changelog detail
stevejgordon Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Api/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ Notes](../../RELEASENOTES.md).

## Unreleased

* Revert optimize performance of `TraceContextPropagator.Extract` introduced
in #5749 to resolve #6158.
([#6161](https://github.com/open-telemetry/opentelemetry-dotnet/pull/6161))

## 1.11.1

Released 2025-Jan-22
Expand Down
136 changes: 24 additions & 112 deletions src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Buffers;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Context.Propagation;
Expand Down Expand Up @@ -76,7 +76,7 @@ public override PropagationContext Extract<T>(PropagationContext context, T carr
var tracestateCollection = getter(carrier, TraceState);
if (tracestateCollection?.Any() ?? false)
{
TryExtractTracestate(tracestateCollection, out tracestate);
TryExtractTracestate(tracestateCollection.ToArray(), out tracestate);
}

return new PropagationContext(
Expand Down Expand Up @@ -220,37 +220,31 @@ internal static bool TryExtractTraceparent(string traceparent, out ActivityTrace
return true;
}

internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollection, out string tracestateResult)
internal static bool TryExtractTracestate(string[] tracestateCollection, out string tracestateResult)
{
tracestateResult = string.Empty;

char[]? rentedArray = null;
Span<char> traceStateBuffer = stackalloc char[128]; // 256B
Span<char> keyLookupBuffer = stackalloc char[96]; // 192B (3x32 keys)
int keys = 0;
int charsWritten = 0;

try
if (tracestateCollection != null)
{
foreach (var tracestateItem in tracestateCollection)
var keySet = new HashSet<string>();
var result = new StringBuilder();
for (int i = 0; i < tracestateCollection.Length; ++i)
{
var tracestate = tracestateItem.AsSpan();
int position = 0;

while (position < tracestate.Length)
var tracestate = tracestateCollection[i].AsSpan();
int begin = 0;
while (begin < tracestate.Length)
{
int length = tracestate.Slice(position).IndexOf(',');
int length = tracestate.Slice(begin).IndexOf(',');
ReadOnlySpan<char> listMember;

if (length != -1)
{
listMember = tracestate.Slice(position, length).Trim();
position += length + 1;
listMember = tracestate.Slice(begin, length).Trim();
begin += length + 1;
}
else
{
listMember = tracestate.Slice(position).Trim();
position = tracestate.Length;
listMember = tracestate.Slice(begin).Trim();
begin = tracestate.Length;
}

// https://github.com/w3c/trace-context/blob/master/spec/20-http_request_header_format.md#tracestate-header-field-values
Expand All @@ -261,7 +255,7 @@ internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollecti
continue;
}

if (keys >= 32)
if (keySet.Count >= 32)
{
// https://github.com/w3c/trace-context/blob/master/spec/20-http_request_header_format.md#list
// test_tracestate_member_count_limit
Expand Down Expand Up @@ -292,107 +286,25 @@ internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollecti
}

// ValidateKey() call above has ensured the key does not contain upper case letters.

var duplicationCheckLength = Math.Min(key.Length, 3);

if (keys > 0)
{
// Fast path check of first three chars for potential duplicated keys
var potentialMatchingKeyPosition = 1;
var found = false;
for (int i = 0; i < keys * 3; i += 3)
{
if (keyLookupBuffer.Slice(i, duplicationCheckLength).SequenceEqual(key.Slice(0, duplicationCheckLength)))
{
found = true;
break;
}

potentialMatchingKeyPosition++;
}

// If the fast check has found a possible duplicate, we need to do a full check
if (found)
{
var bufferToCompare = traceStateBuffer.Slice(0, charsWritten);

// We know which key is the first possible duplicate, so skip to that key
// by slicing to the position after the appropriate comma.
for (int i = 1; i < potentialMatchingKeyPosition; i++)
{
var commaIndex = bufferToCompare.IndexOf(',');

if (commaIndex > -1)
{
bufferToCompare.Slice(commaIndex);
}
}

int existingIndex = -1;
while ((existingIndex = bufferToCompare.IndexOf(key)) > -1)
{
if ((existingIndex > 0 && bufferToCompare[existingIndex - 1] != ',') || bufferToCompare[existingIndex + key.Length] != '=')
{
continue; // this is not a key
}

return false; // test_tracestate_duplicated_keys
}
}
}

// Store up to the first three characters of the key for use in the duplicate lookup fast path
var startKeyLookupIndex = keys > 0 ? keys * 3 : 0;
key.Slice(0, duplicationCheckLength).CopyTo(keyLookupBuffer.Slice(startKeyLookupIndex));

// Check we have capacity to write the key and value
var requiredCapacity = charsWritten > 0 ? listMember.Length + 1 : listMember.Length;

while (charsWritten + requiredCapacity > traceStateBuffer.Length)
if (!keySet.Add(key.ToString()))
{
GrowBuffer(ref rentedArray, ref traceStateBuffer);
// test_tracestate_duplicated_keys
return false;
}

if (charsWritten > 0)
if (result.Length > 0)
{
traceStateBuffer[charsWritten++] = ',';
result.Append(',');
}

listMember.CopyTo(traceStateBuffer.Slice(charsWritten));
charsWritten += listMember.Length;

keys++;
result.Append(listMember.ToString());
}
}

tracestateResult = traceStateBuffer.Slice(0, charsWritten).ToString();

return true;
tracestateResult = result.ToString();
}
finally
{
if (rentedArray is not null)
{
ArrayPool<char>.Shared.Return(rentedArray);
rentedArray = null;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static void GrowBuffer(ref char[]? array, ref Span<char> buffer)
{
var newBuffer = ArrayPool<char>.Shared.Rent(buffer.Length * 2);

buffer.CopyTo(newBuffer.AsSpan());

if (array is not null)
{
ArrayPool<char>.Shared.Return(array);
}

array = newBuffer;
buffer = array.AsSpan();
}
return true;
}

private static byte HexCharToByte(char c)
Expand Down
Loading