diff --git a/src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs b/src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs index 500740cc6b..92f9f8c3e6 100644 --- a/src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs +++ b/src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs @@ -258,6 +258,80 @@ public void Should_set_activity_in_context() } } + class PropagationDataSampling + { + readonly ActivityFactory activityFactory = new(new InstrumentationOptions()); + + TestingActivityListener nsbActivityListener; + + [OneTimeSetUp] + public void Setup() => nsbActivityListener = TestingActivityListener.SetupNServiceBusDiagnosticListener(ActivitySamplingResult.PropagationData); + + [OneTimeTearDown] + public void TearDown() => nsbActivityListener.Dispose(); + + [Test] + public void Should_create_activity_but_skip_header_tag_promotion() + { + var messageHeaders = new Dictionary { { Headers.SagaId, Guid.NewGuid().ToString() } }; + var messageContext = CreateMessageContext(messageHeaders); + + var activity = activityFactory.StartIncomingPipelineActivity(messageContext); + + Assert.That(activity, Is.Not.Null, "PropagationData should still create an activity"); + var tags = activity.Tags.ToImmutableDictionary(); + using (Assert.EnterMultipleScope()) + { + Assert.That(activity.IsAllDataRequested, Is.False); + Assert.That(tags.ContainsKey(ActivityTags.SagaId), Is.False, "should not promote headers to tags when not all data requested"); + Assert.That(tags[ActivityTags.NativeMessageId], Is.EqualTo(messageContext.NativeMessageId), "native message id tag should always be set"); + } + } + + [Test] + public void Should_not_add_exception_event_or_legacy_tags_on_error() + { + var activity = activityFactory.StartIncomingPipelineActivity(CreateMessageContext()); + Assert.That(activity, Is.Not.Null); + + activityFactory.RecordError(activity, new Exception("boom"), new ContextBag()); + + var tags = activity.Tags.ToImmutableDictionary(); + using (Assert.EnterMultipleScope()) + { + Assert.That(activity.Status, Is.EqualTo(ActivityStatusCode.Error), "status should always be set"); + Assert.That(tags.ContainsKey(ActivityTags.ErrorType), Is.True, "error type tag should always be set"); + Assert.That(tags.ContainsKey("otel.status_code"), Is.False, "legacy tags should be skipped when not all data requested"); + Assert.That(activity.Events, Is.Empty, "no exception event should be recorded when not all data requested"); + } + } + + [Test] + public void Should_not_update_activity_from_recoverability_action() + { + var activity = activityFactory.StartIncomingPipelineActivity(CreateMessageContext()); + Assert.That(activity, Is.Not.Null); + var originalDisplayName = activity.DisplayName; + + activityFactory.UpdateActivityFromRecoverabilityAction(activity, new ImmediateRetry(), "receiveAddress"); + + using (Assert.EnterMultipleScope()) + { + Assert.That(activity.DisplayName, Is.EqualTo(originalDisplayName), "display name should not change when not all data requested"); + Assert.That(activity.Tags.ToImmutableDictionary().ContainsKey(ActivityTags.RecoverabilityAction), Is.False); + } + } + + static MessageContext CreateMessageContext(Dictionary? messageHeaders = null) => + new( + Guid.NewGuid().ToString(), + messageHeaders ?? [], + Array.Empty(), + new TransportTransaction(), + "receiver", + new ContextBag()); + } + class StartHandlerActivity : ActivityFactoryTests { [Test] diff --git a/src/NServiceBus.Core.Tests/OpenTelemetry/Helpers/TestingActivityListener.cs b/src/NServiceBus.Core.Tests/OpenTelemetry/Helpers/TestingActivityListener.cs index b72d087a6f..3450ca7548 100644 --- a/src/NServiceBus.Core.Tests/OpenTelemetry/Helpers/TestingActivityListener.cs +++ b/src/NServiceBus.Core.Tests/OpenTelemetry/Helpers/TestingActivityListener.cs @@ -7,24 +7,25 @@ class TestingActivityListener : IDisposable { readonly ActivityListener activityListener; - public static TestingActivityListener SetupNServiceBusDiagnosticListener() => SetupDiagnosticListener(ActivitySources.Main.Name); + public static TestingActivityListener SetupNServiceBusDiagnosticListener(ActivitySamplingResult samplingResult = ActivitySamplingResult.AllData) => + SetupDiagnosticListener(ActivitySources.Main.Name, samplingResult); - public static TestingActivityListener SetupDiagnosticListener(string sourceName) + public static TestingActivityListener SetupDiagnosticListener(string sourceName, ActivitySamplingResult samplingResult = ActivitySamplingResult.AllData) { - var testingListener = new TestingActivityListener(sourceName); + var testingListener = new TestingActivityListener(sourceName, samplingResult); ActivitySource.AddActivityListener(testingListener.activityListener); return testingListener; } - TestingActivityListener(string sourceName = null) + TestingActivityListener(string sourceName = null, ActivitySamplingResult samplingResult = ActivitySamplingResult.AllData) { // do not rely on activities from the notifications as tests are run in parallel activityListener = new ActivityListener { ShouldListenTo = source => string.IsNullOrEmpty(sourceName) || source.Name == sourceName, - Sample = (ref ActivityCreationOptions _) => ActivitySamplingResult.AllData, - SampleUsingParentId = (ref ActivityCreationOptions options) => ActivitySamplingResult.AllData + Sample = (ref ActivityCreationOptions _) => samplingResult, + SampleUsingParentId = (ref ActivityCreationOptions options) => samplingResult }; } public void Dispose() => activityListener?.Dispose(); diff --git a/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityExtensions.cs b/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityExtensions.cs index 3f784962a9..44ecf4e4be 100644 --- a/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityExtensions.cs +++ b/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityExtensions.cs @@ -21,7 +21,7 @@ static bool TryGetRecordingPipelineActivity(this ContextBag pipelineContext, str { if (Activity.Current is not null // Cheaper to check than searching the pipeline context to start with. If there is no ambient activity, there can't be an activity in the context. && pipelineContext.TryGet(activityKey, out activity) // Search activity in context bag - && activity is { IsAllDataRequested: true }) // do not apply "expensive" work on non-recording activities + && activity is { IsAllDataRequested: true }) // skip expensive work when no listener asked for full data (IsAllDataRequested, not Recorded/sampled) { return true; } diff --git a/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityFactory.cs b/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityFactory.cs index c8a958733f..970645c362 100644 --- a/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityFactory.cs +++ b/src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityFactory.cs @@ -71,12 +71,18 @@ sealed class ActivityFactory(InstrumentationOptions options) : IActivityFactory return activity; } + // Baggage/tracestate propagation is correctness, not enrichment, so it must run regardless of sampling. ContextPropagation.PropagateContextFromHeaders(activity, headers); activity.SetIdFormat(ActivityIdFormat.W3C); activity.AddTag(ActivityTags.NativeMessageId, nativeMessageId); - ActivityDecorator.PromoteHeadersToTags(activity, headers); + // IsAllDataRequested is false when a listener sampled this as PropagationData-only: it wants the + // activity to exist for context propagation but won't read anything beyond that, so skip the tag work. + if (activity.IsAllDataRequested) + { + ActivityDecorator.PromoteHeadersToTags(activity, headers); + } return activity; } @@ -170,6 +176,13 @@ sealed class ActivityFactory(InstrumentationOptions options) : IActivityFactory public void UpdateActivityFromRecoverabilityAction(Activity activity, RecoverabilityAction recoverabilityAction, string receiveAddress) { + // Nothing below is read unless a listener asked for full data (IsAllDataRequested), so bail out early + // rather than building tags and DisplayName strings for an activity nobody will inspect. + if (!activity.IsAllDataRequested) + { + return; + } + if (recoverabilityAction is ImmediateRetry) { activity.AddTag(ActivityTags.RecoverabilityAction, "immediate_retry"); @@ -210,16 +223,18 @@ public void RecordError(Activity activity, Exception exception, ContextBag conte activity.SetStatus(ActivityStatusCode.Error, exception.Message); activity.SetTag(ActivityTags.ErrorType, exception.GetType().FullName); - LegacyExceptionTags.SetLegacyStatusTags(activity, exception); - if (!exception.Data.Contains(ExceptionRecordedFlag)) { if (Options.ExceptionRecordingMode == ExceptionRecordingMode.Logs) { Logger.Error($"An exception occurred while executing '{activity.DisplayName}'.", exception); } - else + else if (activity.IsAllDataRequested) { + // Recording the exception on the activity (stack trace event + legacy tags) only matters + // if a listener asked for full data; skip it otherwise. The Logs branch above is a separate + // capture path and stays unconditional regardless of IsAllDataRequested. + LegacyExceptionTags.SetLegacyStatusTags(activity, exception); activity.AddException(exception, LegacyExceptionTags.EscapedTagList); }