Hello,
We are trying to enable communication logging (low level HTTP logging) for requests and responses processed with WebClient
. There is a good tutorial on how to approach the simplest case which I've followed to get the initial results - Link to Baeldung.
However, HTTP logging in our company is a bit more complex simply just calling SLF4J. In certain cases we need to provide extra context paired to request/response, so that other tooling can correctly process those logs (some internal IDs). We've identified the attributes
feature as potentially the cleanest way to pass additional context to the underlying library, seeing how they are present on WebClient
API as well as both Netty and Jetty libraries. However, it seems that the attributes are not copied from WebClient
model to the Netty/Jetty model.
As a workaround, we are currently wrapping the Request/Response spec to add additional behavior on WebClient
side.
So the question is - are attributes the right way of solving cases like this and how would one go about adding support for copying attributes from WebClient
request to underlying library request?
I've found no relevant discussions and this seems to be a useful enhancement.
Comment From: rstoyanchev
Can you clarify what you mean by "pass additional context to the underlying library"? Attributes can be used to pass data down through the WebClient filter chain for the current request independent of what thread they execute on. If you want to propagate context through multiple requests, you're better off with the Reactor Context. There is also guidance on how to bridge the Reactor Context to MDC logging.
Comment From: tstavinoha
Basically what I'd like to do is to pass attributes further than WebClient filter chain, and into the underlying libraries attributes, because those libraries provide listeners for which it is quite easy to add "raw" HTTP request/response logging. It would be for request-response pairs, not for multiple requests.
WebClient's API exposes methods for setting headers (RequestHeadersSpec.headers), body (RequestBodySpec.body) and uri (UriSpec.uri), and they are all copied to the underlying libraries model, for example Jetty Request's getHeaders, getBody, getURI. But it seems that attributes are not copied in a same manner, and that would be very useful in this case. Jetty's request model has a Map
Including a small diagram:
Comment From: rstoyanchev
I see now. We could copy WebClient
attributes to the attributes of the underlying request where available. The Jetty Request
has such a map. Apache HttpComponents has an HttpClientContext
that can hold attributes. The Reactor Netty HttpClientRequest
doesn't have attributes but we could copy to Netty Channel
attributes although looking at DefaultAttributeMap.java#L88 that may be a little expensive to do automatically, without knowing if that's necessary.
Perhaps for Netty we can create a single attribute that contains the whole WebClient attribute map. Or perhaps maybe it becomes a config option on the individual ClientHttpConnector
implementation level to choose whether to pass attributes like that or maybe even which attributes. What do you think?
Comment From: tstavinoha
Those are valid concerns and options.
My opinion is that if you are setting attributes to a request, obviously you plan on doing something with them and they should be available during the whole lifecycle of the request. I wouldn't treat them any different from how headers behave.
I agree with your point that there may be use cases where performance is a top priority and copying attributes is not, even if they are set to the WebClient request (eg. they could only be used by WebClient filters and not required in the underlying library). For such cases users should be able to opt-out of copying attributes when setting up the connector (I think that the majority of library users will not have these performance concerns, hence opt-out behaviour). Also, all-or-nothing sounds good enough for most cases to me (as opposed to a fine-grained attribute filter).
What would be the right starting point for implementing such a feature? How would you design this?
Comment From: szakal
Hi,
What would be the right starting point for implementing such a feature? How would you design this?
Could you provide any recommendation on how to start proceeding with this feature request?
Comment From: rstoyanchev
Probably start by exposing an attributes map on ClientHttpRequest
and having it populated from the ClientRequest
attributes. Then add applyAttributes()
in AbstractClientHttpRequest
, following the same pattern as for headers and cookies. Each sub-class can then decide what to do with attributes this. For some it would be a straight-forward population of a similar map. For Netty, probably a single attribute with a Map value. Also all this should be opted into through a property on each the ClientHttpConnector
implementation.
Comment From: rstoyanchev
@tstavinoha something to mention, as of 5.3 with 7adeb461e0464b0d07c2e8fbe36ce0a22f0a7b5e, there is now callback access to the underlying native request. It would be useful to know how this helps your use case or reversely.
For 5.3.5 with #26656 we also have a correlation id on the ClientHttpResponse
which in the case of Reactor Netty helps to correlate logging from Reactor Netty to the logPrefix
in ClientRequest
and ClientResponse
.
Comment From: sambuccid
Hi @rstoyanchev , I want to work on this issue but it is my first contribution to Spring Framework(and to an open source project in general) so i would need a bit of help.
I had a look at the code and started implementing the change, but i got stuck with some questions:
* I Couldn't understand how would it work for Netty, the channel
seems to be not exposed by the HttpClientRequest
so where can i write the attributes/Map value?
* How do i add a property on each ClientHttpConnector
implementation to opt out the functionality? is there some other property in the class that i can follow? or you meant to opt out the functionality in the implementations of AbstractClientHttpRequest
?
Thanks for any help David
Comment From: divyajnu08
Ok sure thanks very much.
On Wed, 24 Nov 2021, 18:33 sambuccid, @.***> wrote:
Yea sure, i stopped working on this after i submitted the pullrequest but seems that the pullrequest needs some more work, so feel free to start from there
Il giorno mar 23 nov 2021 alle ore 12:20 Divya Srivastava ***@***.*** ha scritto:
Hi sambuccid , can i work with you on this issue ? It will help me learn contributing to open source.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub < https://github.com/spring-projects/spring-framework/issues/26208#issuecomment-976458583 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/AGMNAYVZOQT75IL7QAYPRPLUNOBKRANCNFSM4UMECN4A
. Triage notifications on the go with GitHub Mobile for iOS < https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android < https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spring-projects/spring-framework/issues/26208#issuecomment-977858290, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIS5ZA2SO2DZ6SZ7URZX7C3UNTPDNANCNFSM4UMECN4A .
Comment From: christian-german
Hi Spring team, If this one's still opened, I'd be happy to help!
Comment From: kunalvarpe
Is there any help required on this issue?
Comment From: rstoyanchev
Closing as superseded by #29958.