[API Proposal]: Allow HttpClientLoggerHandler in different places of the handler pipeline

[radek-vlacil]

Background and motivation

Currently the set of AddExtendedHttpClientLogging methods is eventually calling the AddLogger method with wrapHandlersPipeline == true, as visible here. This forces the HttpClientLoggerHandler to be added to the top of the handler chain, as visible here.

This does not fit use cases where all the outgoing requests from the HttpClient pipeline should be tracked. If people for example would want to log Polly resilience handler retries. I would like to therefore propose new overloads for the extension methods which would surface the wrapHandlersPipeline bool parameter. This way any caller could select which behavior is desired.

API Proposal

namespace Microsoft.Extensions.DependencyInjection;

public static class HttpClientLoggingHttpClientBuilderExtensions
{
    // Existing methods.
    // They register the HttpClientLoggingHandler as the top-most handler in the pipeline.
    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder);
    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section);
    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure);

    // New methods have a new parameter "bool wrapHandlersPipeline".
    // When wrapHandlersPipeline = true the HttpClientLoggingHandler is registered as the top-most handler.
    // When wrapHandlersPipeline = true it is registered at the place of the method invocation.
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, bool wrapHandlersPipeline);
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, IConfigurationSection section, bool wrapHandlersPipeline);
+    public static IHttpClientBuilder AddExtendedHttpClientLogging(this IHttpClientBuilder builder, Action<LoggingOptions> configure, bool wrapHandlersPipeline);
}

This design matches the approach used for registering HttpClientLogger in dotnet/runtime. See the AddLogger method.

There’s also a set of extension methods for IServiceCollection that registers HttpClientLoggingHandler for all HttpClient instances. These methods use the ConfigureHttpClientDefaults method under the hood, which causes the logging handler to be registered before any delegating handlers added for a named HttpClient. We won’t add overloads with the wrapHandlersPipeline parameter for these methods, as that would make their behavior confusing. It’s better to keep these methods as they are and register HttpClientLoggingHandler as the top-most handler in the pipeline. The code sample below shows scenarios where this could lead to confusing behavior. Adding the wrapHandlersPipeline parameter to these methods would also expose the implementation detail that they rely on ConfigureHttpClientDefaults internally.

services.ConfigureHttpClientDefaults(builder => builder.AddSomeHandler("A"));

services.AddHttpClient("client-1").AddSomeHandler("C1");

// The logging handler will be registered as part of default HttpClient configuration. It means that it would be put between
// handlers A and B.
// For named clients client-1 and client-2, it will be put before the first handler of this clients. It doesn't matter where the 
// AddExtendedHttpClientLogging(IServiceCollection) is invoked. The final handler configuration for client-1 and client-2 will be the following:
// client-1: A -- LoggingHandler -- B -- C1
// client-2: A -- LoggingHandler -- B -- C2
//
// The behavior that might be not obvious or confusing:
// - AddExtendedHttpClientLogging uses ConfigureHttpClientDefaults internally.
//   As a result, the logging handler is inserted between handlers registered
//   in earlier and later calls to ConfigureHttpClientDefaults.
// - For named HttpClient registrations it is different. The logging handler will always be added before the first handler of any named HttpClient configuraiton.
services.AddExtendedHttpClientLogging(wrapHandlersPipeline: false);

services.AddHttpClient("client-2").AddSomeHandler("C2");

services.ConfigureHttpClientDefaults(builder => builder.AddSomeHandler("B"));

However, if we decide that the IServiceCollection extension methods will always register it as the top-most handler, then this flag would have no effect in those methods. This is a downside we'll need to properly document.

API Usage

services
    .AddHttpClient("client")
    .AddExtendedHttpClientLogging(wrapHandlersPipeline: false);

Risks

The new methods (with the wrapHandlersPipeline parameter) would either make the existing ones (without the parameter) unusable or they always be used with the wrapHandlersPipeline parameter set to false.

Alternative Designs

namespace Microsoft.Extensions.Http.Logging;

public class LoggingOptions
{
+    /// <summary>
+    /// Gets or sets a value indicating whether to register the logging handler as the top-most handler in the pipeline.
+    /// </summary>
+    public bool WrapHandlersPipeline { get; set; } = true;
}

Instead of adding a set of overloads, we can introduce a flag that controls whether HttpClientLoggingHandler should be registered as the top-most handler in the pipeline.