Improve AoT validation and fix OpenTelemetryProtocol AoT warnings

[martincostello]

Fixes #5518

Changes

• Apply changes from Fix native AoT test scripts opentelemetry-dotnet-contrib#1540.
• Minor refactoring to make it easier to add net9.0 at a later date (see Add net9.0 to AoT test matrix #5519).
• Fix native Aot warnings in OpenTelemetry.Exporter.OpenTelemetryProtocol.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

/cc @eerhardt

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.60%. Comparing base (6250307) to head (be60873).
Report is 170 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5520      +/-   ##
==========================================
+ Coverage   83.38%   85.60%   +2.22%     
==========================================
  Files         297      289       -8     
  Lines       12531    12588      +57     
==========================================
+ Hits        10449    10776     +327     
+ Misses       2082     1812     -270     

Flag	Coverage Δ
unittests	?
unittests-Instrumentation-Experimental	24.14% <ø> (?)
unittests-Instrumentation-Stable	24.16% <ø> (?)
unittests-Solution-Experimental	85.53% <ø> (?)
unittests-Solution-Stable	85.55% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 62 files with indirect coverage changes

[martincostello]

I've pushed up a commit that resolves the AoT warnings - whether they're the correct fixes is a different matter, so happy to update them with the correct changes if needed.

[martincostello]

Test failure is dotnet/runtime#100766.

[eerhardt]

Fix native Aot warnings in OpenTelemetry.Exporter.OpenTelemetryProtocol.

Do we know why these warnings weren't being caught by the existing infrastructure? cc @Yun-Ting

[martincostello]

I wondered the same - my guess is that the filter used to kick off that job in CI doesn't cater for basically "any code or project change".

[eerhardt]

Do we know why these warnings weren't being caught by the existing infrastructure?

From doing a little bit of checking, it appears to be 2 things that aren't there today that could have caught this:

1. Today we are only checking if ($line -like "*analysis warning IL*"), but the AOT "warnings" are being treated as "errors". When I publish the app by itself, it emits Trim analysis error IL2091.
2. We were never checking that dotnet publish passed with exit code 0. But with this PR we are now.

So these checks should be fixing the problems in the current test.

[eerhardt]

Thanks, @martincostello, for catching this. The AOT compatibility test looks like it should catch errors going forward now.

[CodeBlanch]

@martincostello What code is generating these warnings is it DelegatingOptionsFactory?

[martincostello]

From #5518:

  LondonTravel.Skill failed with errors (45.7s) → artifacts\bin\LondonTravel.Skill\release\LondonTravel.Skill.dll
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`2<IConfiguration,!!0>): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`2<IConfiguration,!!0>)' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`2<IConfiguration,!!0>): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`2<IConfiguration,!!0>)' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.<>c__DisplayClass0_0`1.<RegisterOptionsFactory>b__0(IServiceProvider): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.<>c__DisplayClass0_0`1' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`4<IServiceProvider,IConfiguration,String,!!0>): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`4<IServiceProvider,IConfiguration,String,!!0>)' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`4<IServiceProvider,IConfiguration,String,!!0>): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.RegisterOptionsFactory<T>(IServiceCollection,Func`4<IServiceProvider,IConfiguration,String,!!0>)' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.<>c__DisplayClass1_0`1.<RegisterOptionsFactory>b__0(IServiceProvider): 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'T' of 'Microsoft.Extensions.DependencyInjection.DelegatingOptionsFactoryServiceCollectionExtensions.<>c__DisplayClass1_0`1' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]
    ILC : Trim analysis error IL2091: Microsoft.Extensions.Options.DelegatingOptionsFactory`1: 'TOptions' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in 'Microsoft.Extensions.Options.IOptionsFactory`1'. The generic parameter 'TOptions' of 'Microsoft.Extensions.Options.DelegatingOptionsFactory`1' 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:\Coding\martincostello\alexa-london-travel\src\LondonTravel.Skill\LondonTravel.Skill.csproj]

You can also see the errors here.

[martincostello]

Oh sorry, you meant the SYSLIB* warnings - I don't know - @eerhardt presumably found them locally when working on his commit.

[eerhardt]

These warnings are documented in https://learn.microsoft.com/en-us/dotnet/fundamentals/syslib-diagnostics/syslib1100-1118. The code that is emitting those warnings is from the Configuration Binder Source Generator. It is happening because the IOtlpExporterOptions public property is an interface, which can't be instantiated. So the config binder emits a warning saying "FYI - I can't create one of these objects".

opentelemetry-dotnet/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Builder/OtlpExporterBuilderOptions.cs

Lines 57 to 63 in c7c7a69

	public IOtlpExporterOptions DefaultOptions => this.DefaultOptionsInstance;
	public IOtlpExporterOptions LoggingOptions => this.LoggingOptionsInstance;
	public IOtlpExporterOptions MetricsOptions => this.MetricsOptionsInstance;
	public IOtlpExporterOptions TracingOptions => this.TracingOptionsInstance;

Here this is safe to suppress because we are creating instances of those objects in the ctor (which is then created through the DelegatingOptionsFactory:

opentelemetry-dotnet/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Builder/OtlpExporterBuilder.cs

Lines 183 to 198 in c7c7a69

	services!.RegisterOptionsFactory((sp, configuration, name) => new OtlpExporterBuilderOptions(
	configuration,
	/* Note: We don't use name for SdkLimitOptions. There should only be
	one provider for a given service collection so SdkLimitOptions is
	treated as a single default instance. */
	sp.GetRequiredService<IOptionsMonitor<SdkLimitOptions>>().CurrentValue,
	sp.GetRequiredService<IOptionsMonitor<ExperimentalOptions>>().Get(name),
	/* Note: We allow LogRecordExportProcessorOptions,
	MetricReaderOptions, & ActivityExportProcessorOptions to be null
	because those only exist if the corresponding signal is turned on.
	Currently this extension turns on all signals so they will always be
	there but that may change in the future so it is handled
	defensively. */
	sp.GetService<IOptionsMonitor<LogRecordExportProcessorOptions>>()?.Get(name),
	sp.GetService<IOptionsMonitor<MetricReaderOptions>>()?.Get(name),
	sp.GetService<IOptionsMonitor<ActivityExportProcessorOptions>>()?.Get(name)));

The way the source generated code works is this:

            if (AsConfigWithChildren(configuration.GetSection("DefaultOptions")) is IConfigurationSection section8)
            {
                global::OpenTelemetry.Exporter.IOtlpExporterOptions? temp10 = instance.DefaultOptions;
                if (temp10 is not null)
                {
                    BindCore(section8, ref temp10, defaultValueIfNotFound: false, binderOptions);
                }
                else
                {
                    throw new InvalidOperationException("Cannot create instance of type 'OpenTelemetry.Exporter.IOtlpExporterOptions' because it is missing a public instance constructor.");
                }
            }

It grabs the property first, and if it is a valid object, it binds the configuration to that object. If the object is null and someone set configuration for that object, it throws because an interface isn't instantiable.

[CodeBlanch]

@eerhardt Is there any way to suppress these closer to the thing causing the issue(s) rather than at the project level? I tried to do this...

#pragma warning disable SYSLIB1100, SYSLIB1101
        services!.Configure<OtlpExporterBuilderOptions>(name, configuration!);
#pragma warning restore SYSLIB1100, SYSLIB1101

...but it doesn't seem to work.

[eerhardt]

According to dotnet/runtime#88865 this is supposed to work, but doesn't for some reason.

I've opened dotnet/runtime#100785 for this issue. EDIT: this was a duplicate of dotnet/runtime#92509

[CodeBlanch]

LGTM

[martincostello]

Confirmed fixed in my original repro project with 1.8.1-alpha.0.13 from the prerelease feed.

@CodeBlanch Is it possible to get a stable patch release containing this fix?

[CodeBlanch]

@martincostello Can you give me a little info on the impact? Basically without this change OtlpExporter can't be used in applications deployed with AoT compile/trimming?

[martincostello]

That's correct - the application won't compile due to errors from the AoT compiler. The only workaround is to turn off trimming/native AoT.

[CodeBlanch]

@martincostello We discussed this on our SIG meeting yesterday. We're going to put out a 1.8.1 release including this fix and one other one. That other one is almost done, OK if this release happens in a day or two?

[martincostello]

Sure, that works for me. Thanks!

[martincostello]

@CodeBlanch Thanks for shipping the fix!