Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ public async Task Should_propagate_baggage_to_headers()
)
.Run();

Assert.That(context.BaggageHeader, Is.EqualTo("key3 = , key2 = value2, key1 = value1"));
// Default (backwards-compatible) propagation produces the legacy comma-separated, percent-encoded format.
// The W3C OWS format ("key3 = , key2 = value2, key1 = value1") is produced only when the
// NServiceBus.Core.OpenTelemetry.UseDistributedContextPropagator AppContext switch is enabled (default in v11).
Assert.That(context.BaggageHeader, Is.EqualTo("key3=,key2=value2,key1=value1"));
}

public class TestEndpoint : EndpointConfigurationBuilder
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,33 @@
namespace NServiceBus.Core.Tests.OpenTelemetry;

using System;
using System.Collections.Generic;
using System.Diagnostics;
using Extensibility;
using NUnit.Framework;

[TestFixture]
public class ContextPropagationIncompatibilityTests
public class ContextPropagationCompatibilityTests
{
[SetUp]
public void EnableDistributedContextPropagator()
{
AppContext.SetSwitch(LegacyContextPropagation.UseDistributedContextPropagatorSwitchName, true);
LegacyContextPropagation.ResetUseDistributedContextPropagator();
}

[TearDown]
public void ResetDistributedContextPropagator()
{
AppContext.SetSwitch(LegacyContextPropagation.UseDistributedContextPropagatorSwitchName, false);
Comment thread
tmasternak marked this conversation as resolved.
LegacyContextPropagation.ResetUseDistributedContextPropagator();
}

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 LegacyWrite = LegacyContextPropagation.PropagateContextToHeaders;
static readonly Reader LegacyRead = LegacyContextPropagation.PropagateContextFromHeaders;
static readonly Writer NewWrite = ContextPropagation.PropagateContextToHeaders;
static readonly Reader NewRead = ContextPropagation.PropagateContextFromHeaders;

Expand All @@ -22,7 +37,7 @@ public class ContextPropagationIncompatibilityTests
// 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.
// "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)
Expand Down Expand Up @@ -81,4 +96,21 @@ public void New_propagation_loses_leading_whitespace_in_a_value()
"new propagation strips the leading whitespace from the value");
}
}
}

