[logging] UseOpenTelemetry extension & WithLogging default behavior (#…

…5072)

src/OpenTelemetry.Extensions.Hosting/.publicApi/Experimental/PublicAPI.Unshipped.txt

@@ -1,2 +1,3 @@
 OpenTelemetry.OpenTelemetryBuilder.WithLogging() -> OpenTelemetry.OpenTelemetryBuilder!
 OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>! configure) -> OpenTelemetry.OpenTelemetryBuilder!
+OpenTelemetry.OpenTelemetryBuilder.WithLogging(System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>? configureBuilder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configureOptions) -> OpenTelemetry.OpenTelemetryBuilder!

src/OpenTelemetry.Extensions.Hosting/CHANGELOG.md

@@ -12,6 +12,11 @@
   APIs.
   ([#4958](https://github.com/open-telemetry/opentelemetry-dotnet/pull/4958))
 
+* The `OpenTelemetryBuilder.WithLogging` experimental API method will now
+  register an `ILoggerProvider` named 'OpenTelemetry' into the
+  `IServiceCollection` to enable `ILoggerFactory` integration.
+  ([#5072](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5072))
+
 ## 1.7.0-alpha.1
 
 Released 2023-Oct-16

src/OpenTelemetry.Extensions.Hosting/OpenTelemetryBuilder.cs

@@ -16,6 +16,7 @@
 
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Diagnostics.Metrics;
+using Microsoft.Extensions.Logging;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Logs;
 using OpenTelemetry.Metrics;
@@ -146,10 +147,17 @@ public OpenTelemetryBuilder WithTracing(Action<TracerProviderBuilder> configure)
     /// Adds logging services into the builder.
     /// </summary>
     /// <remarks>
-    /// <para><b>WARNING</b>: This is an experimental API which might change or be removed in the future. Use at your own risk.</para>
-    /// Note: This is safe to be called multiple times and by library authors.
+    /// <para><b>WARNING</b>: This is an experimental API which might change or
+    /// be removed in the future. Use at your own risk.</para>
+    /// Notes:
+    /// <list type="bullet">
+    /// <item>This is safe to be called multiple times and by library authors.
     /// Only a single <see cref="LoggerProvider"/> will be created for a given
-    /// <see cref="IServiceCollection"/>.
+    /// <see cref="IServiceCollection"/>.</item>
+    /// <item>This method automatically registers an <see
+    /// cref="ILoggerProvider"/> named 'OpenTelemetry' into the <see
+    /// cref="IServiceCollection"/>.</item>
+    /// </list>
     /// </remarks>
     /// <returns>The supplied <see cref="OpenTelemetryBuilder"/> for chaining
     /// calls.</returns>
@@ -159,16 +167,22 @@ public OpenTelemetryBuilder WithTracing(Action<TracerProviderBuilder> configure)
     /// Adds logging services into the builder.
     /// </summary>
     /// <remarks>
-    /// Note: This is safe to be called multiple times and by library authors.
+    /// Notes:
+    /// <list type="bullet">
+    /// <item>This is safe to be called multiple times and by library authors.
     /// Only a single <see cref="LoggerProvider"/> will be created for a given
-    /// <see cref="IServiceCollection"/>.
+    /// <see cref="IServiceCollection"/>.</item>
+    /// <item>This method automatically registers an <see
+    /// cref="ILoggerProvider"/> named 'OpenTelemetry' into the <see
+    /// cref="IServiceCollection"/>.</item>
+    /// </list>
     /// </remarks>
     /// <returns>The supplied <see cref="OpenTelemetryBuilder"/> for chaining
     /// calls.</returns>
     internal
 #endif
         OpenTelemetryBuilder WithLogging()
-        => this.WithLogging(b => { });
+        => this.WithLogging(configureBuilder: null, configureOptions: null);
 
 #if EXPOSE_EXPERIMENTAL_FEATURES
     /// <summary>
@@ -195,9 +209,40 @@ OpenTelemetryBuilder WithLogging(Action<LoggerProviderBuilder> configure)
     {
         Guard.ThrowIfNull(configure);
 
-        var builder = new LoggerProviderBuilderBase(this.Services);
+        return this.WithLogging(configureBuilder: configure, configureOptions: null);
+    }
 
-        configure(builder);
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    /// <summary>
+    /// Adds logging services into the builder.
+    /// </summary>
+    /// <remarks><inheritdoc cref="WithLogging()" path="/remarks"/></remarks>
+    /// <param name="configureBuilder">Optional <see
+    /// cref="LoggerProviderBuilder"/> configuration callback.</param>
+    /// <param name="configureOptions">Optional <see
+    /// cref="OpenTelemetryLoggerOptions"/> configuration callback.</param>
+    /// <returns>The supplied <see cref="OpenTelemetryBuilder"/> for chaining
+    /// calls.</returns>
+    public
+#else
+    /// <summary>
+    /// Adds logging services into the builder.
+    /// </summary>
+    /// <remarks><inheritdoc cref="WithLogging()" path="/remarks"/></remarks>
+    /// <param name="configureBuilder">Optional <see
+    /// cref="LoggerProviderBuilder"/> configuration callback.</param>
+    /// <param name="configureOptions">Optional <see
+    /// cref="OpenTelemetryLoggerOptions"/> configuration callback.</param>
+    /// <returns>The supplied <see cref="OpenTelemetryBuilder"/> for chaining
+    /// calls.</returns>
+    internal
+#endif
+        OpenTelemetryBuilder WithLogging(
+            Action<LoggerProviderBuilder>? configureBuilder,
+            Action<OpenTelemetryLoggerOptions>? configureOptions)
+    {
+        this.Services.AddLogging(
+            logging => logging.UseOpenTelemetry(configureBuilder, configureOptions));
 
         return this;
     }

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

@@ -40,3 +40,6 @@ override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(double value,
 override OpenTelemetry.Metrics.AlwaysOnExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>> tags) -> bool
 override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(double value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>> tags) -> bool
 override OpenTelemetry.Metrics.TraceBasedExemplarFilter.ShouldSample(long value, System.ReadOnlySpan<System.Collections.Generic.KeyValuePair<string!, object?>> tags) -> bool
+static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder) -> Microsoft.Extensions.Logging.ILoggingBuilder!
+static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>! configure) -> Microsoft.Extensions.Logging.ILoggingBuilder!
+static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.UseOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder! builder, System.Action<OpenTelemetry.Logs.LoggerProviderBuilder!>? configureBuilder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions!>? configureOptions) -> Microsoft.Extensions.Logging.ILoggingBuilder!

src/OpenTelemetry/CHANGELOG.md

@@ -64,6 +64,18 @@
   [#4563](https://github.com/open-telemetry/opentelemetry-dotnet/issues/4563).
   ([#5089](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5089))
 
+* Added the `ILoggingBuilder.UseOpenTelemetry` experimental API extension for
+  registering OpenTelemetry `ILogger` integration using `LoggerProviderBuilder`
+  which supports the full DI (`IServiceCollection` \ `IServiceProvider`) API
+  surface (mirrors tracing & metrics).
+  ([#5072](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5072))
+
+* Changed the `ILoggingBuilder` registration extensions (`AddOpenTelemetry` &
+  `UseOpenTelemetry`) to fire the optional `OpenTelemetryLoggerOptions`
+  configuration delegate AFTER the "Logging:OpenTelemetry" `IConfiguration`
+  section has been applied.
+  ([#5072](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5072))
+
 ## 1.7.0-alpha.1
 
 Released 2023-Oct-16

src/OpenTelemetry/Logs/ILogger/OpenTelemetryLoggingExtensions.cs

@@ -17,6 +17,9 @@
 #if NET6_0_OR_GREATER
 using System.Diagnostics.CodeAnalysis;
 #endif
+#if EXPOSE_EXPERIMENTAL_FEATURES
+using System.ComponentModel;
+#endif
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
@@ -48,6 +51,11 @@ public static class OpenTelemetryLoggingExtensions
     /// </remarks>
     /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
     /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    // todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")]
+    // Note: We hide AddOpenTelemetry from IDEs using EditorBrowsable when UseOpenTelemetry is present to reduce confusion.
+    [EditorBrowsable(EditorBrowsableState.Never)]
+#endif
     public static ILoggingBuilder AddOpenTelemetry(
         this ILoggingBuilder builder)
         => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: null);
@@ -59,11 +67,101 @@ public static ILoggingBuilder AddOpenTelemetry(
     /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
     /// <param name="configure">Optional configuration action.</param>
     /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    // todo: [Obsolete("Call UseOpenTelemetry instead this method will be removed in a future version.")]
+    // Note: We hide AddOpenTelemetry from IDEs using EditorBrowsable when UseOpenTelemetry is present to reduce confusion.
+    [EditorBrowsable(EditorBrowsableState.Never)]
+#endif
     public static ILoggingBuilder AddOpenTelemetry(
         this ILoggingBuilder builder,
         Action<OpenTelemetryLoggerOptions>? configure)
         => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: configure);
 
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks>
+    /// <para><b>WARNING</b>: This is an experimental API which might change or be removed in the future. Use at your own risk.</para>
+    /// Note: This is safe to be called multiple times and by library authors.
+    /// Only a single <see cref="OpenTelemetryLoggerProvider"/> will be created
+    /// for a given <see cref="IServiceCollection"/>.
+    /// </remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    public
+#else
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks>
+    /// Note: This is safe to be called multiple times and by library authors.
+    /// Only a single <see cref="OpenTelemetryLoggerProvider"/> will be created
+    /// for a given <see cref="IServiceCollection"/>.
+    /// </remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    internal
+#endif
+        static ILoggingBuilder UseOpenTelemetry(
+        this ILoggingBuilder builder)
+        => AddOpenTelemetryInternal(builder, configureBuilder: null, configureOptions: null);
+
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks><inheritdoc cref="UseOpenTelemetry(ILoggingBuilder)" path="/remarks"/></remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <param name="configure">Optional <see cref="LoggerProviderBuilder"/> configuration action.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    public
+#else
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks><inheritdoc cref="UseOpenTelemetry(ILoggingBuilder)" path="/remarks"/></remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <param name="configure"><see cref="LoggerProviderBuilder"/> configuration action.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    internal
+#endif
+        static ILoggingBuilder UseOpenTelemetry(
+        this ILoggingBuilder builder,
+        Action<LoggerProviderBuilder> configure)
+    {
+        Guard.ThrowIfNull(configure);
+
+        return AddOpenTelemetryInternal(builder, configureBuilder: configure, configureOptions: null);
+    }
+
+#if EXPOSE_EXPERIMENTAL_FEATURES
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks><inheritdoc cref="UseOpenTelemetry(ILoggingBuilder)" path="/remarks"/></remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <param name="configureBuilder">Optional <see cref="LoggerProviderBuilder"/> configuration action.</param>
+    /// <param name="configureOptions">Optional <see cref="OpenTelemetryLoggerOptions"/> configuration action.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    public
+#else
+    /// <summary>
+    /// Adds an OpenTelemetry logger named 'OpenTelemetry' to the <see cref="ILoggerFactory"/>.
+    /// </summary>
+    /// <remarks><inheritdoc cref="UseOpenTelemetry(ILoggingBuilder)" path="/remarks"/></remarks>
+    /// <param name="builder">The <see cref="ILoggingBuilder"/> to use.</param>
+    /// <param name="configureBuilder">Optional <see cref="LoggerProviderBuilder"/> configuration action.</param>
+    /// <param name="configureOptions">Optional <see cref="OpenTelemetryLoggerOptions"/> configuration action.</param>
+    /// <returns>The supplied <see cref="ILoggingBuilder"/> for call chaining.</returns>
+    internal
+#endif
+        static ILoggingBuilder UseOpenTelemetry(
+        this ILoggingBuilder builder,
+        Action<LoggerProviderBuilder>? configureBuilder,
+        Action<OpenTelemetryLoggerOptions>? configureOptions)
+        => AddOpenTelemetryInternal(builder, configureBuilder, configureOptions);
+
     private static ILoggingBuilder AddOpenTelemetryInternal(
         ILoggingBuilder builder,
         Action<LoggerProviderBuilder>? configureBuilder,
@@ -75,19 +173,19 @@ private static ILoggingBuilder AddOpenTelemetryInternal(
 
         var services = builder.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);
 
         services.AddOpenTelemetrySharedProviderBuilderServices();
 
+        if (configureOptions != null)
+        {
+            // Note: Order is important here so that user-supplied delegate
+            // fires AFTER the options are bound to Logging:OpenTelemetry
+            // configuration.
+            services.Configure(configureOptions);
+        }
+
         var loggingBuilder = new LoggerProviderBuilderBase(services).ConfigureBuilder(
             (sp, logging) =>
             {

test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetryServicesExtensionsTests.cs

@@ -381,7 +381,7 @@ public void AddOpenTelemetry_WithLogging_DisposalTest()
     }
 
     [Fact]
-    public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest()
+    public void AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest()
     {
         bool configureBuilderCalled = false;
 
@@ -416,14 +416,8 @@ public async Task AddOpenTelemetry_WithLogging_HostConfigurationHonoredTest()
 
         var host = builder.Build();
 
-        Assert.False(configureBuilderCalled);
-
-        await host.StartAsync();
-
         Assert.True(configureBuilderCalled);
 
-        await host.StopAsync();
-
         host.Dispose();
     }