diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java index 533658beb..cfae6daa6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java @@ -84,7 +84,7 @@ public class TraceWebClientAutoConfiguration { protected static class TraceInterceptorConfiguration { @Autowired - private TracingClientHttpRequestInterceptor clientInterceptor; + private BeanFactory beanFactory; @Bean static TraceRestTemplateBeanPostProcessor traceRestTemplateBeanPostProcessor( @@ -95,7 +95,8 @@ public class TraceWebClientAutoConfiguration { @Bean @Order RestTemplateCustomizer traceRestTemplateCustomizer() { - return new TraceRestTemplateCustomizer(this.clientInterceptor); + return new TraceRestTemplateCustomizer( + new LazyTracingClientHttpRequestInterceptor(this.beanFactory)); } } @@ -247,9 +248,9 @@ class RestTemplateInterceptorInjector { class TraceRestTemplateCustomizer implements RestTemplateCustomizer { - private final TracingClientHttpRequestInterceptor interceptor; + private final ClientHttpRequestInterceptor interceptor; - TraceRestTemplateCustomizer(TracingClientHttpRequestInterceptor interceptor) { + TraceRestTemplateCustomizer(ClientHttpRequestInterceptor interceptor) { this.interceptor = interceptor; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java index 4630c2a35..2ef8eca9e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java @@ -33,6 +33,7 @@ import org.apache.commons.logging.LogFactory; import org.reactivestreams.Publisher; import org.reactivestreams.Subscription; import reactor.core.CoreSubscriber; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.annotation.Nullable; import reactor.util.context.Context; @@ -41,14 +42,22 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.cloud.sleuth.instrument.reactor.ReactorSleuth; +import org.springframework.core.ParameterizedTypeReference; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseCookie; +import org.springframework.http.ResponseEntity; +import org.springframework.http.client.reactive.ClientHttpResponse; +import org.springframework.util.MultiValueMap; import org.springframework.web.client.RestClientException; +import org.springframework.web.reactive.function.BodyExtractor; import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.ExchangeFilterFunction; import org.springframework.web.reactive.function.client.ExchangeFunction; +import org.springframework.web.reactive.function.client.ExchangeStrategies; import org.springframework.web.reactive.function.client.WebClient; +import org.springframework.web.reactive.function.client.WebClientResponseException; /** * {@link BeanPostProcessor} to wrap a {@link WebClient} instance into its trace @@ -294,17 +303,107 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { this.done = true; try { // decorate response body - this.actual - .onNext(ClientResponse.from(response) - .body(response.bodyToFlux(DataBuffer.class) - .transform(this.scopePassingTransformer)) - .build()); + this.actual.onNext(wrapped(response)); } finally { terminateSpan(response, null); } } + // TODO: Remove once fixed + // https://github.com/spring-projects/spring-framework/issues/23366 + private ClientResponse wrapped(ClientResponse response) { + return new ClientResponse() { + @Override + public HttpStatus statusCode() { + try { + return response.statusCode(); + } + catch (IllegalArgumentException ex) { + return null; + } + } + + @Override + public int rawStatusCode() { + return response.rawStatusCode(); + } + + @Override + public Headers headers() { + return response.headers(); + } + + @Override + public MultiValueMap cookies() { + return response.cookies(); + } + + @Override + public ExchangeStrategies strategies() { + return response.strategies(); + } + + @Override + public T body( + BodyExtractor extractor) { + return response.body(extractor); + } + + @Override + public Mono bodyToMono(Class elementClass) { + return response.bodyToMono(elementClass); + } + + @Override + public Mono bodyToMono( + ParameterizedTypeReference typeReference) { + return response.bodyToMono(typeReference); + } + + @Override + public Flux bodyToFlux(Class elementClass) { + return (Flux) response.bodyToFlux(DataBuffer.class) + .transform(scopePassingTransformer); + } + + @Override + public Flux bodyToFlux( + ParameterizedTypeReference typeReference) { + return (Flux) response.bodyToFlux(DataBuffer.class) + .transform(scopePassingTransformer); + } + + @Override + public Mono> toEntity(Class bodyType) { + return response.toEntity(bodyType); + } + + @Override + public Mono> toEntity( + ParameterizedTypeReference typeReference) { + return response.toEntity(typeReference); + } + + @Override + public Mono>> toEntityList( + Class elementType) { + return response.toEntityList(elementType); + } + + @Override + public Mono>> toEntityList( + ParameterizedTypeReference typeReference) { + return response.toEntityList(typeReference); + } + + @Override + public Mono createException() { + return response.createException(); + } + }; + } + @Override public void onError(Throwable t) { try { @@ -350,7 +449,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { void terminateSpan(@Nullable ClientResponse clientResponse, @Nullable Throwable throwable) { - if (clientResponse == null || tryStatusCode(clientResponse) == null) { + if (clientResponse == null) { if (log.isDebugEnabled()) { log.debug("No response was returned. Will close the span [" + this.span + "]"); @@ -358,8 +457,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { handleReceive(this.span, this.ws, clientResponse, throwable); return; } - boolean error = clientResponse.statusCode().is4xxClientError() - || clientResponse.statusCode().is5xxServerError(); + int statusCode = statusCodeAsInt(clientResponse); + boolean error = isError(statusCode); if (error) { if (log.isDebugEnabled()) { log.debug( @@ -367,19 +466,31 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { + this.span + "]"); } throwable = new RestClientException("Status code of the response is [" - + clientResponse.statusCode().value() - + "] and the reason is [" - + clientResponse.statusCode().getReasonPhrase() + "]"); + + statusCode + "] and the reason is [" + + reasonPhrase(clientResponse) + "]"); } handleReceive(this.span, this.ws, clientResponse, throwable); } - private HttpStatus tryStatusCode(ClientResponse clientResponse) { + private String reasonPhrase(ClientResponse clientResponse) { try { - return clientResponse.statusCode(); + return clientResponse.statusCode().getReasonPhrase(); } - catch (Exception ex) { - return null; + catch (IllegalArgumentException ex) { + return ""; + } + } + + private boolean isError(int code) { + return code >= 400; + } + + private int statusCodeAsInt(ClientResponse response) { + try { + return response.rawStatusCode(); + } + catch (Exception dontCare) { + return 0; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java index fe75e40a9..8c8658296 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; -import brave.http.HttpTracing; import feign.Client; import feign.Feign; import feign.Retryer; @@ -45,8 +44,7 @@ final class SleuthFeignBuilder { return new LazyClient(beanFactory); } catch (BeansException ex) { - return TracingFeignClient.create(beanFactory.getBean(HttpTracing.class), - new Client.Default(null, null)); + return new LazyClient(beanFactory, new Client.Default(null, null)); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java index 5deba1e38..4ed89e513 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java @@ -378,6 +378,22 @@ public class WebClientTests { .contains("CLIENT"); } + @Test + @SuppressWarnings("unchecked") + public void shouldNotBreakWhenCustomStatusCodeIsSetViaWebClient() { + Span span = this.tracer.nextSpan().name("foo").start(); + + try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) { + this.webClient.get() + .uri("http://localhost:" + this.port + "/customstatuscode").exchange() + .block(); + } + finally { + span.finish(); + } + then(this.tracer.currentSpan()).isNull(); + } + @Test @Ignore("Flakey on CI") public void shouldReportTraceForCancelledRequestViaWebClient() { @@ -665,6 +681,12 @@ public class WebClientTests { return traceId; } + @RequestMapping(value = "/customstatuscode", method = RequestMethod.GET) + public ResponseEntity customStatusCode() { + this.span = this.tracer.currentSpan(); + return ResponseEntity.status(499).build(); + } + @RequestMapping("/") public Map home(@RequestHeader HttpHeaders headers) { Map map = new HashMap<>();