root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions 12 Packages Projects Releases Wiki Activity

Ignore trailing slash when recording Web metrics 

Fixes gh-18207
This commit is contained in:
Madhura Bhave 2020-01-13 13:31:16 -08:00
parent 8edffc8ed7
commit e60194c7d5
11 changed files with 139 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java
View File

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

3
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/reactive/WebFluxMetricsAutoConfiguration.java
View File

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

2
spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfiguration.java
View File

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

14
spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/reactive/WebFluxMetricsAutoConfigurationTests.java
View File

@ -33,6 +33,7 @@ import org.springframework.boot.test.system.CapturedOutput;
import org.springframework.boot.test.system.OutputCaptureExtension;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.test.web.reactive.server.WebTestClient;
import static org.assertj.core.api.Assertions.assertThat;
@ -43,6 +44,7 @@ import static org.mockito.Mockito.mock;
*
* @author Brian Clozel
* @author Dmytro Nosan
* @author Madhura Bhave
*/
@ExtendWith(OutputCaptureExtension.class)
class WebFluxMetricsAutoConfigurationTests {
@ -55,9 +57,21 @@ class WebFluxMetricsAutoConfigurationTests {
this.contextRunner.run((context) -> {
assertThat(context).getBeans(MetricsWebFilter.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
void shouldNotOverrideCustomTagsProvider() {
this.contextRunner.withUserConfiguration(CustomWebFluxTagsProviderConfig.class)

14
spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java
View File

@ -48,6 +48,7 @@ import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.Ordered;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
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 Dmytro Nosan
* @author Tadaya Tsuyukubo
* @author Madhura Bhave
*/
@ExtendWith(OutputCaptureExtension.class)
class WebMvcMetricsAutoConfigurationTests {
@ -79,12 +81,24 @@ class WebMvcMetricsAutoConfigurationTests {
void definesTagsProviderAndFilterWhenMeterRegistryIsPresent() {
this.contextRunner.run((context) -> {
assertThat(context).hasSingleBean(DefaultWebMvcTagsProvider.class);
assertThat(ReflectionTestUtils.getField(context.getBean(DefaultWebMvcTagsProvider.class),
"ignoreTrailingSlash")).isEqualTo(true);
assertThat(context).hasSingleBean(FilterRegistrationBean.class);
assertThat(context.getBean(FilterRegistrationBean.class).getFilter())
.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
void tagsProviderBacksOff() {
this.contextRunner.withUserConfiguration(TagsProviderConfiguration.class).run((context) -> {

14
spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/DefaultWebFluxTagsProvider.java
View File

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

24
spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/WebFluxTags.java
View File

@ -16,6 +16,8 @@
package org.springframework.boot.actuate.metrics.web.reactive.server;
import java.util.regex.Pattern;
import io.micrometer.core.instrument.Tag;
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 Pattern TRAILING_SLASH_PATTERN = Pattern.compile("/$");
private WebFluxTags() {
}
@ -87,9 +91,27 @@ public final class WebFluxTags {
* @return the uri tag derived from the 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);
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();
if (status != null) {

16
spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/DefaultWebMvcTagsProvider.java
View File

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

18
spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java
View File

@ -94,9 +94,27 @@ public final class WebMvcTags {
* @return the uri tag derived from the request
*/
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) {
String pattern = getMatchingPattern(request);
if (pattern != null) {
if (ignoreTrailingSlash) {
pattern = TRAILING_SLASH_PATTERN.matcher(pattern).replaceAll("");
}
return Tag.of("uri", pattern);
}
if (response != null) {

16
spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java
View File

@ -37,6 +37,7 @@ import static org.assertj.core.api.Assertions.assertThat;
* Tests for {@link MetricsWebFilter}
*
* @author Brian Clozel
* @author Madhura Bhave
*/
class MetricsWebFilterTests {
@ -50,7 +51,7 @@ class MetricsWebFilterTests {
void setup() {
MockClock clock = new MockClock();
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);
}
@ -102,6 +103,19 @@ class MetricsWebFilterTests {
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) {
PathPatternParser parser = new PathPatternParser();
MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path).build());

15
spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterTests.java
View File

@ -289,6 +289,14 @@ class WebMvcMetricsFilterTests {
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 })
@Retention(RetentionPolicy.RUNTIME)
@Timed(percentiles = 0.95)
@ -356,7 +364,7 @@ class WebMvcMetricsFilterTests {
@Bean
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);
}
@ -380,6 +388,11 @@ class WebMvcMetricsFilterTests {
return id.toString();
}
@GetMapping("/simple/{id}")
String simpleMapping(@PathVariable Long id) {
return id.toString();
}
@Timed
@Timed(value = "my.long.request", extraTags = { "region", "test" }, longTask = true)
@GetMapping("/callable/{id}")