Ability to disable default http request logging

[dpk83]

Currently adding HTTP clients also enables logging of http requests by injecting the LoggingHttpMessageHandler. It currently provides no mechanism to disable the logging of these http client requests. These logs contains URLs in plain text which can contain sensitive information.

The only way to avoid spitting out the http logs are by setting the log level for System.Net.Http.HttpClient to none. While this provides some solution it's not ideal and has a few issues

• It disables spitting out the logs but services still end up paying the cost because all of the processing to capture and emit the logs from http client library is still being done
• While this disables the logs, the http client library also adds information to scopes. The scopes however can't be disabled via log level so whatever other logs are being added by the service ends up containing the scopes. These scopes may end up containing sensitive information which again service doesn't have ability to disable.

What's needed is an ability to disable auto injecting of this logging components as part of adding http client so there is a proper way to disable these logs.

The solution should such that it can be done at a central place rather than disabling one by one for all http client factories.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently adding HTTP clients also enables logging of http requests by injecting the LoggingHttpMessageHandler. It currently provides no mechanism to disable the logging of these http client requests. These logs contains URLs in plain text which can contain sensitive information.

The only way to avoid spitting out the http logs are by setting the log level for System.Net.Http.HttpClient to none. While this provides some solution it's not ideal and has a few issues

• It disables spitting out the logs but services still end up paying the cost because all of the processing to capture and emit the logs from http client library is still being done
• While this disables the logs, the http client library also adds information to scopes. The scopes however can't be disabled via log level so whatever other logs are being added by the service ends up containing the scopes. These scopes may end up containing sensitive information which again service doesn't have ability to disable.

What's needed is an ability to disable auto injecting of this logging components as part of adding http client so there is a proper way to disable these logs.

The solution should such that it can be done at a central place rather than disabling one by one for all http client factories.

Author:	dpk83
Assignees:	-
Labels:	area-System.Net.Http
Milestone:	-

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently adding HTTP clients also enables logging of http requests by injecting the LoggingHttpMessageHandler. It currently provides no mechanism to disable the logging of these http client requests. These logs contains URLs in plain text which can contain sensitive information.

The only way to avoid spitting out the http logs are by setting the log level for System.Net.Http.HttpClient to none. While this provides some solution it's not ideal and has a few issues

• It disables spitting out the logs but services still end up paying the cost because all of the processing to capture and emit the logs from http client library is still being done
• While this disables the logs, the http client library also adds information to scopes. The scopes however can't be disabled via log level so whatever other logs are being added by the service ends up containing the scopes. These scopes may end up containing sensitive information which again service doesn't have ability to disable.

What's needed is an ability to disable auto injecting of this logging components as part of adding http client so there is a proper way to disable these logs.

The solution should such that it can be done at a central place rather than disabling one by one for all http client factories.

Author:	dpk83
Assignees:	-
Labels:	untriaged, area-Extensions-HttpClientFactory
Milestone:	-

[tarekgh]

@dotnet/ncl could you please triage this issue and decide if this is something you'll address in .NET 8.0?

CC @joperezr

[CarnaViire]

Related to/part of #77312

Optimistically putting it to 8.0.
I'm currently considering something like

public static IHttpClientBuilder ConfigureLogging(this IHttpClientBuilder builder, Action<LoggingOptionsBuilder> configure) {}

public class LoggingOptionsBuilder
{
    public string Name { get; }
    public IServiceCollection Services { get; }
    public LoggingOptionsBuilder ClearProviders() {} // removes all logging
    public LoggingOptionsBuilder AddDefaultProviders() {} // adds the default logging (LoggingHttpMessageHandler + LoggingScopeHttpMessageHandler)

    // and then potential extensions... these are just out of my head, not fully thought through
    public LoggingOptionsBuilder AddClientHandler() {} // adds LoggingHttpMessageHandler
    public LoggingOptionsBuilder AddLogicalHandler() {} // adds LoggingScopeHttpMessageHandler
    public LoggingOptionsBuilder AddMinimal() {} // one-liner
    public LoggingOptionsBuilder AddMinimal(Func<HttpRequestMessage, HttpResponseMessage?, TimeSpan, string> getLogString) {} // configured one-liner
    public LoggingOptionsBuilder AddCustom(Action<HttpRequestMessage>? before = null, Action<HttpRequestMessage, HttpResponseMessage?, TimeSpan>? after = null, LogLevel level = LogLevel.Information) {}
}

The solution should such that it can be done at a central place rather than disabling one by one for all http client factories.

@dpk83 Note that we don't have a notion of "global" settings in HttpClientFactory at the moment. All settings are per name/type.
We could consider some kind of additional API in the future to contain the list of defaults, but it definitely won't be part of 8.0.

