[AOT] Annotate APIs which call IServiceCollection.TryAddSingleton for trim compatibility

[vitek-karas]

Towards #3429

IServiceCollection.TryAddSingleton<T> has trim annotation on T, this needs to be propagated to the caller to make it trim compatible.

This change just propagates the annotation to the signature of the method calling the TryAddSingleton. The trim tooling will make sure that the annotation is fulfilled by the caller of the OTel public methods with this change.

Note that these changes don't affect the outcome of the EnsureAotCompatibility test. This is because that test uses .NET 7 AOT compiler which has a bug and it doesn't report the warning it should have in these cases. Running the same test as a Trim compatibility test instead (/p:PublishAot=false /p:PublishTrimmed=true) will show warnings for each of the change in this PR. The .NET 8 version of the AOT compiler is fixed and will also report these warnings, so we need to fix these for both trimming and AOT compatibility.

I'd be open to adding a EnsureTrimCompatibility test, but it will make CI slower as it takes similar amount of time to the AOT compatibility test which is already causing CI slowdowns. @Yun-Ting has been discussing possibility of moving the AOT test into its own CI leg, adding the trim test there would be much less disruptive.

Merge requirement checklist

• CONTRIBUTING guidelines followed (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)

@Yun-Ting @eerhardt

[utpilla]

@Yun-Ting has been discussing possibility of moving the AOT test into its own CI leg, adding the trim test there would be much less disruptive.

+1

[codecov]

Codecov Report

Merging #4688 (3f47f4d) into main (13bd341) will increase coverage by 0.07%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head 3f47f4d differs from pull request most recent head 9012de6. Consider uploading reports for the commit 9012de6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4688      +/-   ##
==========================================
+ Coverage   85.11%   85.18%   +0.07%     
==========================================
  Files         314      314              
  Lines       12735    12735              
==========================================
+ Hits        10839    10848       +9     
+ Misses       1896     1887       -9     

Files Changed	Coverage Δ
...endencyInjectionLoggerProviderBuilderExtensions.cs	100.00% <ø> (ø)
...pendencyInjectionMeterProviderBuilderExtensions.cs	100.00% <ø> (ø)
...endencyInjectionTracerProviderBuilderExtensions.cs	100.00% <ø> (ø)
...ry/Logs/Builder/LoggerProviderBuilderExtensions.cs	97.91% <ø> (ø)
.../Metrics/Builder/MeterProviderBuilderExtensions.cs	83.76% <ø> (ø)
...y/Trace/Builder/TracerProviderBuilderExtensions.cs	98.79% <ø> (ø)

... and 6 files with indirect coverage changes

[vitek-karas]

I created #4690 to track the work make a new CI leg for AOT/trim compatibility tests.

[utpilla]

The trim tooling will make sure that the annotation is fulfilled by the caller of the OTel public methods with this change.

@vitek-karas Could you explain a bit more as to what this means for the users of our APIs?

[vitek-karas]

The trim tooling will make sure that the annotation is fulfilled by the caller of the OTel public methods with this change.

@vitek-karas Could you explain a bit more as to what this means for the users of our APIs?

For users which don't trim/AOT their app there's no effect at all (the attribute is completely ignored by compiler and runtime).
For users which do trim/AOT the trimmer/AOT-compiler will check what is the type passed to the annotated T. If it's a specific type, the tool will preserve the necessary parts and there's nothing for the user to do - it just works. If it's another generic type (so somebody has a generic helper) they would get a warning and would need to propagate the annotation same way I did in this change. For example:

void RegisterInstrumentation<TInstrumentation>(LoggerProviderBuilder builder)
{
    builder.AddInstrumentation<TInstrumentation>(); // IL#### - the annotation on TInstrumentation doesn't match those required by AddInstrumentation
}

Fixed:

void RegisterInstrumentation<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructor)] TInstrumentation>(LoggerProviderBuilder builder)
{
    builder.AddInstrumentation<TInstrumentation>(); // no warning
}

This is a standard behavior of many other APIs which are annotated with the DynamicallyAccessedMembers attribute, so it's likely that users which do trim/AOT would already be familiar with how it works.

[vitek-karas]

The test failure is a timed out AOT publish test. I ran it locally multiple times and it passes just fine. It also passed on Linux (in 1.5 minutes, on Windows it timed out after 4 minutes).