NServiceBus.Transport.AzureServiceBus icon indicating copy to clipboard operation
NServiceBus.Transport.AzureServiceBus copied to clipboard

Application Insights Tracing Context Lost on Message Receive

Open jglien opened this issue 3 years ago • 2 comments

Describe the bug Using the native NServiceBus Pub/Sub pattern in two separate asp.net apps, when messages are received, we see that the Azure Service Bus SDK (presumably) has added the Diagnostic-Id message header, however this header appears to be ignored and Activity.Current is null.

NServiceBus uses Microsoft's SDK, according to Microsoft Docs, this should work out-of-the-box.

Official documentation suggests implementing your own tracing, which would be appropriate for a non-asp.net service, of course, but looks like it won't play nice with the the asp.net App Insights configuration (via .AddApplicationInsightsTelemetry()).

To Reproduce Use the native NServiceBus Pub/Sub pattern with two asp.net apps using application insights via .AddApplicationInsightsTelemetry(). Have code handling Https request in first app send a message, observe addition of Diagnostic-Id message header on receiving end in second app. Observe operation ID set in senders app insights logs, observe operation ID not automagically applied on receivers logs. Observe Activity.Current is null on receiving end.

Expected behavior Application Insights logs on both ends should automagically have trace context applied via Activity.Current.

Versions:

  • NuGet package: 2.0.2
  • OS: Docker: Alpine Linux and MacOS
  • .NET Version net6.0

jglien avatar Mar 23 '22 22:03 jglien

It is possible to do this the following, but shouldn't be necessary.

public async Task Handle(MyEvent @event, IMessageHandlerContext context)
{
	var activity = Activity.Current;
	if (activity == null && context.MessageHeaders.TryGetValue("Diagnostic-Id", out var objectId) && objectId is string diagnosticId)
	{
		activity = new Activity("NServiceBus.Receive");
		activity.SetParentId(diagnosticId);
	}

	if (activity == null)
	{
		_logger.LogWarning("No Activity found, tracing will not work :(");
		await ProcessMessageAsync(@event);
	}
	else
	{
		using (var operation = _telemtryClient.StartOperation<RequestTelemetry>(activity))
		{
			await ProcessMessageAsync(@event);
		}
	}
}

jglien avatar Mar 23 '22 23:03 jglien

@jglien We do not yet support OpenTelemetry. A community contribution exists maintained by @jbogard which is available at https://github.com/jbogard/NServiceBus.Extensions.Diagnostics

However, here you run into the problem that the header keys might be incompatible. I did a quick search on Diagnostic-Id and It might already resolve your current issue.

The magic happens at https://github.com/jbogard/NServiceBus.Extensions.Diagnostics/blob/master/src/NServiceBus.Extensions.Diagnostics/IncomingPhysicalMessageDiagnostics.cs#L48

This is a type of "native integration" with Azure Service Bus. OpenTelemetry diagnostics identifiers are added and maintained by Azure Service Bus or by NServiceBus.Extensions.Diagnostics and these can be incompatible or conflicting.

In that regard it is likely wise to only have OpenTelemetry enabled on NServiceBus (as that will always propagate into the configured transport) or the native messaging client. However, its up to each native client how they propagate the identifiers.

ramonsmits avatar Mar 24 '22 13:03 ramonsmits