HttpClientBuilderExtensions LogRequestResponse Method Uses Same Options Across Multiple HttpClients

[gurudusty]

I really enjoy using this library. However, I've encountered an issue with registering custom RequestLoggingOptions for multiple HttpClients using HttpClientBuilderExtensions.LogRequestResponse in an ASP.NET Core environment on .NET 6.
Despite specifying different options, all HttpClients seem to use the first configured options.

Steps to Reproduce:

Using the ASP.NET Core sample app, if I create an additional service/interface MyOtherService / IMyOtherService and register it as follows, both clients use the same configuration options:

// In Startup.cs
services
   .AddHttpClient<IMyService, MyService>()
   .CorrelateRequests("X-Correlation-ID")
   .LogRequestResponse(p => { p.LogMode = LogMode.LogAll; });
services
   .AddHttpClient<IMyOtherService, MyOtherService>()
   .CorrelateRequests("X-Correlation-ID")
   .LogRequestResponse(p => { p.LogMode = LogMode.LogNone; });

Expected Behavior:

Each HttpClient should use its specified LogRequestResponse options.

Actual Behavior:

Both HttpClients use the configuration options specified for the first HttpClient.

Potential Cause:

I believe this issue stems from how LogRequestResponse handles HttpMessageHandler resolution, only registering one service factory instance.

Suggested Solution:

I've propose a change in the method to ensure AddHttpMessageHandler fetches named options and creates a new instance directly, detailed below. However, this may lead to the creation of a handler instance on every request. I've adjusted it to use keyed services for .NET 8+ environments that should mitigate this issue.

// Proposed changes in HttpClientBuilderExtensions.cs
public static class HttpClientBuilderExtensions { 
        public static IHttpClientBuilder LogRequestResponse(this IHttpClientBuilder builder,
            Action<RequestLoggingOptions> configureOptions = null)
        {
            if (configureOptions == null)
                configureOptions = options => { };
            if (builder is null)
                throw new ArgumentNullException(nameof(builder));
            
            builder.Services.Configure(builder.Name, configureOptions);
#if NET8_0_OR_GREATER
            builder.Services.TryAddKeyedTransient<LoggingDelegatingHandler>(builder.Name, (s, k) =>
            {
              var opt = s.GetRequiredService<IOptionsSnapshot<RequestLoggingOptions>>();
              return new LoggingDelegatingHandler(opt.Get((string)k), default, true);
            });
            builder.AddHttpMessageHandler(s => s.GetRequiredKeyedService<LoggingDelegatingHandler>(builder.Name));
#else
            builder.AddHttpMessageHandler(s =>
            {
              var o = s.GetRequiredService<IOptionsSnapshot<RequestLoggingOptions>>().Get(builder.Name);
              return new LoggingDelegatingHandler(o, forHttpClientFactory: true);
            });
#endif
}

Environment Details:

ASP.NET Core on .NET 6

Pull Request:

I will attach a new pull request shortly

Would appreciate any feedback on this issue and the proposed solution. Thanks for all of your great work.