ContextPropagation throws an ArgumentNullException when the Activity.Current.Baggage contains a key with value null

[Asopus]

Describe the bug

Description

Method Uri.EscapeDataString called on Line 24 of the ContextPropagation class throws an ArgumentNullException when the baggage property of the activity parameter contains a key with value null

Expected behavior

No ArgumentNullException

Actual behavior

ArgumentNullException

Versions

Observed on version 8.1.6

Steps to reproduce

Made an example simply for illustration. Start an endpoint with learning transport and register a behavior that adds a key with value null to the baggage property of the current activity. Send a command and in the handler of that command, send a second command:

using NServiceBus.Pipeline;
using System.Diagnostics;

internal partial class Program
{
    private static async Task Main(string[] args)
    {
        var endpointConfiguration = new EndpointConfiguration("Program");
        endpointConfiguration.UseTransport<LearningTransport>();
        endpointConfiguration.Pipeline.Register(typeof(BaggageBehavior), nameof(BaggageBehavior));

        var endpoint = await Endpoint.Start(endpointConfiguration);

        await endpoint.SendLocal(new TestCommand());

        Console.ReadKey();
    }

    class BaggageBehavior : Behavior<IInvokeHandlerContext>
    {
        static DiagnosticSource diagnosticSource = new DiagnosticListener("TestSource");
        public override async Task Invoke(IInvokeHandlerContext context, Func<Task> next)
        {
            using (var activity = new Activity("TestSource"))
            {
                diagnosticSource.StartActivity(activity, null);
                Activity.Current.AddBaggage("test", null);
                await next();
            }
        }
    }

    class TestHandler : IHandleMessages<TestCommand>, IHandleMessages<TestCommand2>
    {
        public async Task Handle(TestCommand message, IMessageHandlerContext context)
        {
            await Console.Out.WriteLineAsync($"{nameof(TestCommand)} handled");
            await context.SendLocal(new TestCommand2());
        }

        public async Task Handle(TestCommand2 message, IMessageHandlerContext context)
        {
            await Console.Out.WriteLineAsync($"{nameof(TestCommand2)} handled");
        }
    }

    class TestCommand : ICommand { }
    class TestCommand2 : ICommand { }
}

await context.SendLocal(new TestCommand2()); throws an ArgumentNullException with stacktrace:

2024-03-25 15:30:48.917 INFO  Immediate Retry is going to retry message '2c4f63c2-73ae-4f68-aa6d-b13e00edf11d' because of an exception:
System.ArgumentNullException: Value cannot be null. (Parameter 'stringToEscape')
   at System.ArgumentNullException.Throw(String paramName)
   at System.ArgumentNullException.ThrowIfNull(Object argument, String paramName)
   at System.UriHelper.EscapeString(String stringToEscape, Boolean checkExistingEscaped, SearchValues`1 noEscape)
   at NServiceBus.ContextPropagation.<>c.<PropagateContextToHeaders>b__0_0(KeyValuePair`2 item) in /_/src/NServiceBus.Core/OpenTelemetry/Tracing/ContextPropagation.cs:line 24
   at System.Linq.Enumerable.SelectEnumerableIterator`2.MoveNext()
   at System.String.Join(String separator, IEnumerable`1 values)
   at NServiceBus.ContextPropagation.PropagateContextToHeaders(Activity activity, Dictionary`2 headers) in /_/src/NServiceBus.Core/OpenTelemetry/Tracing/ContextPropagation.cs:line 24
   at NServiceBus.RoutingToDispatchConnector.Invoke(IRoutingContext context, Func`2 stage) in /_/src/NServiceBus.Core/Pipeline/Outgoing/RoutingToDispatchConnector.cs:line 25
   at NServiceBus.AttachSenderRelatedInfoOnMessageBehavior.Invoke(IRoutingContext context, Func`2 next) in /_/src/NServiceBus.Core/Pipeline/Outgoing/AttachSenderRelatedInfoOnMessageBehavior.cs:line 43
   at NServiceBus.OutgoingPhysicalToRoutingConnector.Invoke(IOutgoingPhysicalMessageContext context, Func`2 stage) in /_/src/NServiceBus.Core/Pipeline/Outgoing/OutgoingPhysicalToRoutingConnector.cs:line 14
   at NServiceBus.SerializeMessageConnector.Invoke(IOutgoingLogicalMessageContext context, Func`2 stage) in /_/src/NServiceBus.Core/Pipeline/Outgoing/SerializeMessageConnector.cs:line 44
   at NServiceBus.SendConnector.Invoke(IOutgoingSendContext context, Func`2 stage) in /_/src/NServiceBus.Core/Routing/Routers/SendConnector.cs:line 23
   at NServiceBus.MessageOperations.SendMessage(IBehaviorContext context, Type messageType, Object message, SendOptions options) in /_/src/NServiceBus.Core/Unicast/MessageOperations.cs:line 137

Relevant log output

No response

Additional Information

No response

[boblangley]

@Asopus Thank you for the reproduction. Can you explain how you ran across this in the wild?

[Asopus]

Hi @boblangley

Before our upgrade to version 8, we used version 6 that doesn't support opentelemetry out of the box. So we basically have a behavior that uses the System.Diagnostics.Activity.Current to compile some metadata about our processes and export it to Azure Application Insights. In some cases the behavior adds a key with value null to the bagage (which is perfectly valid in our context), causing the reported ArgumentNullException further down the line.

Since NServiceBus doesn't really have control over what is put in the System.Diagnostics.Activity.Current baggage, is this something that can be fixed or do you see this as intended/desired behavior when using it in an NServiceBus context?

[Asopus]

@boblangley, any updates on this?

[awright18]

@Asopus We've reviewed your issue and added it to our backlog. It's eligible to be picked up for a future release, but we can't provide a firm timeline.