microsoft / ApplicationInsights-dotnet-server

Microsoft Application Insights for .NET Web Applications
https://azure.microsoft.com/services/application-insights/
133 stars 67 forks source link

Dependency Parent IDs not set correctly in 2.8.0 #1277

Closed aarondandy closed 4 years ago

aarondandy commented 4 years ago

Dependency Parent IDs not set correctly in 2.8.0. Reverting from 2.8.0 back to 2.7.1 is a workaround for me.

Repro Steps

  1. Upgrade from 2.7.1 to 2.8.0 in either 2.2 project or a 3.0 project
  2. Run application, sending telemetry to AI

Actual Behavior

A request with a Request Id of |af71200102f65e46b9c58cd203474526.f0f605963ec9474d. will have a dependency (SQL for example) with a Parent Id of 00-af71200102f65e46b9c58cd203474526-f0f605963ec9474d-00

Expected Behavior

The Parent Id matches the Request Id of the parent.

Version Info

SDK Version (version of https://www.nuget.org/packages/Microsoft.ApplicationInsights.AspNetCore) : 2.8.0 .NET Core Version (TargetFramework in your .csproj file) : 3.0 or 2.2
How Application was onboarded with SDK(Installed Nugets/VisualStudio/Azure WebAppExtension) : nuget OS : Windows Hosting Info (IIS/Azure Web Apps/Running From Visual Studio etc) : Yes.

cijothomas commented 4 years ago

Can you share a repro app for us to investigate?

aarondandy commented 4 years ago

Sure, here is how I put together a minimum reproduction:

  1. Create a new aspnetcore webapp of some kind (2.2 or 3.0)
  2. Add a reference to Microsoft.ApplicationInsights.AspNetCore 2.8.0
  3. Set your AI key in the APPINSIGHTS_INSTRUMENTATIONKEY environment variable
  4. Do something that would be tracked as a dependency:
using var conn = new SqlConnection(connectionString);
var junk = await conn.QueryAsync("SELECT * FROM Pewp"); // Dapper, ADO.NET, EF core, doesn't matter

It seems like this does not affect all dependency types. I notice it with SQL and ServiceBus but HTTP seems OK. With 2.8.0 the parent ID values will contain the 00- prefix and suffix while on 2.7.1 it will match the Parent's ID with whatever the other format is. I see this issue in Azure as well as within Visual Studio.

cijothomas commented 4 years ago

@aarondandy Got it! Thanks for sharing!

For SQL the parent id is set to Activity.Id, which will be in new w3c format of 00. https://github.com/microsoft/ApplicationInsights-dotnet-server/blob/develop/Src/DependencyCollector/Shared/Implementation/SqlClientDiagnostics/SqlClientDiagnosticSourceListener.cs#L350

@lmolkova This should be fixed to set parentid in |trace.span. format https://github.com/microsoft/ApplicationInsights-dotnet-server/blob/develop/Src/Common/W3C/W3CUtilities.cs#L49?

mqtwinter commented 4 years ago

I just noticed this issue yesterday when looking at the github code and was left scratching my head.

What's in master right now:

telemetry.Context.Operation.ParentId = activity.ParentId;

What's in develop right now:

telemetry.Context.Operation.ParentId = activity.Id;

It looks like at some point someone found and fixed this bug.

(Sorry I'm not git/github expert, so maybe this was obvious to everyone else. I also don't know how to find exactly when/where this was fixed.)

aarondandy commented 4 years ago

Just to be clear, this is a problem on netcore 2.2 as well which is where I first noticed it.

cijothomas commented 4 years ago

@mqtwinter The actual file in master is: https://github.com/microsoft/ApplicationInsights-dotnet-server/blob/master/Src/DependencyCollector/Shared/Implementation/SqlClientDiagnostics/SqlClientDiagnosticSourceListener.cs#L350

This (https://github.com/microsoft/ApplicationInsights-dotnet-server/blob/master/Src/DependencyCollector/Shared/Implementation/SqlClientDiagnosticSourceListener.cs) is a leftover file somehow not deleted in a refactorings.

cijothomas commented 4 years ago

@aarondandy Yes the issue is arising from using new System.Diagnostics.DiagnosticSource, and switching to new W3C Format in Activity. (https://docs.microsoft.com/en-us/azure/azure-monitor/app/correlation#enable-w3c-distributed-tracing-support-for-aspnet-core-apps)

You can try the workaround by running the following snippet in your startup.cs configureservices.

Activity.DefaultIdFormat = ActivityIdFormat.Hierarchical;
  Activity.ForceDefaultIdFormat = true;
cijothomas commented 4 years ago

Tagging as bug. @lmolkova What are your thoughts on making a fix for this as a .1 hotfix release?

cijothomas commented 4 years ago

This will be fixed and released as a ".1" release.

aarondandy commented 4 years ago

Microsoft.ApplicationInsights.AspNetCore 2.8.1 seems to fix this issue.