If you have to have a global switch, currently you can remove the logging by calling IServiceCollection.RemoveAll<IHttpMessageHandlerBuilderFilter>() after all registrations.

[dpk83]

IServiceCollection.RemoveAll<IHttpMessageHandlerBuilderFilter>()

The problem with this is that it removes all IHttpMessageHandlerBuilderFilter. What is needed is a way to remove just LoggingHttpMessageHandler.

[CarnaViire]

@dpk83 If you didn't add any custom implementations of IHttpMessageHandlerBuilderFilter manually, the only one registered would be Microsoft.Extensions.Http.LoggingHttpMessageHandlerBuilderFilter which adds LoggingHttpMessageHandler. If you need to have additional custom IHttpMessageHandlerBuilderFilters, you can register them after calling RemoveAll. Alternatively, if you want to have a more "fail-proof" workaround, you can do

for (int i = builder.Services.Count - 1; i >= 0; i--)
{
    ServiceDescriptor? descriptor = builder.Services[i];
    if (descriptor.ServiceType == typeof(IHttpMessageHandlerBuilderFilter) && descriptor.ImplementationType?.Assembly == typeof(IHttpClientFactory).Assembly)
    {
#if DEBUG
        if (descriptor.ImplementationType?.FullName != "Microsoft.Extensions.Http.LoggingHttpMessageHandlerBuilderFilter")
        {
            throw new Exception($"Unexpected IHttpMessageHandlerBuilderFilter: {descriptor.ImplementationType?.FullName}");
        }
#endif
        builder.Services.RemoveAt(i);
    }
}

(based on RemoveAll implementation)

But I wonder why single-point is so important. Would such code be invoked by your library, or users of your library (which may not be the nicest way to do that)?

[dpk83]

But I wonder why single-point is so important. Would such code be invoked by your library, or users of your library (which may not be the nicest way to do that)?

That's because this removal will be invoked from a common library that's used by many services, so there is no control on the order. So, even if the library removes it and it the caller places AddHttpClient call after the library call that removed the filter, then the caller is unknowingly bringing back the filter.

[CarnaViire]

I see. How impactful do you believe this issue is? As I mentioned, I am afraid factory-wide defaults/settings most possibly won't fit into 8.0, given the load on the team, so we need to understand the priorities.

Do I understand you correctly that you believe per-client logging config is not needed for your scenario?

[CarnaViire]

My current WIP draft for the API:

    // existing
    public static class HttpClientFactoryServiceCollectionExtensions
    {
        // new
        public static IHttpClientBuilder AddHttpClientDefaults(this IServiceCollection services) {}
    }

    // existing
    public static class HttpClientBuilderExtensions
    {
        // new
        public static IHttpClientBuilder ConfigureLogging(this IHttpClientBuilder builder, Action<IHttpClientLoggingOptions> configure) {}
    }

    public interface IHttpClientLoggingOptions
    {
        IHttpClientLoggingOptions ClearProviders(); // removes all logging
        IHttpClientLoggingOptions AddDefaultProviders(); // adds the default logging (LoggingHttpMessageHandler + LoggingScopeHttpMessageHandler)
        IHttpClientLoggingOptions AddRequestStartProvider(Func<HttpRequestMessage, string> getRequestStartMessage, LogLevel level = LogLevel.Information);
        IHttpClientLoggingOptions AddRequestEndProvider(Func<HttpRequestMessage, TimeSpan, HttpResponseMessage?, Exception?, string> getRequestEndMessage, LogLevel level = LogLevel.Information);
    }

Then the usage for central-point logging disabling would look like

    services.AddHttpClientDefaults().ConfigureLogging(o => o.ClearProviders()); // disables logging by default

    services.AddHttpClient("no-log"); // this client will not have any logging
    services.AddHttpClient("with-log").ConfigureLogging(o => o.AddDefaultProviders()); // one-client-only return of the logging
    services.AddHttpClient("still-no-log"); // this client also will not have any logging

How does this sound @dpk83?

Also, is custom logging (additionally present in the API) something you could be potentially interested in?
E.g.

services.AddHttpClientDefaults()
    .ConfigureLogging(o =>
    {
        o.AddRequestEndProvider((request, duration, response, ex) => $"{request.Method} {request.RequestUri} - {response.StatusCode} in {duration.TotalMilliseconds}ms");
    });

