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

[logs] Mitigate unwanted object creation during configuration reload #5514

Merged
merged 19 commits into from
Apr 12, 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
1 change: 1 addition & 0 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,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
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
1 change: 1 addition & 0 deletions src/Shared/Options/DelegatingOptionsFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,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 @@ -62,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 @@ -7,6 +7,7 @@
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Xunit;

namespace OpenTelemetry.Logs.Tests;
Expand Down Expand Up @@ -297,11 +298,100 @@ public void CircularReferenceTest(bool requestLoggerProviderDirectly)
Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency);
}

private class TestLogProcessor : BaseProcessor<LogRecord>
[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<IConfiguration>(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<IOptionsMonitor<OpenTelemetryLoggerOptions>>();

Assert.NotNull(optionsMonitor.CurrentValue);
}

if (useOptionsSnapshot)
{
using var scope = sp.CreateScope();

var optionsSnapshot = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<OpenTelemetryLoggerOptions>>();

Assert.NotNull(optionsSnapshot.Value);
}

var loggerFactory = sp.GetRequiredService<ILoggerFactory>();

Assert.Equal(1, delegateInvocationCount);

root.Reload();

Assert.Equal(1, delegateInvocationCount);
}

[Fact]
public void MixedOptionsUsageTest()
{
var root = new ConfigurationBuilder().Build();

var services = new ServiceCollection();

services.AddSingleton<IConfiguration>(root);

services.AddLogging(logging => logging
.AddConfiguration(root.GetSection("logging"))
.AddOpenTelemetry(options =>
{
options.AddProcessor(new TestLogProcessor());
}));

using var sp = services.BuildServiceProvider();

var loggerFactory = sp.GetRequiredService<ILoggerFactory>();

var optionsMonitor = sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue;
var options = sp.GetRequiredService<IOptions<OpenTelemetryLoggerOptions>>().Value;

Assert.True(ReferenceEquals(options, optionsMonitor));

using var scope = sp.CreateScope();

var optionsSnapshot = scope.ServiceProvider.GetRequiredService<IOptionsSnapshot<OpenTelemetryLoggerOptions>>().Value;
Assert.True(ReferenceEquals(options, optionsSnapshot));
}

private sealed class TestLogProcessor : BaseProcessor<LogRecord>
{
public bool Disposed;

protected override void Dispose(bool disposing)
{
this.Disposed = true;

base.Dispose(disposing);
}
}

private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
private sealed class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
{
private readonly ILogger logger;

Expand Down