sentry-dotnet icon indicating copy to clipboard operation
sentry-dotnet copied to clipboard

Don't pass call from ILogger.BeginScope to SentrySdk.PushScope

Open bruno-garcia opened this issue 4 years ago • 10 comments
trafficstars

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.

bruno-garcia avatar Feb 21 '21 02:02 bruno-garcia

Idea: use weakref to track the scope at the time the exception is thrown, and associate them later.

Tyrrrz avatar Feb 22 '21 16:02 Tyrrrz

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.

CteixeiraPW avatar Nov 29 '21 22:11 CteixeiraPW

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 avatar Nov 29 '21 23:11 bruno-garcia

@Bruno-garcia do you have an estimated date for the release of version 4.x?

CteixeiraPW avatar Dec 06 '21 23:12 CteixeiraPW

@CteixeiraPW We don't have a ETA yet. Is this something you're willing to send a PR?

bruno-garcia avatar Dec 07 '21 13:12 bruno-garcia

Relates to #675

bruno-garcia avatar Dec 07 '21 15:12 bruno-garcia

Actually, related to #1492

mattjohnsonpint avatar May 10 '22 21:05 mattjohnsonpint