Skip to content

Commit

Permalink
[main-1.8.0] Backport fixes for 1.8.1 core release (#5543)
Browse files Browse the repository at this point in the history
Co-authored-by: Martin Costello <martin@martincostello.com>
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Co-authored-by: Reiley Yang <reyang@microsoft.com>
  • Loading branch information
4 people authored Apr 17, 2024
1 parent fb74013 commit 34daa1f
Show file tree
Hide file tree
Showing 14 changed files with 217 additions and 16 deletions.
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
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md
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
11 changes: 9 additions & 2 deletions src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs
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

0 comments on commit 34daa1f

Please sign in to comment.