-
Notifications
You must be signed in to change notification settings - Fork 9
Enable native AoT compatibility #357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Mark Grafana.OpenTelemetry.Base as explicitly not AoT compatible due to extensive use of reflection. - Add AoT attribute to `ReflectionHelper` to move warnings to the callers of `CallStaticMethod()`. - Mark Grafana.OpenTelemetry as AoT compatible. Relates to #97.
Enable the use of collection expressions.
Mark various internal classes as `sealed`.
Use some modern C# features.
Attempt to enable native AoT compatibility by using `[DynamicDependency]` to have the trimmer retain the static methods that are called. Not yet tested in an actual native AoT application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enables native Ahead-of-Time (AoT) compilation compatibility for the Grafana.OpenTelemetry.Base and Grafana.OpenTelemetry packages targeting .NET 8.0 and above. The changes add necessary AoT metadata attributes while modernizing the codebase to use C# collection expression syntax.
Key changes:
- Adds
IsAotCompatibleproperty to project files for .NET 8.0+ targets - Introduces
DynamicDependencyandUnconditionalSuppressMessageattributes to preserve reflection-accessed members during trimming - Modernizes array initialization syntax using collection expressions throughout the codebase
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Build.props | Sets LangVersion to latest enabling modern C# features including collection expressions |
| src/Grafana.OpenTelemetry.Base/Grafana.OpenTelemetry.Base.csproj | Adds IsAotCompatible property for net8.0+ targets |
| src/Grafana.OpenTelemetry/Grafana.OpenTelemetry.csproj | Adds IsAotCompatible property for net8.0+ targets |
| src/Grafana.OpenTelemetry.Base/TrimWarnings.cs | New helper class centralizing trim warning constants for consistent AoT attribute usage |
| src/Grafana.OpenTelemetry.Base/ReflectionHelper.cs | Adds RequiresUnreferencedCode attribute and modernizes to collection expression syntax |
| src/Grafana.OpenTelemetry.Base/ResourceDetectors/*.cs | Adds sealed keyword and DynamicDependency attributes to detector initializers; converts to collection expressions |
| src/Grafana.OpenTelemetry.Base/Instrumentations/*.cs | Adds sealed keyword and DynamicDependency attributes to instrumentation initializers; converts to collection expressions |
| src/Grafana.OpenTelemetry.Base/GrafanaOpenTelemetryResourceDetector.cs | Adds sealed keyword and modernizes to collection expression syntax |
| src/Grafana.OpenTelemetry.Base/GrafanaOpenTelemetrySettings.cs | Converts array initialization to collection expression syntax |
| src/Grafana.OpenTelemetry.Base/ExporterSettings/GrafanaCloudConfigurationHelper.cs | Adds sealed keyword to class |
| tests/Grafana.OpenTelemetry.Tests/ReflectionHelperTest.cs | Updates test arrays to use collection expression syntax |
src/Grafana.OpenTelemetry.Base/Instrumentations/MySqlDataInitializer.cs
Outdated
Show resolved
Hide resolved
Fix copy-paste fail.
|
Doing some hacking around with the example app locally, this doesn't work. I removed MVC and EFCore from the sample app and then set
Without a radical redesign, that would undercut the "plug-and-play" design philosophy, I think the most reasonable path forward would be to instead just bubble the "not supported" nature all the way up to the top-most public API surface area that's relevant then we could mark the assembly as AoT-compatible. This wouldn't actually make the library AoT compatible, but it would make it explicit in the relevant code paths and users could potentially use compile-time checks to conditionally include the relevant code paths (or not) in the consuming apps. I'll have a quick look at that in a separate PR now to see if it's trivial enough to do and makes sense. If it's too viral, then we can just leave the explicit "not supported" from #97 present and document it as not supported. |
Changes
Possible approach to allow
Grafana.OpenTelemetry.Baseto work in a native AoT application.Extends #356. Not yet validated the trimmer retains all the appropriate functionality.
Relates to #97.
Merge requirement checklist
Unit tests added/updatedCHANGELOG.mdupdatedChanges in public API reviewed (if applicable)