From e323e05eeac70e6f14de71984c7a3ec1e5c4a580 Mon Sep 17 00:00:00 2001 From: babjo Date: Wed, 29 Jan 2020 23:07:25 +0900 Subject: [PATCH 1/2] Stop time web metrics when autotime is disabled See gh-19981 --- .../metrics/web/TestController.java | 9 +++++++ .../WebMvcMetricsAutoConfigurationTests.java | 26 ++++++++++++++++--- .../web/servlet/WebMvcMetricsFilter.java | 7 +++-- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java index efbae52164..824f0a2c83 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java @@ -16,6 +16,8 @@ package org.springframework.boot.actuate.autoconfigure.metrics.web; +import io.micrometer.core.annotation.Timed; + import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; @@ -24,6 +26,7 @@ import org.springframework.web.bind.annotation.RestController; * * @author Dmytro Nosan * @author Stephane Nicoll + * @author Chanhyeong LEE */ @RestController public class TestController { @@ -43,4 +46,10 @@ public class TestController { return "test2"; } + @Timed + @GetMapping("test3") + public String test3() { + return "test2"; + } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java index 6f99862c15..18df57e945 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.autoconfigure.metrics.web.servlet; +import java.util.Collection; import java.util.Collections; import java.util.EnumSet; @@ -24,6 +25,7 @@ import javax.servlet.Filter; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import io.micrometer.core.instrument.Meter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Timer; @@ -64,6 +66,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. * @author Dmytro Nosan * @author Tadaya Tsuyukubo * @author Madhura Bhave + * @author Chanhyeong LEE */ @ExtendWith(OutputCaptureExtension.class) class WebMvcMetricsAutoConfigurationTests { @@ -157,6 +160,19 @@ class WebMvcMetricsAutoConfigurationTests { }); } + @Test + void timerWorksWithTimedAnnotationsWhenAutoTimeRequestsIsFalse() { + this.contextRunner.withUserConfiguration(TestController.class) + .withConfiguration(AutoConfigurations.of(MetricsAutoConfiguration.class, WebMvcAutoConfiguration.class)) + .withPropertyValues("management.metrics.web.server.request.autotime.enabled=false").run((context) -> { + MeterRegistry registry = getInitializedMeterRegistry(context, "/test3"); + Collection meters = registry.get("http.server.requests").meters(); + assertThat(meters).hasSize(1); + Meter meter = meters.iterator().next(); + assertThat(meter.getId().getTag("uri")).isEqualTo("/test3"); + }); + } + @Test @SuppressWarnings("rawtypes") void longTaskTimingInterceptorIsRegistered() { @@ -167,13 +183,17 @@ class WebMvcMetricsAutoConfigurationTests { .contains(LongTaskTimingHandlerInterceptor.class)); } - private MeterRegistry getInitializedMeterRegistry(AssertableWebApplicationContext context) throws Exception { + private MeterRegistry getInitializedMeterRegistry(AssertableWebApplicationContext context, String... urls) + throws Exception { + if (urls.length == 0) { + urls = new String[] { "/test0", "/test1", "/test2" }; + } assertThat(context).hasSingleBean(FilterRegistrationBean.class); Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); assertThat(filter).isInstanceOf(WebMvcMetricsFilter.class); MockMvc mockMvc = MockMvcBuilders.webAppContextSetup(context).addFilters(filter).build(); - for (int i = 0; i < 3; i++) { - mockMvc.perform(MockMvcRequestBuilders.get("/test" + i)).andExpect(status().isOk()); + for (String url : urls) { + mockMvc.perform(MockMvcRequestBuilders.get(url)).andExpect(status().isOk()); } return context.getBean(MeterRegistry.class); } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java index 7cc029ffaf..6961091714 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java @@ -48,6 +48,7 @@ import org.springframework.web.util.NestedServletException; * * @author Jon Schneider * @author Phillip Webb + * @author Chanhyeong LEE * @since 2.0.0 */ public class WebMvcMetricsFilter extends OncePerRequestFilter { @@ -139,8 +140,10 @@ public class WebMvcMetricsFilter extends OncePerRequestFilter { Set annotations = getTimedAnnotations(handler); Timer.Sample timerSample = timingContext.getTimerSample(); if (annotations.isEmpty()) { - Builder builder = this.autoTimer.builder(this.metricName); - timerSample.stop(getTimer(builder, handler, request, response, exception)); + if (this.autoTimer.isEnabled()) { + Builder builder = this.autoTimer.builder(this.metricName); + timerSample.stop(getTimer(builder, handler, request, response, exception)); + } return; } for (Timed annotation : annotations) { From 10643b770258b503e29a99c2eb3f25b5b35008ea Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 4 Feb 2020 09:58:11 +0100 Subject: [PATCH 2/2] Polish "Stop time web metrics when autotime is disabled" See gh-19981 --- .../autoconfigure/metrics/web/TestController.java | 4 ++-- .../servlet/WebMvcMetricsAutoConfigurationTests.java | 11 ++++++----- .../metrics/web/servlet/WebMvcMetricsFilter.java | 12 ++++++------ 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java index 824f0a2c83..42d81a70dd 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/TestController.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. @@ -49,7 +49,7 @@ public class TestController { @Timed @GetMapping("test3") public String test3() { - return "test2"; + return "test3"; } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java index 18df57e945..d2c1448c5a 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsAutoConfigurationTests.java @@ -71,8 +71,8 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. @ExtendWith(OutputCaptureExtension.class) class WebMvcMetricsAutoConfigurationTests { - private WebApplicationContextRunner contextRunner = new WebApplicationContextRunner().with(MetricsRun.simple()) - .withConfiguration(AutoConfigurations.of(WebMvcMetricsAutoConfiguration.class)); + private final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner() + .with(MetricsRun.simple()).withConfiguration(AutoConfigurations.of(WebMvcMetricsAutoConfiguration.class)); @Test void backsOffWhenMeterRegistryIsMissing() { @@ -183,11 +183,12 @@ class WebMvcMetricsAutoConfigurationTests { .contains(LongTaskTimingHandlerInterceptor.class)); } + private MeterRegistry getInitializedMeterRegistry(AssertableWebApplicationContext context) throws Exception { + return getInitializedMeterRegistry(context, "/test0", "/test1", "/test2"); + } + private MeterRegistry getInitializedMeterRegistry(AssertableWebApplicationContext context, String... urls) throws Exception { - if (urls.length == 0) { - urls = new String[] { "/test0", "/test1", "/test2" }; - } assertThat(context).hasSingleBean(FilterRegistrationBean.class); Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); assertThat(filter).isInstanceOf(WebMvcMetricsFilter.class); diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java index 6961091714..df3fc1f506 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.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. @@ -144,11 +144,11 @@ public class WebMvcMetricsFilter extends OncePerRequestFilter { Builder builder = this.autoTimer.builder(this.metricName); timerSample.stop(getTimer(builder, handler, request, response, exception)); } - return; - } - for (Timed annotation : annotations) { - Builder builder = Timer.builder(annotation, this.metricName); - timerSample.stop(getTimer(builder, handler, request, response, exception)); + } else { + for (Timed annotation : annotations) { + Builder builder = Timer.builder(annotation, this.metricName); + timerSample.stop(getTimer(builder, handler, request, response, exception)); + } } }