Update HttpClient logging APIs per Noah's comments

dotnet · Sep 26, 2023 · 429c3b3 · 429c3b3
1 parent 57c0bd6
commit 429c3b3
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 83 deletions.
diff --git a/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs b/bench/Libraries/Microsoft.Extensions.Http.Diagnostics.PerformanceTests/HttpClientFactory.cs
@@ -45,7 +45,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse(string fil
             .AddFakeRedaction()
             .AddSingleton(_ => NoRemoteCallHandler.Create(fileName))
             .AddHttpClient(nameof(fileName))
-            .AddHttpClientLogging(options =>
+            .AddExtendedHttpClientLogging(options =>
             {
                 options.BodySizeLimit = readLimit;
                 options.ResponseBodyContentTypes.Add(new("application/json"));
@@ -68,7 +68,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll(string fileName
             .AddFakeRedaction()
             .AddSingleton(_ => NoRemoteCallHandler.Create(fileName))
             .AddHttpClient(nameof(fileName))
-            .AddHttpClientLogging(options =>
+            .AddExtendedHttpClientLogging(options =>
             {
                 options.BodySizeLimit = readLimit;
 
@@ -95,7 +95,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogRequest_ChunkedEnco
             .AddFakeRedaction()
             .AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
             .AddHttpClient(nameof(fileName))
-            .AddHttpClientLogging(options =>
+            .AddExtendedHttpClientLogging(options =>
             {
                 options.BodySizeLimit = readLimit;
                 options.RequestBodyContentTypes.Add("application/json");
@@ -118,7 +118,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogResponse_ChunkedEnc
             .AddFakeRedaction()
             .AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
             .AddHttpClient(nameof(fileName))
-            .AddHttpClientLogging(options =>
+            .AddExtendedHttpClientLogging(options =>
             {
                 options.BodySizeLimit = readLimit;
                 options.ResponseBodyContentTypes.Add("application/json");
@@ -141,7 +141,7 @@ public static System.Net.Http.HttpClient CreateWithLoggingLogAll_ChunkedEncoding
             .AddFakeRedaction()
             .AddSingleton(_ => NoRemoteCallNotSeekableHandler.Create(fileName))
             .AddHttpClient(nameof(fileName))
-            .AddHttpClientLogging(options =>
+            .AddExtendedHttpClientLogging(options =>
             {
                 options.BodySizeLimit = readLimit;
 

diff --git a/...ies/Microsoft.Extensions.Http.Diagnostics/Latency/HttpClientLatencyTelemetryExtensions.cs b/...ies/Microsoft.Extensions.Http.Diagnostics/Latency/HttpClientLatencyTelemetryExtensions.cs
@@ -24,20 +24,21 @@ public static class HttpClientLatencyTelemetryExtensions
     /// <remarks>
     /// This extension configures latency information collection globally for all http clients.
     /// </remarks>
-    /// <param name="services">The <see cref="IServiceCollection" />.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services)
+    /// <param name="builder">The client builder.</param>
+    /// <returns>The value of <paramref name="builder"/>.</returns>
+    public static IHttpClientBuilder AddDefaultHttpClientLatencyTelemetry(this IHttpClientBuilder builder)
     {
-        _ = Throw.IfNull(services);
+        _ = Throw.IfNull(builder);
 
+        var services = builder.Services;
         _ = services.RegisterCheckpointNames(HttpCheckpoints.Checkpoints);
         _ = services.AddOptions<HttpClientLatencyTelemetryOptions>();
         _ = services.AddActivatedSingleton<HttpRequestLatencyListener>();
         _ = services.AddActivatedSingleton<HttpClientLatencyContext>();
         _ = services.AddTransient<HttpLatencyTelemetryHandler>();
         _ = services.AddHttpClientLogEnricher<HttpClientLatencyLogEnricher>();
 
-        return services.ConfigureAll<HttpClientFactoryOptions>(
+        _ = services.ConfigureAll<HttpClientFactoryOptions>(
             httpClientOptions =>
             {
                 httpClientOptions
@@ -47,6 +48,8 @@ public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServ
                     httpMessageHandlerBuilder.AdditionalHandlers.Add(handler);
                 });
             });
+
+        return builder;
     }
 
     /// <summary>
@@ -55,21 +58,21 @@ public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServ
     /// <remarks>
     /// This extension configures outgoing request logs auto collection globally for all http clients.
     /// </remarks>
-    /// <param name="services">The <see cref="IServiceCollection" />.</param>
+    /// <param name="builder">The client builder.</param>
     /// <param name="section">The <see cref="IConfigurationSection"/> to use for configuring <see cref="HttpClientLatencyTelemetryOptions"/>.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
+    /// <returns>The value of <paramref name="builder"/>.</returns>
     [DynamicDependency(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicParameterlessConstructor, typeof(LoggingOptions))]
     [UnconditionalSuppressMessage("Trimming",
         "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
         Justification = "Addressed with [DynamicDependency]")]
-    public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services, IConfigurationSection section)
+    public static IHttpClientBuilder AddDefaultHttpClientLatencyTelemetry(this IHttpClientBuilder builder, IConfigurationSection section)
     {
+        _ = Throw.IfNull(builder);
         _ = Throw.IfNull(section);
 
-        _ = services
-            .Configure<HttpClientLatencyTelemetryOptions>(section);
+        _ = builder.Services.Configure<HttpClientLatencyTelemetryOptions>(section);
 
-        return services.AddDefaultHttpClientLatencyTelemetry();
+        return builder.AddDefaultHttpClientLatencyTelemetry();
     }
 
     /// <summary>
@@ -78,16 +81,16 @@ public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServ
     /// <remarks>
     /// This extension configures outgoing request logs auto collection globally for all http clients.
     /// </remarks>
-    /// <param name="services">The <see cref="IServiceCollection" />.</param>
+    /// <param name="builder">The client builder.</param>
     /// <param name="configure">The delegate to configure <see cref="HttpClientLatencyTelemetryOptions"/> with.</param>
-    /// <returns>The value of <paramref name="services"/>.</returns>
-    public static IServiceCollection AddDefaultHttpClientLatencyTelemetry(this IServiceCollection services, Action<HttpClientLatencyTelemetryOptions> configure)
+    /// <returns>The value of <paramref name="builder"/>.</returns>
+    public static IHttpClientBuilder AddDefaultHttpClientLatencyTelemetry(this IHttpClientBuilder builder, Action<HttpClientLatencyTelemetryOptions> configure)
     {
+        _ = Throw.IfNull(builder);
         _ = Throw.IfNull(configure);
 
-        _ = services
-            .Configure(configure);
+        _ = builder.Services.Configure(configure);
 
-        return services.AddDefaultHttpClientLatencyTelemetry();
+        return builder.AddDefaultHttpClientLatencyTelemetry();
     }
 }
diff --git a/...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingHttpClientBuilderExtensions.cs b/...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingHttpClientBuilderExtensions.cs
@@ -31,11 +31,11 @@ public static class HttpClientLoggingHttpClientBuilderExtensions
     /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
     /// </remarks>
     /// <exception cref="ArgumentNullException">Argument <paramref name="builder"/> is <see langword="null"/>.</exception>
-    public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder)
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder)
     {
         _ = Throw.IfNull(builder);
 
-        return AddNamedClientLoggingInternal(builder);
+        return AddExtendedHttpClientLoggingInternal(builder);
     }
 
     /// <summary>
@@ -54,12 +54,12 @@ public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder bu
     [UnconditionalSuppressMessage("Trimming",
         "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
         Justification = "Addressed with [DynamicDependency]")]
-    public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section)
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section)
     {
         _ = Throw.IfNull(builder);
         _ = Throw.IfNull(section);
 
-        return AddNamedClientLoggingInternal(builder, options => options.Bind(section));
+        return AddExtendedHttpClientLoggingInternal(builder, options => options.Bind(section));
     }
 
     /// <summary>
@@ -78,15 +78,15 @@ public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder bu
     [UnconditionalSuppressMessage("Trimming",
         "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
         Justification = "Addressed with [DynamicDependency]")]
-    public static IHttpClientBuilder AddHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure)
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure)
     {
         _ = Throw.IfNull(builder);
         _ = Throw.IfNull(configure);
 
-        return AddNamedClientLoggingInternal(builder, options => options.Configure(configure));
+        return AddExtendedHttpClientLoggingInternal(builder, options => options.Configure(configure));
     }
 
-    private static IHttpClientBuilder AddNamedClientLoggingInternal(IHttpClientBuilder builder, Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder = null)
+    private static IHttpClientBuilder AddExtendedHttpClientLoggingInternal(IHttpClientBuilder builder, Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder = null)
     {
         var optionsBuilder = builder.Services
             .AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>(builder.Name);

diff --git a/...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingServiceCollectionExtensions.cs b/...osoft.Extensions.Http.Diagnostics/Logging/HttpClientLoggingServiceCollectionExtensions.cs
@@ -28,7 +28,7 @@ public static class HttpClientLoggingServiceCollectionExtensions
     /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
     /// </remarks>
     /// <exception cref="ArgumentNullException">Argument <paramref name="services"/> is <see langword="null"/>.</exception>
-    public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services)
+    public static IServiceCollection AddDefaultExtendedHttpClientLogging(this IServiceCollection services)
     {
         _ = Throw.IfNull(services);
 
@@ -62,7 +62,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
     [UnconditionalSuppressMessage("Trimming",
         "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code",
         Justification = "Addressed with [DynamicDependency]")]
-    public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services, IConfigurationSection section)
+    public static IServiceCollection AddDefaultExtendedHttpClientLogging(this IServiceCollection services, IConfigurationSection section)
     {
         _ = Throw.IfNull(services);
         _ = Throw.IfNull(section);
@@ -71,7 +71,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
             .AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>()
             .Bind(section);
 
-        return services.AddDefaultHttpClientLogging();
+        return services.AddDefaultExtendedHttpClientLogging();
     }
 
     /// <summary>
@@ -84,7 +84,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
     /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>.
     /// </remarks>
     /// <exception cref="ArgumentNullException">Any of the arguments is <see langword="null"/>.</exception>
-    public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services, Action<LoggingOptions> configure)
+    public static IServiceCollection AddDefaultExtendedHttpClientLogging(this IServiceCollection services, Action<LoggingOptions> configure)
     {
         _ = Throw.IfNull(services);
         _ = Throw.IfNull(configure);
@@ -93,7 +93,7 @@ public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollec
             .AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>()
             .Configure(configure);
 
-        return services.AddDefaultHttpClientLogging();
+        return services.AddDefaultExtendedHttpClientLogging();
     }
 
     /// <summary>