From 1a8c321a7bb63f1ddd341ea69e47357c9f3937ff Mon Sep 17 00:00:00 2001 From: Dmytro Nosan Date: Thu, 26 Dec 2019 14:02:01 +0200 Subject: [PATCH] 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 --- .../MetricsClientHttpRequestInterceptor.java | 29 ++++++++--- .../MetricsRestTemplateCustomizerTests.java | 48 ++++++++++++++++++- 2 files changed, 70 insertions(+), 7 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/MetricsClientHttpRequestInterceptor.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/MetricsClientHttpRequestInterceptor.java index b5e689866a..1aeee033ad 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/client/MetricsClientHttpRequestInterceptor.java +++ b/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 urlTemplate = new NamedThreadLocal<>("Rest Template URL Template"); + private static final ThreadLocal> 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 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> { + + private UrlTemplateThreadLocal() { + super("Rest Template URL Template"); + } + + @Override + protected Deque initialValue() { + return new LinkedList<>(); + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java index e996d4d763..f1cc5d5496 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java +++ b/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); + } + + } + }