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 7362707f1..2bf50a245 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 @@ -346,7 +346,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { void terminateSpan(@Nullable ClientResponse clientResponse, @Nullable Throwable throwable) { - if (clientResponse == null || clientResponse.statusCode() == null) { + if (clientResponse == null) { if (log.isDebugEnabled()) { log.debug("No response was returned. Will close the span [" + this.span + "]"); @@ -354,8 +354,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { handleReceive(this.span, this.ws, clientResponse, throwable); return; } - boolean error = clientResponse.statusCode().is4xxClientError() - || clientResponse.statusCode().is5xxServerError(); + int statusCode = clientResponse.rawStatusCode(); + boolean error = statusCode >= 400; if (error) { if (log.isDebugEnabled()) { log.debug( @@ -363,9 +363,8 @@ 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 + + "]"); } handleReceive(this.span, this.ws, clientResponse, throwable); } 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 917d6f19c..723d1b8c2 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 @@ -16,20 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web.client.integration; -import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Collectors; - -import javax.servlet.http.HttpServletRequest; - import brave.Span; import brave.Tracer; import brave.Tracing; @@ -50,18 +36,8 @@ import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.nio.client.CloseableHttpAsyncClient; import org.apache.http.impl.nio.client.HttpAsyncClientBuilder; import org.awaitility.Awaitility; -import org.junit.After; -import org.junit.Before; -import org.junit.ClassRule; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; +import org.junit.*; import org.junit.runner.RunWith; -import reactor.netty.http.client.HttpClient; -import reactor.netty.http.client.HttpClientResponse; -import zipkin2.Annotation; -import zipkin2.reporter.Reporter; - import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -94,7 +70,19 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.reactive.function.client.UnknownHttpStatusCodeException; import org.springframework.web.reactive.function.client.WebClient; +import reactor.netty.http.client.HttpClient; +import reactor.netty.http.client.HttpClientResponse; +import zipkin2.Annotation; +import zipkin2.reporter.Reporter; + +import javax.servlet.http.HttpServletRequest; +import java.util.*; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.fail; import static org.assertj.core.api.BDDAssertions.then; @@ -377,46 +365,25 @@ public class WebClientTests { .contains("CLIENT"); } - @Test - @Ignore("Flakey on CI") - public void shouldReportTraceForCancelledRequestViaWebClient() { - Span span = this.tracer.nextSpan().name("foo").start(); - - try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) { - this.webClient.get().uri("http://localhost:" + this.port + "/noresponse") - .retrieve().bodyToMono(String.class).timeout(Duration.ofMillis(0)) - .block(); - } - catch (Exception e) { - - } - finally { - span.finish(); - } - - Awaitility.await().untilAsserted(() -> { - System.out.println("Found spans " + this.reporter.getSpans()); - final Optional clientSpan = this.reporter.getSpans().stream() - .filter(s -> s.kind() == zipkin2.Span.Kind.CLIENT).findFirst(); - then(clientSpan).isPresent(); - then(clientSpan.get().tags()).containsEntry("error", "CANCELLED"); - }); - } - @Test @SuppressWarnings("unchecked") - public void shouldNotBreakWhenCustomStatusCodeIsSetViaWebClient() { + public void shouldWorkWhenCustomStatusCodeIsReturned() throws InterruptedException { 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(); + this.webClient.get().uri("http://localhost:" + this.port + "/issue1462") + .retrieve().bodyToMono(String.class).block(); + } + catch (UnknownHttpStatusCodeException ex) { + } finally { span.finish(); } + then(this.tracer.currentSpan()).isNull(); + then(this.reporter.getSpans()).isNotEmpty().extracting("kind.name") + .contains("CLIENT"); } Object[] parametersForShouldAttachTraceIdWhenCallingAnotherService() { @@ -567,6 +534,11 @@ public class WebClientTests { return new FooController(); } + @Bean + WebClientController webClientController() { + return new WebClientController(); + } + @LoadBalanced @Bean public RestTemplate restTemplate() { @@ -675,7 +647,6 @@ public class WebClientTests { then(traceId).isNotEmpty(); then(parentId).isNotEmpty(); then(spanId).isNotEmpty(); - this.span = this.tracer.currentSpan(); return traceId; } @@ -707,6 +678,17 @@ public class WebClientTests { } + @RestController + public static class WebClientController { + + @RequestMapping(value = "/issue1462", method = RequestMethod.GET) + public ResponseEntity issue1462() { + System.out.println("GOT IT"); + return ResponseEntity.status(499).body("issue1462"); + } + + } + @Configuration public static class SimpleRibbonClientConfiguration {