From 4ff4a9b082e117ea4ed3ebebb69461cfca2d0291 Mon Sep 17 00:00:00 2001 From: anshlykov Date: Wed, 26 Aug 2020 14:42:17 +0300 Subject: [PATCH 1/2] Support @Timed annotation for WebFlux Update `MetricsWebFilter` to support `@Timed` annotations on the WebFlux handler methods or beans. See gh-23112 --- .../web/reactive/server/MetricsWebFilter.java | 52 ++++++- .../MetricsWebFilterTimedAnnotationTests.java | 138 ++++++++++++++++++ 2 files changed, 185 insertions(+), 5 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java index 5486e15884..13f6b2cc9d 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java @@ -16,18 +16,27 @@ package org.springframework.boot.actuate.metrics.web.reactive.server; +import java.lang.reflect.AnnotatedElement; +import java.util.Collections; +import java.util.Set; import java.util.concurrent.TimeUnit; +import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Timer; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.core.Ordered; +import org.springframework.core.annotation.MergedAnnotationCollectors; +import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.Order; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; @@ -68,9 +77,6 @@ public class MetricsWebFilter implements WebFilter { @Override public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { - if (!this.autoTimer.isEnabled()) { - return chain.filter(exchange); - } return chain.filter(exchange).transformDeferred((call) -> filter(exchange, call)); } @@ -97,9 +103,45 @@ public class MetricsWebFilter implements WebFilter { if (cause == null) { cause = exchange.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE); } + Object handler = exchange.getAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE); + Set annotations = getTimedAnnotations(handler); Iterable tags = this.tagsProvider.httpRequestTags(exchange, cause); - this.autoTimer.builder(this.metricName).tags(tags).register(this.registry).record(System.nanoTime() - start, - TimeUnit.NANOSECONDS); + long duration = System.nanoTime() - start; + if (annotations.isEmpty()) { + if (this.autoTimer.isEnabled()) { + this.autoTimer.builder(this.metricName).tags(tags).register(this.registry).record(duration, + TimeUnit.NANOSECONDS); + } + } + else { + for (Timed annotation : annotations) { + Timer.builder(annotation, this.metricName).tags(tags).register(this.registry).record(duration, + TimeUnit.NANOSECONDS); + } + } + } + + private Set getTimedAnnotations(Object handler) { + if (!(handler instanceof HandlerMethod)) { + return Collections.emptySet(); + } + return getTimedAnnotations((HandlerMethod) handler); + } + + private Set getTimedAnnotations(HandlerMethod handler) { + Set methodAnnotations = findTimedAnnotations(handler.getMethod()); + if (!methodAnnotations.isEmpty()) { + return methodAnnotations; + } + return findTimedAnnotations(handler.getBeanType()); + } + + private Set findTimedAnnotations(AnnotatedElement element) { + MergedAnnotations annotations = MergedAnnotations.from(element); + if (!annotations.isPresent(Timed.class)) { + return Collections.emptySet(); + } + return annotations.stream(Timed.class).collect(MergedAnnotationCollectors.toAnnotationSet()); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java new file mode 100644 index 0000000000..cbb2ac07cc --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java @@ -0,0 +1,138 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.metrics.web.reactive.server; + +import java.time.Duration; + +import io.micrometer.core.annotation.Timed; +import io.micrometer.core.instrument.MockClock; +import io.micrometer.core.instrument.simple.SimpleConfig; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; + +import org.springframework.boot.actuate.metrics.AutoTimer; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.util.ReflectionUtils; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.reactive.HandlerMapping; +import org.springframework.web.util.pattern.PathPatternParser; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Unit tests for {@link MetricsWebFilter} for handlers with {@link Timed} annotation. + * + * @author Andrey Shlykov + */ +class MetricsWebFilterTimedAnnotationTests { + + private static final String REQUEST_METRICS_NAME = "http.server.requests"; + + private static final String REQUEST_METRICS_NAME_PERCENTILE = REQUEST_METRICS_NAME + ".percentile"; + + private SimpleMeterRegistry registry; + + private MetricsWebFilter webFilter; + + @BeforeEach + void setup() { + MockClock clock = new MockClock(); + this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); + this.webFilter = new MetricsWebFilter(this.registry, new DefaultWebFluxTagsProvider(true), REQUEST_METRICS_NAME, + AutoTimer.ENABLED); + } + + @Test + void filterAddsStandardTags() { + MockServerWebExchange exchange = createExchange("timedHandler"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + } + + @Test + void filterAddsExtraTags() { + MockServerWebExchange exchange = createExchange("timedExtraTagsHandler"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + assertMetricsContainsTag("tag1", "value1"); + assertMetricsContainsTag("tag2", "value2"); + } + + @Test + void filterAddsExtraTagsAndException() { + MockServerWebExchange exchange = createExchange("timedExtraTagsHandler"); + this.webFilter.filter(exchange, (serverWebExchange) -> Mono.error(new IllegalStateException("test error"))) + .onErrorResume((t) -> { + exchange.getResponse().setRawStatusCode(500); + return exchange.getResponse().setComplete(); + }).block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "500"); + assertMetricsContainsTag("exception", "IllegalStateException"); + assertMetricsContainsTag("tag1", "value1"); + assertMetricsContainsTag("tag2", "value2"); + } + + @Test + void filterAddsPercentileMeters() { + MockServerWebExchange exchange = createExchange("timedPercentilesHandler"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.95").gauge().value()).isNotZero(); + assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.5").gauge().value()).isNotZero(); + } + + private MockServerWebExchange createExchange(String handlerName) { + PathPatternParser parser = new PathPatternParser(); + HandlerMethod handlerMethod = new HandlerMethod(this, ReflectionUtils.findMethod(this.getClass(), handlerName)); + MockServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/projects/spring-boot").build()); + exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, + parser.parse("/projects/{project}")); + exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE, handlerMethod); + return exchange; + } + + private void assertMetricsContainsTag(String tagKey, String tagValue) { + assertThat(this.registry.get(REQUEST_METRICS_NAME).tag(tagKey, tagValue).timer().count()).isEqualTo(1); + } + + @Timed + Mono timedHandler() { + return Mono.just("test"); + } + + @Timed(extraTags = { "tag1", "value1", "tag2", "value2" }) + Mono timedExtraTagsHandler() { + return Mono.just("test"); + } + + @Timed(percentiles = { 0.5, 0.95 }) + Mono timedPercentilesHandler() { + return Mono.just("test"); + } + +} From 15fe41868e66209b898a3933866d22d8432267f8 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 8 Apr 2021 22:45:34 -0700 Subject: [PATCH 2/2] Polish 'Support @Timed annotation for WebFlux' Extract common method logic to a `HandlerMethodTimedAnnotations` utility class and also add some caching. Also move the test code into a single class. See gh-23112 --- .../method/HandlerMethodTimedAnnotations.java | 64 ++++++++ .../web/reactive/server/MetricsWebFilter.java | 43 ++---- .../web/servlet/WebMvcMetricsFilter.java | 30 +--- .../HandlerMethodTimedAnnotationsTests.java | 87 +++++++++++ .../server/MetricsWebFilterTests.java | 77 ++++++++++ .../MetricsWebFilterTimedAnnotationTests.java | 138 ------------------ 6 files changed, 240 insertions(+), 199 deletions(-) create mode 100644 spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotations.java create mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotationsTests.java delete mode 100644 spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotations.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotations.java new file mode 100644 index 0000000000..09bb1e4b01 --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotations.java @@ -0,0 +1,64 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.metrics.web.method; + +import java.lang.reflect.AnnotatedElement; +import java.util.Collections; +import java.util.Map; +import java.util.Set; + +import io.micrometer.core.annotation.Timed; + +import org.springframework.core.annotation.MergedAnnotationCollectors; +import org.springframework.core.annotation.MergedAnnotations; +import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.web.method.HandlerMethod; + +/** + * Utility used to obtain {@link Timed @Timed} annotations from a {@link HandlerMethod}. + * + * @author Phillip Webb + * @since 2.5.0 + */ +public final class HandlerMethodTimedAnnotations { + + private static Map> cache = new ConcurrentReferenceHashMap<>(); + + private HandlerMethodTimedAnnotations() { + } + + public static Set get(HandlerMethod handler) { + Set methodAnnotations = findTimedAnnotations(handler.getMethod()); + if (!methodAnnotations.isEmpty()) { + return methodAnnotations; + } + return findTimedAnnotations(handler.getBeanType()); + } + + private static Set findTimedAnnotations(AnnotatedElement element) { + Set result = cache.get(element); + if (result != null) { + return result; + } + MergedAnnotations annotations = MergedAnnotations.from(element); + result = (!annotations.isPresent(Timed.class)) ? Collections.emptySet() + : annotations.stream(Timed.class).collect(MergedAnnotationCollectors.toAnnotationSet()); + cache.put(element, result); + return result; + } + +} diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java index 13f6b2cc9d..47f21a4db0 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilter.java @@ -16,7 +16,6 @@ package org.springframework.boot.actuate.metrics.web.reactive.server; -import java.lang.reflect.AnnotatedElement; import java.util.Collections; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -25,14 +24,14 @@ import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.Timer.Builder; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; import org.springframework.boot.actuate.metrics.AutoTimer; +import org.springframework.boot.actuate.metrics.web.method.HandlerMethodTimedAnnotations; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.core.Ordered; -import org.springframework.core.annotation.MergedAnnotationCollectors; -import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.Order; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.web.method.HandlerMethod; @@ -100,48 +99,24 @@ public class MetricsWebFilter implements WebFilter { } private void record(ServerWebExchange exchange, Throwable cause, long start) { - if (cause == null) { - cause = exchange.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE); - } + cause = (cause != null) ? cause : exchange.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE); Object handler = exchange.getAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE); - Set annotations = getTimedAnnotations(handler); + Set annotations = (handler instanceof HandlerMethod) + ? HandlerMethodTimedAnnotations.get((HandlerMethod) handler) : Collections.emptySet(); Iterable tags = this.tagsProvider.httpRequestTags(exchange, cause); long duration = System.nanoTime() - start; if (annotations.isEmpty()) { if (this.autoTimer.isEnabled()) { - this.autoTimer.builder(this.metricName).tags(tags).register(this.registry).record(duration, - TimeUnit.NANOSECONDS); + Builder builder = this.autoTimer.builder(this.metricName); + builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS); } } else { for (Timed annotation : annotations) { - Timer.builder(annotation, this.metricName).tags(tags).register(this.registry).record(duration, - TimeUnit.NANOSECONDS); + Builder builder = Timer.builder(annotation, this.metricName); + builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS); } } } - private Set getTimedAnnotations(Object handler) { - if (!(handler instanceof HandlerMethod)) { - return Collections.emptySet(); - } - return getTimedAnnotations((HandlerMethod) handler); - } - - private Set getTimedAnnotations(HandlerMethod handler) { - Set methodAnnotations = findTimedAnnotations(handler.getMethod()); - if (!methodAnnotations.isEmpty()) { - return methodAnnotations; - } - return findTimedAnnotations(handler.getBeanType()); - } - - private Set findTimedAnnotations(AnnotatedElement element) { - MergedAnnotations annotations = MergedAnnotations.from(element); - if (!annotations.isPresent(Timed.class)) { - return Collections.emptySet(); - } - return annotations.stream(Timed.class).collect(MergedAnnotationCollectors.toAnnotationSet()); - } - } 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 d8d3d06085..c95ace093c 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 @@ -17,7 +17,6 @@ package org.springframework.boot.actuate.metrics.web.servlet; import java.io.IOException; -import java.lang.reflect.AnnotatedElement; import java.util.Collections; import java.util.Set; @@ -33,9 +32,8 @@ import io.micrometer.core.instrument.Timer.Builder; import io.micrometer.core.instrument.Timer.Sample; import org.springframework.boot.actuate.metrics.AutoTimer; +import org.springframework.boot.actuate.metrics.web.method.HandlerMethodTimedAnnotations; import org.springframework.boot.web.servlet.error.ErrorAttributes; -import org.springframework.core.annotation.MergedAnnotationCollectors; -import org.springframework.core.annotation.MergedAnnotations; import org.springframework.http.HttpStatus; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.method.HandlerMethod; @@ -130,7 +128,8 @@ public class WebMvcMetricsFilter extends OncePerRequestFilter { private void record(TimingContext timingContext, HttpServletRequest request, HttpServletResponse response, Throwable exception) { Object handler = getHandler(request); - Set annotations = getTimedAnnotations(handler); + Set annotations = (handler instanceof HandlerMethod) + ? HandlerMethodTimedAnnotations.get((HandlerMethod) handler) : Collections.emptySet(); Timer.Sample timerSample = timingContext.getTimerSample(); if (annotations.isEmpty()) { if (this.autoTimer.isEnabled()) { @@ -150,29 +149,6 @@ public class WebMvcMetricsFilter extends OncePerRequestFilter { return request.getAttribute(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE); } - private Set getTimedAnnotations(Object handler) { - if (!(handler instanceof HandlerMethod)) { - return Collections.emptySet(); - } - return getTimedAnnotations((HandlerMethod) handler); - } - - private Set getTimedAnnotations(HandlerMethod handler) { - Set methodAnnotations = findTimedAnnotations(handler.getMethod()); - if (!methodAnnotations.isEmpty()) { - return methodAnnotations; - } - return findTimedAnnotations(handler.getBeanType()); - } - - private Set findTimedAnnotations(AnnotatedElement element) { - MergedAnnotations annotations = MergedAnnotations.from(element); - if (!annotations.isPresent(Timed.class)) { - return Collections.emptySet(); - } - return annotations.stream(Timed.class).collect(MergedAnnotationCollectors.toAnnotationSet()); - } - private Timer getTimer(Builder builder, Object handler, HttpServletRequest request, HttpServletResponse response, Throwable exception) { return builder.tags(this.tagsProvider.getTags(request, response, handler, exception)).register(this.registry); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotationsTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotationsTests.java new file mode 100644 index 0000000000..9297ad231a --- /dev/null +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/method/HandlerMethodTimedAnnotationsTests.java @@ -0,0 +1,87 @@ +/* + * Copyright 2012-2021 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.metrics.web.method; + +import java.lang.reflect.Method; +import java.util.Set; + +import io.micrometer.core.annotation.Timed; +import org.junit.jupiter.api.Test; + +import org.springframework.util.ReflectionUtils; +import org.springframework.web.method.HandlerMethod; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link HandlerMethodTimedAnnotations}. + * + * @author Phillip Webb + */ +class HandlerMethodTimedAnnotationsTests { + + @Test + void getWhenNoneReturnsEmptySet() { + Object bean = new None(); + Method method = ReflectionUtils.findMethod(bean.getClass(), "handle"); + Set annotations = HandlerMethodTimedAnnotations.get(new HandlerMethod(bean, method)); + assertThat(annotations).isEmpty(); + } + + @Test + void getWhenOnMethodReturnsMethodAnnotations() { + Object bean = new OnMethod(); + Method method = ReflectionUtils.findMethod(bean.getClass(), "handle"); + Set annotations = HandlerMethodTimedAnnotations.get(new HandlerMethod(bean, method)); + assertThat(annotations).extracting(Timed::value).containsOnly("y", "z"); + } + + @Test + void getWhenNonOnMethodReturnsBeanAnnotations() { + Object bean = new OnBean(); + Method method = ReflectionUtils.findMethod(bean.getClass(), "handle"); + Set annotations = HandlerMethodTimedAnnotations.get(new HandlerMethod(bean, method)); + assertThat(annotations).extracting(Timed::value).containsOnly("y", "z"); + } + + static class None { + + void handle() { + } + + } + + @Timed("x") + static class OnMethod { + + @Timed("y") + @Timed("z") + void handle() { + } + + } + + @Timed("y") + @Timed("z") + static class OnBean { + + void handle() { + } + + } + +} diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java index 56341e5a87..1dc4819176 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTests.java @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.metrics.web.reactive.server; import java.io.EOFException; import java.time.Duration; +import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; @@ -31,6 +32,8 @@ import org.springframework.boot.actuate.metrics.AutoTimer; import org.springframework.boot.web.reactive.error.ErrorAttributes; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.util.ReflectionUtils; +import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.util.pattern.PathPatternParser; @@ -46,6 +49,8 @@ class MetricsWebFilterTests { private static final String REQUEST_METRICS_NAME = "http.server.requests"; + private static final String REQUEST_METRICS_NAME_PERCENTILE = REQUEST_METRICS_NAME + ".percentile"; + private SimpleMeterRegistry registry; private MetricsWebFilter webFilter; @@ -157,6 +162,59 @@ class MetricsWebFilterTests { assertMetricsContainsTag("outcome", "UNKNOWN"); } + @Test + void filterAddsStandardTags() { + MockServerWebExchange exchange = createTimedHandlerMethodExchange("timed"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + } + + @Test + void filterAddsExtraTags() { + MockServerWebExchange exchange = createTimedHandlerMethodExchange("timedExtraTags"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + assertMetricsContainsTag("tag1", "value1"); + assertMetricsContainsTag("tag2", "value2"); + } + + @Test + void filterAddsExtraTagsAndException() { + MockServerWebExchange exchange = createTimedHandlerMethodExchange("timedExtraTags"); + this.webFilter.filter(exchange, (serverWebExchange) -> Mono.error(new IllegalStateException("test error"))) + .onErrorResume((ex) -> { + exchange.getResponse().setRawStatusCode(500); + return exchange.getResponse().setComplete(); + }).block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "500"); + assertMetricsContainsTag("exception", "IllegalStateException"); + assertMetricsContainsTag("tag1", "value1"); + assertMetricsContainsTag("tag2", "value2"); + } + + @Test + void filterAddsPercentileMeters() { + MockServerWebExchange exchange = createTimedHandlerMethodExchange("timedPercentiles"); + this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) + .block(Duration.ofSeconds(30)); + assertMetricsContainsTag("uri", "/projects/{project}"); + assertMetricsContainsTag("status", "200"); + assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.95").gauge().value()).isNotZero(); + assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.5").gauge().value()).isNotZero(); + } + + private MockServerWebExchange createTimedHandlerMethodExchange(String methodName) { + MockServerWebExchange exchange = createExchange("/projects/spring-boot", "/projects/{project}"); + exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE, + new HandlerMethod(this, ReflectionUtils.findMethod(Handlers.class, methodName))); + return exchange; + } + private MockServerWebExchange createExchange(String path, String pathPattern) { PathPatternParser parser = new PathPatternParser(); MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(path).build()); @@ -168,4 +226,23 @@ class MetricsWebFilterTests { assertThat(this.registry.get(REQUEST_METRICS_NAME).tag(tagKey, tagValue).timer().count()).isEqualTo(1); } + static class Handlers { + + @Timed + Mono timed() { + return Mono.just("test"); + } + + @Timed(extraTags = { "tag1", "value1", "tag2", "value2" }) + Mono timedExtraTags() { + return Mono.just("test"); + } + + @Timed(percentiles = { 0.5, 0.95 }) + Mono timedPercentiles() { + return Mono.just("test"); + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java deleted file mode 100644 index cbb2ac07cc..0000000000 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/reactive/server/MetricsWebFilterTimedAnnotationTests.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.actuate.metrics.web.reactive.server; - -import java.time.Duration; - -import io.micrometer.core.annotation.Timed; -import io.micrometer.core.instrument.MockClock; -import io.micrometer.core.instrument.simple.SimpleConfig; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import reactor.core.publisher.Mono; - -import org.springframework.boot.actuate.metrics.AutoTimer; -import org.springframework.mock.http.server.reactive.MockServerHttpRequest; -import org.springframework.mock.web.server.MockServerWebExchange; -import org.springframework.util.ReflectionUtils; -import org.springframework.web.method.HandlerMethod; -import org.springframework.web.reactive.HandlerMapping; -import org.springframework.web.util.pattern.PathPatternParser; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * Unit tests for {@link MetricsWebFilter} for handlers with {@link Timed} annotation. - * - * @author Andrey Shlykov - */ -class MetricsWebFilterTimedAnnotationTests { - - private static final String REQUEST_METRICS_NAME = "http.server.requests"; - - private static final String REQUEST_METRICS_NAME_PERCENTILE = REQUEST_METRICS_NAME + ".percentile"; - - private SimpleMeterRegistry registry; - - private MetricsWebFilter webFilter; - - @BeforeEach - void setup() { - MockClock clock = new MockClock(); - this.registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); - this.webFilter = new MetricsWebFilter(this.registry, new DefaultWebFluxTagsProvider(true), REQUEST_METRICS_NAME, - AutoTimer.ENABLED); - } - - @Test - void filterAddsStandardTags() { - MockServerWebExchange exchange = createExchange("timedHandler"); - this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) - .block(Duration.ofSeconds(30)); - assertMetricsContainsTag("uri", "/projects/{project}"); - assertMetricsContainsTag("status", "200"); - } - - @Test - void filterAddsExtraTags() { - MockServerWebExchange exchange = createExchange("timedExtraTagsHandler"); - this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) - .block(Duration.ofSeconds(30)); - assertMetricsContainsTag("uri", "/projects/{project}"); - assertMetricsContainsTag("status", "200"); - assertMetricsContainsTag("tag1", "value1"); - assertMetricsContainsTag("tag2", "value2"); - } - - @Test - void filterAddsExtraTagsAndException() { - MockServerWebExchange exchange = createExchange("timedExtraTagsHandler"); - this.webFilter.filter(exchange, (serverWebExchange) -> Mono.error(new IllegalStateException("test error"))) - .onErrorResume((t) -> { - exchange.getResponse().setRawStatusCode(500); - return exchange.getResponse().setComplete(); - }).block(Duration.ofSeconds(30)); - assertMetricsContainsTag("uri", "/projects/{project}"); - assertMetricsContainsTag("status", "500"); - assertMetricsContainsTag("exception", "IllegalStateException"); - assertMetricsContainsTag("tag1", "value1"); - assertMetricsContainsTag("tag2", "value2"); - } - - @Test - void filterAddsPercentileMeters() { - MockServerWebExchange exchange = createExchange("timedPercentilesHandler"); - this.webFilter.filter(exchange, (serverWebExchange) -> exchange.getResponse().setComplete()) - .block(Duration.ofSeconds(30)); - assertMetricsContainsTag("uri", "/projects/{project}"); - assertMetricsContainsTag("status", "200"); - assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.95").gauge().value()).isNotZero(); - assertThat(this.registry.get(REQUEST_METRICS_NAME_PERCENTILE).tag("phi", "0.5").gauge().value()).isNotZero(); - } - - private MockServerWebExchange createExchange(String handlerName) { - PathPatternParser parser = new PathPatternParser(); - HandlerMethod handlerMethod = new HandlerMethod(this, ReflectionUtils.findMethod(this.getClass(), handlerName)); - MockServerWebExchange exchange = MockServerWebExchange - .from(MockServerHttpRequest.get("/projects/spring-boot").build()); - exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, - parser.parse("/projects/{project}")); - exchange.getAttributes().put(HandlerMapping.BEST_MATCHING_HANDLER_ATTRIBUTE, handlerMethod); - return exchange; - } - - private void assertMetricsContainsTag(String tagKey, String tagValue) { - assertThat(this.registry.get(REQUEST_METRICS_NAME).tag(tagKey, tagValue).timer().count()).isEqualTo(1); - } - - @Timed - Mono timedHandler() { - return Mono.just("test"); - } - - @Timed(extraTags = { "tag1", "value1", "tag2", "value2" }) - Mono timedExtraTagsHandler() { - return Mono.just("test"); - } - - @Timed(percentiles = { 0.5, 0.95 }) - Mono timedPercentilesHandler() { - return Mono.just("test"); - } - -}