Auto-configure Logbook interceptor for Spring RestClient - #2368
Auto-configure Logbook interceptor for Spring RestClient#2368kalayciburak wants to merge 2 commits into
Conversation
Register a RestClientCustomizer that applies LogbookClientHttpRequestInterceptor to RestClient.Builder instances, matching existing HttpClient and Feign auto-configuration support. Fixes zalando#2355
| RestClient.class, | ||
| RestClientCustomizer.class | ||
| }) | ||
| @ConditionalOnBean(LogbookClientHttpRequestInterceptor.class) |
There was a problem hiding this comment.
I tested the PR and the customizer is not getting configured because of a non match of this condition. It is because the LogbookClientHttpRequestInterceptor bean is configured in the same class sibling to the logbookRestClientCustomizer. Therefore it is not guaranteed that the interceptor bean is registered before the customizer.
Removing this condition solves the problem. And the condition on the RestClient class already implicitly ensures that the ClientHttpRequestInterceptor is also present, so the LogbookClientHttpRequestInterceptor applies.
There was a problem hiding this comment.
yes, dropped the sibling condition. RestClient class check is enough
|
Hi @kalayciburak I'd like to add another test: package org.zalando.logbook.autoconfigure;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.restclient.RestClientCustomizer;
import org.zalando.logbook.spring.LogbookClientHttpRequestInterceptor;
import static org.assertj.core.api.Assertions.assertThat;
@LogbookTest
class RestClientAutoConfigurationTest {
@Autowired
@Qualifier("logbookRestClientCustomizer")
private RestClientCustomizer customizer;
@Autowired
private LogbookClientHttpRequestInterceptor interceptor;
@Test
void shouldAutoConfigureRestClientCustomizer() {
assertThat(customizer).isNotNull();
assertThat(interceptor).isNotNull();
}
} |
The interceptor is registered in the same auto-config class, so the condition is not guaranteed. RestClient on the classpath already implies the interceptor type. Add RestClientAutoConfigurationTest for the full context.
|
added RestClientAutoConfigurationTest |
|
👍 |
|
@kalayciburak could you make sure that all commits in the PR are signed please? |
prashantpiyush1111
left a comment
prashantpiyush1111
left a comment
There was a problem hiding this comment.
The RestClient auto-configuration looks correct, and the added tests cover interceptor registration, application to RestClient.Builder, back-off behavior, and full application-context configuration. I did not find any code-level issues. The build is passing; only minor documentation/CI maintenance improvements remain.
|
@kalayciburak we can't merge the PR if all commits are not signed |
4 participants
Description
Register a
RestClientCustomizerthat appliesLogbookClientHttpRequestInterceptorto SpringRestClient.Builderinstances.This mirrors the existing auto-configuration support for Apache HttpClient 4/5 and Feign, so outbound
RestClienttraffic is logged automatically when Logbook's Spring Boot starter is on the classpath.Motivation and Context
Fixes #2355
RestClientis the preferred outbound HTTP client in Spring Boot 4.x (Logbook 4.x target). Without this customizer, outbound calls are silent unless developers wire the interceptor manually.Types of changes
Checklist:
Test plan
mvn -pl logbook-spring-boot-autoconfigure test -Dtest=RestClientTest— 3/3 GREENmvn -pl logbook-spring-boot-autoconfigure test— 101/101 GREEN (incl. JaCoCo)