Suppress AOT analyzer warning in Redis instrumentation

[eerhardt]

The AOT analyzer warns about AOT incompatible code inside of a runtime check for RuntimeFeature.IsDynamicCodeSupported. See dotnet/linker#2715.

Suppress this warning until the AOT analyzer is fixed.

Although this repo doesn't have the AOT analyzer enabled yet, this code can be copied into other repos that do enable the analyzer - ex. dotnet/aspire. Porting this change into the otel source to make taking future updates easier. It will also be useful once this repo targets .NET 8 and enables the AOT analyzer.

@sebastienros @reyang @CodeBlanch @utpilla

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.00%. Comparing base (71655ce) to head (1fcdc23).
Report is 170 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1625      +/-   ##
==========================================
- Coverage   73.91%   71.00%   -2.92%     
==========================================
  Files         267        5     -262     
  Lines        9615      269    -9346     
==========================================
- Hits         7107      191    -6916     
+ Misses       2508       78    -2430     

Flag	Coverage Δ
unittests-Instrumentation.StackExchangeRedis	71.00% <ø> (?)

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

Files	Coverage Δ
...mentation/RedisProfilerEntryToActivityConverter.cs	38.26% <ø> (-41.54%)	⬇️

... and 261 files with indirect coverage changes

[Yun-Ting]

Hi @eerhardt, it seems this project is already included as one of the TrimmerRootAssembly for the AOT TestApp.
Could you help me understand why the AOT TestApp doesn't emit warnings from this project?

[eerhardt]

Hi @eerhardt, it seems this project is already included as one of the TrimmerRootAssembly for the AOT TestApp. Could you help me understand why the AOT TestApp doesn't emit warnings from this project?

The difference comes because of the 2 different analysis tools: https://devblogs.microsoft.com/dotnet/creating-aot-compatible-libraries/#analyzing-net-libraries. The case where this matters is the "Roslyn Analyzers" case, which has the bug I linked above (See dotnet/linker#2715). The Roslyn Analyzers don't understand/respect the if (RuntimeFeature.IsDynamicCodeSupported) check that goes around this code, but will once that bug is fixed.

The AotCompatibility.TestApp uses the 2nd approach in that article: "Publishing a test application for AOT", which DOES understand/respect the if (RuntimeFeature.IsDynamicCodeSupported) check.

So the scenario where this matters is when the Roslyn Analyzers are used on this code. This isn't the case in this repo, but this code is being copied to other repos ex. dotnet/aspire, which the Roslyn Analyzers are being used. Porting this change into the otel source to make taking future updates easier. It will also be useful once this repo targets .NET 8 and enables the AOT analyzer.

[Yun-Ting]

So the scenario where this matters is when the Roslyn Analyzers are used on this code. This isn't the case in this repo, but this code is being copied to other repos ex. dotnet/aspire, which the Roslyn Analyzers are being used. Porting this change into the otel source to make taking future updates easier. It will also be useful once this repo targets .NET 8 and enables the AOT analyzer.

Would it be possible to suppress the warnings in the repos which use Roslyn Analyzers and also consumes OpenTelemetry.Instrumentation.StackExchangeRedis until the bug is fixed?

[eerhardt]

Would it be possible to suppress the warnings in the repos which use Roslyn Analyzers

Yes. We are changing the "vendored"/"copied" code in the dotnet/aspire repo. See https://github.com/dotnet/aspire/pull/3104/files#diff-d4c41f6b9838aef61efa0008d9d139a76456adbbf35db653936dc21441427511R210.

This PR is porting this change back to the "main" copy of this code so it is easier to keep the copies in sync.

[CodeBlanch]

LGTM