From 619133848a44bb688ebc47240a173490c03354eb Mon Sep 17 00:00:00 2001 From: Csaba Kos Date: Thu, 9 Jan 2020 17:07:09 -0600 Subject: [PATCH] WebClient tracing should respect skipPattern. (#1517) --- .../TraceWebClientBeanPostProcessor.java | 88 +++++++++++++------ ...ilterFunctionHttpClientResponseTests.java} | 12 +-- .../client/integration/WebClientTests.java | 19 +++- 3 files changed, 86 insertions(+), 33 deletions(-) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/{TraceExchangeFilterFunctionHttpAdapterTests.java => TraceExchangeFilterFunctionHttpClientResponseTests.java} (77%) 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 b39bf7fb2..f558fb57c 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 @@ -142,7 +142,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { HttpTracing httpTracing; - HttpClientHandler handler; + HttpClientHandler handler; TraceContext.Injector injector; @@ -158,25 +158,23 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { @Override public Mono filter(ClientRequest request, ExchangeFunction next) { - ClientRequest.Builder builder = ClientRequest.from(request); + HttpClientRequest wrapper = new HttpClientRequest(request); if (log.isDebugEnabled()) { log.debug("Instrumenting WebClient call"); } - Span span = handler().handleSend(injector(), builder, request, - tracer().nextSpan()); + Span span = handler().handleSend(wrapper); if (log.isDebugEnabled()) { log.debug("Handled send of " + span); } - return new MonoWebClientTrace(next, builder.build(), this, span); + return new MonoWebClientTrace(next, wrapper.buildRequest(), this, span); } @SuppressWarnings("unchecked") - HttpClientHandler handler() { + HttpClientHandler handler() { if (this.handler == null) { - this.handler = HttpClientHandler.create( - this.beanFactory.getBean(HttpTracing.class), - new TraceExchangeFilterFunction.HttpAdapter()); + this.handler = HttpClientHandler + .create(this.beanFactory.getBean(HttpTracing.class)); } return this.handler; } @@ -211,7 +209,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { final Tracer tracer; - final HttpClientHandler handler; + final HttpClientHandler handler; final TraceContext.Injector injector; @@ -251,7 +249,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { final Span span; - final HttpClientHandler handler; + final HttpClientHandler handler; final Function, ? extends Publisher> scopePassingTransformer; @@ -359,7 +357,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { if (log.isTraceEnabled()) { log.trace("Handling receive"); } - this.handler.handleReceive(clientResponse, throwable, clientSpan); + this.handler.handleReceive(new HttpClientResponse(clientResponse), + throwable, clientSpan); if (log.isTraceEnabled()) { log.trace("Closed scope"); } @@ -403,35 +402,70 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction { } - static final class HttpAdapter - extends brave.http.HttpClientAdapter { + static final class HttpClientRequest extends brave.http.HttpClientRequest { - @Override - public String method(ClientRequest request) { - return request.method().name(); + private final ClientRequest delegate; + + private final ClientRequest.Builder builder; + + HttpClientRequest(ClientRequest delegate) { + this.delegate = delegate; + this.builder = ClientRequest.from(delegate); } @Override - public String url(ClientRequest request) { - return request.url().toString(); + public Object unwrap() { + return delegate; } @Override - public String requestHeader(ClientRequest request, String name) { - Object result = request.headers().getFirst(name); - return result != null ? result.toString() : null; + public String method() { + return delegate.method().name(); } @Override - public Integer statusCode(ClientResponse response) { - int result = statusCodeAsInt(response); - return result != 0 ? result : null; + public String path() { + return delegate.url().getPath(); } @Override - public int statusCodeAsInt(ClientResponse response) { + public String url() { + return delegate.url().toString(); + } + + @Override + public String header(String name) { + return delegate.headers().getFirst(name); + } + + @Override + public void header(String name, String value) { + builder.header(name, value); + } + + ClientRequest buildRequest() { + return builder.build(); + } + + } + + static final class HttpClientResponse extends brave.http.HttpClientResponse { + + private final ClientResponse delegate; + + HttpClientResponse(ClientResponse delegate) { + this.delegate = delegate; + } + + @Override + public Object unwrap() { + return delegate; + } + + @Override + public int statusCode() { try { - return response.rawStatusCode(); + return delegate.rawStatusCode(); } catch (Exception dontCare) { return 0; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpAdapterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpClientResponseTests.java similarity index 77% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpAdapterTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpClientResponseTests.java index 9295fbc6b..031edc235 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpAdapterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpClientResponseTests.java @@ -22,16 +22,17 @@ import org.mockito.BDDMockito; import org.springframework.web.reactive.function.client.ClientResponse; -public class TraceExchangeFilterFunctionHttpAdapterTests { +public class TraceExchangeFilterFunctionHttpClientResponseTests { @Test public void should_return_0_when_invalid_status_code_is_returned() { ClientResponse clientResponse = BDDMockito.mock(ClientResponse.class); BDDMockito.given(clientResponse.rawStatusCode()) .willThrow(new IllegalStateException("Boom")); - TraceExchangeFilterFunction.HttpAdapter adapter = new TraceExchangeFilterFunction.HttpAdapter(); + TraceExchangeFilterFunction.HttpClientResponse response = new TraceExchangeFilterFunction.HttpClientResponse( + clientResponse); - Integer statusCode = adapter.statusCodeAsInt(clientResponse); + Integer statusCode = response.statusCode(); BDDAssertions.then(statusCode).isZero(); } @@ -40,9 +41,10 @@ public class TraceExchangeFilterFunctionHttpAdapterTests { public void should_return_status_code_when_valid_status_code_is_returned() { ClientResponse clientResponse = BDDMockito.mock(ClientResponse.class); BDDMockito.given(clientResponse.rawStatusCode()).willReturn(200); - TraceExchangeFilterFunction.HttpAdapter adapter = new TraceExchangeFilterFunction.HttpAdapter(); + TraceExchangeFilterFunction.HttpClientResponse response = new TraceExchangeFilterFunction.HttpClientResponse( + clientResponse); - Integer statusCode = adapter.statusCodeAsInt(clientResponse); + Integer statusCode = response.statusCode(); BDDAssertions.then(statusCode).isEqualTo(200); } 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 992144d61..23a066368 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 @@ -102,7 +102,8 @@ import static org.assertj.core.api.BDDAssertions.then; @SpringBootTest(classes = WebClientTests.TestConfiguration.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @TestPropertySource(properties = { "spring.sleuth.http.legacy.enabled=true", - "spring.application.name=fooservice", "feign.hystrix.enabled=false" }) + "spring.application.name=fooservice", "feign.hystrix.enabled=false", + "spring.sleuth.web.client.skip-pattern=/skip.*" }) @DirtiesContext public class WebClientTests { @@ -398,6 +399,17 @@ public class WebClientTests { .contains("CLIENT"); } + @Test + public void shouldRespectSkipPattern() { + this.webClient.get().uri("http://localhost:" + this.port + "/skip").retrieve() + .bodyToMono(String.class).block(); + then(this.reporter.getSpans()).isEmpty(); + + this.webClient.get().uri("http://localhost:" + this.port + "/doNotSkip") + .retrieve().bodyToMono(String.class).block(); + then(this.reporter.getSpans()).isNotEmpty(); + } + Object[] parametersForShouldAttachTraceIdWhenCallingAnotherService() { return new Object[] { (ResponseEntityProvider) (tests) -> tests.testFeignInterface.headers(), @@ -700,6 +712,11 @@ public class WebClientTests { return ResponseEntity.status(499).body("issue1462"); } + @RequestMapping(value = { "/skip", "/doNotSkip" }, method = RequestMethod.GET) + String skip() { + return "ok"; + } + } @Configuration