Generate AotHelper properties at compile time
A couple of different options
Use built in attributes
The RuntimeFeature.IsDynamicCodeSupported Property looks like it might do what we want in many cases (I think we're using AotHelper.IsNativeAot where we should probably be checking for Trimming compatibility instead in some cases). Need to do a search for uses of AotHelper.IsNativeAot and a bit of a review of these.
Generate these at compile time
For this AotHelper, instead of:
https://github.com/getsentry/sentry-dotnet/blob/217c11cea611a5f26e4a1abfbb7df37f34695722/src/Sentry/Internal/AotHelper.cs#L18-L19
Could we instead write an attribute at compile time? I imagine it's a lot cheaper (worth measuring) than trying to get a stack trace on a static constructor:
Something like this: https://github.com/getsentry/sentry-dotnet/blob/217c11cea611a5f26e4a1abfbb7df37f34695722/src/Sentry/buildTransitive/Sentry.targets#L8-L29
Here's a PR that added writing the attribute at build time as a reference: https://github.com/getsentry/sentry-dotnet/pull/1739
Originally posted by @bruno-garcia in https://github.com/getsentry/sentry-dotnet/pull/2920#discussion_r1409457272
References
- More info available here: https://devblogs.microsoft.com/dotnet/creating-aot-compatible-libraries/
For reference: https://github.com/dotnet/runtime/issues/94380
I assume this would cover some previously edge cases where the SDK wrongly assumes NativeAOT on unsupported platforms (i.e. UWP)?
I assume this would cover some previously edge cases where the SDK wrongly assumes NativeAOT on unsupported platforms (i.e. UWP)?
I think initially when we were building AOT support, we identified and tried to avoid (at runtime) code that would throw errors when compiling AOT. It may well be trimming, rather than AOT, that prevents those code blocks from running however and we should probably change our runtime guards to check for trimming rather than AOT.
Currently if NET8_0_OR_GREATER we check to see if trimming is enabled and, if not, we assume IsNativeAot is also enabled. For previous runtimes we always set IsNativeAot to disabled but still check whether trimming is enabled:
https://github.com/getsentry/sentry-dotnet/blob/9f36d22f3b216351d5957b74e831e8b92636e6ba/src/Sentry/Internal/AotHelper.cs#L13-L34
I think that's a problem though. It's entirely possible to build net8.0 apps that are trimmed but not compiled AOT.
Then we have code like this: https://github.com/getsentry/sentry-dotnet/blob/233e95b5a2104819c735a29490c1aaf6b336f1fd/src/Sentry/Internal/DebugStackTrace.cs#L179-L181
So it doesn't execute some potentially problematic code if IsNativeAot is set. On net8.0 that kind of works because there is an equivalence between trimming being enabled and native aot being detected. However on UWP that code would be executed even when trimming was enabled - which I think is what causes our problems on UWP.
If I get that right then we could get around this issue on UWP by updating the check instead of having to rely on a dedicated package?
If I get that right then we could get around this issue on UWP by updating the check instead of having to rely on a dedicated package?
Possibly. We still wouldn't be able to hook up the unhandled exception handler automatically as that relies on reflection... but that's not a huge issue (people can copy/paste 6-7 lines of code to hook that up)... and I think there is some device information we can't capture without a dedicated UWP library since gathering that relies on Windows Native libraries.
Proposal - Source generator to read msbuild propertygroup values that can be read at runtime by the SentrySDK
#3313 would also benefit from this solution.
cc @jamescrosswell
Initial thoughts on how we could potentially do this via source generation - https://github.com/aritchie/BuildPropertyCapture
This will give us truth values for PublishTrimmed and PublishAot values. There are several issues at the moment which are listed in the readme of the project at the moment.
Initial thoughts on how we could potentially do this via source generation - https://github.com/aritchie/BuildPropertyCapture
Nice! We would want to vendor that into the sentry-dotnet repo (like we've done here and here) to minimise dependencies.
When I copy stuff from my own personal repos, I don't bother with an attribution.txt file etc. I've generally just left a comment in those cases.
I did it in a separate repo to prove it out. There are a few issues that still have to be figured out and are documented in the readme. Once it is finished, it would be moved to a sentry project
@jamescrosswell additional thoughts on this topic. Adding them here for historical context. What if we asked users to call a source generated UseSentryAot in cases where they really want AOT to work?
We can also technically suppress even generating the method if <PublishAot> isn't set to true. Thoughts?
That doesn't sound super reliable. Users could still AOT compile apps that called UseSentry rather than UseSentryAot - so we'd still need to include the current logic that disables certain features based on our runtime estimate of whether AOT compilation has been used or not... so I don't think this would buy us anything.
Possibly we just invest time and energy into trying to make those non-AOT compatible features AOT compatible instead (#3323 is the main one).
So we can use module initializers on non-mobile targets. We would only need these source generated versions for MAUI/ios/android/etc. Thoughts?
@bruno-garcia @Flash0ver fyi for additional comments/thoughts
I also think that (Incremental) Source Generators could be a great fit.
Either with a [ModuleInitializer] overwriting static IsTrimmed and IsAotCompiled properties.
Or alternatively, since .NET SDK 9.0.200 C# Interceptors are stable, which also enable Native AOT for ASP.NET Core Minimal APIs. We could generate GeneratedInit and GeneratedUseSentry as slim wrappers within file-local types, which additionally set e.g. SentryOptions accordingly, and intercept the Init and UseSentry invocations from user code.
Perhaps we could utilize partial methods, and now with C# 13 also partial properties, but I guess we don't want to require any explicit manual infrastructural plumbing in user code.
We could ship this Generator in the core package. This would increase the size of the core package slightly.
I haven't tried yet if Roslyn's AnalyzerConfigOptions also provide MSBuild options passed through the SDK (e.g. -p:PublishTrimmed=true or -p:PublishAot=true) ... but I guess it does.
@Flash0ver For context, most of this has already been achieved here: https://github.com/aritchie/BuildPropertyCapture - We've just been thinking about how best to initialize it when it comes to Android/iOS where module initializers do not operate.
My current thinking is source generating the dictionary to set values in the SentrySdk class.
SentrySdk.BuildProperties = new Dictionary<string, string>
{
{ "Property1": "Value" }
};
My current thinking is source generating the dictionary to set values in the SentrySdk class.
What would be calling that code though? When/where would SentrySdk.BuildProperties be initialised/populated? This is happening when the user is building their app. Static constructor on a partial class maybe (partial so that we can reference it from the code for the Sentry SDK)?
@jamescrosswell I actually didn't have any idea how to do initialize it without module initializers or a source generator UseSentryAot that we spoke about, BUT.... I just tested module initializers on both iOS & Android. They both work... so now we can give this a real trial