RestClient - Consider adding request/response logs

[michaelisvy]

Hi,
I commonly add logs to the RestClient based on this nice example from Craig Walls: https://github.com/habuma/logging-interceptor/tree/main

Would it make sense to add request/response logging capabilities as part of the RestClient? That way, it would be simple for everyone to set it up.

Thanks!

[rstoyanchev]

Most of the code in Craig's example is to wrap the response for buffering, and the same can be applied through the BufferingClientHttpRequestFactory. The actual logging is quite straight forward:

@Bean
RestClient.Builder restClientBuilder() {
	ClientHttpRequestInterceptor interceptor = (req, reqBody, ex) -> {
		ClientHttpResponse response = ex.execute(req, reqBody);
		if (LOGGER.isDebugEnabled()) {
			LOGGER.debug("Request body: \n===========\n{}\n===========", new String(reqBody, StandardCharsets.UTF_8));
			LOGGER.debug("Response body:\n===========\n{}\n===========", response.getBody());
		}
		return response;
	};
	return RestClient.builder()
			.requestFactory(new BufferingClientHttpRequestFactory(new JdkClientHttpRequestFactory()))
			.requestInterceptor(interceptor);
}

However, this forces explicit creation of the underlying HttpClientRequestFactory, and that may not be desirable if you want to just add logging, but otherwise let the request factory be configured elsewhere. The other issue is that use of interception involves the InterceptingClientHttpRequestFactory, which also buffers the request, and that causes an unnecessary use of an additional buffering OutputStream and additional copy for the request body.

We can look to improve buffering by making it more of an internal concern, the way InterceptingClientHttpRequestFactory is applied. Doing so would allow us to handle interception and buffering concerns together as needed. On the Builder we can allow buffering to be enabled, with an additional callback to potentially make decisions per request. The BufferingClientHttpRequestFactory could be deprecated.

[rstoyanchev]

The above commit closes #33763, but incorrectly referenced this issue.

[michaelisvy]

thanks!
I was thinking it would be great to have something as simple to configure as the logger we have in Spring AI. I'm using this with Spring AI:

this.chatClient = builder
        .defaultSystem("You are a helpful assistant writing in formal English")
        .defaultAdvisors(new SimpleLoggerAdvisor())
        .build();

Not sure if we could have something like that with the RestClient?

return RestClient.builder()
			.requestInterceptor(new SimpleLoggerInterceptor());

[rstoyanchev]

A built-in interceptor becomes more complex for what should be a simple task. Take AbstractRequestLoggingFilter and sub-classes on the server side for example. You need to account for different logging frameworks, flexible choices for what to log and what not to, etc. Also, logging content comes with a buffering trade-off that arguably should be a conscious decision.

I think we should make buffering easy to enable. The actual logging then should be simple. I created #33785, and will close this for now.

[michaelisvy]

sounds good, thanks!