search

getsentry / sentry-dotnet

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

Enhanced Stack Frames with Native AOT #3323

Open jamescrosswell opened 3 weeks ago

jamescrosswell commented 3 weeks ago

Continuing on from https://github.com/getsentry/sentry-dotnet/pull/3322 we need to implement Michal's other suggetions:

In the end, someone will need to do the work to enable the AOT/trim/single file safety analyzers and address all the warnings, but this is enough to have a golden path that makes sample/StackTrace project produce the same results under native AOT as under JIT.

Necessary fixes:

GetMethodBody under AOT throws. This will not return anything useful under Mono with ILStrip either. The code simply needs to deal with it. Runtime reflection stack doesn't guarantee referential equality of MemberInfos (except for TypeInfo). This is also true on JIT-based runtimes, but there's even less caching on native AOT. Use operator ==. The reflection to read TupleElementNamesAttribute is trim-unfriendly and cannot be analyzed. Replace with no-reflection. Some fallout from the previous point because the nullable annotations on the thing returned by reflection were wrong.

Finally we should be able to simplify this: https://github.com/getsentry/sentry-dotnet/blob/233e95b5a2104819c735a29490c1aaf6b336f1fd/src/Sentry/Internal/DebugStackTrace.cs#L179-L181

bitsandfoxes commented 4 days ago

Just for my understanding - and to help prioritizing this, is this just about cleaning up the check? Or would we actually get better stack frames?

jamescrosswell commented 4 days ago

Just for my understanding - and to help prioritizing this, is this just about cleaning up the check? Or would we actually get better stack frames?

If we can get Ben.Demystifier working with AOT, we should get better stack frames yes... that code above could be changed to:

 var frames = (_options.StackTraceMode == StackTraceMode.Enhanced) 
     ? EnhancedStackTrace.GetFrames(stackTrace).Select(p => new RealStackFrame(p)) 
     : stackTrace.GetFrames().Select(p => new RealStackFrame(p));
bitsandfoxes commented 4 days ago

If we can get Ben.Demystifier working with AOT

I'm failing to keep track of how to get there. Is https://github.com/getsentry/sentry-dotnet/issues/3230 the only missing part?

jamescrosswell commented 4 hours ago

I'm failing to keep track of how to get there. Is #3230 the only missing part?

Yes I think so. I added some detail to that issue as well.