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.

See gh-19381
pull/19681/head
Dmytro Nosan 5 years ago committed by Brian Clozel
parent 3f3bac98cb
commit 25838b4794

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.io.IOException;
import java.net.URI; import java.net.URI;
import java.util.Deque;
import java.util.LinkedList;
import java.util.Map; import java.util.Map;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -41,7 +43,7 @@ import org.springframework.web.util.UriTemplateHandler;
*/ */
class MetricsClientHttpRequestInterceptor implements ClientHttpRequestInterceptor { 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; private final MeterRegistry meterRegistry;
@ -96,22 +98,24 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto
finally { finally {
getTimeBuilder(request, response).register(this.meterRegistry).record(System.nanoTime() - startTime, getTimeBuilder(request, response).register(this.meterRegistry).record(System.nanoTime() - startTime,
TimeUnit.NANOSECONDS); TimeUnit.NANOSECONDS);
if (urlTemplate.get().isEmpty()) {
urlTemplate.remove(); urlTemplate.remove();
} }
} }
}
UriTemplateHandler createUriTemplateHandler(UriTemplateHandler delegate) { UriTemplateHandler createUriTemplateHandler(UriTemplateHandler delegate) {
return new UriTemplateHandler() { return new UriTemplateHandler() {
@Override @Override
public URI expand(String url, Map<String, ?> arguments) { public URI expand(String url, Map<String, ?> arguments) {
urlTemplate.set(url); urlTemplate.get().push(url);
return delegate.expand(url, arguments); return delegate.expand(url, arguments);
} }
@Override @Override
public URI expand(String url, Object... arguments) { public URI expand(String url, Object... arguments) {
urlTemplate.set(url); urlTemplate.get().push(url);
return delegate.expand(url, arguments); return delegate.expand(url, arguments);
} }
@ -120,8 +124,21 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto
private Timer.Builder getTimeBuilder(HttpRequest request, ClientHttpResponse response) { private Timer.Builder getTimeBuilder(HttpRequest request, ClientHttpResponse response) {
return this.autoTimer.builder(this.metricName) 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"); .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<>();
}
}
} }

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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; package org.springframework.boot.actuate.metrics.web.client;
import java.io.IOException;
import java.net.URI; import java.net.URI;
import java.net.URISyntaxException; import java.net.URISyntaxException;
@ -29,7 +30,11 @@ import org.junit.jupiter.api.Test;
import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.boot.actuate.metrics.AutoTimer;
import org.springframework.http.HttpMethod; import org.springframework.http.HttpMethod;
import org.springframework.http.HttpRequest;
import org.springframework.http.MediaType; 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.MockRestServiceServer;
import org.springframework.test.web.client.match.MockRestRequestMatchers; import org.springframework.test.web.client.match.MockRestRequestMatchers;
import org.springframework.test.web.client.response.MockRestResponseCreators; import org.springframework.test.web.client.response.MockRestResponseCreators;
@ -107,4 +112,45 @@ class MetricsRestTemplateCustomizerTests {
this.mockServer.verify(); 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);
}
}
} }

Loading…
Cancel
Save