(something that was asked in #44411)

[noahfalk]

@JamesNK - Heads up, I wasn't aware before but I just discovered this issue. I see @CarnaViire and @dpk83 already appear to have some work in flight around this scenario (#87247)

[JamesNK]

I think the work here can be simplified. We could make improvements to the HTTP logging built into Microsoft.Extensions.Http now but there isn't anything stopping us from doing that in the future.

The goal right now is to allow someone to remove default http request logging for a client. They can then hook in their own logging library. Below are a couple of ideas for doing that:

The first idea is not to do anything. It's possible to disable logging relatively cleanly today:

services.AddHttpClient("contoso").ConfigureHttpMessageHandlerBuilder(b =>
{
    // Remove the logger handlers added by the filter. Fortunately, they're both public, so it is a simple test on the type.
    for (var i = b.AdditionalHandlers.Count - 1; i >= 0; i--)
    {
        var handlerType = b.AdditionalHandlers[i].GetType();
        if (handlerType == typeof(LoggingScopeHttpMessageHandler) || handlerType == typeof(LoggingHttpMessageHandler))
        {
            b.AdditionalHandlers.RemoveAt(i);
        }
    }
});

This code removes the two handlers added by the logging filter. This logic doesn't interfere with any other configuration.

It's not an elegant solution, but only a small number of people writing their own HTTP client logging libraries will want to do this. For example, someone who wants to add their own HTTP logging functionality can include this code in the same extension method that adds their new logger. It's hidden from the end user.

The second idea is to add a property to HttpMessageHandlerBuilder:

namespace Microsoft.Extensions.Http;

public abstract class HttpMessageHandlerBuilder
{
+    public virtual bool SuppressHttpLogging { get; set; }
}

Usage:

services.AddHttpClient("contoso").ConfigureHttpMessageHandlerBuilder(b =>
{
    b.SuppressHttpLogging = true;
});

Setting this property would do the same thing as the first sample, remove the added logging handlers, but the code would be internal to Microsoft.Extensions.Http.

[dpk83]

@CarnaViire I like your proposal as we do need the ability to disable the logging centrally

[JamesNK]

@dpk83 The techniques I discussed can be done centrally.

[dpk83]

@dpk83 The techniques I discussed can be done centrally.

How to you ensure that someone else calling AddHttpClient doesn't end up adding the message handler back on that http client.

Also, for the library cases like ours we don't create a http client and expose our extensions on service collection, how do we invoke ConfigureHttpMessageHandlerBuilder given we won't have access to the http client builder?

[JamesNK]

How to you ensure that someone else calling AddHttpClient doesn't end up adding the message handler back on that http client.

Are you concerned that someone wants to maliciously add the default logging handlers back after they have been removed? What is the scenario that someone do that?

Also, for the library cases like ours we don't create a http client and expose our extensions on service collection, how do we invoke ConfigureHttpMessageHandlerBuilder given we won't have access to the http client builder?

ConfigureHttpMessageHandlerBuilder is just a helper method for configuring HttpMessageHandlerBuilder for a specific client. Globally configuring HttpMessageHandlerBuilder isn't difficult.

[dpk83]

Globally configuring HttpMessageHandlerBuilder isn't difficult.

I didn't see that in what you shared. Also, if it is globally configured can it be explicitly overwritten for a specific client if needed?

Basically all of the functionalities that @CarnaViire 's solution is providing if that can be provided with the solution you are proposing then I think we are fine with either approach

[JamesNK]

I didn't see that in what you shared. Also, if it is globally configured can it be explicitly overwritten for a specific client if needed?

I think so. It would probably depend on what order things are done when configuring services.

Is turning logging off globally and then re-enabling per-client important for you? Do you have scenarios where you would need that feature?

Basically all of the functionalities that @CarnaViire 's solution is providing if that can be provided with the solution you are proposing then I think we are fine with either approach

My proposal is different. It is just what is asked for in the original issue: a way to turn http logging off.

I'll write some samples to verify.

[dpk83]

I think so. It would probably depend on what order things are done when configuring services.

That's the thing, order shouldn't matter. CarnaVire's proposal supports that

[JamesNK]

I’m trying to understand what you need and create a solution for it.

What is the scenario where you want to globally turn logging off and then selectively turn it back on?

[geeknoid]

Hey, @JamesNK , I tried to apply your first proposal and it doesn't work for me. I must be doing something wrong. Here's a PR with the change: dotnet/extensions#4054.

If I put a breakpoint in the RemoveDefaultLogging function, I see that the RemoveAt call is never invoked from any of our test suites, indicating something is likely not right...

[JamesNK]

Merged dotnet/extensions#4060. It provides a workaround for .NET 8.

[CarnaViire]

Thank you @JamesNK! I'll move this one to 9.0 then

[CarnaViire]

Duplicate of #77312

[CarnaViire]

This issue will be solved by #77312 and #87914