HttpLogging redaction and enrichment#50163
Conversation
@wtgodbe any idea why it's complaining about modifying the Unshipped file? Is the release branch more strict about that? edit Figured this out, the error message is just incomplete. It doesn't want us modifying even the Unshipped file in release branches since we shouldn't be adding APIs in servicing. However that doesn't account for rc2 work which is going into release/8.0 until rc2 branches. I'll clarify the error in the script, but it won't fix it, we'll need to override it. |
Co-authored-by: Sébastien Ros <[email protected]>
| } | ||
|
|
||
| if (loggingFields.HasFlag(HttpLoggingFields.ResponseBody)) | ||
| if (loggingFields.HasFlag(HttpLoggingFields.ResponseBody) || _interceptors.Length > 0) |
There was a problem hiding this comment.
Why is the response body captured if there is an interceptor? Is there a situation where this wouldn't be wanted?
Add comment?
There was a problem hiding this comment.
The response body isn't captured until after interceptors run, but we need to hook it in advance so that an interceptor can decide to capture the body or not.
There was a problem hiding this comment.
That seems like a limitation of interceptors. Someone might want to redact fields with an interceptor, but an interceptor forces the body to be captured which can have a big performance overhead.
Have you considered a property on the interceptor interface to control whether the body is captured? bool CanCaptureResponseBody or bool SuppressCaptureResponseBody
There was a problem hiding this comment.
We're not always capturing the response body, we're only shimming it so we can see when it starts and make the decision to capture it. This should not cause significant overhead.
|
I've updated the description to include two recent requirements changes based on feedback from the Extensions folks, CombineLogs and exception handling. |
|
@wtgodbe ready to merge. |
|
@Tratcher, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work! Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers. This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement. Thanks! |
This adds extensibility to the logging middleware so that what gets logged can be granularly customized per request.
Description
See the API review notes at #31844 (comment)
Fixes #31844, contributes to dotnet/extensions#4330
Fixes #49989, CopyTo/Async not logging the request body.
Fixes #49063, Clarify when the request body is not fully logged.
IHttpLoggingInterceptor:
Duration:
Combine Logs:
bool HttpLoggingOptions.CombineLogs { get; set; }has been added to support writing one unified log of request, request body, response, and response body data at the end of the response. The extensions components require this.Exception handling: