Skip to content
Open
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
74 changes: 74 additions & 0 deletions src/NServiceBus.Core.Tests/OpenTelemetry/ActivityFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> { { 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<string, string>? messageHeaders = null) =>
new(
Guid.NewGuid().ToString(),
messageHeaders ?? [],
Array.Empty<byte>(),
new TransportTransaction(),
"receiver",
new ContextBag());
}

class StartHandlerActivity : ActivityFactoryTests
{
[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ActivityContext> _) => ActivitySamplingResult.AllData,
SampleUsingParentId = (ref ActivityCreationOptions<string> options) => ActivitySamplingResult.AllData
Sample = (ref ActivityCreationOptions<ActivityContext> _) => samplingResult,
SampleUsingParentId = (ref ActivityCreationOptions<string> options) => samplingResult
};
}
public void Dispose() => activityListener?.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
23 changes: 19 additions & 4 deletions src/NServiceBus.Core/OpenTelemetry/Tracing/ActivityFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Comment on lines 226 to 230
}
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);
}

Expand Down
Loading