Merge branch 'main' into 4466-bug-httpactivityfeature

src/OpenTelemetry.Api.ProviderBuilderExtensions/AssemblyInfo.cs

@@ -9,6 +9,8 @@
 
 #if !EXPOSE_EXPERIMENTAL_FEATURES
 [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
 

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/.publicApi/Experimental/PublicAPI.Unshipped.txt

@@ -0,0 +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<OpenTelemetry.Exporter.OtlpExporterOptions!, OpenTelemetry.Logs.LogRecordExportProcessorOptions!>? configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
+static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, string? name, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!>? configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder!
+static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!, OpenTelemetry.Logs.LogRecordExportProcessorOptions!>! configureExporterAndProcessor) -> OpenTelemetry.Logs.LoggerProviderBuilder!
+static OpenTelemetry.Logs.OtlpLogExporterHelperExtensions.AddOtlpExporter(this OpenTelemetry.Logs.LoggerProviderBuilder! builder, System.Action<OpenTelemetry.Exporter.OtlpExporterOptions!>! configureExporter) -> OpenTelemetry.Logs.LoggerProviderBuilder!

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

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj

@@ -34,10 +34,11 @@
     <!-- Note: When '$(ExposeExperimentalFeatures)' == 'false' these links are
     NOT required because this project sees API + SDK internals -->
     <Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" RequiresExposedExperimentalFeatures="true" />
-    <Compile Include="$(RepoRoot)\src\Shared\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
+    <Compile Include="$(RepoRoot)\src\Shared\DiagnosticDefinitions.cs" Link="Includes\DiagnosticDefinitions.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" RequiresExposedExperimentalFeatures="true" />
+    <Compile Include="$(RepoRoot)\src\Shared\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\StatusHelper.cs" Link="Includes\StatusHelper.cs" RequiresExposedExperimentalFeatures="true" />

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs

@@ -7,6 +7,7 @@
 #if EXPOSE_EXPERIMENTAL_FEATURES
 using System.ComponentModel;
 #endif
+using System.Diagnostics;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
@@ -169,9 +170,15 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
 
         var services = builder.Services;
 
-        // Note: This will bind logger options element (eg "Logging:OpenTelemetry") to OpenTelemetryLoggerOptions
+        // Note: This will bind logger options element (e.g., "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();
 
         if (configureOptions != null)
@@ -206,10 +213,45 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
 
         services.TryAddEnumerable(
             ServiceDescriptor.Singleton<ILoggerProvider, OpenTelemetryLoggerProvider>(
-                sp => new OpenTelemetryLoggerProvider(
-                    sp.GetRequiredService<LoggerProvider>(),
-                    sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
-                    disposeProvider: false)));
+                sp =>
+                {
+                    var state = sp.GetRequiredService<LoggerProviderBuilderSdk>();
+
+                    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 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
+                         * and stop the loop.
+                         */
+                        provider = sp.GetRequiredService<LoggerProvider>();
+                        Debug.Assert(provider == state.Provider, "state.Provider did not match resolved LoggerProvider.");
+                    }
+
+                    return new OpenTelemetryLoggerProvider(
+                        provider,
+                        sp.GetRequiredService<IOptionsMonitor<OpenTelemetryLoggerOptions>>().CurrentValue,
+                        disposeProvider: false);
+                }));
 
         return builder;
 

src/OpenTelemetry/OpenTelemetry.csproj

@@ -6,7 +6,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="Microsoft.Extensions.Logging" />
     <PackageReference Include="Microsoft.Extensions.Logging.Configuration" />
   </ItemGroup>
 

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<LogRecord>

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpLogExporterTests.cs

