[OpenTelemetry] Address feedback

Address Copilot code review feedback.

Author: martincostello

src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs

@@ -69,10 +69,10 @@ public override PropagationContext Extract<T>(PropagationContext context, T carr
             }
 
             string? tracestate = null;
-            var tracestateCollection = getter(carrier, TraceState);
-            if (tracestateCollection != null && HasAnyValues(tracestateCollection))
+            TryExtractTracestate(getter(carrier, TraceState), out var extractedTracestate, out var hasTraceState);
+            if (hasTraceState)
             {
-                TryExtractTracestate(tracestateCollection, out tracestate);
+                tracestate = extractedTracestate;
             }
 
             return new PropagationContext(
@@ -216,23 +216,83 @@ internal static bool TryExtractTraceparent(string traceparent, out ActivityTrace
         return true;
     }
 
-    internal static bool TryExtractTracestate(string[] tracestateCollection, out string tracestateResult)
-        => TryExtractTracestate((IEnumerable<string>)tracestateCollection, out tracestateResult);
+    internal static bool TryExtractTracestate(string[]? tracestateCollection, out string tracestateResult)
+        => TryExtractTracestate((IEnumerable<string>?)tracestateCollection, out tracestateResult);
 
-    internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollection, out string tracestateResult)
+    internal static bool TryExtractTracestate(IEnumerable<string>? tracestateCollection, out string tracestateResult)
+        => TryExtractTracestate(tracestateCollection, out tracestateResult, out _);
+
+    private static bool TryExtractTracestate(IEnumerable<string>? tracestateCollection, out string tracestateResult, out bool hasTraceState)
     {
         tracestateResult = string.Empty;
+        hasTraceState = false;
 
         if (tracestateCollection == null)
         {
             return true;
         }
 
-        if (TryGetSingleValue(tracestateCollection, out var singleTraceState))
+        if (tracestateCollection is IList<string> list)
+        {
+            if (list.Count == 0)
+            {
+                return true;
+            }
+
+            hasTraceState = true;
+            if (list.Count == 1)
+            {
+                return TryExtractSingleTracestate(list[0], out tracestateResult);
+            }
+
+            return TryExtractMultipleTracestate(list, out tracestateResult);
+        }
+
+        if (tracestateCollection is IReadOnlyList<string> readOnlyList)
+        {
+            if (readOnlyList.Count == 0)
+            {
+                return true;
+            }
+
+            hasTraceState = true;
+            if (readOnlyList.Count == 1)
+            {
+                return TryExtractSingleTracestate(readOnlyList[0], out tracestateResult);
+            }
+
+            return TryExtractMultipleTracestate(readOnlyList, out tracestateResult);
+        }
+
+        using var enumerator = tracestateCollection.GetEnumerator();
+        if (!enumerator.MoveNext())
+        {
+            return true;
+        }
+
+        hasTraceState = true;
+        var singleTraceState = enumerator.Current;
+        if (!enumerator.MoveNext())
         {
             return TryExtractSingleTracestate(singleTraceState, out tracestateResult);
         }
 
+        return TryExtractMultipleTracestate(EnumerateFrom(singleTraceState, enumerator), out tracestateResult);
+    }
+
+    private static IEnumerable<string> EnumerateFrom(string first, IEnumerator<string> enumerator)
+    {
+        yield return first;
+
+        do
+        {
+            yield return enumerator.Current;
+        }
+        while (enumerator.MoveNext());
+    }
+
+    private static bool TryExtractMultipleTracestate(IEnumerable<string> tracestateCollection, out string tracestateResult)
+    {
         var keySet = new HashSet<string>();
         var result = new StringBuilder();
 
@@ -268,13 +328,15 @@ internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollecti
                 {
                     // https://github.com/w3c/trace-context/blob/master/spec/20-http_request_header_format.md#list
                     // test_tracestate_member_count_limit
+                    tracestateResult = string.Empty;
                     return false;
                 }
 
                 var keyLength = listMember.IndexOf('=');
                 if (keyLength == listMember.Length || keyLength == -1)
                 {
                     // Missing key or value in tracestate
+                    tracestateResult = string.Empty;
                     return false;
                 }
 
@@ -284,20 +346,23 @@ internal static bool TryExtractTracestate(IEnumerable<string> tracestateCollecti
                     // test_tracestate_key_illegal_characters in https://github.com/w3c/trace-context/blob/master/test/test.py
                     // test_tracestate_key_length_limit
                     // test_tracestate_key_illegal_vendor_format
+                    tracestateResult = string.Empty;
                     return false;
                 }
 
                 var value = listMember.Slice(keyLength + 1);
                 if (!ValidateValue(value))
                 {
                     // test_tracestate_value_illegal_characters
+                    tracestateResult = string.Empty;
                     return false;
                 }
 
                 // ValidateKey() call above has ensured the key does not contain upper case letters.
                 if (!keySet.Add(key.ToString()))
                 {
                     // test_tracestate_duplicated_keys
+                    tracestateResult = string.Empty;
                     return false;
                 }
 
@@ -502,29 +567,6 @@ private static int GetKeyHashCode(ReadOnlySpan<char> key)
 #endif
     }
 
