open-telemetry / opentelemetry-dotnet

The OpenTelemetry .NET Client
https://opentelemetry.io
Apache License 2.0
3.22k stars 764 forks source link

OpenTelemetry .NET SDK is not AOT safe #3429

Closed timmydo closed 1 year ago

timmydo commented 2 years ago

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:

Unhandled Exception: System.TypeInitializationException: A type initializer threw an exception. To determine which type, inspect the InnerException's StackTrace property.
 ---> System.TypeInitializationException: A type initializer threw an exception. To determine which type, inspect the InnerException's StackTrace property.
 ---> System.Reflection.MissingMetadataException: 'OpenTelemetry.Context.AsyncLocalRuntimeContextSlot<System.Int32>' is missing metadata. For more information, please visit http://go.microsoft.com/fwlink/?LinkID=392859
   at System.Reflection.Runtime.General.TypeUnifier.WithVerifiedTypeHandle(RuntimeConstructedGenericTypeInfo, RuntimeTypeInfo[]) + 0x15a
   at System.Reflection.Runtime.General.TypeUnifier.GetConstructedGenericTypeWithTypeHandle(RuntimeTypeInfo, RuntimeTypeInfo[]) + 0x3b
   at System.Reflection.Runtime.TypeInfos.RuntimeTypeInfo.MakeGenericType(Type[]) + 0x40d
   at OpenTelemetry.Context.RuntimeContext.RegisterSlot[T](String) + 0x161
   at OpenTelemetry.SuppressInstrumentationScope..cctor() + 0x1d
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x16a
   --- End of inner exception stack trace ---
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x26b
   at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnGCStaticBase(StaticClassConstructionContext*, Object) + 0x1c
   at OpenTelemetry.SuppressInstrumentationScope.get_IsSuppressed() + 0x1b
   at OpenTelemetry.Sdk.get_SuppressInstrumentation() + 0x15
   at OpenTelemetry.Logs.OpenTelemetryLoggerProvider..cctor() + 0x10
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x16a
   --- End of inner exception stack trace ---
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x26b
   at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnNonGCStaticBase(StaticClassConstructionContext*, IntPtr) + 0x1c
   at OpenTelemetry.Logs.OpenTelemetryLoggerProvider..ctor(IOptionsMonitor`1) + 0x29

libraries:

    <PackageReference Update="OpenTelemetry" Version="[1.2.0-rc3]" />
    <PackageReference Update="OpenTelemetry.Api" Version="[1.2.0-rc3]" />
    <PackageReference Update="OpenTelemetry.Exporter.Console" Version="[1.2.0-rc3]" />
    <PackageReference Update="OpenTelemetry.Exporter.Prometheus" Version="[1.2.0-rc3]" />

more information: https://docs.microsoft.com/en-us/dotnet/core/deploying/native-aot

eerhardt commented 1 year ago

@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 https://github.com/dotnet/aspnetcore/issues/45910. Ensuring Open Telemetry works in these NativeAOT ASP.NET apps is part of this goal.

The issue is this following code:

https://github.com/open-telemetry/opentelemetry-dotnet/blob/a878ca3eead7d1b84c3e4c379c81b665b553c41b/src/OpenTelemetry.Api/Context/RuntimeContext.cs#L33-L55

This is not NativeAOT-compatible since it using Reflection to MakeGenericType on the ContextSlotType. The NativeAOT compiler has no idea that it needs to generate native code for the AsyncLocalRuntimeContextSlot<int> type. And thus we get an exception at runtime above.

You also get warnings about this method when you publish with -p:PublishAot=true.

ILC : Trim analysis warning IL2055: OpenTelemetry.Context.RuntimeContext.RegisterSlot<T>(String): Call to 'System.Type.MakeGenericType(Type[])' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type. [C:\DotNetTest\Net8Api\Net8Api.csproj]
ILC : AOT analysis warning IL3050: OpenTelemetry.Context.RuntimeContext.RegisterSlot<T>(String): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\DotNetTest\Net8Api\Net8Api.csproj]
ILC : Trim analysis warning IL2075: OpenTelemetry.Context.RuntimeContext.RegisterSlot<T>(String): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors' in call to 'System.Type.GetConstructor(Type[])'. The return value of method 'OpenTelemetry.Context.RuntimeContext.ContextSlotType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\DotNetTest\Net8Api\Net8Api.csproj]

What can we do to solve this? We need to not use MakeGenericType, and instead have a static call to the RuntimeContextSlot<T> that we are trying to create.

Do we really need the full extensibility of public static Type ContextSlotType { get; set; }? Would it be acceptable to remove that API and instead have a static enum that could be set for the 3 types of context slots? Then RegisterSlot could look something like:

    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;
            }
        }

cc @LakshanF @agocke @tarekgh

cijothomas commented 1 year ago

Tagging for consideration in 1.5.0

reyang commented 1 year ago

+1, I think OpenTelemetry .NET 1.5 should be AOT safe.

agocke commented 1 year ago

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:

<IsTrimmable>true</IsTrimmable>
<EnableAotAnalyzer>true</EnableAotAnalyzer>

This should catch the vast majority of trimming and AOT problems.

eerhardt commented 1 year ago

and compiling for >= .NET 6

Note that most Open Telemetry libraries don't target net6 today. Most of them just target netstandard2.0.

Also for EnableAotAnalyzer, you will need to target net7.0 instead, because more APIs were annotated with RequiresDynamicCode in net7.0.

eerhardt commented 1 year ago

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

ILC : Trim analysis warning IL2026: OpenTelemetry.Exporter.ConsoleTagTransformer.TransformArrayTag(String,Array): Using member 'System.Text.Json.JsonSerializer.Serialize<Array>(Array,JsonSerializerOptions)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved. [C:\DotNetTest\Net8Api\Net8Api.csproj]
ILC : AOT analysis warning IL3050: OpenTelemetry.Exporter.ConsoleTagTransformer.TransformArrayTag(String,Array): Using member 'System.Text.Json.JsonSerializer.Serialize<Array>(Array,JsonSerializerOptions)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. JSON serialization and deserialization might require types that cannot be statically analyzed and might need runtime code generation. Use System.Text.Json source generation for native AOT applications. [C:\DotNetTest\Net8Api\Net8Api.csproj]

Coming from this line of code: https://github.com/open-telemetry/opentelemetry-dotnet/blob/d0829fff2f229b67c884526cb4943b99f6c75f29/src/OpenTelemetry.Exporter.Console/ConsoleTagTransformer.cs#L37-L38

reyang commented 1 year ago

@agocke @eerhardt Thanks for the help here! Here is a tiny PR #4191.

reyang commented 1 year ago

What is the timeline for OpenTelemetry 1.5? Is it scheduled before November 2023 – by the time .NET 8 ships?

Yes.

cijothomas commented 1 year ago

What is the timeline for OpenTelemetry 1.5? Is it scheduled before November 2023 – by the time .NET 8 ships?

Yes.

We plan to ship 1.5 in next 3 months (june), followed by 1.6 (around Oct). https://github.com/open-telemetry/opentelemetry-dotnet/milestone/36

utpilla commented 1 year ago

@Yun-Ting would be working on this issue.

alanwest commented 1 year ago

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.

TheXenocide commented 1 year ago

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.

eerhardt commented 1 year ago

@Yun-Ting @reyang - can this issue now be closed? I believe all the OpenTelemetry libraries in this repo are now AOT-compatible.

Yun-Ting commented 1 year ago

@eerhardt thanks for keeping track of this. @utpilla, could you help in closing this issue? Thank you.

missisjana commented 1 year ago

I am getting an error when using

        <PackageReference Include="OpenTelemetry" Version="1.6.0" />
        <PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.6.0" />

Error:

Unknown error in export method: {0}{System.TypeInitializationException: A type initializer threw an exception. To determine which type, inspect the InnerException's StackTrace property.
 ---> System.PlatformNotSupportedException: Dynamic code generation is not supported on this platform.
   at System.Reflection.Emit.ReflectionEmitThrower.ThrowPlatformNotSupportedException() + 0x2b
   at OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.MetricItemExtensions.CreateRepeatedFieldOfMetricSetCountAction() + 0x8b
   at OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.MetricItemExtensions..cctor() + 0x6e
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0xb9
   --- End of inner exception stack trace ---
   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0xb0
   at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnGCStaticBase(StaticClassConstructionContext*, Object) + 0xd
   at OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.MetricItemExtensions.AddMetrics(ExportMetricsServiceRequest request, Resource processResource, Batch`1& metrics) + 0x11a
   at OpenTelemetry.Exporter.OtlpMetricExporter.Export(??? metrics) + 0xd9}
