Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/verifyaotcompat.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand All @@ -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 }}

1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
1 change: 1 addition & 0 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down
26 changes: 17 additions & 9 deletions build/test-aot-compatibility.ps1
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
5 changes: 5 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public OpenTelemetryLoggerOptions SetResourceBuilder(ResourceBuilder resourceBui
Guard.ThrowIfNull(resourceBuilder);

this.ResourceBuilder = resourceBuilder;

return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
{
Expand Down Expand Up @@ -249,7 +256,7 @@ private static ILoggingBuilder AddOpenTelemetryInternal(

return new OpenTelemetryLoggerProvider(
provider,
sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value,
disposeProvider: false);
}));

Expand Down
8 changes: 8 additions & 0 deletions src/Shared/Options/DelegatingOptionsFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
{
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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!;
}
}
47 changes: 47 additions & 0 deletions src/Shared/Options/SingletonOptionsManager.cs
Original file line number Diff line number Diff line change
@@ -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()
{
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>$(TargetFrameworksForAotCompatibilityTests)</TargetFramework>
<TargetFrameworks>$(TargetFrameworksForAotCompatibilityTests)</TargetFrameworks>
<PublishAot>true</PublishAot>
<TrimmerSingleWarn>false</TrimmerSingleWarn>
<SelfContained>true</SelfContained>
Expand Down
Loading
Loading