Using DistributedContextPropagator instead of hand-written baggage propagator (#7820)

* initial migration to the DistributedContextPropagator for W3C compatibility

* new propagator with backwards compatiblity tests

* fixing the startnewtrace header progagation

* refactor ContextPropagation.cs

* small tweak

* warning fixes

* att tests reflect supported baggage and tracestate formats

Author: tmasternak

src/NServiceBus.AcceptanceTests/Core/OpenTelemetry/Metrics/When_envelope_handler_succeeds.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.ApplicationInsights.Extensibility;
 using NServiceBus;
 using NServiceBus.AcceptanceTesting;
 using NServiceBus.AcceptanceTests.Core.OpenTelemetry;

src/NServiceBus.AcceptanceTests/Core/OpenTelemetry/Traces/When_ambient_trace_in_message_session.cs

@@ -15,7 +15,7 @@ public async Task Should_attach_to_ambient_trace()
         using var externalActivitySource = new ActivitySource("external trace source");
         using var _ = TestingActivityListener.SetupDiagnosticListener(externalActivitySource.Name); // need to have a registered listener for activities to be created
 
-        const string wrapperActivityTraceState = "test trace state";
+        const string wrapperActivityTraceState = "tracekey=traceValue";
 
         var context = await Scenario.Define<Context>()
             .WithEndpoint<EndpointWithAmbientActivity>(b => b

src/NServiceBus.AcceptanceTests/Core/OpenTelemetry/Traces/When_incoming_message_has_baggage_header.cs

@@ -17,7 +17,7 @@ public async Task Should_propagate_baggage_to_activity()
                 {
                     var sendOptions = new SendOptions();
                     sendOptions.RouteToThisEndpoint();
-                    sendOptions.SetHeader(Headers.DiagnosticsBaggage, "key1=value1,key2=value2,key3=");
+                    sendOptions.SetHeader(Headers.DiagnosticsBaggage, "key1=value1,key2=value2,key3=value3");
                     await session.Send(new SomeMessage(), sendOptions);
                 })
             )
@@ -29,7 +29,7 @@ public async Task Should_propagate_baggage_to_activity()
 
         VerifyBaggageItem("key1", "value1");
         VerifyBaggageItem("key2", "value2");
-        VerifyBaggageItem("key3", "");
+        VerifyBaggageItem("key3", "value3");
         return;
 
         void VerifyBaggageItem(string key, string expectedValue)

src/NServiceBus.AcceptanceTests/Core/OpenTelemetry/Traces/When_outgoing_activity_has_baggage.cs

@@ -33,7 +33,7 @@ public async Task Should_propagate_baggage_to_headers()
             )
             .Run();
 
-        Assert.That(context.BaggageHeader, Is.EqualTo("key3=,key2=value2,key1=value1"));
+        Assert.That(context.BaggageHeader, Is.EqualTo("key3 = , key2 = value2, key1 = value1"));
     }
 
     public class TestEndpoint : EndpointConfigurationBuilder

src/NServiceBus.Core.Tests/OpenTelemetry/ContextPropagationIncompatibilityTests.cs

