sentry-dotnet
sentry-dotnet copied to clipboard
Don't pass call from ILogger.BeginScope to SentrySdk.PushScope
We've added a nice API to add data to an event through the instance of the exception.
To extend that to add attachments, we have the problem that attachments live on the Scope. And the code pulling data from the Exception only has SentryEvent, and no scope.
The API such as WithScope doesn't work properly on ASP.NET Core due to "scope lock" which was a work around to keep breadcrumbs around even though the ILogger interface BeginScope mapped to the SDK PushScope. To avoid dropping these breadcrumbs. Ideally we'd simply not map BeingScope to PushScope and simply abandon "scope lock". The negative side effect here is that adding an attachment with WithScope with a locked scope actually means the attachment never goes away, and each new call adds yet another attachment.
The unified API also includes overloads to the public API to pass a local scope, or mutate a clone. Such as:
Sentry.CaptureException(exception, s => s.SetTag() which isn't in .NET yet since lots of use cases in .NET are done through the logger integration anyway: BeginScope and structured logging.
Idea: use weakref to track the scope at the time the exception is thrown, and associate them later.
Hello @bruno-garcia Do you guys have an idea when this enhancement will be released?
We have some standards in my organization about when we must create an entry to the logs. For us is required to create a new log entry in every write operation to the DB. After using "WithScope" to include some data/tags and create a new log entry about this operation, the following log entries include the same data (we don't want that to happen). We want to use sentry to replace our internal logger (an internal app) and our exception handler for over 100 applications (WebForms, ASP .NET 5, React). So far this issue is what is keeping us waiting to move forward with you product.
I think the simplest solution for this is to remove the call from ILogger.BeginScope to SentrySdk.PushScope.
And remove the ScopeLock thing after that.
Possibly this could be done and shipped as part of the version 4.0.0 of the SDK. Combined with other changes. @SimonCropp thoughts?
@Bruno-garcia do you have an estimated date for the release of version 4.x?
@CteixeiraPW We don't have a ETA yet. Is this something you're willing to send a PR?
Relates to #675
Actually, related to #1492