search

getsentry / sentry-xamarin

Sentry for Xamarin Native and Xamarin.Forms
https://docs.sentry.io/platforms/dotnet/guides/xamarin/
44 stars 11 forks source link

Line numbers for Android #134

Closed mattjohnsonpint closed 1 year ago

mattjohnsonpint commented 1 year ago

With Sentry.Xamarin 1.4.6, we are using Sentry 3.26.2, which (when configured) will upload symbols to Sentry during a release build. This part appears to work fine.

In testing, I found that filenames and line numbers are appearing for iOS Xamarin apps now, but not for Android Xamarin apps.

I believe this is because we aren't sending any of the DebugImage information in the stack frames with the exceptions. We do this for Android on MAUI (or any net6.0-android or net7.0-android app), but not for traditional Xamarin apps.

It should be possible to fix this, either by sharing source code from Sentry .NET (DebugStackTrace, AndroidAssemblyReader, etc.) or by modifying the code in Sentry to detect Android environment at runtime instead of compiling it only for net6.0-android.

mattjohnsonpint commented 1 year ago

Upstream issue: https://github.com/getsentry/sentry-dotnet/issues/2126

lucas-zimerman commented 1 year ago

If it doesn't require a lot of changes, sharing the source code could ideal, unless the environment detection doesn't have a huge impact in performance for the other platforms.

mattjohnsonpint commented 1 year ago

@vaind - thoughts?

mattjohnsonpint commented 1 year ago

I've been doing some experimentation, and I think a good approach would be to split Sentry.Android.AssemblyReader namespace out to its own Nuget package, targeting netstandard2.0. We'd then take a dependency on that from both Sentry/net6.0-android and Sentry.Xamarin/monodroid9.0.

(I did try just creating a monodroid9.0 target of Sentry, but that's not possible with the dotnet SDK).

To share the code, we can add SentryOptions.AssemblyReader, of type Func<string, PEReader?>?, and wire it to the Android assembly reader during initialization.

I think the assembly reader should be in its own GitHub repository also, so it can rev versions independently. That will also move the apk tests so they don't need to run on every Sentry build. Any objections?

vaind commented 1 year ago

Sounds good. Not sure if a split to another repo is necessary though. Is there currently only a single nuget package shipped from the sentry-dotnet repo?

mattjohnsonpint commented 1 year ago

No, there are several. Just they all version and deploy together by default. I can probably work around that though. I'll give it a try. 👍

vaind commented 1 year ago

Do we need different versions though? Maybe it's ok to push even though there were no changes. I guess that also happens for other packages