-    private static bool HasAnyValues(IEnumerable<string> values)
-    {
-#if NET
-        if (values.TryGetNonEnumeratedCount(out var count))
-        {
-            return count > 0;
-        }
-#endif
-
-        if (values is ICollection<string> collection)
-        {
-            return collection.Count > 0;
-        }
-
-        if (values is IReadOnlyCollection<string> readOnlyCollection)
-        {
-            return readOnlyCollection.Count > 0;
-        }
-
-        using var enumerator = values.GetEnumerator();
-        return enumerator.MoveNext();
-    }
-
     private static bool TryGetSingleValue(IEnumerable<string>? values, out string value)
     {
         value = string.Empty;

test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTests.cs

@@ -161,6 +161,26 @@ public void Extract_SupportsEnumerableCarrierValues()
         Assert.Equal("k1=v1,k2=v2", actual.ActivityContext.TraceState);
     }
 
+    [Fact]
+    public void Extract_EnumeratesEnumerableTracestateValuesOnce()
+    {
+        var tracestateValues = new SingleUseEnumerableCarrierValues("  k1=v1 , k2=v2  ");
+        var headers = new Dictionary<string, IEnumerable<string>>
+        {
+            [TraceParent] = new EnumerableCarrierValues($"00-{TraceId}-{SpanId}-01"),
+            [TraceState] = tracestateValues,
+        };
+
+        var target = new TraceContextPropagator();
+        var actual = target.Extract(default, headers, static (carrier, name) =>
+            carrier.TryGetValue(name, out var value) ? value : Empty);
+
+        Assert.Equal(ActivityTraceId.CreateFromString(TraceId.AsSpan()), actual.ActivityContext.TraceId);
+        Assert.Equal(ActivitySpanId.CreateFromString(SpanId.AsSpan()), actual.ActivityContext.SpanId);
+        Assert.Equal("k1=v1,k2=v2", actual.ActivityContext.TraceState);
+        Assert.Equal(1, tracestateValues.EnumerationCount);
+    }
+
     [Fact]
     public void Extract_IgnoresMultipleEnumerableTraceparentValues()
     {
@@ -226,7 +246,7 @@ public void TryExtractTracestate_SingleHeaderRejectsDuplicateLongKeys()
     [Fact]
     public void TryExtractTracestate_NullCollectionReturnsEmpty()
     {
-        Assert.True(TraceContextPropagator.TryExtractTracestate((IEnumerable<string>)null!, out var actual));
+        Assert.True(TraceContextPropagator.TryExtractTracestate((IEnumerable<string>?)null, out var actual));
         Assert.Empty(actual);
     }
 
@@ -455,4 +475,24 @@ public IEnumerator<string> GetEnumerator()
 
         IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
     }
+
+    private sealed class SingleUseEnumerableCarrierValues(params string[] values) : IEnumerable<string>
+    {
+        public int EnumerationCount { get; private set; }
+
+        public IEnumerator<string> GetEnumerator()
+        {
+            if (this.EnumerationCount++ > 0)
+            {
+                throw new InvalidOperationException("Sequence was enumerated multiple times.");
+            }
+
+            foreach (var value in values)
+            {
+                yield return value;
+            }
+        }
+
+        IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();
+    }
 }