Modify WebClient tracing to include the uri template in generated span names (#1940)

Spans names are in the form '<http method> <uri template>'
(e.g. 'GET /path/{id}'). This naming strategy matches the span names
generated by TraceWebFilter.

The improve naming strategy is not applied to RestTemplate as the uri
template is not readily available to interceptors and RestTemplate is
in maintenance mode

spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunction.java

@@ -38,10 +38,12 @@ import org.springframework.cloud.sleuth.http.HttpClientRequest;
 import org.springframework.cloud.sleuth.http.HttpClientResponse;
 import org.springframework.cloud.sleuth.instrument.reactor.TraceContextPropagator;
 import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.http.HttpMethod;
 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.WebClient;
 
 /**
  * Trace representation of {@link ExchangeFilterFunction}.
@@ -53,6 +55,8 @@ public final class TraceExchangeFilterFunction implements ExchangeFilterFunction
 
 	private static final Log log = LogFactory.getLog(TraceExchangeFilterFunction.class);
 
+	private static final String URI_TEMPLATE_ATTRIBUTE = WebClient.class.getName() + ".uriTemplate";
+
 	final ConfigurableApplicationContext springContext;
 
 	// Lazy initialized fields
@@ -155,12 +159,18 @@ public final class TraceExchangeFilterFunction implements ExchangeFilterFunction
 
 		final CurrentTraceContext currentTraceContext;
 
+		final HttpMethod method;
+
+		final String httpRoute;
+
 		TraceWebClientSubscriber(CoreSubscriber<? super ClientResponse> actual, Context ctx, Span clientSpan,
 				TraceContext parent, MonoWebClientTrace mono) {
 			this.actual = actual;
 			this.parent = parent;
 			this.handler = mono.handler;
 			this.currentTraceContext = mono.currentTraceContext;
+			this.method = mono.request.method();
+			this.httpRoute = (String) mono.request.attribute(URI_TEMPLATE_ATTRIBUTE).orElse(null);
 			this.context = this.parent != null && !this.parent.equals(ctx.getOrDefault(TraceContext.class, null))
 					? ctx.put(TraceContext.class, this.parent) : ctx;
 			set(clientSpan);
@@ -186,8 +196,7 @@ public final class TraceExchangeFilterFunction implements ExchangeFilterFunction
 					if (log.isTraceEnabled()) {
 						log.trace("OnNext finally");
 					}
-					// TODO: is there a way to read the request at response time?
-					this.handler.handleReceive(new ClientResponseWrapper(response), span);
+					this.handler.handleReceive(new ClientResponseWrapper(response, this.method, this.httpRoute), span);
 				}
 			}
 		}
@@ -330,6 +339,11 @@ public final class TraceExchangeFilterFunction implements ExchangeFilterFunction
 			return delegate.url().getPath();
 		}
 
+		@Override
+		public String route() {
+			return (String) delegate.attribute(URI_TEMPLATE_ATTRIBUTE).orElse(null);
+		}
+
 		@Override
 		public String url() {
 			return delegate.url().toString();
@@ -355,8 +369,24 @@ public final class TraceExchangeFilterFunction implements ExchangeFilterFunction
 
 		final ClientResponse delegate;
 
-		ClientResponseWrapper(ClientResponse delegate) {
+		final HttpMethod method;
+
+		final String httpRoute;
+
+		ClientResponseWrapper(ClientResponse delegate, HttpMethod method, String httpRoute) {
 			this.delegate = delegate;
+			this.method = method;
+			this.httpRoute = httpRoute;
+		}
+
+		@Override
+		public String method() {
+			return method.name();
+		}
+
+		@Override
+		public String route() {
+			return httpRoute;
 		}
 
 		@Override

spring-cloud-sleuth-instrumentation/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceExchangeFilterFunctionHttpClientResponseTests.java

@@ -29,7 +29,7 @@ public class TraceExchangeFilterFunctionHttpClientResponseTests {
 	public void should_return_0_when_invalid_status_code_is_returned() {
 		ClientResponse clientResponse = BDDMockito.mock(ClientResponse.class);
 		BDDMockito.given(clientResponse.rawStatusCode()).willReturn(-1);
-		ClientResponseWrapper response = new ClientResponseWrapper(clientResponse);
+		ClientResponseWrapper response = new ClientResponseWrapper(clientResponse, null, null);
 
 		Integer statusCode = response.statusCode();
 
@@ -40,7 +40,7 @@ public class TraceExchangeFilterFunctionHttpClientResponseTests {
 	public void should_return_status_code_when_valid_status_code_is_returned() {
 		ClientResponse clientResponse = BDDMockito.mock(ClientResponse.class);
 		BDDMockito.given(clientResponse.rawStatusCode()).willReturn(200);
-		ClientResponseWrapper response = new ClientResponseWrapper(clientResponse);
+		ClientResponseWrapper response = new ClientResponseWrapper(clientResponse, null, null);
 
 		Integer statusCode = response.statusCode();
 

tests/common/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/integration/sampled/WebClientTests.java

@@ -68,6 +68,7 @@ import org.springframework.http.ResponseEntity;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.TestPropertySource;
+import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestMethod;
@@ -252,6 +253,25 @@ public abstract class WebClientTests {
 				.collect(Collectors.toList())).isNotEmpty().contains("CLIENT");
 	}
 
+	@Test
+	@SuppressWarnings("unchecked")
+	public void shouldUseUriTemplateInSpanName() {
+		Span span = this.tracer.nextSpan().name("foo").start();
+
+		try (Tracer.SpanInScope ws = this.tracer.withSpan(span)) {
+			this.webClientBuilder.baseUrl("http://localhost:" + this.port).build().get()
+					.uri("/prefix/{variable}/suffix", "value").retrieve().bodyToMono(String.class)
+					.block(Duration.ofSeconds(5));
+		}
+		finally {
+			span.end();
+		}
+
+		thenThereIsNoCurrentSpan();
+		then(this.spans.reportedSpans().stream().filter(r -> r.getKind() == Span.Kind.CLIENT).map(r -> r.getName())
+				.collect(Collectors.toList())).isNotEmpty().contains("GET /prefix/{variable}/suffix");
+	}
+
 	/**
 	 * Cancel before {@link Subscription#request(long)} means a network request was never
 	 * sent
@@ -549,6 +569,11 @@ public abstract class WebClientTests {
 			return "ok";
 		}
 
+		@RequestMapping(value = "/prefix/{variable}/suffix", method = RequestMethod.GET)
+		String pathVariable(@PathVariable("variable") String variable) {
+			return "variable = " + variable;
+		}
+
 	}
 
 	@Configuration(proxyBeanMethods = false)