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 5486e15884..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,18 +16,26 @@ package org.springframework.boot.actuate.metrics.web.reactive.server; +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 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.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 +76,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)); } @@ -94,12 +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 = (handler instanceof HandlerMethod) + ? HandlerMethodTimedAnnotations.get((HandlerMethod) handler) : Collections.emptySet(); 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()) { + Builder builder = this.autoTimer.builder(this.metricName); + builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS); + } + } + else { + for (Timed annotation : annotations) { + Builder builder = Timer.builder(annotation, this.metricName); + builder.tags(tags).register(this.registry).record(duration, TimeUnit.NANOSECONDS); + } + } } } 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"); + } + + } + }