From 0a89f4a37d188822fe6a22cfab505a44cd3bcc9f Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 24 Oct 2022 15:26:57 -0700 Subject: [PATCH 1/2] Fix circular reference issue in LoggerProvider buildup. --- ...viderBuilderServiceCollectionExtensions.cs | 4 ++ .../Logs/Builder/LoggerProviderBuilderSdk.cs | 2 + .../Logs/ExportLogRecordProcessorOptions.cs | 3 + src/OpenTelemetry/Logs/LoggerProviderSdk.cs | 7 ++- .../LoggerProviderBuilderExtensionsTests.cs | 57 +++++++++++++++++-- 5 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry/Internal/Builder/ProviderBuilderServiceCollectionExtensions.cs b/src/OpenTelemetry/Internal/Builder/ProviderBuilderServiceCollectionExtensions.cs index ec643e62d50..c70d092df14 100644 --- a/src/OpenTelemetry/Internal/Builder/ProviderBuilderServiceCollectionExtensions.cs +++ b/src/OpenTelemetry/Internal/Builder/ProviderBuilderServiceCollectionExtensions.cs @@ -20,6 +20,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection.Extensions; using OpenTelemetry.Internal; +using OpenTelemetry.Logs; using OpenTelemetry.Metrics; using OpenTelemetry.Trace; @@ -31,6 +32,9 @@ public static IServiceCollection AddOpenTelemetryLoggerProviderBuilderServices(t { services.AddOpenTelemetryProviderBuilderServices(); + services.TryAddSingleton(); + services.RegisterOptionsFactory(configuration => new ExportLogRecordProcessorOptions(configuration)); + return services; } diff --git a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderSdk.cs b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderSdk.cs index 921b5524c40..398474e30a3 100644 --- a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderSdk.cs +++ b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderSdk.cs @@ -67,6 +67,8 @@ public LoggerProviderBuilderSdk() var services = new ServiceCollection(); services.AddOpenTelemetryLoggerProviderBuilderServices(); + services.AddSingleton( + sp => throw new NotSupportedException("External LoggerProvider created through Sdk.CreateLoggerProviderBuilder cannot be accessed using service provider.")); this.services = services; this.ownsServices = true; diff --git a/src/OpenTelemetry/Logs/ExportLogRecordProcessorOptions.cs b/src/OpenTelemetry/Logs/ExportLogRecordProcessorOptions.cs index e445599f51e..84a166f74a1 100644 --- a/src/OpenTelemetry/Logs/ExportLogRecordProcessorOptions.cs +++ b/src/OpenTelemetry/Logs/ExportLogRecordProcessorOptions.cs @@ -21,6 +21,9 @@ namespace OpenTelemetry.Logs; +/// +/// Options for configuring either a or . +/// public class ExportLogRecordProcessorOptions { private BatchExportLogRecordProcessorOptions? batchExportProcessorOptions; diff --git a/src/OpenTelemetry/Logs/LoggerProviderSdk.cs b/src/OpenTelemetry/Logs/LoggerProviderSdk.cs index ac321bfad13..1133553f820 100644 --- a/src/OpenTelemetry/Logs/LoggerProviderSdk.cs +++ b/src/OpenTelemetry/Logs/LoggerProviderSdk.cs @@ -21,6 +21,7 @@ using System.Diagnostics; using System.Text; using System.Threading; +using Microsoft.Extensions.DependencyInjection; using OpenTelemetry.Internal; using OpenTelemetry.Resources; @@ -49,6 +50,9 @@ public LoggerProviderSdk( { Debug.Assert(serviceProvider != null, "serviceProvider was null"); + var state = serviceProvider!.GetRequiredService(); + state.RegisterProvider(nameof(LoggerProvider), this); + OpenTelemetrySdkEventSource.Log.LoggerProviderSdkEvent("Building LoggerProviderSdk."); this.ServiceProvider = serviceProvider!; @@ -60,9 +64,6 @@ public LoggerProviderSdk( Debug.Assert(this.ownedServiceProvider != null, "ownedServiceProvider was null"); } - var state = new LoggerProviderBuilderState(serviceProvider!); - state.RegisterProvider(nameof(LoggerProvider), this); - CallbackHelper.InvokeRegisteredConfigureStateCallbacks( serviceProvider!, state); diff --git a/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs index 74a3cc54fc1..71a7b9f5cf6 100644 --- a/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/LoggerProviderBuilderExtensionsTests.cs @@ -140,7 +140,7 @@ public void LoggerProviderBuilderAddInstrumentationTest() using (var provider = Sdk.CreateLoggerProviderBuilder() .AddInstrumentation() - .AddInstrumentation((sp, provider) => new CustomInstrumentation(provider)) + .AddInstrumentation((sp, provider) => new CustomInstrumentation() { Provider = provider }) .Build() as LoggerProviderSdk) { Assert.NotNull(provider); @@ -160,6 +160,56 @@ public void LoggerProviderBuilderAddInstrumentationTest() Assert.True(((CustomInstrumentation)instrumentation[1]).Disposed); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void LoggerProviderNestedResolutionUsingBuilderTest(bool callNestedConfigure) + { + bool innerTestExecuted = false; + + using var provider = Sdk.CreateLoggerProviderBuilder() + .ConfigureServices(services => + { + if (callNestedConfigure) + { + services.ConfigureOpenTelemetryLogging(); + } + }) + .ConfigureBuilder((sp, builder) => + { + innerTestExecuted = true; + Assert.Throws(() => sp.GetService()); + }) + .Build(); + + Assert.True(innerTestExecuted); + + Assert.Throws(() => provider.GetServiceProvider()?.GetService()); + } + + [Fact] + public void LoggerProviderNestedResolutionUsingConfigureTest() + { + bool innerTestExecuted = false; + + var serviceCollection = new ServiceCollection(); + + serviceCollection.ConfigureOpenTelemetryLogging(builder => + { + builder.ConfigureBuilder((sp, builder) => + { + innerTestExecuted = true; + Assert.Throws(() => sp.GetService()); + }); + }); + + using var serviceProvider = serviceCollection.BuildServiceProvider(); + + var resolvedProvider = serviceProvider.GetRequiredService(); + + Assert.True(innerTestExecuted); + } + private sealed class CustomInstrumentation : IDisposable { public bool Disposed; @@ -169,11 +219,6 @@ public CustomInstrumentation() { } - public CustomInstrumentation(LoggerProvider provider) - { - this.Provider = provider; - } - public void Dispose() { this.Disposed = true; From 5bb03e55d98b097a5470c441b7ccd41e53b2c7a2 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 24 Oct 2022 15:34:00 -0700 Subject: [PATCH 2/2] Support loading ExportLogRecordProcessorOptions envvars from IConfiguration. --- ...atchExportLogRecordProcessorOptionsTest.cs | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 test/OpenTelemetry.Tests/Logs/BatchExportLogRecordProcessorOptionsTest.cs diff --git a/test/OpenTelemetry.Tests/Logs/BatchExportLogRecordProcessorOptionsTest.cs b/test/OpenTelemetry.Tests/Logs/BatchExportLogRecordProcessorOptionsTest.cs new file mode 100644 index 00000000000..aca94f68bd5 --- /dev/null +++ b/test/OpenTelemetry.Tests/Logs/BatchExportLogRecordProcessorOptionsTest.cs @@ -0,0 +1,132 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using Microsoft.Extensions.Configuration; +using Xunit; + +namespace OpenTelemetry.Logs.Tests +{ + public class BatchExportLogRecordProcessorOptionsTest : IDisposable + { + public BatchExportLogRecordProcessorOptionsTest() + { + ClearEnvVars(); + } + + public void Dispose() + { + ClearEnvVars(); + GC.SuppressFinalize(this); + } + + [Fact] + public void BatchExportProcessorOptions_Defaults() + { + var options = new BatchExportLogRecordProcessorOptions(); + + Assert.Equal(30000, options.ExporterTimeoutMilliseconds); + Assert.Equal(512, options.MaxExportBatchSize); + Assert.Equal(2048, options.MaxQueueSize); + Assert.Equal(5000, options.ScheduledDelayMilliseconds); + } + + [Fact] + public void BatchExportProcessorOptions_EnvironmentVariableOverride() + { + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey, "1"); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.MaxExportBatchSizeEnvVarKey, "2"); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.MaxQueueSizeEnvVarKey, "3"); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ScheduledDelayEnvVarKey, "4"); + + var options = new BatchExportLogRecordProcessorOptions(); + + Assert.Equal(1, options.ExporterTimeoutMilliseconds); + Assert.Equal(2, options.MaxExportBatchSize); + Assert.Equal(3, options.MaxQueueSize); + Assert.Equal(4, options.ScheduledDelayMilliseconds); + } + + [Fact] + public void ExportLogRecordProcessorOptions_UsingIConfiguration() + { + var values = new Dictionary() + { + [BatchExportLogRecordProcessorOptions.MaxQueueSizeEnvVarKey] = "1", + [BatchExportLogRecordProcessorOptions.MaxExportBatchSizeEnvVarKey] = "2", + [BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey] = "3", + [BatchExportLogRecordProcessorOptions.ScheduledDelayEnvVarKey] = "4", + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(values) + .Build(); + + var parentOptions = new ExportLogRecordProcessorOptions(configuration); + + Assert.Equal(1, parentOptions.BatchExportProcessorOptions.MaxQueueSize); + Assert.Equal(2, parentOptions.BatchExportProcessorOptions.MaxExportBatchSize); + Assert.Equal(3, parentOptions.BatchExportProcessorOptions.ExporterTimeoutMilliseconds); + Assert.Equal(4, parentOptions.BatchExportProcessorOptions.ScheduledDelayMilliseconds); + + var options = new BatchExportLogRecordProcessorOptions(configuration); + + Assert.Equal(1, options.MaxQueueSize); + Assert.Equal(2, options.MaxExportBatchSize); + Assert.Equal(3, options.ExporterTimeoutMilliseconds); + Assert.Equal(4, options.ScheduledDelayMilliseconds); + } + + [Fact] + public void BatchExportProcessorOptions_InvalidPortEnvironmentVariableOverride() + { + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey, "invalid"); + + Assert.Throws(() => new BatchExportLogRecordProcessorOptions()); + } + + [Fact] + public void BatchExportProcessorOptions_SetterOverridesEnvironmentVariable() + { + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey, "123"); + + var options = new BatchExportLogRecordProcessorOptions + { + ExporterTimeoutMilliseconds = 89000, + }; + + Assert.Equal(89000, options.ExporterTimeoutMilliseconds); + } + + [Fact] + public void BatchExportProcessorOptions_EnvironmentVariableNames() + { + Assert.Equal("OTEL_BLRP_EXPORT_TIMEOUT", BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey); + Assert.Equal("OTEL_BLRP_MAX_EXPORT_BATCH_SIZE", BatchExportLogRecordProcessorOptions.MaxExportBatchSizeEnvVarKey); + Assert.Equal("OTEL_BLRP_MAX_QUEUE_SIZE", BatchExportLogRecordProcessorOptions.MaxQueueSizeEnvVarKey); + Assert.Equal("OTEL_BLRP_SCHEDULE_DELAY", BatchExportLogRecordProcessorOptions.ScheduledDelayEnvVarKey); + } + + private static void ClearEnvVars() + { + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ExporterTimeoutEnvVarKey, null); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.MaxExportBatchSizeEnvVarKey, null); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.MaxQueueSizeEnvVarKey, null); + Environment.SetEnvironmentVariable(BatchExportLogRecordProcessorOptions.ScheduledDelayEnvVarKey, null); + } + } +}