Modify HttpClientFactory based HttpClient request logging

[johnkors]

Hi,

I saw this issue for overriding the default logging / altering the log statements emitted when doing requests using a HttpClientFactory based HttpClient, which recommends re-implementing the IHttpMessageHandlerBuilderFilter for this.

I wanted 2 alterations from the default logging, to reduce the logs:

1. Don't emit 2 log statements for each request, but only one (the default logging emits a start-req, and a end-req log statement)
2. In the single emitted log statement, include the request url.

So reducing this:

- "Sending HTTP request {HttpMethod} {Uri}");
- "Received HTTP response headers after {ElapsedMilliseconds}ms - {StatusCode}"

to

- $"{HttpMethod} {Uri} - {StatusCode} in {ElapsedMilliseconds}ms"

I was surprised by the amount of code I had to write to be able to do this. Is there no easier way to just modify the default log statements..?

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

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

[CarnaViire]

Triage: the solution for that could be to introduce a switch to turn off the default logging. In that case you wouldn't need to write your own IHttpMessageHandlerBuilderFilter to remove it, you would be able to just turn it off and add a new custom one. This is a direction we should consider in Future. If you run into this, please upvote the top post to help us prioritize. Thanks!

[timba]

I also would like to give an example why we need to have possibility to change the default logging output. HTTP logging of Information level exposes full Uri to logs. Many web APIs accept sensitive information via query string (passwords, tokens, usernames, etc). This means that Information level HTTP logs may expose sensitive information which is a potential risk. The only option here for developers is to remove default logging handler, implement custom one and add it to the handlers list, which is quite annoying, as @johnkors mentioned.

Also, I'd propose to look at the HTTP logging from another perspective. If we look into HTTP protocol, TLS secured (HTTPS) messages don't expose query string, only domain is opened, the rest is encrypted (including query string). Taking into account that HTTP logging of Information level may be considered as HTTP messaging logging, I'd suggest to remove query string from this level at all in case of HTTPS messages.

[johnkors]

Decided to create a (opinionated) nuget for what I want. A better API for replacing the default logger sounds interesting, at least if you cannot make the default logging more configurable than it is today.

https://github.com/johnkors/MinimalHttpLogger

It aggregates the default 4 log statements into what I prefer:

Default logger:

info: Start processing HTTP request GET https://www.google.com/
info: Sending HTTP request GET https://www.google.com/
info: Received HTTP response headers after 188.6041ms - 200
info: End processing HTTP request after 188.8026ms - 200

MinimalHttpLogger:

info: GET https://www.google.com/ - 200 OK in 186.4883ms

[CarnaViire]

Duplicate of #77312