search

getsentry / sentry-dotnet

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

Configurable default behaviour for SentryStackTraceFactory.IsSystemModuleName #342

Closed dave-yotta closed 1 month ago

dave-yotta commented 4 years ago

Please mark the type framework used:

Please mark the type of the runtime used:

Please mark the NuGet packages used:

Took me a while to figure out what was going on. Added to SentryOptions.InAppInclude "Alloy" (our application prefix), however other modules (e.g. MongoDB) were still coming up as "InApp" in the events being sent to sentry.

This is because (confusingly) any module name not matched in InAppInclude or InAppExclude (you might say, any modules we do not recognise specifically) are counted as not system modules, i.e. InApp = true.

https://github.com/getsentry/sentry-dotnet/blob/9affdb5d8e6a38ead535770e64c0356d7a8a3f02/src/Sentry/Extensibility/SentryStackTraceFactory.cs#L201

I expected that when I told the SDK that I know where my app code is (using InAppInclude), it would not include anything else. I've had to add a BeforeSend hook that goes through all the frames and changes the value of InApp.

It would be possible to add a default behaviour in the SentryOptions like InAppDefault to be used when the other supplied conditions are not hit.

bruno-garcia commented 4 years ago

@dave-yotta Makes sense to me and that's a fair request.

The SDK has to have some sensible defaults so for the simple use-case of just initializing the SDK without providing any config, it's nice to be able to hide all System. Microsoft. etc and mark everything else as InApp.

We could introduce an option called: InAppExcludeByDefault to that it shows close to InAppExclude on intellisense and describing that it will mark frames that don't match the InAppInclude list will be marked as not in-app.

Would you be willing to contribute with a PR for this?

bitsandfoxes commented 1 month ago

I think this functionality is well covered and provided via https://github.com/getsentry/sentry-dotnet/blob/d955f6fb53464bd54b5378d441e458e426376b63/src/Sentry/SentryOptions.cs#L264 and https://github.com/getsentry/sentry-dotnet/blob/d955f6fb53464bd54b5378d441e458e426376b63/src/Sentry/SentryOptions.cs#L251 that accept a StringOrRegex, giving users control over what they consider as part of their app.

dave-yotta commented 1 month ago

Do you mean like this?

InAppInclude = "^AppNamespace.*$";
InAppExclude = "^(?!.*AppNamespace).*$";
jamescrosswell commented 1 month ago

Do you mean like this?

@dave-yotta you could do it like that, but there's some redundancy there. The code that decides whether something is inapp or not looks like this: https://github.com/getsentry/sentry-dotnet/blob/34b55980cd9f84d3922f80ff70c87b76d38ae01d/src/Sentry/SentryStackFrame.cs#L214-L216

Your InAppInclude is basically just checking whether the namespace starts with AppNamespace... If you use a string (rather than a regular expression) that's exactly what Sentry would check for anyway.

So we envisaged people would typically use it like this:

InAppInclude = "AppNamespace"; // Anything in the "AppNamespace" root namespace will be considered InApp
InAppExclude = new Regex(".*", RegexOptions.Compiled); // Matches everything... i.e. if it doesn't match the InAppInclude then it's not InApp