From f20670851e71fe4c8c628480dfb254eeb8ecd59d Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Wed, 29 Nov 2023 16:59:34 -0800 Subject: [PATCH 01/28] draft --- .../.publicApi/Stable/PublicAPI.Unshipped.txt | 1 + .../ILogger/OpenTelemetryLoggingExtensions.cs | 67 ++++++++++++++++--- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt index e69de29bb2d..b895503c61e 100644 --- a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -0,0 +1 @@ +static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder) -> Microsoft.Extensions.Logging.ILoggingBuilder! diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 1dfbd6fe408..45d57e4bfa6 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -17,6 +17,7 @@ #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif +using System.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; @@ -64,6 +65,18 @@ public static ILoggingBuilder AddOpenTelemetry( Action? configure) => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: configure); + /// + /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the . + /// + /// + /// The to use. + /// Optional configure LoggerProviderBuilder action. + /// The supplied for call chaining. + public static ILoggingBuilder AddOpenTelemetry( + this ILoggingBuilder builder, + Action? configureBuilder) + => AddOpenTelemetryInternal(builder, configureBuilder: configureBuilder, configureOptions: null); + private static ILoggingBuilder AddOpenTelemetryInternal( ILoggingBuilder builder, Action? configureBuilder, @@ -75,17 +88,20 @@ private static ILoggingBuilder AddOpenTelemetryInternal( var services = builder.Services; + // Note: This will bind logger options element (e.g., "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions + RegisterLoggerProviderOptions(services); + if (configureOptions != null) { - // TODO: Move this below the RegisterLoggerProviderOptions call so - // that user-supplied delegate fires AFTER the options are bound to - // Logging:OpenTelemetry configuration. services.Configure(configureOptions); } - // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions - RegisterLoggerProviderOptions(services); - + /* Note: This ensures IConfiguration is available when using + * IServiceCollections NOT attached to a host. For example when + * performing: + * + * new ServiceCollection().AddLogging(b => b.AddOpenTelemetry()) + */ services.AddOpenTelemetrySharedProviderBuilderServices(); var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder( @@ -112,10 +128,41 @@ private static ILoggingBuilder AddOpenTelemetryInternal( services.TryAddEnumerable( ServiceDescriptor.Singleton( - sp => new OpenTelemetryLoggerProvider( - sp.GetRequiredService(), - sp.GetRequiredService>().CurrentValue, - disposeProvider: false))); + sp => + { + var state = sp.GetRequiredService(); + + var provider = state.Provider; + if (provider == null) + { + /* + * Note: + * + * There is a possibility of a circular reference when + * accessing LoggerProvider from the IServiceProvider. + * + * If LoggerProvider is the first thing accessed, and + * it requires some service which accesses ILogger + * (for example, IHttpClientFactory), then the + * OpenTelemetryLoggerProvider will try to access the + * LoggerProvider inside the initial access to + * LoggerProvider. + * + * This check uses the provider reference captured on + * LoggerProviderBuilderSdk during construction of + * LoggerProviderSdk to detect if a provider has + * already been created to give to + * OpenTelemetryLoggerProvider. + */ + provider = sp.GetRequiredService(); + Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider."); + } + + return new OpenTelemetryLoggerProvider( + provider, + sp.GetRequiredService>().CurrentValue, + disposeProvider: false); + })); return builder; From e11d4bba1a0eb3c44e6c10dac010236c35a57247 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 12:37:18 -0800 Subject: [PATCH 02/28] add OTLP exporter to the LoggerProvider --- .../.publicApi/Stable/PublicAPI.Unshipped.txt | 1 + .../OtlpLogExporterHelperExtensions.cs | 97 +++++++++++++++++++ .../.publicApi/Stable/PublicAPI.Unshipped.txt | 2 +- .../ILogger/OpenTelemetryLoggingExtensions.cs | 1 - .../OtlpLogExporterTests.cs | 15 +++ 5 files changed, 114 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt index f90e2e4dfbd..32a83f681e1 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -2,5 +2,6 @@ OpenTelemetry.Exporter.OtlpLogExporter OpenTelemetry.Exporter.OtlpLogExporter.OtlpLogExporter(OpenTelemetry.Exporter.OtlpExporterOptions! options) -> void override OpenTelemetry.Exporter.OtlpLogExporter.Export(in OpenTelemetry.Batch logRecordBatch) -> OpenTelemetry.ExportResult +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string! name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 49431a3b1a4..ce5011af0fd 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -119,6 +119,68 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( }); } + /// + /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. + /// + /// builder to use. + /// Name which is used when retrieving options. + /// Callback action for + /// configuring and . + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter( + this LoggerProviderBuilder builder, + string name, + Action configureExporterAndProcessor) + { + var finalOptionsName = name ?? Options.DefaultName; + + builder.ConfigureServices(services => + { + OtlpExporterOptions.RegisterOtlpExporterOptionsFactory(services); + services.RegisterOptionsFactory(configuration => new SdkLimitOptions(configuration)); + }); + + return builder.AddProcessor(sp => + { + OtlpExporterOptions exporterOptions; + LogRecordExportProcessorOptions processorOptions; + + if (name == null) + { + // If we are NOT using named options we create a new + // instance always. The reason for this is + // OtlpExporterOptions is shared by all signals. Without a + // name, delegates for all signals will mix together. See: + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/4043 + exporterOptions = sp.GetRequiredService>().Create(finalOptionsName); + processorOptions = sp.GetRequiredService>().Create(finalOptionsName); + + // Configuration delegate is executed inline on the fresh instance. + configureExporterAndProcessor?.Invoke(exporterOptions, processorOptions); + } + else + { + // When using named options we can properly utilize Options + // API to create or reuse an instance. + exporterOptions = sp.GetRequiredService>().Get(finalOptionsName); + processorOptions = sp.GetRequiredService>().Get(finalOptionsName); + } + + // Note: Not using finalOptionsName here for SdkLimitOptions. + // There should only be one provider for a given service + // collection so SdkLimitOptions is treated as a single default + // instance. + var sdkOptionsManager = sp.GetRequiredService>().CurrentValue; + + return BuildOtlpLogExporterProcessor( + exporterOptions, + processorOptions, + sdkOptionsManager, + sp); + }); + } + internal static BaseProcessor BuildOtlpLogExporter( IServiceProvider sp, OtlpExporterOptions exporterOptions, @@ -165,6 +227,41 @@ internal static BaseProcessor BuildOtlpLogExporter( } } + private static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExporterOptions exporterOptions, LogRecordExportProcessorOptions processorOptions, SdkLimitOptions sdkLimitOptions, IServiceProvider sp) + { + /* + * Note: + * + * We don't currently enable IHttpClientFactory for OtlpLogExporter. + * + * The DefaultHttpClientFactory requires the ILoggerFactory in its ctor: + * https://github.com/dotnet/runtime/blob/fa40ecf7d36bf4e31d7ae968807c1c529bac66d6/src/libraries/Microsoft.Extensions.Http/src/DefaultHttpClientFactory.cs#L64 + * + * This creates a circular reference: ILoggerFactory -> + * OpenTelemetryLoggerProvider -> OtlpLogExporter -> IHttpClientFactory + * -> ILoggerFactory + * + * exporterOptions.TryEnableIHttpClientFactoryIntegration(sp, + * "OtlpLogExporter"); + */ + + BaseExporter otlpExporter = new OtlpLogExporter(exporterOptions, sdkLimitOptions, experimentalOptions: new()); + + if (processorOptions.ExportProcessorType == ExportProcessorType.Simple) + { + return new SimpleLogRecordExportProcessor(otlpExporter); + } + else + { + return new BatchLogRecordExportProcessor( + otlpExporter, + processorOptions.BatchExportProcessorOptions.MaxQueueSize, + processorOptions.BatchExportProcessorOptions.ScheduledDelayMilliseconds, + processorOptions.BatchExportProcessorOptions.ExporterTimeoutMilliseconds, + processorOptions.BatchExportProcessorOptions.MaxExportBatchSize); + } + } + private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, string? name, string finalName) { // Note: If OtlpExporter has been registered for tracing and/or metrics diff --git a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt index b11e3fd13ec..67458e44e6d 100644 --- a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -1,2 +1,2 @@ -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder) -> Microsoft.Extensions.Logging.ILoggingBuilder! +static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder) -> Microsoft.Extensions.Logging.ILoggingBuilder! OpenTelemetry.Metrics.Metric.MeterTags.get -> System.Collections.Generic.IEnumerable>? diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index dcb89ef2047..290e6cafe72 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -193,7 +193,6 @@ private static ILoggingBuilder AddOpenTelemetryInternal( services.Configure(configureOptions); } - var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder( (sp, logging) => { diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 090a950b75c..761adc30e77 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -42,6 +42,21 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests { private static readonly SdkLimitOptions DefaultSdkLimitOptions = new(); + [Fact] + public void AddOtlpExporterWithNamedLogRecordExportProcessorOptions() + { + IServiceCollection services = new ServiceCollection(); + services.AddLogging(builder => builder.UseOpenTelemetry()); + + services.Configure("myOtlpOptions", p => p.ExportProcessorType = ExportProcessorType.Simple); + services.ConfigureOpenTelemetryLoggerProvider(builder => builder.AddOtlpExporter(name: "MyOtlpOptions", configureExporterAndProcessor: null)); + + using var serviceProvider = services.BuildServiceProvider(); + + var provider = serviceProvider.GetRequiredService(); + Assert.NotNull(provider); + } + [Fact] public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse() { From 36e85600226b0eb7a914574b6c030533dfbc6ede Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 14:17:14 -0800 Subject: [PATCH 03/28] api doc --- src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt index 67458e44e6d..01e38e9263f 100644 --- a/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -1,2 +1 @@ -static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action? configureBuilder) -> Microsoft.Extensions.Logging.ILoggingBuilder! OpenTelemetry.Metrics.Metric.MeterTags.get -> System.Collections.Generic.IEnumerable>? From 6723a0434530edee0d843c16160a8d1b1c458bc1 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 15:30:03 -0800 Subject: [PATCH 04/28] extension APIs and tests --- .../.publicApi/Stable/PublicAPI.Unshipped.txt | 6 +- .../OtlpLogExporterHelperExtensions.cs | 98 ++++++++++++++++++- .../OtlpLogExporterTests.cs | 47 +++++++-- 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt index 32a83f681e1..41aa34fd340 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -2,6 +2,10 @@ OpenTelemetry.Exporter.OtlpLogExporter OpenTelemetry.Exporter.OtlpLogExporter.OtlpLogExporter(OpenTelemetry.Exporter.OtlpExporterOptions! options) -> void override OpenTelemetry.Exporter.OtlpLogExporter.Export(in OpenTelemetry.Batch logRecordBatch) -> OpenTelemetry.ExportResult -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string! name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index ce5011af0fd..44ad36310af 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -119,18 +119,109 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( }); } + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) + => AddOtlpExporter(builder, name: null, configureExporter: null); + + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// Callback action for configuring . + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) + => AddOtlpExporter(builder, name: null, configureExporter: configureExporter); + + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// Callback action for + /// configuring and . + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) + => AddOtlpExporter(builder, name: null, configureExporterAndProcessor: configureExporterAndProcessor); + /// /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. /// /// builder to use. /// Name which is used when retrieving options. + /// Callback action for configuring . + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter( + this LoggerProviderBuilder builder, + string? name, + Action? configureExporter) + { + var finalOptionsName = name ?? Options.DefaultName; + + builder.ConfigureServices(services => + { + if (name != null && configureExporter != null) + { + // If we are using named options we register the + // configuration delegate into options pipeline. + services.Configure(finalOptionsName, configureExporter); + } + + OtlpExporterOptions.RegisterOtlpExporterOptionsFactory(services); + services.RegisterOptionsFactory(configuration => new SdkLimitOptions(configuration)); + }); + + return builder.AddProcessor(sp => + { + OtlpExporterOptions exporterOptions; + + if (name == null) + { + // If we are NOT using named options we create a new + // instance always. The reason for this is + // OtlpExporterOptions is shared by all signals. Without a + // name, delegates for all signals will mix together. See: + // https://github.com/open-telemetry/opentelemetry-dotnet/issues/4043 + exporterOptions = sp.GetRequiredService>().Create(finalOptionsName); + + // Configuration delegate is executed inline on the fresh instance. + configureExporter?.Invoke(exporterOptions); + } + else + { + // When using named options we can properly utilize Options + // API to create or reuse an instance. + exporterOptions = sp.GetRequiredService>().Get(finalOptionsName); + } + + // Note: Not using finalOptionsName here for SdkLimitOptions. + // There should only be one provider for a given service + // collection so SdkLimitOptions is treated as a single default + // instance. + var sdkOptionsManager = sp.GetRequiredService>().CurrentValue; + + return BuildOtlpLogExporterProcessor( + exporterOptions, + sp.GetRequiredService>().Get(finalOptionsName), + sdkOptionsManager); + }); + } + + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// Name which is used when retrieving options. /// Callback action for /// configuring and . /// The instance of to chain the calls. public static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, - string name, + string? name, Action configureExporterAndProcessor) { var finalOptionsName = name ?? Options.DefaultName; @@ -176,8 +267,7 @@ public static LoggerProviderBuilder AddOtlpExporter( return BuildOtlpLogExporterProcessor( exporterOptions, processorOptions, - sdkOptionsManager, - sp); + sdkOptionsManager); }); } @@ -227,7 +317,7 @@ internal static BaseProcessor BuildOtlpLogExporter( } } - private static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExporterOptions exporterOptions, LogRecordExportProcessorOptions processorOptions, SdkLimitOptions sdkLimitOptions, IServiceProvider sp) + internal static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExporterOptions exporterOptions, LogRecordExportProcessorOptions processorOptions, SdkLimitOptions sdkLimitOptions) { /* * Note: diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 761adc30e77..20ab5f7d7ea 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -42,19 +42,52 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests { private static readonly SdkLimitOptions DefaultSdkLimitOptions = new(); - [Fact] - public void AddOtlpExporterWithNamedLogRecordExportProcessorOptions() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ResolutionOrderTest(bool requestLoggerProviderDirectly) { IServiceCollection services = new ServiceCollection(); - services.AddLogging(builder => builder.UseOpenTelemetry()); - services.Configure("myOtlpOptions", p => p.ExportProcessorType = ExportProcessorType.Simple); - services.ConfigureOpenTelemetryLoggerProvider(builder => builder.AddOtlpExporter(name: "MyOtlpOptions", configureExporterAndProcessor: null)); + services.AddLogging(builder => builder.AddOpenTelemetry()); using var serviceProvider = services.BuildServiceProvider(); - var provider = serviceProvider.GetRequiredService(); - Assert.NotNull(provider); + if (requestLoggerProviderDirectly) + { + var provider = serviceProvider.GetRequiredService(); + Assert.NotNull(provider); + } + else + { + var factory = serviceProvider.GetRequiredService(); + Assert.NotNull(factory); + } + } + + [Fact] + public void AddOtlpExporterWithNamedOptions() + { + int defaultExporterOptionsConfigureOptionsInvocations = 0; + int namedExporterOptionsConfigureOptionsInvocations = 0; + + using var loggerProvider = Sdk.CreateLoggerProviderBuilder() + .ConfigureServices(services => + { + services.Configure(o => defaultExporterOptionsConfigureOptionsInvocations++); + services.Configure(o => defaultExporterOptionsConfigureOptionsInvocations++); + + services.Configure("Exporter2", o => namedExporterOptionsConfigureOptionsInvocations++); + services.Configure("Exporter2", o => namedExporterOptionsConfigureOptionsInvocations++); + + services.Configure("Exporter3", o => namedExporterOptionsConfigureOptionsInvocations++); + services.Configure("Exporter3", o => namedExporterOptionsConfigureOptionsInvocations++); + }) + .AddOtlpExporter() + .Build(); + + Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); + Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); } [Fact] From 3653fd32015c6687659ba0fd292dfd00c422af7d Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 16:18:39 -0800 Subject: [PATCH 05/28] more tests --- .../OtlpLogExporterTests.cs | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 20ab5f7d7ea..7f9ac94d90f 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -90,6 +90,89 @@ public void AddOtlpExporterWithNamedOptions() Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); } + [Fact] + public void UserHttpFactoryCalledWhenUsingHttpProtobuf() + { + OtlpExporterOptions options = new OtlpExporterOptions(); + + var defaultFactory = options.HttpClientFactory; + + int invocations = 0; + options.Protocol = OtlpExportProtocol.HttpProtobuf; + options.HttpClientFactory = () => + { + invocations++; + return defaultFactory(); + }; + + using (var exporter = new OtlpLogExporter(options)) + { + Assert.Equal(1, invocations); + } + + using (var provider = Sdk.CreateLoggerProviderBuilder() + .AddOtlpExporter(o => + { + o.Protocol = OtlpExportProtocol.HttpProtobuf; + o.HttpClientFactory = options.HttpClientFactory; + }) + .Build()) + { + Assert.Equal(2, invocations); + } + + options.HttpClientFactory = null; + Assert.Throws(() => + { + using var exporter = new OtlpLogExporter(options); + }); + + options.HttpClientFactory = () => null; + Assert.Throws(() => + { + using var exporter = new OtlpLogExporter(options); + }); + } + + [Fact] + public void AddOtlpExporterSetsDefaultBatchExportProcessor() + { + if (Environment.Version.Major == 3) + { + // Adding the OtlpExporter creates a GrpcChannel. + // This switch must be set before creating a GrpcChannel when calling an insecure HTTP/2 endpoint. + // See: https://docs.microsoft.com/aspnet/core/grpc/troubleshoot#call-insecure-grpc-services-with-net-core-client + AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true); + } + + var loggerProvider = Sdk.CreateLoggerProviderBuilder() + .AddOtlpExporter() + .Build(); + + CheckProcessorDefaults(); + + loggerProvider.Dispose(); + + void CheckProcessorDefaults() + { + var bindingFlags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic; + + var processor = typeof(BaseProcessor) + .Assembly + .GetType("OpenTelemetry.Logs.LoggerProviderSdk") + .GetProperty("Processor", bindingFlags) + .GetValue(loggerProvider) as BatchExportProcessor; + + Assert.NotNull(processor); + + var scheduledDelayMilliseconds = typeof(BatchExportProcessor) + .GetField("scheduledDelayMilliseconds", bindingFlags) + .GetValue(processor); + + Assert.Equal(5000, scheduledDelayMilliseconds); + } + } + [Fact] public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse() { From 64f8df9d3550f78a65b504715879c9529d14df2c Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 16:31:52 -0800 Subject: [PATCH 06/28] EXPOSE_EXPERIMENTAL_FEATURES --- .../OtlpLogExporterHelperExtensions.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 44ad36310af..99f088b13bf 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -119,13 +119,14 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( }); } +#if EXPOSE_EXPERIMENTAL_FEATURES /// /// Adds an OTLP exporter to the LoggerProvider. /// /// builder to use. /// The instance of to chain the calls. public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) - => AddOtlpExporter(builder, name: null, configureExporter: null); + => AddOtlpExporter(builder, name: null, configureExporter: null); /// /// Adds an OTLP exporter to the LoggerProvider. @@ -351,6 +352,7 @@ internal static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExpor processorOptions.BatchExportProcessorOptions.MaxExportBatchSize); } } +#endif private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, string? name, string finalName) { From 4255964fe4b7c5719e846b6067effde9b0096016 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 1 Dec 2023 16:44:19 -0800 Subject: [PATCH 07/28] public api files --- .../.publicApi/Experimental/PublicAPI.Unshipped.txt | 8 ++++++++ .../.publicApi/Stable/PublicAPI.Unshipped.txt | 8 +------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt index e69de29bb2d..36d3908f74d 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -0,0 +1,8 @@ +#nullable enable +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt index 41aa34fd340..e7672c30f06 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -2,10 +2,4 @@ OpenTelemetry.Exporter.OtlpLogExporter OpenTelemetry.Exporter.OtlpLogExporter.OtlpLogExporter(OpenTelemetry.Exporter.OtlpExporterOptions! options) -> void override OpenTelemetry.Exporter.OtlpLogExporter.Export(in OpenTelemetry.Batch logRecordBatch) -> OpenTelemetry.ExportResult -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! + From 3e2aff06b5d0e03a3e8714d2c7669d51fdd70822 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 8 Dec 2023 14:35:36 -0800 Subject: [PATCH 08/28] fix macro --- .../OtlpLogExporterHelperExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 99f088b13bf..9f4a6365df5 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -271,6 +271,7 @@ public static LoggerProviderBuilder AddOtlpExporter( sdkOptionsManager); }); } +#endif internal static BaseProcessor BuildOtlpLogExporter( IServiceProvider sp, @@ -352,7 +353,6 @@ internal static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExpor processorOptions.BatchExportProcessorOptions.MaxExportBatchSize); } } -#endif private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, string? name, string finalName) { From 7f28ee5c72532399979e80837df226f1937d56da Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 8 Dec 2023 16:13:13 -0800 Subject: [PATCH 09/28] fixed public api file --- .../.publicApi/Stable/PublicAPI.Unshipped.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt index e7672c30f06..f90e2e4dfbd 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Stable/PublicAPI.Unshipped.txt @@ -2,4 +2,5 @@ OpenTelemetry.Exporter.OtlpLogExporter OpenTelemetry.Exporter.OtlpLogExporter.OtlpLogExporter(OpenTelemetry.Exporter.OtlpExporterOptions! options) -> void override OpenTelemetry.Exporter.OtlpLogExporter.Export(in OpenTelemetry.Batch logRecordBatch) -> OpenTelemetry.ExportResult - +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! From c828fcca243b8f32e464565c92be0e32f820c227 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 8 Dec 2023 16:46:54 -0800 Subject: [PATCH 10/28] api/test --- .../.publicApi/Experimental/PublicAPI.Unshipped.txt | 2 -- .../OtlpLogExporterTests.cs | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt index 36d3908f74d..649b641595a 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -4,5 +4,3 @@ static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this O static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, string? name, System.Action? configure) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 72105dae83f..117c6447020 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -69,6 +69,8 @@ public void AddOtlpExporterWithNamedOptions() services.Configure("Exporter3", o => namedExporterOptionsConfigureOptionsInvocations++); }) .AddOtlpExporter() + .AddOtlpExporter("Exporter2", o => { }) + .AddOtlpExporter("Exporter3", o => { }) .Build(); Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); From 6c4d67b8e94d5f813950677b013c97c394d17ec6 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Fri, 8 Dec 2023 17:05:23 -0800 Subject: [PATCH 11/28] flag for namedoptions test --- .../OtlpLogExporterTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 117c6447020..482929d9a0a 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -50,6 +50,7 @@ public void ResolutionOrderTest(bool requestLoggerProviderDirectly) } } +#if EXPOSE_EXPERIMENTAL_FEATURES [Fact] public void AddOtlpExporterWithNamedOptions() { @@ -76,6 +77,7 @@ public void AddOtlpExporterWithNamedOptions() Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); } +#endif [Fact] public void UserHttpFactoryCalledWhenUsingHttpProtobuf() From 7276894b87aa836025c2a53f02ffb8375a3cd834 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Mon, 11 Dec 2023 13:45:32 -0800 Subject: [PATCH 12/28] test flag --- .../OtlpLogExporterTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 482929d9a0a..117c6447020 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -50,7 +50,6 @@ public void ResolutionOrderTest(bool requestLoggerProviderDirectly) } } -#if EXPOSE_EXPERIMENTAL_FEATURES [Fact] public void AddOtlpExporterWithNamedOptions() { @@ -77,7 +76,6 @@ public void AddOtlpExporterWithNamedOptions() Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); } -#endif [Fact] public void UserHttpFactoryCalledWhenUsingHttpProtobuf() From 7a7e9624c5d01cc5928422283c063bbcf4ccca6e Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Mon, 11 Dec 2023 14:20:08 -0800 Subject: [PATCH 13/28] mark apis internal when !EXPOSE_EXPERIMENTAL_FEATURES --- .../OtlpLogExporterHelperExtensions.cs | 67 +++++++++++++++++-- 1 file changed, 61 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index f65acefc6e8..c993f906808 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -110,39 +110,80 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( /// /// Adds an OTLP exporter to the LoggerProvider. /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. /// The instance of to chain the calls. - public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) + public +#else + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// The instance of to chain the calls. + internal +#endif + static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) => AddOtlpExporter(builder, name: null, configureExporter: null); +#if EXPOSE_EXPERIMENTAL_FEATURES /// /// Adds an OTLP exporter to the LoggerProvider. /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. /// Callback action for configuring . /// The instance of to chain the calls. - public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) + public +#else + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// builder to use. + /// Callback action for configuring . + /// The instance of to chain the calls. + internal +#endif + static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) => AddOtlpExporter(builder, name: null, configureExporter: configureExporter); +#if EXPOSE_EXPERIMENTAL_FEATURES /// /// Adds an OTLP exporter to the LoggerProvider. /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. /// Callback action for /// configuring and . /// The instance of to chain the calls. - public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) + public +#else + internal +#endif + static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) => AddOtlpExporter(builder, name: null, configureExporterAndProcessor: configureExporterAndProcessor); +#if EXPOSE_EXPERIMENTAL_FEATURES /// /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. /// Name which is used when retrieving options. /// Callback action for configuring . /// The instance of to chain the calls. - public static LoggerProviderBuilder AddOtlpExporter( + public +#else + /// + /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. + /// + /// builder to use. + /// Name which is used when retrieving options. + /// Callback action for configuring . + /// The instance of to chain the calls. + internal +#endif + static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, string? name, Action? configureExporter) @@ -198,6 +239,19 @@ public static LoggerProviderBuilder AddOtlpExporter( }); } +#if EXPOSE_EXPERIMENTAL_FEATURES + /// + /// Adds an OTLP exporter to the LoggerProvider. + /// + /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. + /// builder to use. + /// Name which is used when retrieving options. + /// Callback action for + /// configuring and . + /// The instance of to chain the calls. + public +#else /// /// Adds an OTLP exporter to the LoggerProvider. /// @@ -207,7 +261,9 @@ public static LoggerProviderBuilder AddOtlpExporter( /// configuring and . /// The instance of to chain the calls. - public static LoggerProviderBuilder AddOtlpExporter( + internal +#endif + static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, string? name, Action configureExporterAndProcessor) @@ -258,7 +314,6 @@ public static LoggerProviderBuilder AddOtlpExporter( sdkOptionsManager); }); } -#endif internal static BaseProcessor BuildOtlpLogExporter( IServiceProvider sp, From 6cf59f04ea4216fe6259a9ab6102986b35912fb0 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Mon, 11 Dec 2023 15:10:24 -0800 Subject: [PATCH 14/28] test --- src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs index 2145d23c19e..9a5d1fdf9be 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs @@ -8,6 +8,7 @@ [assembly: InternalsVisibleTo("OpenTelemetry.Api.ProviderBuilderExtensions.Tests" + AssemblyInfo.PublicKey)] #if !EXPOSE_EXPERIMENTAL_FEATURES +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Console" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting" + AssemblyInfo.PublicKey)] #endif From 35406aefac68efa0960d5a0e891af459202c4260 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Mon, 11 Dec 2023 15:35:11 -0800 Subject: [PATCH 15/28] Revert "test" This reverts commit 6cf59f04ea4216fe6259a9ab6102986b35912fb0. --- src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs index 9a5d1fdf9be..2145d23c19e 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs @@ -8,7 +8,6 @@ [assembly: InternalsVisibleTo("OpenTelemetry.Api.ProviderBuilderExtensions.Tests" + AssemblyInfo.PublicKey)] #if !EXPOSE_EXPERIMENTAL_FEATURES -[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Console" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting" + AssemblyInfo.PublicKey)] #endif From 1f99014779cf241a82deaada4a3d368c56412c1b Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Mon, 11 Dec 2023 15:40:51 -0800 Subject: [PATCH 16/28] internal visible to for project and test --- src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs index 2145d23c19e..68ff9c29706 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs @@ -6,8 +6,10 @@ [assembly: InternalsVisibleTo("OpenTelemetry" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Tests" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Api.ProviderBuilderExtensions.Tests" + AssemblyInfo.PublicKey)] +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests" + AssemblyInfo.PublicKey)] #if !EXPOSE_EXPERIMENTAL_FEATURES +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Console" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting" + AssemblyInfo.PublicKey)] #endif From fa4d229b55c963999ce3ec42fb292aa8d1c1eccc Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 12 Dec 2023 14:51:33 -0800 Subject: [PATCH 17/28] Add ExperimentalAttribute decorations and clean up a few nits. --- .../Experimental/PublicAPI.Unshipped.txt | 1 - ...etry.Exporter.OpenTelemetryProtocol.csproj | 3 ++- .../OtlpLogExporterHelperExtensions.cs | 26 ++++++++++++++----- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt index 649b641595a..f4cb24cb080 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,4 +1,3 @@ -#nullable enable static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj index 92329c814d6..bed60120451 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj @@ -34,10 +34,11 @@ - + + diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index c993f906808..565f33f8b41 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -4,6 +4,9 @@ #nullable enable using System.Diagnostics; +#if EXPOSE_EXPERIMENTAL_FEATURES && NET8_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -113,6 +116,9 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. /// The instance of to chain the calls. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif public #else /// @@ -133,6 +139,9 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) /// builder to use. /// Callback action for configuring . /// The instance of to chain the calls. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif public #else /// @@ -156,6 +165,9 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, /// configuring and . /// The instance of to chain the calls. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif public #else internal @@ -172,6 +184,9 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, /// Name which is used when retrieving options. /// Callback action for configuring . /// The instance of to chain the calls. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif public #else /// @@ -250,6 +265,9 @@ static LoggerProviderBuilder AddOtlpExporter( /// configuring and . /// The instance of to chain the calls. +#if NET8_0_OR_GREATER + [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] +#endif public #else /// @@ -321,15 +339,11 @@ internal static BaseProcessor BuildOtlpLogExporter( LogRecordExportProcessorOptions processorOptions, Func, BaseExporter>? configureExporterInstance = null) { - if (sp == null) - { - throw new ArgumentNullException(nameof(sp)); - } - + Debug.Assert(sp != null, "sp was null"); Debug.Assert(exporterOptions != null, "exporterOptions was null"); Debug.Assert(processorOptions != null, "processorOptions was null"); - var config = sp.GetRequiredService(); + var config = sp!.GetRequiredService(); var sdkLimitOptions = new SdkLimitOptions(config); var experimentalOptions = new ExperimentalOptions(config); From 04a036ce12578ed63a85a99ef8a2a8a37172c37a Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 12 Dec 2023 16:08:16 -0800 Subject: [PATCH 18/28] Tweaks and fixes. --- .../Experimental/PublicAPI.Unshipped.txt | 2 +- .../OtlpLogExporterHelperExtensions.cs | 131 +++++++++--------- 2 files changed, 68 insertions(+), 65 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt index f4cb24cb080..e6bd747c9de 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt @@ -1,5 +1,5 @@ static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder) -> OpenTelemetry.Logs.LoggerProviderBuilder! -static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! +static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder! static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder! diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 565f33f8b41..9bde5bb44aa 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -153,7 +153,7 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) internal #endif static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) - => AddOtlpExporter(builder, name: null, configureExporter: configureExporter); + => AddOtlpExporter(builder, name: null, configureExporter); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -173,7 +173,7 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, internal #endif static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) - => AddOtlpExporter(builder, name: null, configureExporterAndProcessor: configureExporterAndProcessor); + => AddOtlpExporter(builder, name: null, configureExporterAndProcessor); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -181,8 +181,8 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, /// /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. - /// Name which is used when retrieving options. - /// Callback action for configuring . + /// Optional name which is used when retrieving options. + /// Optional callback action for configuring . /// The instance of to chain the calls. #if NET8_0_OR_GREATER [Experimental(DiagnosticDefinitions.LoggerProviderExperimentalApi, UrlFormat = DiagnosticDefinitions.ExperimentalApiUrlFormat)] @@ -193,8 +193,8 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. /// /// builder to use. - /// Name which is used when retrieving options. - /// Callback action for configuring . + /// Optional name which is used when retrieving options. + /// Optional callback action for configuring . /// The instance of to chain the calls. internal #endif @@ -214,8 +214,7 @@ static LoggerProviderBuilder AddOtlpExporter( services.Configure(finalOptionsName, configureExporter); } - OtlpExporterOptions.RegisterOtlpExporterOptionsFactory(services); - services.RegisterOptionsFactory(configuration => new SdkLimitOptions(configuration)); + RegisterOptions(services); }); return builder.AddProcessor(sp => @@ -245,12 +244,14 @@ static LoggerProviderBuilder AddOtlpExporter( // There should only be one provider for a given service // collection so SdkLimitOptions is treated as a single default // instance. - var sdkOptionsManager = sp.GetRequiredService>().CurrentValue; + var sdkLimitOptions = sp.GetRequiredService>().CurrentValue; - return BuildOtlpLogExporterProcessor( + return BuildOtlpLogExporter( + sp, exporterOptions, sp.GetRequiredService>().Get(finalOptionsName), - sdkOptionsManager); + sdkLimitOptions, + sp.GetRequiredService>().Get(finalOptionsName)); }); } @@ -260,8 +261,8 @@ static LoggerProviderBuilder AddOtlpExporter( /// /// WARNING: This is an experimental API which might change or be removed in the future. Use at your own risk. /// builder to use. - /// Name which is used when retrieving options. - /// Callback action for + /// Optional name which is used when retrieving options. + /// Optional callback action for /// configuring and . /// The instance of to chain the calls. @@ -274,8 +275,8 @@ static LoggerProviderBuilder AddOtlpExporter( /// Adds an OTLP exporter to the LoggerProvider. /// /// builder to use. - /// Name which is used when retrieving options. - /// Callback action for + /// Optional name which is used when retrieving options. + /// Optional callback action for /// configuring and . /// The instance of to chain the calls. @@ -284,20 +285,15 @@ static LoggerProviderBuilder AddOtlpExporter( static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, string? name, - Action configureExporterAndProcessor) + Action? configureExporterAndProcessor) { var finalOptionsName = name ?? Options.DefaultName; - builder.ConfigureServices(services => - { - OtlpExporterOptions.RegisterOtlpExporterOptionsFactory(services); - services.RegisterOptionsFactory(configuration => new SdkLimitOptions(configuration)); - }); + builder.ConfigureServices(RegisterOptions); return builder.AddProcessor(sp => { OtlpExporterOptions exporterOptions; - LogRecordExportProcessorOptions processorOptions; if (name == null) { @@ -307,29 +303,31 @@ static LoggerProviderBuilder AddOtlpExporter( // name, delegates for all signals will mix together. See: // https://github.com/open-telemetry/opentelemetry-dotnet/issues/4043 exporterOptions = sp.GetRequiredService>().Create(finalOptionsName); - processorOptions = sp.GetRequiredService>().Create(finalOptionsName); - - // Configuration delegate is executed inline on the fresh instance. - configureExporterAndProcessor?.Invoke(exporterOptions, processorOptions); } else { // When using named options we can properly utilize Options // API to create or reuse an instance. exporterOptions = sp.GetRequiredService>().Get(finalOptionsName); - processorOptions = sp.GetRequiredService>().Get(finalOptionsName); } + var processorOptions = sp.GetRequiredService>().Get(finalOptionsName); + + // Configuration delegate is executed inline. + configureExporterAndProcessor?.Invoke(exporterOptions, processorOptions); + // Note: Not using finalOptionsName here for SdkLimitOptions. // There should only be one provider for a given service // collection so SdkLimitOptions is treated as a single default // instance. - var sdkOptionsManager = sp.GetRequiredService>().CurrentValue; + var sdkLimitOptions = sp.GetRequiredService>().CurrentValue; - return BuildOtlpLogExporterProcessor( + return BuildOtlpLogExporter( + sp, exporterOptions, processorOptions, - sdkOptionsManager); + sdkLimitOptions, + sp.GetRequiredService>().Get(finalOptionsName)); }); } @@ -340,43 +338,31 @@ internal static BaseProcessor BuildOtlpLogExporter( Func, BaseExporter>? configureExporterInstance = null) { Debug.Assert(sp != null, "sp was null"); - Debug.Assert(exporterOptions != null, "exporterOptions was null"); - Debug.Assert(processorOptions != null, "processorOptions was null"); var config = sp!.GetRequiredService(); var sdkLimitOptions = new SdkLimitOptions(config); var experimentalOptions = new ExperimentalOptions(config); - BaseExporter otlpExporter = new OtlpLogExporter( - exporterOptions!, - sdkLimitOptions, - experimentalOptions); - - if (configureExporterInstance != null) - { - otlpExporter = configureExporterInstance(otlpExporter); - } - - if (processorOptions!.ExportProcessorType == ExportProcessorType.Simple) - { - return new SimpleLogRecordExportProcessor(otlpExporter); - } - else - { - var batchOptions = processorOptions.BatchExportProcessorOptions; - - return new BatchLogRecordExportProcessor( - otlpExporter, - batchOptions.MaxQueueSize, - batchOptions.ScheduledDelayMilliseconds, - batchOptions.ExporterTimeoutMilliseconds, - batchOptions.MaxExportBatchSize); - } + return BuildOtlpLogExporter(sp!, exporterOptions, processorOptions, sdkLimitOptions, experimentalOptions, configureExporterInstance); } - internal static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExporterOptions exporterOptions, LogRecordExportProcessorOptions processorOptions, SdkLimitOptions sdkLimitOptions) + private static BaseProcessor BuildOtlpLogExporter( + IServiceProvider sp, + OtlpExporterOptions exporterOptions, + LogRecordExportProcessorOptions processorOptions, + SdkLimitOptions sdkLimitOptions, + ExperimentalOptions experimentalOptions, + Func, BaseExporter>? configureExporterInstance = null) { + // Note: sp is not currently used by this method but it should be used + // at some point for IHttpClientFactory integration. + Debug.Assert(sp != null, "sp was null"); + Debug.Assert(exporterOptions != null, "exporterOptions was null"); + Debug.Assert(processorOptions != null, "processorOptions was null"); + Debug.Assert(sdkLimitOptions != null, "sdkLimitOptions was null"); + Debug.Assert(experimentalOptions != null, "experimentalOptions was null"); + /* * Note: * @@ -393,23 +379,40 @@ internal static BaseProcessor BuildOtlpLogExporterProcessor(OtlpExpor * "OtlpLogExporter"); */ - BaseExporter otlpExporter = new OtlpLogExporter(exporterOptions, sdkLimitOptions, experimentalOptions: new()); + BaseExporter otlpExporter = new OtlpLogExporter( + exporterOptions!, + sdkLimitOptions!, + experimentalOptions!); - if (processorOptions.ExportProcessorType == ExportProcessorType.Simple) + if (configureExporterInstance != null) + { + otlpExporter = configureExporterInstance(otlpExporter); + } + + if (processorOptions!.ExportProcessorType == ExportProcessorType.Simple) { return new SimpleLogRecordExportProcessor(otlpExporter); } else { + var batchOptions = processorOptions.BatchExportProcessorOptions; + return new BatchLogRecordExportProcessor( otlpExporter, - processorOptions.BatchExportProcessorOptions.MaxQueueSize, - processorOptions.BatchExportProcessorOptions.ScheduledDelayMilliseconds, - processorOptions.BatchExportProcessorOptions.ExporterTimeoutMilliseconds, - processorOptions.BatchExportProcessorOptions.MaxExportBatchSize); + batchOptions.MaxQueueSize, + batchOptions.ScheduledDelayMilliseconds, + batchOptions.ExporterTimeoutMilliseconds, + batchOptions.MaxExportBatchSize); } } + private static void RegisterOptions(IServiceCollection services) + { + OtlpExporterOptions.RegisterOtlpExporterOptionsFactory(services); + services.RegisterOptionsFactory(configuration => new SdkLimitOptions(configuration)); + services.RegisterOptionsFactory(configuration => new ExperimentalOptions(configuration)); + } + private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, string? name, string finalName) { // Note: If OtlpExporter has been registered for tracing and/or metrics From d09f1b0dabf49af4e35540f91d4a172143562315 Mon Sep 17 00:00:00 2001 From: Yun-Ting Date: Wed, 13 Dec 2023 14:49:02 -0800 Subject: [PATCH 19/28] changelog --- src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md index 284ce9c8e64..959fe7c87ee 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* **Experimental (pre-release builds only):** Added + `LoggerProviderBuilder.AddOtlpExporter` registration extensions. + [#5103](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5103) + ## 1.7.0 Released 2023-Dec-08 From c19868d7acb19d593a34e1f9e0f2e784223fd044 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 18 Dec 2023 15:46:35 -0800 Subject: [PATCH 20/28] Code review. --- .../AssemblyInfo.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs index 68ff9c29706..c6a15f659eb 100644 --- a/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs +++ b/src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs @@ -6,11 +6,11 @@ [assembly: InternalsVisibleTo("OpenTelemetry" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Tests" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Api.ProviderBuilderExtensions.Tests" + AssemblyInfo.PublicKey)] -[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests" + AssemblyInfo.PublicKey)] #if !EXPOSE_EXPERIMENTAL_FEATURES -[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Exporter.Console" + AssemblyInfo.PublicKey)] +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol" + AssemblyInfo.PublicKey)] +[assembly: InternalsVisibleTo("OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests" + AssemblyInfo.PublicKey)] [assembly: InternalsVisibleTo("OpenTelemetry.Extensions.Hosting" + AssemblyInfo.PublicKey)] #endif From e208fbcf8ece49f4f96fc2df58f334c21cc4739b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Mon, 18 Dec 2023 16:31:10 -0800 Subject: [PATCH 21/28] Code review. --- .../ILogger/OpenTelemetryLoggingExtensions.cs | 10 ++-- .../OtlpLogExporterTests.cs | 23 --------- .../OpenTelemetryLoggingExtensionsTests.cs | 47 +++++++++++++++++++ 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index cf2964c80cb..42c1beaf39c 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -174,11 +174,11 @@ private static ILoggingBuilder AddOpenTelemetryInternal( RegisterLoggerProviderOptions(services); /* Note: This ensures IConfiguration is available when using - * IServiceCollections NOT attached to a host. For example when - * performing: - * - * new ServiceCollection().AddLogging(b => b.AddOpenTelemetry()) - */ + * IServiceCollections NOT attached to a host. For example when + * performing: + * + * new ServiceCollection().AddLogging(b => b.AddOpenTelemetry()) + */ services.AddOpenTelemetrySharedProviderBuilderServices(); if (configureOptions != null) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 117c6447020..3d2714b2717 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -27,29 +27,6 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests { private static readonly SdkLimitOptions DefaultSdkLimitOptions = new(); - [Theory] - [InlineData(true)] - [InlineData(false)] - public void ResolutionOrderTest(bool requestLoggerProviderDirectly) - { - IServiceCollection services = new ServiceCollection(); - - services.AddLogging(builder => builder.AddOpenTelemetry()); - - using var serviceProvider = services.BuildServiceProvider(); - - if (requestLoggerProviderDirectly) - { - var provider = serviceProvider.GetRequiredService(); - Assert.NotNull(provider); - } - else - { - var factory = serviceProvider.GetRequiredService(); - Assert.NotNull(factory); - } - } - [Fact] public void AddOtlpExporterWithNamedOptions() { diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index ddf0c27a6ec..18cffad9a6f 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -266,7 +266,54 @@ public void VerifyExceptionIsThrownWhenImplementationFactoryIsNull() Assert.Throws(() => sp.GetRequiredService() as LoggerProviderSdk); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void CircularReferenceTest(bool requestLoggerProviderDirectly) + { + var services = new ServiceCollection(); + + services.AddLogging(logging => logging.AddOpenTelemetry()); + + services.ConfigureOpenTelemetryLoggerProvider(builder => builder.AddProcessor()); + + using var sp = services.BuildServiceProvider(); + + if (requestLoggerProviderDirectly) + { + var provider = sp.GetRequiredService(); + Assert.NotNull(provider); + } + else + { + var factory = sp.GetRequiredService(); + Assert.NotNull(factory); + } + } + private class TestLogProcessor : BaseProcessor { } + + private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor + { + private readonly ILogger logger; + + public TestLogProcessorWithILoggerFactoryDependency(ILoggerFactory loggerFactory) + { + // Note: It is NOT recommended to log from inside a processor. This + // test is meant to mirror someone injecting IHttpClientFactory + // (which itself uses ILoggerFactory) as part of an exporter. That + // is a more realistic scenario but needs a dependency to do that so + // here we approximate the graph. + this.logger = loggerFactory.CreateLogger("MyLogger"); + } + + protected override void Dispose(bool disposing) + { + this.logger.LogInformation("Dispose called"); + + base.Dispose(disposing); + } + } } From c4e795fa8d129aca15d1a0b65c39a43df132e84b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 19 Dec 2023 11:42:50 -0800 Subject: [PATCH 22/28] Clean up. --- .../OtlpLogExporterTests.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 3d2714b2717..224cf985654 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -90,12 +90,6 @@ public void UserHttpFactoryCalledWhenUsingHttpProtobuf() { using var exporter = new OtlpLogExporter(options); }); - - options.HttpClientFactory = () => null; - Assert.Throws(() => - { - using var exporter = new OtlpLogExporter(options); - }); } [Fact] From 2f51df2e9124eea109b86aba113de8181dc8688d Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 19 Dec 2023 11:49:04 -0800 Subject: [PATCH 23/28] Code review and test improvements. --- .../OtlpLogExporterTests.cs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 224cf985654..018c9f01117 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -30,28 +30,31 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests [Fact] public void AddOtlpExporterWithNamedOptions() { - int defaultExporterOptionsConfigureOptionsInvocations = 0; - int namedExporterOptionsConfigureOptionsInvocations = 0; + int defaultConfigureExporterOptionsInvocations = 0; + int namedConfigureExporterOptionsInvocations = 0; using var loggerProvider = Sdk.CreateLoggerProviderBuilder() .ConfigureServices(services => { - services.Configure(o => defaultExporterOptionsConfigureOptionsInvocations++); - services.Configure(o => defaultExporterOptionsConfigureOptionsInvocations++); + services.Configure(o => defaultConfigureExporterOptionsInvocations++); + services.Configure(o => defaultConfigureExporterOptionsInvocations++); + services.Configure(o => defaultConfigureExporterOptionsInvocations++); - services.Configure("Exporter2", o => namedExporterOptionsConfigureOptionsInvocations++); - services.Configure("Exporter2", o => namedExporterOptionsConfigureOptionsInvocations++); + services.Configure("Exporter2", o => namedConfigureExporterOptionsInvocations++); + services.Configure("Exporter2", o => namedConfigureExporterOptionsInvocations++); + services.Configure("Exporter2", o => namedConfigureExporterOptionsInvocations++); - services.Configure("Exporter3", o => namedExporterOptionsConfigureOptionsInvocations++); - services.Configure("Exporter3", o => namedExporterOptionsConfigureOptionsInvocations++); + services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); + services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); + services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); }) .AddOtlpExporter() .AddOtlpExporter("Exporter2", o => { }) .AddOtlpExporter("Exporter3", o => { }) .Build(); - Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); - Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); + Assert.Equal(3, defaultConfigureExporterOptionsInvocations); + Assert.Equal(6, namedConfigureExporterOptionsInvocations); } [Fact] From e17f4e7ddb228b453c4939d243b3282c05e17e62 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 19 Dec 2023 11:52:30 -0800 Subject: [PATCH 24/28] Nits --- .../OtlpLogExporterHelperExtensions.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 9bde5bb44aa..3a0e4f03f17 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -129,7 +129,7 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( internal #endif static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) - => AddOtlpExporter(builder, name: null, configureExporter: null); + => AddOtlpExporter(builder, name: null, configureExporter: null); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -152,8 +152,8 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) /// The instance of to chain the calls. internal #endif - static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) - => AddOtlpExporter(builder, name: null, configureExporter); + static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporter) + => AddOtlpExporter(builder, name: null, configureExporter); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -172,8 +172,8 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, #else internal #endif - static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) - => AddOtlpExporter(builder, name: null, configureExporterAndProcessor); + static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, Action configureExporterAndProcessor) + => AddOtlpExporter(builder, name: null, configureExporterAndProcessor); #if EXPOSE_EXPERIMENTAL_FEATURES /// @@ -198,7 +198,7 @@ static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder, /// The instance of to chain the calls. internal #endif - static LoggerProviderBuilder AddOtlpExporter( + static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, string? name, Action? configureExporter) @@ -282,7 +282,7 @@ static LoggerProviderBuilder AddOtlpExporter( /// The instance of to chain the calls. internal #endif - static LoggerProviderBuilder AddOtlpExporter( + static LoggerProviderBuilder AddOtlpExporter( this LoggerProviderBuilder builder, string? name, Action? configureExporterAndProcessor) From 66ea5e39a620eea2d9f492a8d5eccb62d187614b Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 19 Dec 2023 11:58:37 -0800 Subject: [PATCH 25/28] Test improvements. --- .../OtlpLogExporterTests.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 018c9f01117..9d338f8ec7a 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -33,6 +33,9 @@ public void AddOtlpExporterWithNamedOptions() int defaultConfigureExporterOptionsInvocations = 0; int namedConfigureExporterOptionsInvocations = 0; + int defaultConfigureSdkLimitsOptionsInvocations = 0; + int namedConfigureSdkLimitsOptionsInvocations = 0; + using var loggerProvider = Sdk.CreateLoggerProviderBuilder() .ConfigureServices(services => { @@ -47,6 +50,10 @@ public void AddOtlpExporterWithNamedOptions() services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); services.Configure("Exporter3", o => namedConfigureExporterOptionsInvocations++); + + services.Configure(o => defaultConfigureSdkLimitsOptionsInvocations++); + services.Configure("Exporter2", o => namedConfigureSdkLimitsOptionsInvocations++); + services.Configure("Exporter3", o => namedConfigureSdkLimitsOptionsInvocations++); }) .AddOtlpExporter() .AddOtlpExporter("Exporter2", o => { }) @@ -55,6 +62,11 @@ public void AddOtlpExporterWithNamedOptions() Assert.Equal(3, defaultConfigureExporterOptionsInvocations); Assert.Equal(6, namedConfigureExporterOptionsInvocations); + + // Note: SdkLimitOptions does NOT support named options. We only allow a + // single instance for a given IServiceCollection. + Assert.Equal(1, defaultConfigureSdkLimitsOptionsInvocations); + Assert.Equal(0, namedConfigureSdkLimitsOptionsInvocations); } [Fact] From 8f15ca09d7c78e002921d095abfa95f47137aaa8 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 22 Dec 2023 16:14:55 -0800 Subject: [PATCH 26/28] Code review. --- .../ILogger/OpenTelemetryLoggingExtensions.cs | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 42c1beaf39c..6d7afc1ee68 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -226,18 +226,22 @@ private static ILoggingBuilder AddOpenTelemetryInternal( * There is a possibility of a circular reference when * accessing LoggerProvider from the IServiceProvider. * - * If LoggerProvider is the first thing accessed, and - * it requires some service which accesses ILogger - * (for example, IHttpClientFactory), then the - * OpenTelemetryLoggerProvider will try to access the - * LoggerProvider inside the initial access to - * LoggerProvider. + * If LoggerProvider is the first thing accessed, and it + * requires some service which accesses ILogger (for + * example, IHttpClientFactory), then the + * OpenTelemetryLoggerProvider will try to access a new + * (second) LoggerProvider while still in the process of + * building the first one: + * + * LoggerProvider -> IHttpClientFactory -> + * ILoggerFactory -> OpenTelemetryLoggerProvider -> + * LoggerProvider * * This check uses the provider reference captured on * LoggerProviderBuilderSdk during construction of - * LoggerProviderSdk to detect if a provider has - * already been created to give to - * OpenTelemetryLoggerProvider. + * LoggerProviderSdk to detect if a provider has already + * been created to give to OpenTelemetryLoggerProvider + * and stop the loop. */ provider = sp.GetRequiredService(); Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider."); From 11d7f4ca69e998e4347d1e3cba0ab16582888572 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 22 Dec 2023 16:23:11 -0800 Subject: [PATCH 27/28] Code review. --- .../Logs/OpenTelemetryLoggingExtensionsTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs index 18cffad9a6f..cdebc61f176 100644 --- a/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs +++ b/test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs @@ -289,6 +289,12 @@ public void CircularReferenceTest(bool requestLoggerProviderDirectly) var factory = sp.GetRequiredService(); Assert.NotNull(factory); } + + var loggerProvider = sp.GetRequiredService() as LoggerProviderSdk; + + Assert.NotNull(loggerProvider); + + Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency); } private class TestLogProcessor : BaseProcessor From df362e466a13114e0910ff06f4f56a9f19f55f8f Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Fri, 22 Dec 2023 16:51:28 -0800 Subject: [PATCH 28/28] Code review. --- .../OtlpLogExporterHelperExtensions.cs | 57 +++++++++---------- .../IntegrationTest/IntegrationTests.cs | 2 + 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 3a0e4f03f17..85298517f95 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -60,13 +60,18 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( return loggerOptions.AddProcessor(sp => { - var exporterOptions = GetOtlpExporterOptions(sp, name, finalOptionsName); + var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); var processorOptions = sp.GetRequiredService>().Get(finalOptionsName); configure?.Invoke(exporterOptions); - return BuildOtlpLogExporter(sp, exporterOptions, processorOptions); + return BuildOtlpLogExporter( + sp, + exporterOptions, + processorOptions, + GetOptions(sp, Options.DefaultName, Options.DefaultName, (sp, c, n) => new SdkLimitOptions(c)), + GetOptions(sp, name, finalOptionsName, (sp, c, n) => new ExperimentalOptions(c))); }); } @@ -99,13 +104,18 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( return loggerOptions.AddProcessor(sp => { - var exporterOptions = GetOtlpExporterOptions(sp, name, finalOptionsName); + var exporterOptions = GetOptions(sp, name, finalOptionsName, OtlpExporterOptions.CreateOtlpExporterOptions); var processorOptions = sp.GetRequiredService>().Get(finalOptionsName); configureExporterAndProcessor?.Invoke(exporterOptions, processorOptions); - return BuildOtlpLogExporter(sp, exporterOptions, processorOptions); + return BuildOtlpLogExporter( + sp, + exporterOptions, + processorOptions, + GetOptions(sp, Options.DefaultName, Options.DefaultName, (sp, c, n) => new SdkLimitOptions(c)), + GetOptions(sp, name, finalOptionsName, (sp, c, n) => new ExperimentalOptions(c))); }); } @@ -332,22 +342,6 @@ static LoggerProviderBuilder AddOtlpExporter( } internal static BaseProcessor BuildOtlpLogExporter( - IServiceProvider sp, - OtlpExporterOptions exporterOptions, - LogRecordExportProcessorOptions processorOptions, - Func, BaseExporter>? configureExporterInstance = null) - { - Debug.Assert(sp != null, "sp was null"); - - var config = sp!.GetRequiredService(); - - var sdkLimitOptions = new SdkLimitOptions(config); - var experimentalOptions = new ExperimentalOptions(config); - - return BuildOtlpLogExporter(sp!, exporterOptions, processorOptions, sdkLimitOptions, experimentalOptions, configureExporterInstance); - } - - private static BaseProcessor BuildOtlpLogExporter( IServiceProvider sp, OtlpExporterOptions exporterOptions, LogRecordExportProcessorOptions processorOptions, @@ -413,7 +407,12 @@ private static void RegisterOptions(IServiceCollection services) services.RegisterOptionsFactory(configuration => new ExperimentalOptions(configuration)); } - private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, string? name, string finalName) + private static T GetOptions( + IServiceProvider sp, + string? name, + string finalName, + Func createOptionsFunc) + where T : class, new() { // Note: If OtlpExporter has been registered for tracing and/or metrics // then IOptionsFactory will be set by a call to @@ -421,15 +420,15 @@ private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, s // are only using logging, we don't have an opportunity to do that // registration so we manually create a factory. - var optionsFactory = sp.GetRequiredService>(); - if (optionsFactory is not DelegatingOptionsFactory) + var optionsFactory = sp.GetRequiredService>(); + if (optionsFactory is not DelegatingOptionsFactory) { - optionsFactory = new DelegatingOptionsFactory( - (c, n) => OtlpExporterOptions.CreateOtlpExporterOptions(sp, c, n), + optionsFactory = new DelegatingOptionsFactory( + (c, n) => createOptionsFunc(sp, c, n), sp.GetRequiredService(), - sp.GetServices>(), - sp.GetServices>(), - sp.GetServices>()); + sp.GetServices>(), + sp.GetServices>(), + sp.GetServices>()); return optionsFactory.Create(finalName); } @@ -445,6 +444,6 @@ private static OtlpExporterOptions GetOtlpExporterOptions(IServiceProvider sp, s // If we have a valid factory AND we are using named options, we can // safely use the Options API fully. - return sp.GetRequiredService>().Get(finalName); + return sp.GetRequiredService>().Get(finalName); } } diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/IntegrationTest/IntegrationTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/IntegrationTest/IntegrationTests.cs index 0fc08270cdf..11b5b797127 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/IntegrationTest/IntegrationTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/IntegrationTest/IntegrationTests.cs @@ -238,6 +238,8 @@ public void LogExportResultIsSuccess(OtlpExportProtocol protocol, string endpoin sp, exporterOptions, processorOptions, + new SdkLimitOptions(), + new ExperimentalOptions(), configureExporterInstance: otlpExporter => { delegatingExporter = new DelegatingExporter