-
Notifications
You must be signed in to change notification settings - Fork 765
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
OpenTelemetry .NET SDK is not AOT safe #3429
Comments
@reyang @cijothomas @CodeBlanch - I ran into this issue as well trying to use Open Telemetry in an ASP.NET app. We want to enable ASP.NET + NativeAOT in .NET 8.0. See dotnet/aspnetcore#45910. Ensuring Open Telemetry works in these NativeAOT ASP.NET apps is part of this goal. The issue is this following code: opentelemetry-dotnet/src/OpenTelemetry.Api/Context/RuntimeContext.cs Lines 33 to 55 in a878ca3
This is not NativeAOT-compatible since it using Reflection to MakeGenericType on the You also get warnings about this method when you publish with
What can we do to solve this? We need to not use MakeGenericType, and instead have a static call to the Do we really need the full extensibility of public enum ContextSlotType
{
AsyncLocal,
ThreadLocal,
Remoting
}
...
public static RuntimeContextSlot<T> RegisterSlot<T>(string slotName)
{
Guard.ThrowIfNullOrEmpty(slotName);
lock (Slots)
{
if (Slots.ContainsKey(slotName))
{
throw new InvalidOperationException($"Context slot already registered: '{slotName}'");
}
RuntimeContextSlot<T> slot = ContextSlotType switch
{
ContextSlotType.AsyncLocal => new AsyncLocalRuntimeContextSlot<T>(slotName),
ContextSlotType.ThreadLocal => new ThreadLocalRuntimeContextSlot<T>(slotName),
#if NETFRAMEWORK
ContextSlotType.Remoting => new RemotingRuntimeContextSlot<T>(slotName),
#endif
_ => throw new NotSupportedException($"{ContextSlotType} is not supported."),
};
Slots[slotName] = slot;
return slot;
}
} |
Tagging for consideration in 1.5.0 |
+1, I think OpenTelemetry .NET 1.5 should be AOT safe. |
Hey all -- I own Native AOT and would love to help here, so let me know if you have any questions. I agree with @eerhardt that our general recommendation is to try to fix AOT and trimming warnings by removing areas where runtime code generation or reflection are not strictly necessary. If you want to be confident that you're trimming and AOT compatible, I would suggest adding the following properties to your project file and compiling for >= .NET 6:
This should catch the vast majority of trimming and AOT problems. |
Note that most Open Telemetry libraries don't target net6 today. Most of them just target netstandard2.0. Also for |
What is the timeline for OpenTelemetry 1.5? Is it scheduled before November 2023 – by the time .NET 8 ships? Note that there are other warnings in other Open Telemetry than just the one discussed in that issue. Another one I see in my Console exporter app is
Coming from this line of code: opentelemetry-dotnet/src/OpenTelemetry.Exporter.Console/ConsoleTagTransformer.cs Lines 37 to 38 in d0829ff
|
Yes. |
We plan to ship 1.5 in next 3 months (june), followed by 1.6 (around Oct). |
@Yun-Ting would be working on this issue. |
I've moved this from the 1.5.0 milestone. We're continuing to make progress on AOT support, but we will not be able to have full support in the 1.5 timeframe. We still anticipate full support in time for supporting .NET 8. |
The trimming tool can't tell which type Expression.Property(Expression, string propertyName) refers to, so it isn't able to preserve the referenced property. The workaround is to explicitly reference the PropertyInfo from the static type, which the trimmer can see and knows to preserve the property. Contributes to open-telemetry#3429
- ConfigurationBinder.GetValue uses Reflection to bind IConfiguration values to strongly typed objects. ConfigurationExtensions.TryGetStringValue was using ConfigurationBinder to get a string value from IConfiguration, which is causing a warning. However, IConfiguration values are already strings, so this is unnecessary. It is also not performant because calling ConfigurationBinder allocates objects and uses TypeDescriptor. - Additionally, suppress 2 EventSource warnings while I'm making changes. Contributes to open-telemetry#3429
With AOT supported in .NET 8, will OpenTelemetry support work out-of-box for Blazor Client and Server? Currently there appears to be a gap here with regard to App Insights support. |
This ensures that the OpenTelemetry.Instrumentation.EventCounters library stays AOT-compatible. Contributes to open-telemetry/opentelemetry-dotnet#3429
I am getting an error when using
Error:
|
@Yun-Ting @reyang - it looks like we missed annotating the OpenTelemetry.Exporter.OpenTelemetryProtocol library: Lines 16 to 31 in fe78453
This is a pretty important library. We are going to need it in order to use OTLP. This issue shouldn't be closed until that is addressed. |
I did a quick check of the warnings from this library:
This includes the error above from @missisjana. @JamesNK addressed all the cc @vitek-karas |
This addresses the AOT warnings in the last library in this opentelemetry-dotnet. 1. Google.Protobuf v3.19.4 isn't trimming and AOT compatible. We need to upgrade to a newer version. I chose the latest patch in v3.22.x. v3.22.0 was the first version that is AOT compatible. 2. There were 2 places in Exporter.OpenTelemetryProtocol that was using System.Reflection.Emit to set a private field on Protobuf's RepeatedField class in order to return it to a pool. Setting this private field is no longer necessary since v3.22.0 because the Clear method was updated to support this scenario. See protocolbuffers/protobuf@a4fd216 Fix open-telemetry#3429
I've opened #4859 to address this. |
Bug Report
When publishing my app with
dotnet publish --self-contained -p:PublishAot=true -r win-x64 -o out
and then running it, the app crashes on startup with an open telemetry stack:libraries:
more information: https://docs.microsoft.com/en-us/dotnet/core/deploying/native-aot
The text was updated successfully, but these errors were encountered: