Merge branch '2.2.x'

Closes gh-19701
pull/19711/head
Madhura Bhave 5 years ago
commit d1f51e064b

@ -243,6 +243,11 @@ public class MetricsProperties {
*/ */
private String metricName = "http.server.requests"; private String metricName = "http.server.requests";
/**
* Whether the trailing slash should be ignored when recording metrics.
*/
private boolean ignoreTrailingSlash = true;
/** /**
* Auto-timed request settings. * Auto-timed request settings.
*/ */
@ -261,6 +266,14 @@ public class MetricsProperties {
this.metricName = metricName; this.metricName = metricName;
} }
public boolean isIgnoreTrailingSlash() {
return this.ignoreTrailingSlash;
}
public void setIgnoreTrailingSlash(boolean ignoreTrailingSlash) {
this.ignoreTrailingSlash = ignoreTrailingSlash;
}
} }
} }

@ -59,7 +59,8 @@ public class WebFluxMetricsAutoConfiguration {
@Bean @Bean
@ConditionalOnMissingBean(WebFluxTagsProvider.class) @ConditionalOnMissingBean(WebFluxTagsProvider.class)
public DefaultWebFluxTagsProvider webfluxTagConfigurer() { public DefaultWebFluxTagsProvider webfluxTagConfigurer() {
return new DefaultWebFluxTagsProvider(); return new DefaultWebFluxTagsProvider(
this.properties.getWeb().getServer().getRequest().isIgnoreTrailingSlash());
} }
@Bean @Bean

@ -71,7 +71,7 @@ public class WebMvcMetricsAutoConfiguration {
@Bean @Bean
@ConditionalOnMissingBean(WebMvcTagsProvider.class) @ConditionalOnMissingBean(WebMvcTagsProvider.class)
public DefaultWebMvcTagsProvider webMvcTagsProvider() { public DefaultWebMvcTagsProvider webMvcTagsProvider() {
return new DefaultWebMvcTagsProvider(); return new DefaultWebMvcTagsProvider(this.properties.getWeb().getServer().getRequest().isIgnoreTrailingSlash());
} }
@Bean @Bean

@ -33,6 +33,7 @@ import org.springframework.boot.test.system.CapturedOutput;
import org.springframework.boot.test.system.OutputCaptureExtension; import org.springframework.boot.test.system.OutputCaptureExtension;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.test.web.reactive.server.WebTestClient;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
@ -43,6 +44,7 @@ import static org.mockito.Mockito.mock;
* *
* @author Brian Clozel * @author Brian Clozel
* @author Dmytro Nosan * @author Dmytro Nosan
* @author Madhura Bhave
*/ */
@ExtendWith(OutputCaptureExtension.class) @ExtendWith(OutputCaptureExtension.class)
class WebFluxMetricsAutoConfigurationTests { class WebFluxMetricsAutoConfigurationTests {
@ -55,9 +57,21 @@ class WebFluxMetricsAutoConfigurationTests {
this.contextRunner.run((context) -> { this.contextRunner.run((context) -> {
assertThat(context).getBeans(MetricsWebFilter.class).hasSize(1); assertThat(context).getBeans(MetricsWebFilter.class).hasSize(1);
assertThat(context).getBeans(DefaultWebFluxTagsProvider.class).hasSize(1); assertThat(context).getBeans(DefaultWebFluxTagsProvider.class).hasSize(1);
assertThat(ReflectionTestUtils.getField(context.getBean(DefaultWebFluxTagsProvider.class),
"ignoreTrailingSlash")).isEqualTo(true);
}); });
} }
@Test
void tagsProviderWhenIgnoreTrailingSlashIsFalse() {
this.contextRunner.withPropertyValues("management.metrics.web.server.request.ignore-trailing-slash=false")
.run((context) -> {
assertThat(context).hasSingleBean(DefaultWebFluxTagsProvider.class);
assertThat(ReflectionTestUtils.getField(context.getBean(DefaultWebFluxTagsProvider.class),
"ignoreTrailingSlash")).isEqualTo(false);
});
}
@Test @Test
void shouldNotOverrideCustomTagsProvider() { void shouldNotOverrideCustomTagsProvider() {
this.contextRunner.withUserConfiguration(CustomWebFluxTagsProviderConfig.class) this.contextRunner.withUserConfiguration(CustomWebFluxTagsProviderConfig.class)

@ -48,6 +48,7 @@ import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Configuration;
import org.springframework.core.Ordered; import org.springframework.core.Ordered;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.test.web.servlet.setup.MockMvcBuilders;
@ -62,6 +63,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
* @author Andy Wilkinson * @author Andy Wilkinson
* @author Dmytro Nosan * @author Dmytro Nosan
* @author Tadaya Tsuyukubo * @author Tadaya Tsuyukubo
* @author Madhura Bhave
*/ */
@ExtendWith(OutputCaptureExtension.class) @ExtendWith(OutputCaptureExtension.class)
class WebMvcMetricsAutoConfigurationTests { class WebMvcMetricsAutoConfigurationTests {
@ -79,12 +81,24 @@ class WebMvcMetricsAutoConfigurationTests {
void definesTagsProviderAndFilterWhenMeterRegistryIsPresent() { void definesTagsProviderAndFilterWhenMeterRegistryIsPresent() {
this.contextRunner.run((context) -> { this.contextRunner.run((context) -> {
assertThat(context).hasSingleBean(DefaultWebMvcTagsProvider.class); assertThat(context).hasSingleBean(DefaultWebMvcTagsProvider.class);
assertThat(ReflectionTestUtils.getField(context.getBean(DefaultWebMvcTagsProvider.class),
"ignoreTrailingSlash")).isEqualTo(true);
assertThat(context).hasSingleBean(FilterRegistrationBean.class); assertThat(context).hasSingleBean(FilterRegistrationBean.class);
assertThat(context.getBean(FilterRegistrationBean.class).getFilter()) assertThat(context.getBean(FilterRegistrationBean.class).getFilter())
.isInstanceOf(WebMvcMetricsFilter.class); .isInstanceOf(WebMvcMetricsFilter.class);
}); });
} }
@Test
void tagsProviderWhenIgnoreTrailingSlashIsFalse() {
this.contextRunner.withPropertyValues("management.metrics.web.server.request.ignore-trailing-slash=false")
.run((context) -> {
assertThat(context).hasSingleBean(DefaultWebMvcTagsProvider.class);
assertThat(ReflectionTestUtils.getField(context.getBean(DefaultWebMvcTagsProvider.class),
"ignoreTrailingSlash")).isEqualTo(false);
});
}
@Test @Test
void tagsProviderBacksOff() { void tagsProviderBacksOff() {
this.contextRunner.withUserConfiguration(TagsProviderConfiguration.class).run((context) -> { this.contextRunner.withUserConfiguration(TagsProviderConfiguration.class).run((context) -> {

@ -31,10 +31,20 @@ import org.springframework.web.server.ServerWebExchange;
*/ */
public class DefaultWebFluxTagsProvider implements WebFluxTagsProvider { public class DefaultWebFluxTagsProvider implements WebFluxTagsProvider {
private final boolean ignoreTrailingSlash;
public DefaultWebFluxTagsProvider() {
this(false);
}
public DefaultWebFluxTagsProvider(boolean ignoreTrailingSlash) {
this.ignoreTrailingSlash = ignoreTrailingSlash;
}
@Override @Override
public Iterable<Tag> httpRequestTags(ServerWebExchange exchange, Throwable exception) { public Iterable<Tag> httpRequestTags(ServerWebExchange exchange, Throwable exception) {
return Arrays.asList(WebFluxTags.method(exchange), WebFluxTags.uri(exchange), WebFluxTags.exception(exception), return Arrays.asList(WebFluxTags.method(exchange), WebFluxTags.uri(exchange, this.ignoreTrailingSlash),
WebFluxTags.status(exchange), WebFluxTags.outcome(exchange)); WebFluxTags.exception(exception), WebFluxTags.status(exchange), WebFluxTags.outcome(exchange));
} }
} }

@ -16,6 +16,8 @@
package org.springframework.boot.actuate.metrics.web.reactive.server; package org.springframework.boot.actuate.metrics.web.reactive.server;
import java.util.regex.Pattern;
import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Tag;
import org.springframework.boot.actuate.metrics.http.Outcome; import org.springframework.boot.actuate.metrics.http.Outcome;
@ -48,6 +50,8 @@ public final class WebFluxTags {
private static final Tag EXCEPTION_NONE = Tag.of("exception", "None"); private static final Tag EXCEPTION_NONE = Tag.of("exception", "None");
private static final Pattern TRAILING_SLASH_PATTERN = Pattern.compile("/$");
private WebFluxTags() { private WebFluxTags() {
} }
@ -87,9 +91,27 @@ public final class WebFluxTags {
* @return the uri tag derived from the exchange * @return the uri tag derived from the exchange
*/ */
public static Tag uri(ServerWebExchange exchange) { public static Tag uri(ServerWebExchange exchange) {
return uri(exchange, false);
}
/**
* Creates a {@code uri} tag based on the URI of the given {@code exchange}. Uses the
* {@link HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} best matching pattern if
* available. Falling back to {@code REDIRECTION} for 3xx responses, {@code NOT_FOUND}
* for 404 responses, {@code root} for requests with no path info, and {@code UNKNOWN}
* for all other requests.
* @param exchange the exchange
* @param ignoreTrailingSlash whether to ignore the trailing slash
* @return the uri tag derived from the exchange
*/
public static Tag uri(ServerWebExchange exchange, boolean ignoreTrailingSlash) {
PathPattern pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); PathPattern pathPattern = exchange.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE);
if (pathPattern != null) { if (pathPattern != null) {
return Tag.of("uri", pathPattern.getPatternString()); String patternString = pathPattern.getPatternString();
if (ignoreTrailingSlash) {
patternString = TRAILING_SLASH_PATTERN.matcher(patternString).replaceAll("");
}
return Tag.of("uri", patternString);
} }
HttpStatus status = exchange.getResponse().getStatusCode(); HttpStatus status = exchange.getResponse().getStatusCode();
if (status != null) { if (status != null) {

@ -30,16 +30,26 @@ import io.micrometer.core.instrument.Tags;
*/ */
public class DefaultWebMvcTagsProvider implements WebMvcTagsProvider { public class DefaultWebMvcTagsProvider implements WebMvcTagsProvider {
private final boolean ignoreTrailingSlash;
public DefaultWebMvcTagsProvider() {
this(false);
}
public DefaultWebMvcTagsProvider(boolean ignoreTrailingSlash) {
this.ignoreTrailingSlash = ignoreTrailingSlash;
}
@Override @Override
public Iterable<Tag> getTags(HttpServletRequest request, HttpServletResponse response, Object handler, public Iterable<Tag> getTags(HttpServletRequest request, HttpServletResponse response, Object handler,
Throwable exception) { Throwable exception) {
return Tags.of(WebMvcTags.method(request), WebMvcTags.uri(request, response), WebMvcTags.exception(exception), return Tags.of(WebMvcTags.method(request), WebMvcTags.uri(request, response, this.ignoreTrailingSlash),
WebMvcTags.status(response), WebMvcTags.outcome(response)); WebMvcTags.exception(exception), WebMvcTags.status(response), WebMvcTags.outcome(response));
} }
@Override @Override
public Iterable<Tag> getLongRequestTags(HttpServletRequest request, Object handler) { public Iterable<Tag> getLongRequestTags(HttpServletRequest request, Object handler) {
return Tags.of(WebMvcTags.method(request), WebMvcTags.uri(request, null)); return Tags.of(WebMvcTags.method(request), WebMvcTags.uri(request, null, this.ignoreTrailingSlash));
} }
} }

@ -94,9 +94,27 @@ public final class WebMvcTags {
* @return the uri tag derived from the request * @return the uri tag derived from the request
*/ */
public static Tag uri(HttpServletRequest request, HttpServletResponse response) { public static Tag uri(HttpServletRequest request, HttpServletResponse response) {
return uri(request, response, false);
}
/**
* Creates a {@code uri} tag based on the URI of the given {@code request}. Uses the
* {@link HandlerMapping#BEST_MATCHING_PATTERN_ATTRIBUTE} best matching pattern if
* available. Falling back to {@code REDIRECTION} for 3xx responses, {@code NOT_FOUND}
* for 404 responses, {@code root} for requests with no path info, and {@code UNKNOWN}
* for all other requests.
* @param request the request
* @param response the response
* @param ignoreTrailingSlash whether to ignore the trailing slash
* @return the uri tag derived from the request
*/
public static Tag uri(HttpServletRequest request, HttpServletResponse response, boolean ignoreTrailingSlash) {
if (request != null) { if (request != null) {
String pattern = getMatchingPattern(request); String pattern = getMatchingPattern(request);
if (pattern != null) { if (pattern != null) {
if (ignoreTrailingSlash) {
pattern = TRAILING_SLASH_PATTERN.matcher(pattern).replaceAll("");
}
return Tag.of("uri", pattern); return Tag.of("uri", pattern);
} }
if (response != null) { if (response != null) {

@ -37,6 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link MetricsWebFilter} * Tests for {@link MetricsWebFilter}
* *
* @author Brian Clozel * @author Brian Clozel
* @author Madhura Bhave
*/ */
class MetricsWebFilterTests { class MetricsWebFilterTests {
@ -50,7 +51,7 @@ class MetricsWebFilterTests {
void setup() { void setup() {
MockClock clock = new MockClock(); MockClock clock = new MockClock();
this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock);
this.webFilter = new MetricsWebFilter(this.registry, new DefaultWebFluxTagsProvider(), REQUEST_METRICS_NAME, this.webFilter = new MetricsWebFilter(this.registry, new DefaultWebFluxTagsProvider(true), REQUEST_METRICS_NAME,
AutoTimer.ENABLED); AutoTimer.ENABLED);
} }
@ -102,6 +103,19 @@ class MetricsWebFilterTests {
assertMetricsContainsTag("status", "500"); assertMetricsContainsTag("status", "500");
} }
@Test
void trailingSlashShouldNotRecordDuplicateMetrics() {
MockServerWebExchange exchange1 = createExchange("/projects/spring-boot", "/projects/{project}");
MockServerWebExchange exchange2 = createExchange("/projects/spring-boot", "/projects/{project}/");
this.webFilter.filter(exchange1, (serverWebExchange) -> exchange1.getResponse().setComplete())
.block(Duration.ofSeconds(30));
this.webFilter.filter(exchange2, (serverWebExchange) -> exchange2.getResponse().setComplete())
.block(Duration.ofSeconds(30));
assertThat(this.registry.get(REQUEST_METRICS_NAME).tag("uri", "/projects/{project}").timer().count())
.isEqualTo(2);
assertThat(this.registry.get(REQUEST_METRICS_NAME).tag("status", "200").timer().count()).isEqualTo(2);
}
private MockServerWebExchange createExchange(String path, String pathPattern) { private MockServerWebExchange createExchange(String path, String pathPattern) {
PathPatternParser parser = new PathPatternParser(); PathPatternParser parser = new PathPatternParser();
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path).build()); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path).build());

@ -289,6 +289,14 @@ class WebMvcMetricsFilterTests {
assertThat(this.prometheusRegistry.scrape()).contains("le=\"30.0\""); assertThat(this.prometheusRegistry.scrape()).contains("le=\"30.0\"");
} }
@Test
void trailingSlashShouldNotRecordDuplicateMetrics() throws Exception {
this.mvc.perform(get("/api/c1/simple/10")).andExpect(status().isOk());
this.mvc.perform(get("/api/c1/simple/10/")).andExpect(status().isOk());
assertThat(this.registry.get("http.server.requests").tags("status", "200", "uri", "/api/c1/simple/{id}").timer()
.count()).isEqualTo(2);
}
@Target({ ElementType.METHOD }) @Target({ ElementType.METHOD })
@Retention(RetentionPolicy.RUNTIME) @Retention(RetentionPolicy.RUNTIME)
@Timed(percentiles = 0.95) @Timed(percentiles = 0.95)
@ -356,7 +364,7 @@ class WebMvcMetricsFilterTests {
@Bean @Bean
WebMvcMetricsFilter webMetricsFilter(MeterRegistry registry, WebApplicationContext ctx) { WebMvcMetricsFilter webMetricsFilter(MeterRegistry registry, WebApplicationContext ctx) {
return new WebMvcMetricsFilter(registry, new DefaultWebMvcTagsProvider(), "http.server.requests", return new WebMvcMetricsFilter(registry, new DefaultWebMvcTagsProvider(true), "http.server.requests",
AutoTimer.ENABLED); AutoTimer.ENABLED);
} }
@ -380,6 +388,11 @@ class WebMvcMetricsFilterTests {
return id.toString(); return id.toString();
} }
@GetMapping("/simple/{id}")
String simpleMapping(@PathVariable Long id) {
return id.toString();
}
@Timed @Timed
@Timed(value = "my.long.request", extraTags = { "region", "test" }, longTask = true) @Timed(value = "my.long.request", extraTags = { "region", "test" }, longTask = true)
@GetMapping("/callable/{id}") @GetMapping("/callable/{id}")

Loading…
Cancel
Save