Support nested requests in MetricsClientHttpRequestInterceptor

Prior to this commit, requests made by `HttpRequestInterceptor`
instances configured on `RestTemplate` would not be recorded
properly.

This commit ensures that nested requests are recorded separately.

Closes gh-20231

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/MetricsClientHttpRequestInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,8 @@ package org.springframework.boot.actuate.metrics.web.client;
 
 import java.io.IOException;
 import java.net.URI;
+import java.util.Deque;
+import java.util.LinkedList;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
@@ -41,7 +43,7 @@ import org.springframework.web.util.UriTemplateHandler;
  */
 class MetricsClientHttpRequestInterceptor implements ClientHttpRequestInterceptor {
 
-	private static final ThreadLocal<String> urlTemplate = new NamedThreadLocal<>("Rest Template URL Template");
+	private static final ThreadLocal<Deque<String>> urlTemplate = new UrlTemplateThreadLocal();
 
 	private final MeterRegistry meterRegistry;
 
@@ -96,7 +98,9 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto
 		finally {
 			getTimeBuilder(request, response).register(this.meterRegistry).record(System.nanoTime() - startTime,
 					TimeUnit.NANOSECONDS);
-			urlTemplate.remove();
+			if (urlTemplate.get().isEmpty()) {
+				urlTemplate.remove();
+			}
 		}
 	}
 
@@ -105,13 +109,13 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto
 
 			@Override
 			public URI expand(String url, Map<String, ?> arguments) {
-				urlTemplate.set(url);
+				urlTemplate.get().push(url);
 				return delegate.expand(url, arguments);
 			}
 
 			@Override
 			public URI expand(String url, Object... arguments) {
-				urlTemplate.set(url);
+				urlTemplate.get().push(url);
 				return delegate.expand(url, arguments);
 			}
 
@@ -120,8 +124,21 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto
 
 	private Timer.Builder getTimeBuilder(HttpRequest request, ClientHttpResponse response) {
 		return this.autoTimer.builder(this.metricName)
-				.tags(this.tagProvider.getTags(urlTemplate.get(), request, response))
+				.tags(this.tagProvider.getTags(urlTemplate.get().poll(), request, response))
 				.description("Timer of RestTemplate operation");
 	}
 
+	private static final class UrlTemplateThreadLocal extends NamedThreadLocal<Deque<String>> {
+
+		private UrlTemplateThreadLocal() {
+			super("Rest Template URL Template");
+		}
+
+		@Override
+		protected Deque<String> initialValue() {
+			return new LinkedList<>();
+		}
+
+	}
+
 }

spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * Copyright 2012-2020 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package org.springframework.boot.actuate.metrics.web.client;
 
+import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
 
@@ -29,7 +30,11 @@ import org.junit.jupiter.api.Test;
 
 import org.springframework.boot.actuate.metrics.AutoTimer;
 import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpRequest;
 import org.springframework.http.MediaType;
+import org.springframework.http.client.ClientHttpRequestExecution;
+import org.springframework.http.client.ClientHttpRequestInterceptor;
+import org.springframework.http.client.ClientHttpResponse;
 import org.springframework.test.web.client.MockRestServiceServer;
 import org.springframework.test.web.client.match.MockRestRequestMatchers;
 import org.springframework.test.web.client.response.MockRestResponseCreators;
@@ -107,4 +112,45 @@ class MetricsRestTemplateCustomizerTests {
 		this.mockServer.verify();
 	}
 
+	@Test
+	void interceptNestedRequest() {
+		this.mockServer.expect(MockRestRequestMatchers.requestTo("/test/123"))
+				.andExpect(MockRestRequestMatchers.method(HttpMethod.GET))
+				.andRespond(MockRestResponseCreators.withSuccess("OK", MediaType.APPLICATION_JSON));
+
+		RestTemplate nestedRestTemplate = new RestTemplate();
+		MockRestServiceServer nestedMockServer = MockRestServiceServer.createServer(nestedRestTemplate);
+		nestedMockServer.expect(MockRestRequestMatchers.requestTo("/nestedTest/124"))
+				.andExpect(MockRestRequestMatchers.method(HttpMethod.GET))
+				.andRespond(MockRestResponseCreators.withSuccess("OK", MediaType.APPLICATION_JSON));
+		this.customizer.customize(nestedRestTemplate);
+
+		TestInterceptor testInterceptor = new TestInterceptor(nestedRestTemplate);
+		this.restTemplate.getInterceptors().add(testInterceptor);
+
+		this.restTemplate.getForObject("/test/{id}", String.class, 123);
+		this.registry.get("http.client.requests").tags("uri", "/test/{id}").timer();
+		this.registry.get("http.client.requests").tags("uri", "/nestedTest/{nestedId}").timer();
+
+		this.mockServer.verify();
+		nestedMockServer.verify();
+	}
+
+	private static final class TestInterceptor implements ClientHttpRequestInterceptor {
+
+		private final RestTemplate restTemplate;
+
+		private TestInterceptor(RestTemplate restTemplate) {
+			this.restTemplate = restTemplate;
+		}
+
+		@Override
+		public ClientHttpResponse intercept(HttpRequest request, byte[] body, ClientHttpRequestExecution execution)
+				throws IOException {
+			this.restTemplate.getForObject("/nestedTest/{nestedId}", String.class, 124);
+			return execution.execute(request, body);
+		}
+
+	}
+
 }