Add option to use Ben.Demystifier for cleaner and less confusing stack traces

getsentry / sentry-unity

Development of Sentry SDK for Unity

https://docs.sentry.io/platforms/unity/

MIT License

208 stars 52 forks source link

Add option to use Ben.Demystifier for cleaner and less confusing stack traces #675

Open oddgames-david opened 2 years ago

oddgames-david commented 2 years ago

Would be nice to clean up stacktraces using this library (or you guys make plugin architecture for us to override stacktraces?)

bitsandfoxes commented 2 years ago

That's an interesting suggestion, thanks for raising this! Especially since the .NET SDK already has Ben.Demystifier as a submodule. I'll look into how that would work and get back to you.

bitsandfoxes commented 2 years ago

I think I stumbled upon the reason why we're not already doing that: https://github.com/getsentry/sentry-unity/blob/248ca0c4d7ddf6d24185bad92825d036795182b6/src/Sentry.Unity/SentryUnityOptions.cs#L113

oddgames-david commented 2 years ago

Backtrace a unity owned product is using it. Maybe that comment is old?Get Outlook for Android

bitsandfoxes commented 2 years ago

You could try to use the ScriptableConfig to change the StacktraceMode from StackTraceMode.Original to Enhanced (which is the default for the .NET SDK). We would expect this to work on Mono. Let us know what you get on your targetplatform - maybe we can reconsider our approach.

vaind commented 2 years ago

@oddgames-david did you end up setting StackTraceMode.Enhanced mode with mono? What was your experience with the stack traces?

bruno-garcia commented 1 week ago

Ben.Demystifer is available in the Sentry .NET SDK for many years. When we built the Unity SDK, and compiled with IL2CPP. It didn't work.

So events from Mono and IL2CPP can be grouped together, we opt'ed to always use the built-in stack trace format (aka default).

Since then we added Native AOT support on the Sentry SDK for .NET. @jamescrosswell made some changes to Ben.Demystifier to work on AOT. So it's possible we could make this work on IL2CPP now.

jamescrosswell commented 1 week ago

Since then we added Native AOT support on the Sentry SDK for .NET. @jamescrosswell made some changes to Ben.Demystifier to work on AOT. So it's possible we could make this work on IL2CPP now.

Not quite true... I added an issue for us to look into making Ben.Demystifier work better with AOT:

https://github.com/getsentry/sentry-dotnet/issues/3323

That work hasn't been done yet though so Ben.Demystifier is still disabled when using AOT compilation currently.

oddgames-david commented 1 week ago

Just want to let you know that it's OK if the stack traces differ between Mono and IL2CPP for my use case (perhaps you can put a warning in). Even between platforms it can be managable.

bruno-garcia commented 5 days ago

Just want to let you know that it's OK if the stack traces differ between Mono and IL2CPP for my use case (perhaps you can put a warning in). Even between platforms it can be managable.

That's fair, and it's what we did in .NET at the end. Since JIT apps will have demystifier, and AOT ones won't.

There are some PRs from @MichalStrehovsky which we could bring to our fork. It's already 11 commits ahead as we've done some improvements over the years.

@oddgames-david is that something you would be interested in contributing?