eerhardt commented 1 year ago

@Yun-Ting @reyang - it looks like we missed annotating the OpenTelemetry.Exporter.OpenTelemetryProtocol library:

https://github.com/open-telemetry/opentelemetry-dotnet/blob/fe78453c03feb8dbe506b2a0284312bdfa1367c5/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj#L16-L31

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.

eerhardt commented 1 year ago

I did a quick check of the warnings from this library:

C:\git\opentelemetry-dotnet\src\OpenTelemetry.Exporter.OpenTelemetryProtocol\Implementation\ActivityExtensions.cs(305): AOT analysis warning IL3050: OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ActivityExtensions.CreateRepeatedFieldOfSpanSetCountAction(): Using member 'System.Reflection.Emit.DynamicMethod.DynamicMethod(String,Type,Type[],Module,Boolean)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Creating a DynamicMethod requires dynamic code. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
C:\git\opentelemetry-dotnet\src\OpenTelemetry.Exporter.OpenTelemetryProtocol\Implementation\MetricItemExtensions.cs(426): AOT analysis warning IL3050: OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.MetricItemExtensions.CreateRepeatedFieldOfMetricSetCountAction(): Using member 'System.Reflection.Emit.DynamicMethod.DynamicMethod(String,Type,Type[],Module,Boolean)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Creating a DynamicMethod requires dynamic code. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\FieldDescriptor.cs(448): Trim analysis warning IL2075: Google.Protobuf.Reflection.FieldDescriptor.CreateAccessor(): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'Google.Protobuf.Reflection.MessageDescriptor.ClrType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonFormatter.cs(915): Trim analysis warning IL2070: Google.Protobuf.JsonFormatter.OriginalEnumValueHelper.GetNameMapping(Type): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Reflection.TypeInfo.DeclaredFields.get'. The parameter 'enumType' of method 'Google.Protobuf.JsonFormatter.OriginalEnumValueHelper.GetNameMapping(Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\SingleFieldAccessor.cs(90): Trim analysis warning IL2072: Google.Protobuf.Reflection.SingleFieldAccessor.SingleFieldAccessor(PropertyInfo,FieldDescriptor): 'name' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Reflection.RuntimeReflectionExtensions.GetRuntimeProperty(Type,String)'. The return value of method 'System.Reflection.MemberInfo.DeclaringType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\SingleFieldAccessor.cs(96): Trim analysis warning IL2072: Google.Protobuf.Reflection.SingleFieldAccessor.SingleFieldAccessor(PropertyInfo,FieldDescriptor): 'name' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Reflection.RuntimeReflectionExtensions.GetRuntimeMethod(Type,String,Type[])'. The return value of method 'System.Reflection.MemberInfo.DeclaringType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\SingleFieldAccessor.cs(112): Trim analysis warning IL2072: Google.Protobuf.Reflection.SingleFieldAccessor.SingleFieldAccessor(PropertyInfo,FieldDescriptor): 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The return value of method 'System.Reflection.PropertyInfo.PropertyType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\ReflectionUtil.cs(126): AOT analysis warning IL3050: Google.Protobuf.Reflection.ReflectionUtil.CreateExtensionHelper(Extension): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\OneofDescriptor.cs(177): Trim analysis warning IL2075: Google.Protobuf.Reflection.OneofDescriptor.CreateAccessor(String): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperty(String)'. The return value of method 'Google.Protobuf.Reflection.MessageDescriptor.ClrType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\OneofDescriptor.cs(186): Trim analysis warning IL2075: Google.Protobuf.Reflection.OneofDescriptor.CreateAccessor(String): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'Google.Protobuf.Reflection.MessageDescriptor.ClrType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]
T:\src\github\protobuf\csharp\src\Google.Protobuf\Reflection\ReflectionUtil.cs(135): AOT analysis warning IL3050: Google.Protobuf.Reflection.ReflectionUtil.GetReflectionHelper(Type,Type): Using member 'System.Type.MakeGenericType(Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime. [C:\git\opentelemetry-dotnet\test\OpenTelemetry.AotCompatibility.TestApp\OpenTelemetry.AotCompatibility.TestApp.csproj]

This includes the error above from @missisjana.

@JamesNK addressed all the Google.Protobuf warnings as of <PackageReference Include="Google.Protobuf" Version="3.22.0" />. If we update our reference to that version, those all go away, and we are only left with the 2 from the OpenTelemetry.Exporter.OpenTelemetryProtocol library.

cc @vitek-karas

eerhardt commented 1 year ago

I've opened https://github.com/open-telemetry/opentelemetry-dotnet/pull/4859 to address this.