From b43bdfb2a2b60f27845656dd38bdf85c64f96013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Kie=C5=82kowicz?= Date: Wed, 4 Oct 2023 20:11:28 +0200 Subject: [PATCH] Fix test_tracestate_duplicated_keys --- src/OpenTelemetry.Api/CHANGELOG.md | 3 +++ .../Context/Propagation/TraceContextPropagator.cs | 10 +++------- .../W3CTraceContextTests.cs | 4 ++-- .../Trace/Propagation/TraceContextPropagatorTest.cs | 8 ++++---- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/OpenTelemetry.Api/CHANGELOG.md b/src/OpenTelemetry.Api/CHANGELOG.md index 6d47071c7e3..4fdffec74ce 100644 --- a/src/OpenTelemetry.Api/CHANGELOG.md +++ b/src/OpenTelemetry.Api/CHANGELOG.md @@ -14,6 +14,9 @@ `trace-flags` field of the `traceparent` header. ([#4893](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4893)) +* Fix `TraceContextPropagator` by propagating duplicated `tracestate` entries. + ([#4937](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4937)) + ## 1.6.0 Released 2023-Sep-05 diff --git a/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs b/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs index 16c2456ad4a..3922f5f6fb1 100644 --- a/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs +++ b/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs @@ -235,7 +235,7 @@ internal static bool TryExtractTracestate(string[] tracestateCollection, out str if (tracestateCollection != null) { - var keySet = new HashSet(); + int validEntries = 0; var result = new StringBuilder(); for (int i = 0; i < tracestateCollection.Length; ++i) { @@ -264,7 +264,7 @@ internal static bool TryExtractTracestate(string[] tracestateCollection, out str continue; } - if (keySet.Count >= 32) + if (validEntries >= 32) { // https://github.com/w3c/trace-context/blob/master/spec/20-http_request_header_format.md#list // test_tracestate_member_count_limit @@ -295,11 +295,6 @@ internal static bool TryExtractTracestate(string[] tracestateCollection, out str } // ValidateKey() call above has ensured the key does not contain upper case letters. - if (!keySet.Add(key.ToString())) - { - // test_tracestate_duplicated_keys - return false; - } if (result.Length > 0) { @@ -307,6 +302,7 @@ internal static bool TryExtractTracestate(string[] tracestateCollection, out str } result.Append(listMember.ToString()); + validEntries++; } } diff --git a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs index 58ec321d600..1effb256171 100644 --- a/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs +++ b/test/OpenTelemetry.Instrumentation.W3cTraceContext.Tests/W3CTraceContextTests.cs @@ -103,11 +103,11 @@ public void W3CTraceContextTestSuiteAsync(string value) if (AspNetCoreHostingVersion.Major <= 6) { - Assert.StartsWith("FAILED (failures=5)", lastLine); + Assert.StartsWith("FAILED (failures=4)", lastLine); } else { - Assert.StartsWith("FAILED (failures=2)", lastLine); + Assert.StartsWith("FAILED (failures=1)", lastLine); } } diff --git a/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs b/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs index f7dae35547f..089aa248517 100644 --- a/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs +++ b/test/OpenTelemetry.Tests/Trace/Propagation/TraceContextPropagatorTest.cs @@ -193,10 +193,10 @@ public void Inject_WithTracestate() public void DuplicateKeys() { // test_tracestate_duplicated_keys - Assert.Empty(CallTraceContextPropagator("foo=1,foo=1")); - Assert.Empty(CallTraceContextPropagator("foo=1,foo=2")); - Assert.Empty(CallTraceContextPropagator(new[] { "foo=1", "foo=1" })); - Assert.Empty(CallTraceContextPropagator(new[] { "foo=1", "foo=2" })); + Assert.Equal("foo=1,foo=1", CallTraceContextPropagator("foo=1,foo=1")); + Assert.Equal("foo=1,foo=2", CallTraceContextPropagator("foo=1,foo=2")); + Assert.Equal("foo=1,foo=1", CallTraceContextPropagator(new[] { "foo=1", "foo=1" })); + Assert.Equal("foo=1,foo=2", CallTraceContextPropagator(new[] { "foo=1", "foo=2" })); } [Fact]