diff --git a/.github/workflows/verifyaotcompat.yml b/.github/workflows/verifyaotcompat.yml index 6a599bb53..e9beff96d 100644 --- 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 index 5a6450697..43d0bdd2b 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -28,6 +28,7 @@ --> + diff --git a/OpenTelemetry.sln b/OpenTelemetry.sln index c9a9a4c3f..7af79a3f8 100644 --- 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 index 37483488f..39d365203 100644 --- a/build/test-aot-compatibility.ps1 +++ b/build/test-aot-compatibility.ps1 @@ -1,24 +1,35 @@ 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) @@ -26,10 +37,7 @@ 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 index f4d9f9ffd..9f07dab60 100644 --- 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 index 8f89cb125..c10ab504d 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj @@ -8,6 +8,9 @@ disable BUILDING_INTERNAL_PERSISTENT_STORAGE;$(DefineConstants) + true + + $(NoWarn);SYSLIB1100;SYSLIB1101 @@ -19,6 +22,7 @@ + diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index f372be6be..4995ad027 100644 --- 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 index 15575f1c7..06b3478e5 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggerOptions.cs @@ -117,6 +117,7 @@ public class OpenTelemetryLoggerOptions Guard.ThrowIfNull(resourceBuilder); this.ResourceBuilder = resourceBuilder; + return this; } diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 6d7afc1ee..6fdf92441 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -173,6 +173,13 @@ public static class OpenTelemetryLoggingExtensions // 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(); + /* Note: This ensures IConfiguration is available when using * IServiceCollections NOT attached to a host. For example when * performing: @@ -192,7 +199,7 @@ public static class OpenTelemetryLoggingExtensions var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder( (sp, logging) => { - var options = sp.GetRequiredService>().CurrentValue; + var options = sp.GetRequiredService>().Value; if (options.ResourceBuilder != null) { @@ -249,7 +256,7 @@ public static class OpenTelemetryLoggingExtensions return new OpenTelemetryLoggerProvider( provider, - sp.GetRequiredService>().CurrentValue, + sp.GetRequiredService>().Value, disposeProvider: false); })); diff --git a/src/Shared/Options/DelegatingOptionsFactory.cs b/src/Shared/Options/DelegatingOptionsFactory.cs index 315b38caa..1b8fe6218 100644 --- a/src/Shared/Options/DelegatingOptionsFactory.cs +++ b/src/Shared/Options/DelegatingOptionsFactory.cs @@ -16,6 +16,9 @@ #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 . /// /// The type of options being requested. +#if NET6_0_OR_GREATER +internal sealed class DelegatingOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] TOptions> : +#else internal sealed class DelegatingOptionsFactory : +#endif IOptionsFactory where TOptions : class { @@ -74,6 +81,7 @@ internal sealed class DelegatingOptionsFactory : public TOptions Create(string name) { TOptions options = this.optionsFactoryFunc(this.configuration, name); + foreach (IConfigureOptions setup in _setups) { if (setup is IConfigureNamedOptions namedSetup) diff --git a/src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs b/src/Shared/Options/DelegatingOptionsFactoryServiceCollectionExtensions.cs index 1140478f7..69c7b6c3b 100644 --- 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( +#endif this IServiceCollection services, Func optionsFactoryFunc) where T : class @@ -33,7 +40,11 @@ internal static class DelegatingOptionsFactoryServiceCollectionExtensions return services!; } +#if NET6_0_OR_GREATER + public static IServiceCollection RegisterOptionsFactory<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>( +#else public static IServiceCollection RegisterOptionsFactory( +#endif this IServiceCollection services, Func optionsFactoryFunc) where T : class @@ -51,6 +62,22 @@ internal static class DelegatingOptionsFactoryServiceCollectionExtensions sp.GetServices>()); }); + return services!; + } + +#if NET6_0_OR_GREATER + public static IServiceCollection DisableOptionsReloading<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>( +#else + public static IServiceCollection DisableOptionsReloading( +#endif + this IServiceCollection services) + where T : class + { + Debug.Assert(services != null, "services was null"); + + services!.TryAddSingleton, SingletonOptionsManager>(); + services!.TryAddScoped, SingletonOptionsManager>(); + return services!; } } diff --git a/src/Shared/Options/SingletonOptionsManager.cs b/src/Shared/Options/SingletonOptionsManager.cs new file mode 100644 index 000000000..c1807183e --- /dev/null +++ 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, IOptionsSnapshot +#else +internal sealed class SingletonOptionsManager : IOptionsMonitor, IOptionsSnapshot +#endif + where TOptions : class +{ + private readonly TOptions instance; + + public SingletonOptionsManager(IOptions 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 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 index bf5db5a04..0b7dffb58 100644 --- a/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj +++ b/test/OpenTelemetry.AotCompatibility.TestApp/OpenTelemetry.AotCompatibility.TestApp.csproj @@ -2,7 +2,7 @@ Exe - $(TargetFrameworksForAotCompatibilityTests) + $(TargetFrameworksForAotCompatibilityTests) true false true diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index cdebc61f1..e0c59ca7b 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -7,6 +7,7 @@ using System.Reflection; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Xunit; namespace OpenTelemetry.Logs.Tests; @@ -297,11 +298,100 @@ public sealed class OpenTelemetryLoggingExtensionsTests Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency); } - private class TestLogProcessor : BaseProcessor + [Theory] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void OptionReloadingTest(bool useOptionsMonitor, bool useOptionsSnapshot) { + var delegateInvocationCount = 0; + + var root = new ConfigurationBuilder().Build(); + + var services = new ServiceCollection(); + + services.AddSingleton(root); + + services.AddLogging(logging => logging + .AddConfiguration(root.GetSection("logging")) + .AddOpenTelemetry(options => + { + delegateInvocationCount++; + + options.AddProcessor(new TestLogProcessor()); + })); + + using var sp = services.BuildServiceProvider(); + + if (useOptionsMonitor) + { + var optionsMonitor = sp.GetRequiredService>(); + + Assert.NotNull(optionsMonitor.CurrentValue); + } + + if (useOptionsSnapshot) + { + using var scope = sp.CreateScope(); + + var optionsSnapshot = scope.ServiceProvider.GetRequiredService>(); + + Assert.NotNull(optionsSnapshot.Value); + } + + var loggerFactory = sp.GetRequiredService(); + + Assert.Equal(1, delegateInvocationCount); + + root.Reload(); + + Assert.Equal(1, delegateInvocationCount); } - private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor + [Fact] + public void MixedOptionsUsageTest() + { + var root = new ConfigurationBuilder().Build(); + + var services = new ServiceCollection(); + + services.AddSingleton(root); + + services.AddLogging(logging => logging + .AddConfiguration(root.GetSection("logging")) + .AddOpenTelemetry(options => + { + options.AddProcessor(new TestLogProcessor()); + })); + + using var sp = services.BuildServiceProvider(); + + var loggerFactory = sp.GetRequiredService(); + + var optionsMonitor = sp.GetRequiredService>().CurrentValue; + var options = sp.GetRequiredService>().Value; + + Assert.True(ReferenceEquals(options, optionsMonitor)); + + using var scope = sp.CreateScope(); + + var optionsSnapshot = scope.ServiceProvider.GetRequiredService>().Value; + Assert.True(ReferenceEquals(options, optionsSnapshot)); + } + + private sealed class TestLogProcessor : BaseProcessor + { + public bool Disposed; + + protected override void Dispose(bool disposing) + { + this.Disposed = true; + + base.Dispose(disposing); + } + } + + private sealed class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor { private readonly ILogger logger;