[main-1.8.0] Backport fixes for 1.8.1 core release (#5543)

CodeBlanch · martincostello · eerhardt · web-flow · commit 34daa1f96fc5 · 2024-04-17T14:17:49.000-07:00
Co-authored-by: Martin Costello &lt;martin@martincostello.com&gt;
Co-authored-by: Eric Erhardt &lt;eric.erhardt@microsoft.com&gt;
Co-authored-by: Reiley Yang &lt;reyang@microsoft.com&gt;
diff --git a/.github/workflows/verifyaotcompat.yml b/.github/workflows/verifyaotcompat.yml
@@ -11,7 +11,7 @@ jobs:
     strategy:
       fail-fast: false # ensures the entire test matrix is run, even if one permutation fails
       matrix:
-        os: [ ubuntu-latest ]
+        os: [ ubuntu-latest, windows-latest ]
         version: [ net8.0 ]
 
     runs-on: ${{ matrix.os }}
@@ -24,4 +24,3 @@ jobs:
     - name: publish AOT testApp, assert static analysis warning count, and run the app
       shell: pwsh
       run: .\build\test-aot-compatibility.ps1 ${{ matrix.version }}
-
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -28,6 +28,7 @@
     -->
 
     <PackageVersion Include="Microsoft.Extensions.Configuration" Version="8.0.0" />
+    <PackageVersion Include="Microsoft.Extensions.Configuration.Binder" Version="8.0.1" />
     <PackageVersion Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Extensions.Diagnostics.Abstractions" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Extensions.Hosting.Abstractions" Version="8.0.0" />
diff --git a/OpenTelemetry.sln b/OpenTelemetry.sln
@@ -299,6 +299,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Options", "Options", "{4949
 	ProjectSection(SolutionItems) = preProject
 		src\Shared\Options\DelegatingOptionsFactory.cs = src\Shared\Options\DelegatingOptionsFactory.cs
 		src\Shared\Options\DelegatingOptionsFactoryServiceCollectionExtensions.cs = src\Shared\Options\DelegatingOptionsFactoryServiceCollectionExtensions.cs
+		src\Shared\Options\SingletonOptionsManager.cs = src\Shared\Options\SingletonOptionsManager.cs
 	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shims", "Shims", "{A0CB9A10-F22D-4E66-A449-74B3D0361A9C}"
diff --git a/build/test-aot-compatibility.ps1 b/build/test-aot-compatibility.ps1
@@ -1,35 +1,43 @@
 param([string]$targetNetFramework)
 
 $rootDirectory = Split-Path $PSScriptRoot -Parent
-$publishOutput = dotnet publish $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj -nodeReuse:false /p:UseSharedCompilation=false /p:ExposeExperimentalFeatures=true
+$publishOutput = dotnet publish $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj --framework $targetNetFramework -nodeReuse:false /p:UseSharedCompilation=false /p:ExposeExperimentalFeatures=true
 
 $actualWarningCount = 0
 
 foreach ($line in $($publishOutput -split "`r`n"))
 {
-    if ($line -like "*analysis warning IL*")
+    if (($line -like "*analysis warning IL*") -or ($line -like "*analysis error IL*"))
     {
         Write-Host $line
-
         $actualWarningCount += 1
     }
 }
 
-pushd $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/bin/Release/$targetNetFramework/linux-x64
+Write-Host "Actual warning count is:", $actualWarningCount
+$expectedWarningCount = 0
+
+if ($LastExitCode -ne 0)
+{
+    Write-Host "There was an error while publishing AotCompatibility Test App. LastExitCode is:", $LastExitCode
+    Write-Host $publishOutput
+}
+
+$runtime = $IsWindows ? "win-x64" : ($IsMacOS ? "macos-x64" : "linux-x64")
+$app = $IsWindows ? "./OpenTelemetry.AotCompatibility.TestApp.exe" : "./OpenTelemetry.AotCompatibility.TestApp"
+
+Push-Location $rootDirectory/test/OpenTelemetry.AotCompatibility.TestApp/bin/Release/$targetNetFramework/$runtime
 
 Write-Host "Executing test App..."
-./OpenTelemetry.AotCompatibility.TestApp
+$app
 Write-Host "Finished executing test App"
 
 if ($LastExitCode -ne 0)
 {
   Write-Host "There was an error while executing AotCompatibility Test App. LastExitCode is:", $LastExitCode
 }
 
-popd
-
-Write-Host "Actual warning count is:", $actualWarningCount
-$expectedWarningCount = 0
+Pop-Location
 
 $testPassed = 0
 if ($actualWarningCount -ne $expectedWarningCount)
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+* Fix native AoT warnings in `OpenTelemetry.Exporter.OpenTelemetryProtocol`.
+  ([#5520](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5520))
+
 ## 1.8.0
 
 Released 2024-Apr-02
diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj
@@ -8,6 +8,9 @@
     <!-- this is temporary. will remove in future PR. -->
     <Nullable>disable</Nullable>
     <DefineConstants>BUILDING_INTERNAL_PERSISTENT_STORAGE;$(DefineConstants)</DefineConstants>
+    <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
+    <!-- SYSLIB1100;SYSLIB1101 - Configuration.Binder: can't create instance and unsupported type -->
+    <NoWarn>$(NoWarn);SYSLIB1100;SYSLIB1101</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
@@ -19,6 +22,7 @@
     <PackageReference Include="Grpc" Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == '$(NetFrameworkMinimumSupportedVersion)'" />
     <PackageReference Include="Google.Protobuf" />
     <PackageReference Include="Grpc.Tools" PrivateAssets="All" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />
   </ItemGroup>
 
   <ItemGroup>
diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+* Fixed an issue in Logging where unwanted objects (processors, exporters, etc.)
+  could be created inside delegates automatically executed by the Options API
+  during configuration reload.
+  ([#5514](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5514))
+
 ## 1.8.0
 
 Released 2024-Apr-02
diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs
@@ -117,6 +117,7 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui
         Guard.ThrowIfNull(resourceBuilder);
 
         this.ResourceBuilder = resourceBuilder;
+
         return this;
     }
 
diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs
@@ -173,6 +173,13 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
         // Note: This will bind logger options element (e.g., "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions
         RegisterLoggerProviderOptions(services);
 
+        // Note: We disable built-in IOptionsMonitor and IOptionsSnapshot
+        // features for OpenTelemetryLoggerOptions as a workaround to prevent
+        // unwanted objects (processors, exporters, etc.) being created by
+        // configuration delegates being re-run during reload of IConfiguration
+        // or from options created while under scopes.
+        services.DisableOptionsReloading<OpenTelemetryLoggerOptions>();
+
         /* Note: This ensures IConfiguration is available when using
          * IServiceCollections NOT attached to a host. For example when
          * performing:
@@ -192,7 +199,7 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
         var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder(
             (sp, logging) =>
             {
-                var options = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue;
+                var options = sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value;
 
                 if (options.ResourceBuilder != null)
                 {
@@ -249,7 +256,7 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
 
                     return new OpenTelemetryLoggerProvider(
                         provider,
-                        sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
+                        sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value,
                         disposeProvider: false);
                 }));
 
diff --git a/src/Shared/Options/DelegatingOptionsFactory.cs b/src/Shared/Options/DelegatingOptionsFactory.cs
@@ -16,6 +16,9 @@ example of how that works.
 #nullable enable
 
 using System.Diagnostics;
+#if NET6_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
 using Microsoft.Extensions.Configuration;
 
 namespace Microsoft.Extensions.Options;
@@ -24,7 +27,11 @@ namespace Microsoft.Extensions.Options;
 /// Implementation of <see cref="IOptionsFactory{TOptions}"/>.
 /// </summary>
 /// <typeparam name="TOptions">The type of options being requested.</typeparam>
+#if NET6_0_OR_GREATER
+internal sealed class DelegatingOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> :
+#else
 internal sealed class DelegatingOptionsFactory<TOptions> :
+#endif
     IOptionsFactory<TOptions>
     where TOptions : class
 {
@@ -74,6 +81,7 @@ public DelegatingOptionsFactory(
     public TOptions Create(string name)
     {
         TOptions options = this.optionsFactoryFunc(this.configuration, name);
+
         foreach (IConfigureOptions<TOptions> setup in _setups)
         {
             if (setup is IConfigureNamedOptions<TOptions> namedSetup)
diff --git a/src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs b/src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs
@@ -4,6 +4,9 @@
 #nullable enable
 
 using System.Diagnostics;
+#if NET6_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Options;
@@ -12,7 +15,11 @@ namespace Microsoft.Extensions.DependencyInjection;
 
 internal static class DelegatingOptionsFactoryServiceCollectionExtensions
 {
+#if NET6_0_OR_GREATER
+    public static IServiceCollection RegisterOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>(
+#else
     public static IServiceCollection RegisterOptionsFactory<T>(
+#endif
         this IServiceCollection services,
         Func<IConfiguration, T> optionsFactoryFunc)
         where T : class
@@ -33,7 +40,11 @@ public static IServiceCollection RegisterOptionsFactory<T>(
         return services!;
     }
 
+#if NET6_0_OR_GREATER
+    public static IServiceCollection RegisterOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>(
+#else
     public static IServiceCollection RegisterOptionsFactory<T>(
+#endif
         this IServiceCollection services,
         Func<IServiceProvider, IConfiguration, string, T> optionsFactoryFunc)
         where T : class
@@ -51,6 +62,22 @@ public static IServiceCollection RegisterOptionsFactory<T>(
                 sp.GetServices<IValidateOptions<T>>());
         });
 
+        return services!;
+    }
+
+#if NET6_0_OR_GREATER
+    public static IServiceCollection DisableOptionsReloading<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>(
+#else
+    public static IServiceCollection DisableOptionsReloading<T>(
+#endif
+        this IServiceCollection services)
+        where T : class
+    {
+        Debug.Assert(services != null, "services was null");
+
+        services!.TryAddSingleton<IOptionsMonitor<T>, SingletonOptionsManager<T>>();
+        services!.TryAddScoped<IOptionsSnapshot<T>, SingletonOptionsManager<T>>();
+
         return services!;
     }
 }
diff --git a/src/Shared/Options/SingletonOptionsManager.cs b/src/Shared/Options/SingletonOptionsManager.cs
@@ -0,0 +1,47 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+#nullable enable
+
+#if NET6_0_OR_GREATER
+using System.Diagnostics.CodeAnalysis;
+#endif
+
+namespace Microsoft.Extensions.Options;
+
+#if NET6_0_OR_GREATER
+internal sealed class SingletonOptionsManager<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> : IOptionsMonitor<TOptions>, IOptionsSnapshot<TOptions>
+#else
+internal sealed class SingletonOptionsManager<TOptions> : IOptionsMonitor<TOptions>, IOptionsSnapshot<TOptions>
+#endif
+    where TOptions : class
+{
+    private readonly TOptions instance;
+
+    public SingletonOptionsManager(IOptions<TOptions> options)
+    {
+        this.instance = options.Value;
+    }
+
+    public TOptions CurrentValue => this.instance;
+
+    public TOptions Value => this.instance;
+
+    public TOptions Get(string? name) => this.instance;
+
+    public IDisposable? OnChange(Action<TOptions, string?> listener)
+        => NoopChangeNotification.Instance;
+
+    private sealed class NoopChangeNotification : IDisposable
+    {
+        private NoopChangeNotification()
+        {
+        }
+
+        public static NoopChangeNotification Instance { get; } = new();
+
+        public void Dispose()
+        {
+        }
+    }
+}
diff --git a/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj b/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj
@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>$(TargetFrameworksForAotCompatibilityTests)</TargetFramework>
+    <TargetFrameworks>$(TargetFrameworksForAotCompatibilityTests)</TargetFrameworks>
     <PublishAot>true</PublishAot>
     <TrimmerSingleWarn>false</TrimmerSingleWarn>
     <SelfContained>true</SelfContained>
diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs

Original file line number	Diff line number	Diff line change
`@@ -117,6 +117,7 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui`
`117`	`117`	`Guard.ThrowIfNull(resourceBuilder);`
`118`	`118`
`119`	`119`	`this.ResourceBuilder = resourceBuilder;`
	`120`	`+`
`120`	`121`	`return this;`
`121`	`122`	`}`
`122`	`123`