sentry-dotnet
sentry-dotnet copied to clipboard
getsentry
Transaction should be enriched from the client
A SentryClient can be used stand-alone. All event data is applied through the client.
This rule is broken for transactions:
https://github.com/getsentry/sentry-dotnet/blob/d25c97f0e5c77b520e30877444e93a9147afb9cd/src/Sentry/Internal/Hub.cs#L223
To fix this, we need to expose Scope as a parameter to CaptureTransaction(...), similarly to how it's done with CaptureEvent(...):
https://github.com/getsentry/sentry-dotnet/blob/d25c97f0e5c77b520e30877444e93a9147afb9cd/src/Sentry/SentryClient.cs#L105
https://github.com/getsentry/sentry-dotnet/blob/d25c97f0e5c77b520e30877444e93a9147afb9cd/src/Sentry/SentryClient.cs#L63
Transaction would then have to be able to extract the right scope to pass it here:
https://github.com/getsentry/sentry-dotnet/blob/49575e98544ec835fd6af1a70c97d14876461690/src/Sentry/TransactionTracer.cs#L228
This is technically a breaking change, but probably low severity, assuming nobody calls CaptureTransaction(...) directly or implements ISentryClient/IHub themselves.
Otherwise, we could maybe get the scope implicitly in CaptureTransaction(...), but I'm not sure what's the best way to do it. Also, it would introduce inconsistency with CaptureEvent(...).
Yeah the client should be getting a scope in order to merge it too, like it does for CaptureEvent already. Sadly it's a breaking change. This will affect only customers using the client directly and since they can't create transactions on it (only through SentrySdk.StartTransaction) this is low priority indeed.
Before waiting for 4.0.0, we should also validate whether scoped data (ex: tags) makes its way through to transactions at all. If not, that's a p1 bug to solve now.
