diff --git a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs index 83bdd6907a..5d425778ac 100644 --- a/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs +++ b/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpLogExporterHelperExtensions.cs @@ -15,7 +15,12 @@ // using Microsoft.Extensions.Logging; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using OpenTelemetry.Exporter; +using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; +using OpenTelemetry.Internal; +using System.Diagnostics; namespace OpenTelemetry.Logs; @@ -45,14 +50,197 @@ public static OpenTelemetryLoggerOptions AddOtlpExporter( => AddOtlpExporterInternal(loggerOptions, configure); /// - /// Adds an OTLP Exporter to the OpenTelemetry . + /// Adds OpenTelemetry Protocol (OTLP) exporter to the LoggerProvider. /// - /// options to use. - /// Callback action for configuring and . - /// The instance of to chain the calls. - public static OpenTelemetryLoggerOptions AddOtlpExporter( - this OpenTelemetryLoggerOptions loggerOptions, + /// builder to use. + /// The instance of to chain the calls. + public static LoggerProviderBuilder AddOtlpExporter(this LoggerProviderBuilder builder) + => AddOtlpExporter(builder, name: null, configureExporter: null); + + /// + /// Adds OpenTelemetry Protocol (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 OpenTelemetry Protocol (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, + sp); + }); + } + + /// + /// 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); + }); + } + + 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); + + 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 OpenTelemetryLoggerOptions AddOtlpExporterInternal( + OpenTelemetryLoggerOptions loggerOptions, + Action configure) { var exporterOptions = new OtlpExporterOptions(); var processorOptions = new LogRecordExportProcessorOptions(); diff --git a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs index 805808488a..e4f540e6f0 100644 --- a/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs +++ b/src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs @@ -190,7 +190,7 @@ OpenTelemetryBuilder WithLogging(Action configure) { Guard.ThrowIfNull(configure); - var builder = new LoggerProviderBuilderBase(this.Services); + var builder = new LoggerProviderBuilderBase(this.Services, addSharedServices: false); configure(builder); diff --git a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs index 5b6c65beab..cc13fa7a6e 100644 --- a/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs +++ b/src/OpenTelemetry/Logs/Builder/LoggerProviderBuilderBase.cs @@ -46,10 +46,21 @@ public LoggerProviderBuilderBase() this.allowBuild = true; } - internal LoggerProviderBuilderBase(IServiceCollection services) + internal LoggerProviderBuilderBase(IServiceCollection services, bool addSharedServices) { Guard.ThrowIfNull(services); + if (addSharedServices) + { + /* 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(); + } + services .AddOpenTelemetryLoggerProviderBuilderServices() .TryAddSingleton(sp => new LoggerProviderSdk(sp, ownsServiceProvider: false)); diff --git a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs index 5776241441..0476aed234 100644 --- a/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs +++ b/src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs @@ -14,6 +14,9 @@ // limitations under the License. // +#nullable enable + +using System.Diagnostics; #if NET6_0_OR_GREATER using System.Diagnostics.CodeAnalysis; #endif @@ -58,7 +61,7 @@ public static ILoggingBuilder AddOpenTelemetry( // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions RegisterLoggerProviderOptions(builder.Services); - new LoggerProviderBuilderBase(builder.Services).ConfigureBuilder( + new LoggerProviderBuilderBase(builder.Services, addSharedServices: true).ConfigureBuilder( (sp, logging) => { var options = sp.GetRequiredService>().CurrentValue; @@ -80,10 +83,40 @@ public static ILoggingBuilder AddOpenTelemetry( builder.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 the 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) than 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; diff --git a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs index 172ec25456..f95252998c 100644 --- a/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs +++ b/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs @@ -41,10 +41,33 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests { private static readonly SdkLimitOptions DefaultSdkLimitOptions = new(); - [Fact] - public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse() - { - bool optionsValidated = false; + [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 AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse() + { + bool optionsValidated = false; AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true); var logRecords = new List(); @@ -893,20 +916,130 @@ public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_ContainsScopeAttributeDoubl Assert.Equal(scopeValue.ToString(), actualScope.Value.DoubleValue.ToString()); } - [Fact] - public void ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfTypeString_ScopeIsIgnored() - { - // Arrange. - var logRecords = new List(1); - using var loggerFactory = LoggerFactory.Create(builder => + [Fact] + public void TestAddOtlpExporter_NamedOptions() { - builder.AddOpenTelemetry(options => + 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() + .AddOtlpExporter("Exporter2", o => { }) + .AddOtlpExporter("Exporter3", (eo, po) => { }) + .Build(); + + Assert.Equal(2, defaultExporterOptionsConfigureOptionsInvocations); + Assert.Equal(4, namedExporterOptionsConfigureOptionsInvocations); + } + + [Fact] + public void UserHttpFactoryCalled() + { + OtlpExporterOptions options = new OtlpExporterOptions(); + + var defaultFactory = options.HttpClientFactory; + + int invocations = 0; + options.Protocol = OtlpExportProtocol.HttpProtobuf; + options.HttpClientFactory = () => { - options.IncludeScopes = true; - options.AddInMemoryExporter(logRecords); + 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); }); - }); - var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests)); + + options.HttpClientFactory = () => null; + Assert.Throws(() => + { + using var exporter = new OtlpLogExporter(options); + }); + } + + [Fact] + public void TestAddOtlpExporter_SetsDefaultBatchExportProcessor() + { + 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 = System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.Instance | System.Reflection.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 ToOtlpLog_WhenOptionsIncludeScopesIsTrue_AndScopeStateIsOfTypeString_ScopeIsIgnored() + { + // Arrange. + var logRecords = new List(1); + using var loggerFactory = LoggerFactory.Create(builder => + { + builder.AddOpenTelemetry(options => + { + options.IncludeScopes = true; + options.AddInMemoryExporter(logRecords); + }); + }); + var logger = loggerFactory.CreateLogger(nameof(OtlpLogExporterTests)); const string scopeState = "Some scope state";