-
Notifications
You must be signed in to change notification settings - Fork 851
Expose wrapHandlersPipeline parameter in AddExtendedHttpClientLogging API #7231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
19a2a6c
930c40d
3da8f98
4373f93
4018e1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Net.Http; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Microsoft.Extensions.DependencyInjection.Extensions; | ||
|
|
@@ -10,6 +11,7 @@ | |
| using Microsoft.Extensions.Http.Logging.Internal; | ||
| using Microsoft.Extensions.Options; | ||
| using Microsoft.Extensions.Telemetry.Internal; | ||
| using Microsoft.Shared.DiagnosticIds; | ||
| using Microsoft.Shared.Diagnostics; | ||
|
|
||
| namespace Microsoft.Extensions.DependencyInjection; | ||
|
|
@@ -34,7 +36,31 @@ public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBu | |
| { | ||
| _ = Throw.IfNull(builder); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder); | ||
| return AddExtendedHttpClientLoggingInternal(builder, configureOptionsBuilder: null, wrapHandlersPipeline: true); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>. | ||
| /// </summary> | ||
| /// <param name="builder">The <see cref="IHttpClientBuilder" />.</param> | ||
| /// <param name="wrapHandlersPipeline"> | ||
| /// Determines the logging behavior when resilience strategies (like retries or hedging) are configured. | ||
| /// When <see langword="true"/>, logs one record per logical request with total duration including all retry attempts. | ||
| /// When <see langword="false"/>, logs each individual attempt separately with per-attempt duration. | ||
| /// </param> | ||
| /// <returns>The value of <paramref name="builder"/>.</returns> | ||
| /// <remarks> | ||
| /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>. | ||
| /// A lot of the information logged by this method (like bodies, methods, host, path, and duration) will be added as enrichment tags to the structured log. Make sure | ||
| /// you have a way of viewing structured logs in order to view this extra information. | ||
| /// </remarks> | ||
| /// <exception cref="ArgumentNullException">Argument <paramref name="builder"/> is <see langword="null"/>.</exception> | ||
| [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] | ||
| public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, bool wrapHandlersPipeline) | ||
| { | ||
| _ = Throw.IfNull(builder); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder, configureOptionsBuilder: null, wrapHandlersPipeline); | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -54,7 +80,33 @@ public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBu | |
| _ = Throw.IfNull(builder); | ||
| _ = Throw.IfNull(section); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Bind(section)); | ||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Bind(section), wrapHandlersPipeline: true); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>. | ||
| /// </summary> | ||
| /// <param name="builder">The <see cref="IHttpClientBuilder" />.</param> | ||
| /// <param name="section">The <see cref="IConfigurationSection"/> to use for configuring <see cref="LoggingOptions"/>.</param> | ||
| /// <param name="wrapHandlersPipeline"> | ||
| /// Determines the logging behavior when resilience strategies (like retries or hedging) are configured. | ||
| /// When <see langword="true"/>, logs one record per logical request with total duration including all retry attempts. | ||
| /// When <see langword="false"/>, logs each individual attempt separately with per-attempt duration. | ||
| /// </param> | ||
| /// <returns>The value of <paramref name="builder"/>.</returns> | ||
| /// <remarks> | ||
| /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>. | ||
| /// A lot of the information logged by this method (like bodies, methods, host, path, and duration) will be added as enrichment tags to the structured log. Make sure | ||
| /// you have a way of viewing structured logs in order to view this extra information. | ||
| /// </remarks> | ||
| /// <exception cref="ArgumentNullException">Any of the arguments is <see langword="null"/>.</exception> | ||
| [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] | ||
| public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section, bool wrapHandlersPipeline) | ||
| { | ||
| _ = Throw.IfNull(builder); | ||
| _ = Throw.IfNull(section); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Bind(section), wrapHandlersPipeline); | ||
| } | ||
|
Comment on lines
+103
to
110
|
||
|
|
||
| /// <summary> | ||
|
|
@@ -74,10 +126,39 @@ public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBu | |
| _ = Throw.IfNull(builder); | ||
| _ = Throw.IfNull(configure); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Configure(configure)); | ||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Configure(configure), wrapHandlersPipeline: true); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Adds an <see cref="IHttpClientAsyncLogger" /> to emit logs for outgoing requests for a named <see cref="HttpClient"/>. | ||
| /// </summary> | ||
| /// <param name="builder">The <see cref="IHttpClientBuilder" />.</param> | ||
| /// <param name="configure">The delegate to configure <see cref="LoggingOptions"/> with.</param> | ||
| /// <param name="wrapHandlersPipeline"> | ||
| /// Determines the logging behavior when resilience strategies (like retries or hedging) are configured. | ||
| /// When <see langword="true"/>, logs one record per logical request with total duration including all retry attempts. | ||
| /// When <see langword="false"/>, logs each individual attempt separately with per-attempt duration. | ||
| /// </param> | ||
| /// <returns>The value of <paramref name="builder"/>.</returns> | ||
| /// <remarks> | ||
| /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>. | ||
| /// A lot of the information logged by this method (like bodies, methods, host, path, and duration) will be added as enrichment tags to the structured log. Make sure | ||
| /// you have a way of viewing structured logs in order to view this extra information. | ||
| /// </remarks> | ||
| /// <exception cref="ArgumentNullException">Any of the arguments is <see langword="null"/>.</exception> | ||
| [Experimental(diagnosticId: DiagnosticIds.Experiments.Telemetry, UrlFormat = DiagnosticIds.UrlFormat)] | ||
| public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure, bool wrapHandlersPipeline) | ||
| { | ||
| _ = Throw.IfNull(builder); | ||
| _ = Throw.IfNull(configure); | ||
|
|
||
| return AddExtendedHttpClientLoggingInternal(builder, options => options.Configure(configure), wrapHandlersPipeline); | ||
| } | ||
|
Comment on lines
+149
to
156
|
||
|
|
||
| private static IHttpClientBuilder AddExtendedHttpClientLoggingInternal(IHttpClientBuilder builder, Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder = null) | ||
| private static IHttpClientBuilder AddExtendedHttpClientLoggingInternal( | ||
| IHttpClientBuilder builder, | ||
| Action<OptionsBuilder<LoggingOptions>>? configureOptionsBuilder, | ||
| bool wrapHandlersPipeline) | ||
| { | ||
| var optionsBuilder = builder.Services | ||
| .AddOptionsWithValidateOnStart<LoggingOptions, LoggingOptionsValidator>(builder.Name); | ||
|
|
@@ -97,6 +178,6 @@ private static IHttpClientBuilder AddExtendedHttpClientLoggingInternal(IHttpClie | |
| .RemoveAllLoggers() | ||
| .AddLogger( | ||
| serviceProvider => serviceProvider.GetRequiredKeyedService<HttpClientLogger>(builder.Name), | ||
| wrapHandlersPipeline: true); | ||
| wrapHandlersPipeline); | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new overload with wrapHandlersPipeline parameter is marked with [Experimental] attribute, but the existing overload without this parameter (line 35) is not, even though both can result in the same behavior (wrapHandlersPipeline: true). This creates an inconsistency where users calling AddExtendedHttpClientLogging(builder, true) get an experimental warning, but users calling AddExtendedHttpClientLogging(builder) (which also uses wrapHandlersPipeline: true internally) do not. Consider either: (1) marking all overloads as experimental for consistency, (2) removing the experimental attribute from the new overloads if the underlying functionality is stable, or (3) providing clear documentation explaining why only the new overloads are experimental.