microsoft / ApplicationInsights-dotnet

ApplicationInsights-dotnet
MIT License
565 stars 287 forks source link

Microsoft.Extensions.Logging.ApplicationInsights adds trace IDs a 2nd time to trace telemetry #2862

Open carlin-q-scott opened 7 months ago

carlin-q-scott commented 7 months ago

Describe the bug

The trace IDs (span, parent, trace) are included in all telemetry by default, but they’re also included in all log messages. So when we send log messages to app insights as traces, they’re on the trace telemetry twice. Both as custom properties, and system properties.

image

They're also on Exception telemetry emitted by the same library.

To Reproduce

Use the Microsoft.Extensions.Logging.ApplicationInsights package and log a warning and an exception.

Activity.Current= new Activity("test");
logger.LogWarning("test trace telemetry");
logger.LogWarning(new Exception("test"), "test Exception telemetry");
cijothomas commented 7 months ago

have you enabled scopes feature? it is likely that traceid and other duplicate info is coming from scopes.

carlin-q-scott commented 7 months ago

Yes, I have scopes enabled because I want my log scopes added to the telemetry.

cijothomas commented 7 months ago

Thanks for confirming! Very good chance that the the duplicate TraceId,SpanId are coming from scopes!

Are you enabling it yourself using this doc? Or it is coming automatically? https://learn.microsoft.com/en-us/aspnet/core/fundamentals/logging/?view=aspnetcore-7.0#automatically-log-scope-with-spanid-traceid-parentid-baggage-and-tags

Not much we can do about it in this repo now. I have seen similar issues reported in OpenTelemetry as well. There are some PRs in asp.net core where it is removing these things from scopes, so it won't end up being duplicated.

carlin-q-scott commented 7 months ago

@cijothomas I'm using the default options, which includes the activity tracing info in logger scope. I was hesitant to remove them via the startup options because maybe they're helpful for looking through the console logs if my telemetry isn't working. But that seems like a weak reason, so I think I will just disable those.

Not much we can do about it in this repo now.

Why is that? I figured the telemetry processor that adds the logger scopes to the telemetry could filter out the telemetry properties because those are clearly duplicate information. Or is that handled by something lower level in asp.net core?

cijothomas commented 7 months ago

Not much we can do about it in this repo now.

Why is that?

I meant to say this repo won't offer a new feature to selectively disable certain scope fields. It is mainly because of OpenTelemetry quickly becoming the instrumentation choice, and not much feature additions are occurring in this repo.

Probably possible to remove the duplicates yourself with a custom telemetry processor, if you can identify then reliably. (in your case, it looks like it should be possible, as you are looking to remove certain fields like traceid,spanid)