getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
590 stars 206 forks source link

SentryScope.Transaction should always be AsyncLocal #3590

Closed jamescrosswell closed 1 month ago

jamescrosswell commented 2 months ago

Originally posted by @jamescrosswell in #3546

Problem

When IsGlobalModeEnabled == true, spans from the UI can get mixed up with transactions in background services, and vice versa.

Originally Global Mode was added so that tags and context that get set in the UI also get applied to exceptions that get thrown in background threads. This makes sense for Events/Exceptions. However, unless they share an AsyncLocal context, it doesn't make sense for spans from one thread to end up under transactions that were created in another thread.

Solution

The "Current" transaction should always be stored as an AsyncLocal, regardless of what kind of scope stack manager is configured for the application more generally.

Compatibility with OpenTelemetry

This would be the same as the behaviour for OpenTelemetry traces, since Activity.Current is declared as static AsyncLocal<Activity>.

bruno-garcia commented 1 month ago

Compatibility with OpenTelemetry

GlobalMode is used on mobile and desktop and I'm not sure how popular, or even if it's compatible, with OTel. I mean I'm sure desktop is, but Mobile I'm not sure.