@@ -27,6 +27,125 @@ public class OtlpLogExporterTests : Http2UnencryptedSupportTests
 {
     private static readonly SdkLimitOptions DefaultSdkLimitOptions = new();
 
+    [Fact]
+    public void AddOtlpExporterWithNamedOptions()
+    {
+        int defaultConfigureExporterOptionsInvocations = 0;
+        int namedConfigureExporterOptionsInvocations = 0;
+
+        int defaultConfigureSdkLimitsOptionsInvocations = 0;
+        int namedConfigureSdkLimitsOptionsInvocations = 0;
+
+        using var loggerProvider = Sdk.CreateLoggerProviderBuilder()
+            .ConfigureServices(services =>
+            {
+                services.Configure<OtlpExporterOptions>(o => defaultConfigureExporterOptionsInvocations++);
+                services.Configure<LogRecordExportProcessorOptions>(o => defaultConfigureExporterOptionsInvocations++);
+                services.Configure<ExperimentalOptions>(o => defaultConfigureExporterOptionsInvocations++);
+
+                services.Configure<OtlpExporterOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);
+                services.Configure<LogRecordExportProcessorOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);
+                services.Configure<ExperimentalOptions>("Exporter2", o => namedConfigureExporterOptionsInvocations++);
+
+                services.Configure<OtlpExporterOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);
+                services.Configure<LogRecordExportProcessorOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);
+                services.Configure<ExperimentalOptions>("Exporter3", o => namedConfigureExporterOptionsInvocations++);
+
+                services.Configure<SdkLimitOptions>(o => defaultConfigureSdkLimitsOptionsInvocations++);
+                services.Configure<SdkLimitOptions>("Exporter2", o => namedConfigureSdkLimitsOptionsInvocations++);
+                services.Configure<SdkLimitOptions>("Exporter3", o => namedConfigureSdkLimitsOptionsInvocations++);
+            })
+            .AddOtlpExporter()
+            .AddOtlpExporter("Exporter2", o => { })
+            .AddOtlpExporter("Exporter3", o => { })
+            .Build();
+
+        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]
+    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<InvalidOperationException>(() =>
+        {
+            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<LogRecord>)
+                .Assembly
+                .GetType("OpenTelemetry.Logs.LoggerProviderSdk")
+                .GetProperty("Processor", bindingFlags)
+                .GetValue(loggerProvider) as BatchExportProcessor<LogRecord>;
+
+            Assert.NotNull(processor);
+
+            var scheduledDelayMilliseconds = typeof(BatchExportProcessor<LogRecord>)
+                .GetField("scheduledDelayMilliseconds", bindingFlags)
+                .GetValue(processor);
+
+            Assert.Equal(5000, scheduledDelayMilliseconds);
+        }
+    }
+
     [Fact]
     public void AddOtlpLogExporterReceivesAttributesWithParseStateValueSetToFalse()
     {

test/OpenTelemetry.Tests/Logs/OpenTelemetryLoggingExtensionsTests.cs

@@ -266,7 +266,60 @@ public void VerifyExceptionIsThrownWhenImplementationFactoryIsNull()
         Assert.Throws<ArgumentNullException>(() => sp.GetRequiredService<LoggerProvider>() 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<TestLogProcessorWithILoggerFactoryDependency>());
+
+        using var sp = services.BuildServiceProvider();
+
+        if (requestLoggerProviderDirectly)
+        {
+            var provider = sp.GetRequiredService<LoggerProvider>();
+            Assert.NotNull(provider);
+        }
+        else
+        {
+            var factory = sp.GetRequiredService<ILoggerFactory>();
+            Assert.NotNull(factory);
+        }
+
+        var loggerProvider = sp.GetRequiredService<LoggerProvider>() as LoggerProviderSdk;
+
+        Assert.NotNull(loggerProvider);
+
+        Assert.True(loggerProvider.Processor is TestLogProcessorWithILoggerFactoryDependency);
+    }
+
     private class TestLogProcessor : BaseProcessor<LogRecord>
     {
     }
+
+    private class TestLogProcessorWithILoggerFactoryDependency : BaseProcessor<LogRecord>
+    {
+        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);
+        }
+    }
 }