From 7e56dc67ebdfd1de7de71a26584313eadd97ce65 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Tue, 5 Feb 2019 17:00:59 +0100 Subject: [PATCH 1/2] Fix regression introduced in #1126 (brave headers propagation) Before #1126, the headers were eagerly set in `TraceExchangeFilterFunction#filter`. After it, the side effect was moved to lazy `MonoWebClientTrace#subscribe`. However, we have everything to instrument the request in `filter`, and it can be done eagerly. --- .../TraceWebClientBeanPostProcessor.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) 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 a274b43c19..015e5f96b7 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 @@ -146,7 +146,17 @@ public static ExchangeFilterFunction create(BeanFactory beanFactory) { @Override public Mono filter(ClientRequest request, ExchangeFunction next) { - return new MonoWebClientTrace(next, request, this); + ClientRequest.Builder builder = ClientRequest.from(request); + if (log.isDebugEnabled()) { + log.debug("Instrumenting WebClient call"); + } + Span span = handler().handleSend(injector(), builder, request, + tracer().nextSpan()); + if (log.isDebugEnabled()) { + log.debug("Handled send of " + span); + } + + return new MonoWebClientTrace(next, builder.build(), this, span); } private static final class MonoWebClientTrace extends Mono { @@ -165,8 +175,10 @@ private static final class MonoWebClientTrace extends Mono { final Function, ? extends Publisher> scopePassingTransformer; + private final Span span; + MonoWebClientTrace(ExchangeFunction next, ClientRequest request, - TraceExchangeFilterFunction parent) { + TraceExchangeFilterFunction parent, Span span) { this.next = next; this.request = request; this.tracer = parent.tracer(); @@ -174,28 +186,16 @@ private static final class MonoWebClientTrace extends Mono { this.injector = parent.injector(); this.tracing = parent.httpTracing().tracing(); this.scopePassingTransformer = parent.scopePassingTransformer; + this.span = span; } @Override public void subscribe(CoreSubscriber subscriber) { - final ClientRequest.Builder builder = ClientRequest.from(this.request); Context context = subscriber.currentContext(); - this.next.exchange(builder.build()).subscribe(new WebClientTracerSubscriber( - subscriber, context, findOrCreateSpan(builder), this)); - } - - private Span findOrCreateSpan(ClientRequest.Builder builder) { - if (log.isDebugEnabled()) { - log.debug("Instrumenting WebClient call"); - } - Span clientSpan = this.handler.handleSend(this.injector, builder, - this.request, this.tracer.nextSpan()); - if (log.isDebugEnabled()) { - log.debug("Handled send of " + clientSpan); - } - return clientSpan; + this.next.exchange(request).subscribe( + new WebClientTracerSubscriber(subscriber, context, span, this)); } static final class WebClientTracerSubscriber From 78c52369335a7cc1b60021925472fa0cd441a528 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Tue, 5 Feb 2019 17:16:40 +0100 Subject: [PATCH 2/2] Add a test --- .../client/integration/WebClientTests.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 7ddb02ddd8..853eee3200 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 @@ -26,6 +26,7 @@ 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; @@ -491,6 +492,28 @@ public void should_wrap_rest_template_builders() { then(this.reporter.getSpans()).extracting("kind.name").contains("CLIENT"); } + @Test + public void should_add_headers_eagerly() { + Span span = this.tracer.nextSpan().name("foo").start(); + + AtomicReference traceId = new AtomicReference<>(); + try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) { + this.webClientBuilder + .filter((request, exchange) -> { + traceId.set(request.headers().getFirst("X-B3-SpanId")); + + return exchange.exchange(request); + }) + .build() + .get().uri("http://localhost:" + this.port + "/traceid") + .retrieve().bodyToMono(String.class).block(); + } + finally { + span.finish(); + } + then(traceId).doesNotHaveValue(null); + } + private String getHeader(ResponseEntity response, String name) { List headers = response.getHeaders().get(name); return headers == null || headers.isEmpty() ? null : headers.get(0);