From 1d8942941e6ced2b0bdf089159ea37994b92ea3b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 26 Jan 2018 20:14:19 -0800 Subject: [PATCH] Normalize micrometer client tag URIs Update `MetricsClientHttpRequestInterceptor` so that captured URIs are normalize to always contain a leading slash. Fixes gh-11798 --- .../MetricsClientHttpRequestInterceptor.java | 9 ++++-- .../MetricsRestTemplateCustomizerTests.java | 32 +++++++++++++------ 2 files changed, 30 insertions(+), 11 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 5984c341258..1c4f79e2820 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-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -97,10 +97,15 @@ class MetricsClientHttpRequestInterceptor implements ClientHttpRequestIntercepto private Timer.Builder getTimeBuilder(HttpRequest request, ClientHttpResponse response) { + String url = ensureLeadingSlash(urlTemplate.get()); return Timer.builder(this.metricName) - .tags(this.tagProvider.getTags(urlTemplate.get(), request, response)) + .tags(this.tagProvider.getTags(url, request, response)) .description("Timer of RestTemplate operation") .publishPercentileHistogram(this.recordPercentiles); } + private String ensureLeadingSlash(String url) { + return (url.startsWith("/") ? url : "/" + url); + } + } 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 59ce80a1c42..b6ddb9c82a1 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 @@ -47,18 +47,23 @@ public class MetricsRestTemplateCustomizerTests { private RestTemplate restTemplate; + private MockRestServiceServer mockServer; + + private MetricsRestTemplateCustomizer customizer; + @Before public void setup() { this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); this.restTemplate = new RestTemplate(); + this.mockServer = MockRestServiceServer.createServer(this.restTemplate); + this.customizer = new MetricsRestTemplateCustomizer(this.registry, + new DefaultRestTemplateExchangeTagsProvider(), "http.client.requests", + true); + this.customizer.customize(this.restTemplate); } @Test public void interceptRestTemplate() { - MetricsRestTemplateCustomizer customizer = new MetricsRestTemplateCustomizer( - this.registry, new DefaultRestTemplateExchangeTagsProvider(), - "http.client.requests", true); - customizer.customize(this.restTemplate); MockRestServiceServer mockServer = MockRestServiceServer .createServer(this.restTemplate); mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) @@ -79,13 +84,22 @@ public class MetricsRestTemplateCustomizerTests { @Test public void avoidDuplicateRegistration() { - MetricsRestTemplateCustomizer customizer = new MetricsRestTemplateCustomizer( - this.registry, new DefaultRestTemplateExchangeTagsProvider(), - "http.client.requests", true); - customizer.customize(this.restTemplate); + this.customizer.customize(this.restTemplate); assertThat(this.restTemplate.getInterceptors()).hasSize(1); - customizer.customize(this.restTemplate); + this.customizer.customize(this.restTemplate); assertThat(this.restTemplate.getInterceptors()).hasSize(1); } + @Test + public void normalizeUriToContainLeadingSlash() { + this.mockServer.expect(MockRestRequestMatchers.requestTo("/test/123")) + .andExpect(MockRestRequestMatchers.method(HttpMethod.GET)) + .andRespond(MockRestResponseCreators.withSuccess("OK", + MediaType.APPLICATION_JSON)); + String result = this.restTemplate.getForObject("test/{id}", String.class, 123); + this.registry.get("http.client.requests").tags("uri", "/test/{id}").timer(); + assertThat(result).isEqualTo("OK"); + this.mockServer.verify(); + } + }