[TestCase(null, "", "")]
[TestCase("", "", "")]
[TestCase(" ", "", " ")]
[TestCase(" x ", "x", " x ")]
[TestCase(" x x ", "x x", " x x ")]
public void ValidateThatLegacyPropagatorPreservesLeadingAndTrailingWhitespaceInBaggageValues(string input, string expectedNew, string expectedLegacy)
{
var outputNew = Transmit(input, NewWrite, NewRead);
var outputLegacy = Transmit(input, LegacyWrite, LegacyRead);

using (Assert.EnterMultipleScope())
{
Assert.That(expectedNew, Is.EqualTo(outputNew), "Native propagator isn't trimming all leading and trailing whitespaces");
Assert.That(expectedLegacy, Is.EqualTo(outputLegacy), "Legacy propagator isn't preserving leading and trailing whitespace for backwards compatibility");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
namespace NServiceBus.Core.Tests.OpenTelemetry;

using System;
using System.Collections.Generic;
using System.Diagnostics;
using Extensibility;
using NUnit.Framework;

[TestFixture]
public class ContextPropagationDefaultBehaviorTests
{
// Without the opt-in switch, the endpoint default must remain the backwards-compatible
// legacy propagator (percent-encoded, comma-separated, whitespace preserved).
[SetUp]
public void EnsureDefault()
{
AppContext.SetSwitch(LegacyContextPropagation.UseDistributedContextPropagatorSwitchName, false);
LegacyContextPropagation.ResetUseDistributedContextPropagator();
}

[Test]
public void Default_uses_legacy_percent_encoded_baggage_format()
{
using var activity = new Activity(ActivityNames.OutgoingMessageActivityName);
activity.SetIdFormat(ActivityIdFormat.W3C);
activity.Start();
activity.AddBaggage("serverNode", "DF 28");

var headers = new Dictionary<string, string>();
ContextPropagation.PropagateContextToHeaders(activity, headers, new ContextBag());

Assert.That(headers[Headers.DiagnosticsBaggage], Is.EqualTo("serverNode=DF%2028"));
}

[Test]
public void Default_does_not_throw_when_baggage_value_is_null()
{
// Reproduces https://github.com/Particular/NServiceBus/issues/6983 on the legacy propagator.
// A null baggage value must not make the legacy propagator call Uri.EscapeDataString(null).
// Calls LegacyContextPropagation directly so the assertion is independent of the AppContext switch.
using var activity = new Activity(ActivityNames.OutgoingMessageActivityName);
activity.SetIdFormat(ActivityIdFormat.W3C);
activity.Start();
activity.AddBaggage("test", null);

var headers = new Dictionary<string, string>();

Assert.DoesNotThrow(() => LegacyContextPropagation.PropagateContextToHeaders(activity, headers, new ContextBag()));
Assert.That(headers[Headers.DiagnosticsBaggage], Is.EqualTo("test="));
}

[Test]
public void Default_round_trip_preserves_value_whitespace()
{
using var outgoing = new Activity(ActivityNames.OutgoingMessageActivityName);
outgoing.SetIdFormat(ActivityIdFormat.W3C);
outgoing.Start();
outgoing.AddBaggage("key1", " leading-and-trailing ");

var headers = new Dictionary<string, string>();
ContextPropagation.PropagateContextToHeaders(outgoing, headers, new ContextBag());

using var incoming = new Activity(ActivityNames.IncomingMessageActivityName);
incoming.SetIdFormat(ActivityIdFormat.W3C);
incoming.Start();
ContextPropagation.PropagateContextFromHeaders(incoming, headers);

// Legacy propagation preserves leading/trailing whitespace via percent-encoding;
// the DistributedContextPropagator (opt-in) would trim it.
Assert.That(incoming.GetBaggageItem("key1"), Is.EqualTo(" leading-and-trailing "));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace NServiceBus.Core.Tests.OpenTelemetry;

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -8,7 +9,7 @@
using NUnit.Framework;

[TestFixture]
public class ContextPropagationTests
public class LegacyContextPropagationTests
{
[Test]
public void Propagate_activity_id_to_header()
Expand Down Expand Up @@ -193,68 +194,35 @@ 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")
.WithHeaderValue("key1 = value1"),
.WithHeaderValue("key1=value1"),

new ContextPropagationTestCase("with multiple keys")
.WithBaggage("key1", "value1")
.WithBaggage("key2", "value2")
.WithHeaderValue("key1 = value1, key2 = value2"),
.WithHeaderValue("key1=value1,key2=value2"),

new ContextPropagationTestCase("with properties that do not have keys")
.WithBaggage("key1", "value1;property1;property2")
.WithHeaderValue("key1 = value1%3Bproperty1%3Bproperty2"),
.WithHeaderValue("key1=value1%3Bproperty1%3Bproperty2"),

new ContextPropagationTestCase("with properties that have keys")
.WithBaggage("key3", "value3; propertyKey=propertyValue")
.WithHeaderValue("key3 = value3%3B%20propertyKey=propertyValue"),
.WithHeaderValue("key3=value3%3B%20propertyKey%3DpropertyValue"),

new ContextPropagationTestCase("with values containing whitespace")
.WithBaggage("serverNode", "DF 28")
.WithHeaderValue("serverNode = DF%2028"),
.WithHeaderValue("serverNode=DF%2028"),

new ContextPropagationTestCase("with values containing unicode")
.WithBaggage("userId", "Amélie")
.WithHeaderValue("userId = Am%C3%A9lie")
.WithHeaderValue("userId=Am%C3%A9lie")
};

public class ContextPropagationTestCase(string caseName)
Expand Down

This file was deleted.

19 changes: 19 additions & 0 deletions src/NServiceBus.Core/OpenTelemetry/Tracing/ContextPropagation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ static class ContextPropagation
{
public static void PropagateContextToHeaders(Activity? activity, Dictionary<string, string> headers, ContextBag contextBag)
{
// TODO: investigate if we need to improve the switch check for better performance
// Removed in v11, see obsolete_v11.cs
if (!LegacyContextPropagation.UseDistributedContextPropagator)
{
LegacyContextPropagation.PropagateContextToHeaders(activity, headers, contextBag);
return;
}

// The following part was intentionally not extracted to a separate class to prevent
// accidental leftovers when because that the legacy propagator will be removed in v11
if (activity is null)
{
return;
Expand All @@ -28,6 +38,15 @@ public static void PropagateContextToHeaders(Activity? activity, Dictionary<stri

public static void PropagateContextFromHeaders(Activity? activity, IDictionary<string, string> headers)
{
// Removed in v11, see obsolete_v11.cs
if (!LegacyContextPropagation.UseDistributedContextPropagator)
{
LegacyContextPropagation.PropagateContextFromHeaders(activity, headers);
return;
}

// The following part was intentionally not extracted to a separate class to prevent
// accidental leftovers when because that the legacy propagator will be removed in v11
if (activity is null)
{
return;
Expand Down
Loading