open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.21k stars 760 forks source link

.net6.0 Opentelemetry changes TraceId in WebApi Action method #3179

Closed vadimgudko closed 3 weeks ago

vadimgudko commented 2 years ago

Bug Report

Runtime version : net6.0

Symptom

I have a .net 6.0 Asp.net WebApi project. When it receives http request from i.e. console app traceId is getting changed: example in attached screen. Also uploaded simple project to demonstrate the issue https://github.com/vadimgudko/DiagnosticsNet6Test.

I expect to have same Activity TraceId in Console app, and in Activity.Current.Id and Activity.Current.ParentID of WebApi. In .net5.0 and netcore3.1 traceId is getting propagated correctly without change.

If I remove Opentelemetry dependency or comment "tracerProvider.Build();" traceId is getting propagated correctly image-2022-03-31-17-59-16-938

cijothomas commented 2 years ago

Is your client propagating trace parent header when it call server?

I'll check repro app anyway.

vadimgudko commented 2 years ago

yes, header traceparent header is getting sent image

cijothomas commented 2 years ago

Haven't investigated this yet. Couple of quick notes: using (var activity = new Activity("console job")) -- this is incorrect. You should use activitySource.StartActivity(), (using the new Activity() won't trigger OpenTelemetry :))

cijothomas commented 2 years ago

It does not look like you have enabled OpenTelemetry instrumentations in the projects, so what you see would be the default behaviors of ASP.NET Core itself, not OpenTelemetry...

vadimgudko commented 2 years ago

Tried with activitySource.StartActivity - still same bug. If I remove/disable OpenTelemetry- it works without bug, so ASP.NET Core itself works fine. I believe there is some trigger/eventlistener in OpenTelemetry which updates traceId to new value

cijothomas commented 2 years ago

Tried with activitySource.StartActivity - still same bug.

That was not the cause of this bug/behavior :) I just shared that as the correct way of starting activity.

Will check the repro in detail, to see if we can explain the behavior you are seeing.

cijothomas commented 2 years ago

@utpilla would you have time to look at this?

utpilla commented 2 years ago

The issue here is with AddSource("*") in the SDK setup of the web app as it interferes with the ASP.NET Core library's creation of activity on an incoming request.

Please refrain from using AddSource("*") or AddSource("Microsoft.AspNetCore) in the SDK setup of your web app.

vadimgudko commented 2 years ago

Thank you for response, but I'd like to have all traces including "Microsoft.AspNetCore" to be exported using Opentelemetry SDK. Is it possible that when OpenTelemetry 1.2.0 is released it would be fixed?

cijothomas commented 2 years ago

https://github.com/dotnet/aspnetcore/blob/9009ac30d4511649ccbcbe5060122fef063bef7d/src/Hosting/Hosting/src/WebHostBuilder.cs#L291 -- This is likely the root cause.

kirtisagar commented 2 years ago

@vadimgudko by adding the conditional compilation for .net6 could fix this issue.

cijothomas commented 2 years ago

@vishweshbankwar Could you investigate this? It looks like the issue is due to the fact that ASP.NET Core 6.0 onwards, it creates ActivitySource, but the instrumentation is not yet handling that special case.

vadimgudko commented 2 years ago

I modified my code to ignore Source "Microsoft.AspNetCore" traceId bug is fixed now but exported traces chain missing one node and chain is broken in zipkin. Instead of having: 1->2->3->4->5, I have: 1->2 4->5

vishweshbankwar commented 2 years ago

@vadimgudko - The issue that you are facing is explained here https://github.com/dotnet/aspnetcore/issues/37471. We do not have a fix for this yet. In your case I think default sampler is causing that. If you set your sampler to AlwaysOnSampler(), it should work as expected i.e. you would get the same traceIds.

You could use https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.AspNetCore instead to collect traces from your web api. However, you would still need to make sure to not add "Microsoft.AspNetCore" source in your code or else you would continue to see the same issue (assuming you use default sampler).

vadimgudko commented 2 years ago

AlwaysOnSampler() - fixed the issue, thanks a lot!

vishweshbankwar commented 2 years ago

Just a clarification note: The issue will surface for any sampler accessing traceid during sampling (explained in 2. of https://github.com/dotnet/aspnetcore/issues/37471)

github-actions[bot] commented 1 month ago

This issue was marked stale due to lack of activity and will be closed in 7 days. Commenting will instruct the bot to automatically remove the label. This bot runs once per day.

github-actions[bot] commented 3 weeks ago

Closed as inactive. Feel free to reopen if this issue is still a concern.