Passing attributes from WebClient to underlying HTTP library

[tstavinoha]

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.

[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.

[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<String, Object>, the same type as RequestHeadersSpec, so theoretically this should be possible.

Including a small diagram:

[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?

[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?

[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?

[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.

[rstoyanchev]

@tstavinoha something to mention, as of 5.3 with 7adeb46, 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.

[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

[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 > < #26208 (comment) >, > 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 <#26208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIS5ZA2SO2DZ6SZ7URZX7C3UNTPDNANCNFSM4UMECN4A> .

[christian-german]

Hi Spring team,
If this one's still opened, I'd be happy to help!

[kunalvarpe]

Is there any help required on this issue?

[rstoyanchev]

Closing as superseded by #29958.