@@ -0,0 +1,84 @@
+namespace NServiceBus.Core.Tests.OpenTelemetry;
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using Extensibility;
+using NUnit.Framework;
+
+[TestFixture]
+public class ContextPropagationIncompatibilityTests
+{
+    delegate void Writer(Activity activity, Dictionary<string, string> headers, ContextBag context);
+    delegate void Reader(Activity activity, IDictionary<string, string> headers);
+
+    static readonly Writer LegacyWrite = LegacyContextPropagator.PropagateContextToHeaders;
+    static readonly Reader LegacyRead = LegacyContextPropagator.PropagateContextFromHeaders;
+    static readonly Writer NewWrite = ContextPropagation.PropagateContextToHeaders;
+    static readonly Reader NewRead = ContextPropagation.PropagateContextFromHeaders;
+
+    // A value exercising every class of special character: structural baggage delimiters
+    // (',' ';' '='), the escape char '%', quotes, brackets, slashes, ampersand, Unicode and
+    // an emoji, plus interior spaces. Deliberately has NO leading/trailing whitespace, so this
+    // value isolates "what happens to special characters" from the separate edge-whitespace
+    // issue covered by New_propagation_loses_leading_whitespace_in_a_value.
+    // This already includes property-like syntax (the ';' and '=' delimiters), so a value such as
+    // "zone=eu;sensitive" is just a subset and needs no separate case here. 
+    const string AllSpecialCharacters = "a b,c;d=e&f'g\"h\\i(j)k{l}m[n]o%p/q?r:s@t~u|v<w>x é ü 😀 z";
+
+    static Dictionary<string, string> Send(string value, Writer write)
+    {
+        using var sender = new Activity(ActivityNames.OutgoingMessageActivityName);
+        sender.SetIdFormat(ActivityIdFormat.W3C);
+        sender.Start();
+        sender.AddBaggage("key", value);
+
+        var headers = new Dictionary<string, string>();
+        write(sender, headers, new ContextBag());
+        sender.Stop();
+        return headers;
+    }
+
+    static string Receive(Dictionary<string, string> headers, Reader read)
+    {
+        using var receiver = new Activity(ActivityNames.IncomingMessageActivityName);
+        receiver.SetIdFormat(ActivityIdFormat.W3C);
+        receiver.Start();
+        read(receiver, headers);
+        return receiver.GetBaggageItem("key");
+    }
+
+    static string Transmit(string value, Writer write, Reader read) => Receive(Send(value, write), read);
+
+    [Test]
+    public void Legacy_sender_to_new_receiver_preserves_the_value()
+    {
+        var received = Transmit(AllSpecialCharacters, LegacyWrite, NewRead);
+        Assert.That(received, Is.EqualTo(AllSpecialCharacters));
+    }
+
+    [Test]
+    public void New_sender_to_legacy_receiver_prepends_a_leading_space_but_keeps_the_special_characters()
+    {
+        var received = Transmit(AllSpecialCharacters, NewWrite, LegacyRead);
+
+        Assert.That(received, Is.EqualTo(" " + AllSpecialCharacters),
+                "ignoring the leading space, every special character round-trips correctly");
+    }
+
+    [Test]
+    public void New_propagation_loses_leading_whitespace_in_a_value()
+    {
+        const string valueWithLeadingSpace = " hasLeadingSpace";
+
+        var legacyRoundTrip = Transmit(valueWithLeadingSpace, LegacyWrite, LegacyRead);
+        var newRoundTrip = Transmit(valueWithLeadingSpace, NewWrite, NewRead);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(legacyRoundTrip, Is.EqualTo(valueWithLeadingSpace),
+                "legacy propagation preserves leading whitespace via percent-encoding");
+            Assert.That(newRoundTrip, Is.EqualTo("hasLeadingSpace"),
+                "new propagation strips the leading whitespace from the value");
+        }
+    }
+}

src/NServiceBus.Core.Tests/OpenTelemetry/ContextPropagationTests.cs

@@ -1,6 +1,5 @@
 namespace NServiceBus.Core.Tests.OpenTelemetry;
 
-using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
@@ -94,7 +93,9 @@ public void Can_propagate_baggage_from_header_to_activity(ContextPropagationTest
             headers[Headers.DiagnosticsBaggage] = testCase.BaggageHeaderValue;
         }
 
-        var activity = new Activity(ActivityNames.IncomingMessageActivityName);
+        using var activity = new Activity(ActivityNames.IncomingMessageActivityName);
+        activity.SetIdFormat(ActivityIdFormat.W3C);
+        activity.Start();
 
         ContextPropagation.PropagateContextFromHeaders(activity, headers);
 
@@ -114,7 +115,9 @@ public void Can_propagate_baggage_from_activity_to_header(ContextPropagationTest
 
         var headers = new Dictionary<string, string>();
 
-        var activity = new Activity(ActivityNames.OutgoingMessageActivityName);
+        using var activity = new Activity(ActivityNames.OutgoingMessageActivityName);
+        activity.SetIdFormat(ActivityIdFormat.W3C);
+        activity.Start();
 
         foreach (var baggageItem in testCase.ExpectedBaggageItems.Reverse())
         {
@@ -131,7 +134,7 @@ public void Can_propagate_baggage_from_activity_to_header(ContextPropagationTest
             {
                 Assert.That(baggageHeaderSet, Is.True, "Should have a baggage header if there is baggage");
 
-                Assert.That(baggageValue, Is.EqualTo(testCase.BaggageHeaderValueWithoutOptionalWhitespace), "baggage header is set but is not correct");
+                Assert.That(baggageValue, Is.EqualTo(testCase.BaggageHeaderValue), "baggage header is set but is not correct");
             }
         }
         else
@@ -146,7 +149,9 @@ public void Can_roundtrip_baggage(ContextPropagationTestCase testCase)
         TestContext.Out.WriteLine($"Baggage header: {testCase.BaggageHeaderValue}");
 
         var outgoingHeaders = new Dictionary<string, string>();
-        var outgoingActivity = new Activity(ActivityNames.OutgoingMessageActivityName);
+        using var outgoingActivity = new Activity(ActivityNames.OutgoingMessageActivityName);
+        outgoingActivity.SetIdFormat(ActivityIdFormat.W3C);
+        outgoingActivity.Start();
 
         foreach (var baggageItem in testCase.ExpectedBaggageItems.Reverse())
         {
@@ -157,7 +162,9 @@ public void Can_roundtrip_baggage(ContextPropagationTestCase testCase)
 
         // Simulate wire transfer
         var incomingHeaders = outgoingHeaders;
-        var incomingActivity = new Activity(ActivityNames.IncomingMessageActivityName);
+        using var incomingActivity = new Activity(ActivityNames.IncomingMessageActivityName);
+        incomingActivity.SetIdFormat(ActivityIdFormat.W3C);
+        incomingActivity.Start();
 
         ContextPropagation.PropagateContextFromHeaders(incomingActivity, incomingHeaders);
 
@@ -170,55 +177,87 @@ public void Can_roundtrip_baggage(ContextPropagationTestCase testCase)
         }
     }
 
+    [Test]
+    public void Can_not_roundtrip_baggage_value_with_optional_whitespaces()
+    {
+        var outgoingHeaders = new Dictionary<string, string>();
+        using var outgoingActivity = new Activity(ActivityNames.OutgoingMessageActivityName);
+        outgoingActivity.SetIdFormat(ActivityIdFormat.W3C);
+        outgoingActivity.Start();
+
+        outgoingActivity.AddBaggage("key1", " value1");
+        outgoingActivity.AddBaggage("key2", "value2 ");
+
+        ContextPropagation.PropagateContextToHeaders(outgoingActivity, outgoingHeaders, new ContextBag());
+
+        // Simulate wire transfer
+        var incomingHeaders = outgoingHeaders;
+        using var incomingActivity = new Activity(ActivityNames.IncomingMessageActivityName);
+        incomingActivity.SetIdFormat(ActivityIdFormat.W3C);
+        incomingActivity.Start();
+
+        ContextPropagation.PropagateContextFromHeaders(incomingActivity, incomingHeaders);
+
+        using (Assert.EnterMultipleScope())
+        {
+            foreach (var baggageItem in outgoingActivity.Baggage)
+            {
+                var key = baggageItem.Key;
+                var actualValue = incomingActivity.GetBaggageItem(key);
+                Assert.That(actualValue, Is.Not.Null, $"Baggage is missing item with key |{key}|");
+                Assert.That(actualValue, Is.EqualTo(baggageItem.Value.Trim()), $"Baggage item |{key}| has the wrong value");
+            }
+        }
+    }
+
     // HINT: Many of these test cases are given as examples in the spec https://www.w3.org/TR/baggage/#example
     static IEnumerable TestCases => new object[]
     {
         new ContextPropagationTestCase("without any baggage"),
 
         new ContextPropagationTestCase("with a single key")
-            .WithBaggage("key1", "value1"),
+            .WithBaggage("key1", "value1")
+            .WithHeaderValue("key1 = value1"),
 
         new ContextPropagationTestCase("with multiple keys")
             .WithBaggage("key1", "value1")
-            .WithBaggage("key2", "value2"),
-
-        new ContextPropagationTestCase("with whitespace")
-            .WithBaggage("key1 ", " value1")
-            .WithBaggage(" key2", "value2 ")
-            .WithBaggage(" key3 ", " value3 "),
+            .WithBaggage("key2", "value2")
+            .WithHeaderValue("key1 = value1, key2 = value2"),
 
         new ContextPropagationTestCase("with properties that do not have keys")
-            .WithBaggage("key1", "value1;property1;property2"),
+            .WithBaggage("key1", "value1;property1;property2")
+            .WithHeaderValue("key1 = value1%3Bproperty1%3Bproperty2"),
 
         new ContextPropagationTestCase("with properties that have keys")
-            .WithBaggage("key3", "value3; propertyKey=propertyValue"),
+            .WithBaggage("key3", "value3; propertyKey=propertyValue")
+            .WithHeaderValue("key3 = value3%3B%20propertyKey=propertyValue"),
 
         new ContextPropagationTestCase("with values containing whitespace")
-            .WithBaggage("serverNode", "DF 28"),
+            .WithBaggage("serverNode", "DF 28")
+            .WithHeaderValue("serverNode = DF%2028"),
 
         new ContextPropagationTestCase("with values containing unicode")
             .WithBaggage("userId", "Amélie")
+            .WithHeaderValue("userId = Am%C3%A9lie")
     };
 
-    public class ContextPropagationTestCase
+    public class ContextPropagationTestCase(string caseName)
     {
-        string caseName;
-        Dictionary<string, string> baggageItems = [];
+        readonly Dictionary<string, string> baggageItems = [];
 
-        public ContextPropagationTestCase(string caseName)
+        public ContextPropagationTestCase WithBaggage(string key, string value)
         {
-            this.caseName = caseName;
+            baggageItems.Add(key, value);
+            return this;
         }
 
-        public ContextPropagationTestCase WithBaggage(string key, string value)
+        public ContextPropagationTestCase WithHeaderValue(string headerValue)
         {
-            baggageItems.Add(key, value);
+            BaggageHeaderValue = headerValue;
             return this;
         }
 
-        public string BaggageHeaderValue => string.Join(",", from kvp in baggageItems select $"{kvp.Key}={Uri.EscapeDataString(kvp.Value)}");
-        public string BaggageHeaderValueWithoutOptionalWhitespace
-            => string.Join(",", from kvp in baggageItems select $"{kvp.Key.Trim()}={Uri.EscapeDataString(kvp.Value)}");
+        public string BaggageHeaderValue { get; private set; }
         public IEnumerable<KeyValuePair<string, string>> ExpectedBaggageItems => from kvp in baggageItems
                                                                                  select new KeyValuePair<string, string>(
                                                                                      kvp.Key.Trim(),

src/NServiceBus.Core.Tests/OpenTelemetry/LegacyContextPropagator.cs

@@ -0,0 +1,87 @@
+#nullable enable
+
+namespace NServiceBus.Core.Tests.OpenTelemetry;
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using Extensibility;
+
+static class LegacyContextPropagator
+{
+    public static void PropagateContextToHeaders(Activity activity, Dictionary<string, string> headers, ContextBag contextBag)
+    {
+        if (activity is null)
+        {
+            return;
+        }
+
+        if (activity.Id is not null)
+        {
+            headers[Headers.DiagnosticsTraceParent] = activity.Id;
+        }
+
+        if (activity.TraceStateString is not null)
+        {
+            headers[Headers.DiagnosticsTraceState] = activity.TraceStateString;
+        }
+
+        // Check whether the startnewtrace setting was set in the context, if so, add it to the headers now the trace parent was added
+        if (contextBag.TryGet<string>(Headers.StartNewTrace, out var headerContent))
+        {
+            headers[Headers.StartNewTrace] = headerContent;
+        }
+
+        var baggage = string.Join(",", activity.Baggage.Select(item => $"{item.Key}={Uri.EscapeDataString(item.Value ?? string.Empty)}"));
+        if (!string.IsNullOrEmpty(baggage))
+        {
+            headers[Headers.DiagnosticsBaggage] = baggage;
+        }
+    }
+
+    public static void PropagateContextFromHeaders(Activity? activity, IDictionary<string, string> headers)
+    {
+        if (activity is null)
+        {
+            return;
+        }
+
+        if (headers.TryGetValue(Headers.DiagnosticsTraceState, out var traceState))
+        {
+            activity.TraceStateString = traceState;
+        }
+
+        if (headers.TryGetValue(Headers.DiagnosticsBaggage, out var baggageValue))
+        {
+            var baggageSpan = baggageValue.AsSpan();
+            // HINT: Iterate in reverse order because Activity baggage is LIFO
+            while (!baggageSpan.IsEmpty)
+            {
+                var lastComma = baggageSpan.LastIndexOf(',');
+                ReadOnlySpan<char> baggageItem;
+
+                if (lastComma >= 0)
+                {
+                    baggageItem = baggageSpan[(lastComma + 1)..];
+                    baggageSpan = baggageSpan[..lastComma];
+                }
+                else
+                {
+                    baggageItem = baggageSpan;
+                    baggageSpan = [];
+                }
+
+                var firstEquals = baggageItem.IndexOf('=');
+                if (firstEquals < 0 || firstEquals >= baggageItem.Length)
+                {
+                    continue;
+                }
+
+                var key = baggageItem[..firstEquals].Trim();
+                var value = baggageItem[(firstEquals + 1)..];
+                activity.AddBaggage(key.ToString(), Uri.UnescapeDataString(value));
+            }
+        }
+    }
+}

src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityExtensions.cs

@@ -41,6 +41,8 @@ public static void SetErrorStatus(this Activity activity, Exception ex)
         activity.SetStatus(ActivityStatusCode.Error, ex.Message);
         activity.SetTag("otel.status_code", "ERROR");
         activity.SetTag("otel.status_description", ex.Message);
+
+
         activity.AddEvent(new ActivityEvent("exception", DateTimeOffset.UtcNow,
             [
                 new KeyValuePair<string, object?>("exception.escaped", true),