From cf17e5708bcc25d8c76829c84b667682a1c77460 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 3 Nov 2017 21:08:04 -0700 Subject: [PATCH] Polish Micrometer 1.0.0-rc.3 upgrade Polish Micrometer 1.0.0-rc.3 upgrade changes. Closes gh-10906 --- .../metrics/MeterBindersConfiguration.java | 1 + .../export/StepRegistryProperties.java | 4 +- .../StepRegistryPropertiesConfigAdapter.java | 2 +- .../atlas/AtlasPropertiesConfigAdapter.java | 2 +- .../export/datadog/DatadogProperties.java | 4 +- .../DatadogPropertiesConfigAdapter.java | 1 + .../export/ganglia/GangliaProperties.java | 6 +- .../GangliaPropertiesConfigAdapter.java | 2 +- .../export/graphite/GraphiteProperties.java | 8 +- .../GraphitePropertiesConfigAdapter.java | 2 +- .../export/influx/InfluxProperties.java | 2 + .../influx/InfluxPropertiesConfigAdapter.java | 3 +- .../prometheus/PrometheusProperties.java | 9 +- .../PrometheusPropertiesConfigAdapter.java | 1 + .../simple/SimpleExportConfiguration.java | 1 + .../export/simple/SimpleProperties.java | 8 +- .../simple/SimplePropertiesConfigAdapter.java | 3 +- .../statsd/StatsdExportConfiguration.java | 4 +- .../export/statsd/StatsdProperties.java | 4 +- .../statsd/StatsdPropertiesConfigAdapter.java | 2 +- .../servlet/WebMvcMetricsConfiguration.java | 8 +- ...ricsAutoConfigurationIntegrationTests.java | 6 +- .../boot/actuate/metrics/TimedUtils.java | 50 ------ .../integration/SpringIntegrationMetrics.java | 151 +++++++++--------- .../servlet/DefaultWebMvcTagsProvider.java | 7 +- .../metrics/web/servlet/WebMvcMetrics.java | 45 ++++-- ...csFilter.java => WebMvcMetricsFilter.java} | 68 ++++---- .../metrics/web/servlet/WebMvcTags.java | 13 +- .../actuate/metrics/MetricsEndpointTests.java | 10 +- .../MetricsEndpointWebIntegrationTests.java | 22 +-- .../MetricsRestTemplateCustomizerTests.java | 13 +- ...=> WebMvcMetricsFilterAutoTimedTests.java} | 105 ++++++------ ...sts.java => WebMvcMetricsFilterTests.java} | 112 ++++++------- .../WebMvcMetricsIntegrationTests.java | 38 +++-- .../spring-boot-dependencies/pom.xml | 8 +- 35 files changed, 372 insertions(+), 353 deletions(-) delete mode 100644 spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/TimedUtils.java rename spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/{MetricsFilter.java => WebMvcMetricsFilter.java} (58%) rename spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/{MetricsFilterAutoTimedTests.java => WebMvcMetricsFilterAutoTimedTests.java} (60%) rename spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/{MetricsFilterTests.java => WebMvcMetricsFilterTests.java} (82%) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterBindersConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterBindersConfiguration.java index 95842d4b79..3801f57cf4 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterBindersConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MeterBindersConfiguration.java @@ -59,4 +59,5 @@ class MeterBindersConfiguration { public ProcessorMetrics processorMetrics() { return new ProcessorMetrics(); } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryProperties.java index b3b83a7be9..41152a7dd5 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryProperties.java @@ -36,7 +36,7 @@ public abstract class StepRegistryProperties { /** * Enable publishing to the backend. */ - private Boolean enabled = true; + private Boolean enabled; /** * The connection timeout for requests to the backend. @@ -67,7 +67,7 @@ public abstract class StepRegistryProperties { this.step = step; } - public Boolean isEnabled() { + public Boolean getEnabled() { return this.enabled; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryPropertiesConfigAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryPropertiesConfigAdapter.java index aede43227b..2a9e56a14d 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryPropertiesConfigAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/StepRegistryPropertiesConfigAdapter.java @@ -53,7 +53,7 @@ public abstract class StepRegistryPropertiesConfigAdapter new StatsdMeterRegistry(statsdConfig, hierarchicalNameMapper, clock); } @@ -63,4 +64,5 @@ public class StatsdExportConfiguration { public HierarchicalNameMapper hierarchicalNameMapper() { return HierarchicalNameMapper.DEFAULT; } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdProperties.java index 949b79e7f0..a4f773f0d0 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdProperties.java @@ -34,7 +34,7 @@ public class StatsdProperties { /** * Enable publishing to the backend. */ - private Boolean enabled = true; + private Boolean enabled; /** * Variant of the StatsD line protocol to use. @@ -68,7 +68,7 @@ public class StatsdProperties { */ private Integer queueSize = Integer.MAX_VALUE; - public Boolean isEnabled() { + public Boolean getEnabled() { return this.enabled; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdPropertiesConfigAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdPropertiesConfigAdapter.java index 4014a9be7a..840be63de5 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdPropertiesConfigAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/statsd/StatsdPropertiesConfigAdapter.java @@ -50,7 +50,7 @@ public class StatsdPropertiesConfigAdapter extends @Override public boolean enabled() { - return get(StatsdProperties::isEnabled, StatsdConfig::enabled); + return get(StatsdProperties::getEnabled, StatsdConfig::enabled); } @Override diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsConfiguration.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsConfiguration.java index 3928ff7f06..050085057f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsConfiguration.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/web/servlet/WebMvcMetricsConfiguration.java @@ -20,8 +20,8 @@ import io.micrometer.core.instrument.MeterRegistry; import org.springframework.boot.actuate.autoconfigure.metrics.MetricsProperties; import org.springframework.boot.actuate.metrics.web.servlet.DefaultWebMvcTagsProvider; -import org.springframework.boot.actuate.metrics.web.servlet.MetricsFilter; import org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetrics; +import org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter; import org.springframework.boot.actuate.metrics.web.servlet.WebMvcTagsProvider; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; @@ -60,7 +60,9 @@ public class WebMvcMetricsConfiguration { } @Bean - public MetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, HandlerMappingIntrospector introspector) { - return new MetricsFilter(controllerMetrics, introspector); + public WebMvcMetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, + HandlerMappingIntrospector introspector) { + return new WebMvcMetricsFilter(controllerMetrics, introspector); } + } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsAutoConfigurationIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsAutoConfigurationIntegrationTests.java index be2a4327df..e324827002 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsAutoConfigurationIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsAutoConfigurationIntegrationTests.java @@ -21,10 +21,12 @@ import java.util.Map; import java.util.Set; import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.MockClock; import io.micrometer.core.instrument.Statistic; import io.micrometer.core.instrument.binder.MeterBinder; import io.micrometer.core.instrument.binder.jvm.JvmMemoryMetrics; import io.micrometer.core.instrument.binder.logging.LogbackMetrics; +import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.Test; import org.junit.runner.RunWith; @@ -90,6 +92,7 @@ public class MetricsAutoConfigurationIntegrationTests { "{\"message\": \"hello\"}", MediaType.APPLICATION_JSON)); assertThat(this.external.getForObject("/api/external", Map.class)) .containsKey("message"); + MockClock.clock(this.registry).add(SimpleConfig.DEFAULT_STEP); assertThat(this.registry.find("http.client.requests").value(Statistic.Count, 1.0) .timer()).isPresent(); } @@ -97,6 +100,7 @@ public class MetricsAutoConfigurationIntegrationTests { @Test public void requestMappingIsInstrumented() { this.loopback.getForObject("/api/people", Set.class); + MockClock.clock(this.registry).add(SimpleConfig.DEFAULT_STEP); assertThat(this.registry.find("http.server.requests").value(Statistic.Count, 1.0) .timer()).isPresent(); } @@ -118,7 +122,7 @@ public class MetricsAutoConfigurationIntegrationTests { @Bean public MeterRegistry registry() { - return new SimpleMeterRegistry(); + return new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); } @Bean diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/TimedUtils.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/TimedUtils.java deleted file mode 100644 index b54ff2f235..0000000000 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/TimedUtils.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright 2012-2017 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 - * - * http://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; - -import java.lang.reflect.AnnotatedElement; -import java.util.Arrays; -import java.util.stream.Stream; - -import io.micrometer.core.annotation.Timed; -import io.micrometer.core.annotation.TimedSet; - -import org.springframework.core.annotation.AnnotationUtils; - -/** - * A utility class for finding {@link Timed} annotations. - * - * @author Jon Schneider - * @since 2.0.0 - */ -public final class TimedUtils { - private TimedUtils() { - } - - public static Stream findTimedAnnotations(AnnotatedElement element) { - Timed t = AnnotationUtils.findAnnotation(element, Timed.class); - if (t != null) - return Stream.of(t); - - TimedSet ts = AnnotationUtils.findAnnotation(element, TimedSet.class); - if (ts != null) { - return Arrays.stream(ts.value()); - } - - return Stream.empty(); - } -} diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/integration/SpringIntegrationMetrics.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/integration/SpringIntegrationMetrics.java index bafb3462ce..a07a8b0c03 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/integration/SpringIntegrationMetrics.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/integration/SpringIntegrationMetrics.java @@ -20,12 +20,22 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.concurrent.TimeUnit; +import java.util.function.ToDoubleFunction; + +import io.micrometer.core.instrument.FunctionCounter; +import io.micrometer.core.instrument.Gauge; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.TimeGauge; +import io.micrometer.core.instrument.binder.MeterBinder; import org.springframework.beans.factory.SmartInitializingSingleton; -import org.springframework.integration.support.management.*; - -import io.micrometer.core.instrument.*; -import io.micrometer.core.instrument.binder.MeterBinder; +import org.springframework.integration.support.management.IntegrationManagementConfigurer; +import org.springframework.integration.support.management.MessageChannelMetrics; +import org.springframework.integration.support.management.MessageHandlerMetrics; +import org.springframework.integration.support.management.MessageSourceMetrics; +import org.springframework.integration.support.management.PollableChannelManagement; /** * A {@link MeterBinder} for Spring Integration metrics. @@ -53,21 +63,18 @@ public class SpringIntegrationMetrics implements MeterBinder, SmartInitializingS @Override public void bindTo(MeterRegistry registry) { - Gauge.builder("spring.integration.channelNames", this.configurer, - c -> c.getChannelNames().length).tags(tags) - .description("The number of spring integration channels") - .register(registry); - - Gauge.builder("spring.integration.handlerNames", configurer, - c -> c.getHandlerNames().length).tags(this.tags) - .description("The number of spring integration handlers") - .register(registry); - - Gauge.builder("spring.integration.sourceNames", configurer, - c -> c.getSourceNames().length).tags(this.tags) - .description("The number of spring integration sources") - .register(registry); - + registerGuage(registry, this.configurer, this.tags, + "spring.integration.channelNames", + "The number of spring integration channels", + (configurer) -> configurer.getChannelNames().length); + registerGuage(registry, this.configurer, this.tags, + "spring.integration.handlerNames", + "The number of spring integration handlers", + (configurer) -> configurer.getHandlerNames().length); + registerGuage(registry, this.configurer, this.tags, + "spring.integration.sourceNames", + "The number of spring integration sources", + (configurer) -> configurer.getSourceNames().length); this.registries.add(registry); } @@ -75,13 +82,10 @@ public class SpringIntegrationMetrics implements MeterBinder, SmartInitializingS for (String source : this.configurer.getSourceNames()) { MessageSourceMetrics sourceMetrics = this.configurer.getSourceMetrics(source); Iterable tagsWithSource = Tags.concat(this.tags, "source", source); - - FunctionCounter - .builder("spring.integration.source.messages", sourceMetrics, - MessageSourceMetrics::getMessageCount) - .tags(tagsWithSource) - .description("The number of successful handler calls") - .register(registry); + registerFunctionCounter(registry, sourceMetrics, tagsWithSource, + "spring.integration.source.messages", + "The number of successful handler calls", + MessageSourceMetrics::getMessageCount); } } @@ -89,33 +93,22 @@ public class SpringIntegrationMetrics implements MeterBinder, SmartInitializingS for (String handler : this.configurer.getHandlerNames()) { MessageHandlerMetrics handlerMetrics = this.configurer .getHandlerMetrics(handler); - - // TODO could use improvement to dynamically commute the handler name with its - // ID, which can change after creation as shown in the - // SpringIntegrationApplication sample. Iterable tagsWithHandler = Tags.concat(this.tags, "handler", handler); - - TimeGauge - .builder("spring.integration.handler.duration.max", handlerMetrics, - TimeUnit.MILLISECONDS, MessageHandlerMetrics::getMaxDuration) - .tags(tagsWithHandler).description("The maximum handler duration") - .register(registry); - - TimeGauge - .builder("spring.integration.handler.duration.min", handlerMetrics, - TimeUnit.MILLISECONDS, MessageHandlerMetrics::getMinDuration) - .tags(tagsWithHandler).description("The minimum handler duration") - .register(registry); - - TimeGauge - .builder("spring.integration.handler.duration.mean", handlerMetrics, - TimeUnit.MILLISECONDS, MessageHandlerMetrics::getMeanDuration) - .tags(tagsWithHandler).description("The mean handler duration") - .register(registry); - - Gauge.builder("spring.integration.handler.activeCount", handlerMetrics, - MessageHandlerMetrics::getActiveCount).tags(tagsWithHandler) - .description("The number of active handlers").register(registry); + registerTimedGauge(registry, handlerMetrics, tagsWithHandler, + "spring.integration.handler.duration.max", + "The maximum handler duration", + MessageHandlerMetrics::getMaxDuration); + registerTimedGauge(registry, handlerMetrics, tagsWithHandler, + "spring.integration.handler.duration.min", + "The minimum handler duration", + MessageHandlerMetrics::getMinDuration); + registerTimedGauge(registry, handlerMetrics, tagsWithHandler, + "spring.integration.handler.duration.mean", + "The mean handler duration", MessageHandlerMetrics::getMeanDuration); + registerGuage(registry, handlerMetrics, tagsWithHandler, + "spring.integration.handler.activeCount", + "The number of active handlers", + MessageHandlerMetrics::getActiveCount); } } @@ -124,33 +117,43 @@ public class SpringIntegrationMetrics implements MeterBinder, SmartInitializingS MessageChannelMetrics channelMetrics = this.configurer .getChannelMetrics(channel); Iterable tagsWithChannel = Tags.concat(this.tags, "channel", channel); - - FunctionCounter - .builder("spring.integration.channel.sendErrors", channelMetrics, - MessageChannelMetrics::getSendErrorCount) - .tags(tagsWithChannel) - .description( - "The number of failed sends (either throwing an exception or rejected by the channel)") - .register(registry); - - FunctionCounter - .builder("spring.integration.channel.sends", channelMetrics, - MessageChannelMetrics::getSendCount) - .tags(tagsWithChannel).description("The number of successful sends") - .register(registry); - + registerFunctionCounter(registry, channelMetrics, tagsWithChannel, + "spring.integration.channel.sendErrors", + "The number of failed sends (either throwing an exception or rejected by the channel)", + MessageChannelMetrics::getSendErrorCount); + registerFunctionCounter(registry, channelMetrics, tagsWithChannel, + "spring.integration.channel.sends", "The number of successful sends", + MessageChannelMetrics::getSendCount); if (channelMetrics instanceof PollableChannelManagement) { - FunctionCounter - .builder("spring.integration.receives", - (PollableChannelManagement) channelMetrics, - PollableChannelManagement::getReceiveCount) - .tags(tagsWithChannel) - .description("The number of messages received") - .register(registry); + registerFunctionCounter(registry, + (PollableChannelManagement) channelMetrics, tagsWithChannel, + "spring.integration.receives", "The number of messages received", + PollableChannelManagement::getReceiveCount); } } } + private void registerGuage(MeterRegistry registry, T object, Iterable tags, + String name, String description, ToDoubleFunction value) { + Gauge.Builder builder = Gauge.builder(name, object, value); + builder.tags(this.tags).description(description).register(registry); + } + + private void registerTimedGauge(MeterRegistry registry, T object, + Iterable tags, String name, String description, + ToDoubleFunction value) { + TimeGauge.Builder builder = TimeGauge.builder(name, object, + TimeUnit.MILLISECONDS, value); + builder.tags(tags).description(description).register(registry); + } + + private void registerFunctionCounter(MeterRegistry registry, T object, + Iterable tags, String name, String description, + ToDoubleFunction value) { + FunctionCounter.builder(name, object, value).tags(tags) + .description(description).register(registry); + } + @Override public void afterSingletonsInstantiated() { this.registries.forEach((registry) -> { diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/DefaultWebMvcTagsProvider.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/DefaultWebMvcTagsProvider.java index 495cbe4403..9d1263a192 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/DefaultWebMvcTagsProvider.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/DefaultWebMvcTagsProvider.java @@ -39,7 +39,7 @@ public class DefaultWebMvcTagsProvider implements WebMvcTagsProvider { */ @Override public Iterable httpLongRequestTags(HttpServletRequest request, Object handler) { - return Arrays.asList(WebMvcTags.method(request), WebMvcTags.uri(request)); + return Arrays.asList(WebMvcTags.method(request), WebMvcTags.uri(request, null)); } /** @@ -52,8 +52,9 @@ public class DefaultWebMvcTagsProvider implements WebMvcTagsProvider { @Override public Iterable httpRequestTags(HttpServletRequest request, HttpServletResponse response, Throwable ex) { - return Arrays.asList(WebMvcTags.method(request), WebMvcTags.uri(request), - WebMvcTags.exception(ex), WebMvcTags.status(response)); + return Arrays.asList(WebMvcTags.method(request), + WebMvcTags.uri(request, response), WebMvcTags.exception(ex), + WebMvcTags.status(response)); } } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetrics.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetrics.java index d7bf324c32..14d52041b2 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetrics.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetrics.java @@ -17,6 +17,7 @@ package org.springframework.boot.actuate.metrics.web.servlet; import java.lang.reflect.AnnotatedElement; +import java.util.Arrays; import java.util.Collections; import java.util.IdentityHashMap; import java.util.Map; @@ -24,20 +25,27 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import io.micrometer.core.annotation.Timed; +import io.micrometer.core.annotation.TimedSet; +import io.micrometer.core.instrument.LongTaskTimer; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Tag; +import io.micrometer.core.instrument.Tags; +import io.micrometer.core.instrument.Timer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.boot.actuate.metrics.TimedUtils; + +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.ObjectUtils; import org.springframework.web.context.request.RequestAttributes; import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.method.HandlerMethod; - -import io.micrometer.core.annotation.Timed; -import io.micrometer.core.instrument.*; +import org.springframework.web.servlet.resource.ResourceHttpRequestHandler; /** * Support class for Spring MVC metrics. @@ -147,7 +155,7 @@ public class WebMvcMetrics { Timer.Builder builder = Timer.builder(config.getName()) .tags(this.tagsProvider.httpRequestTags(request, response, thrown)) .tags(config.getExtraTags()).description("Timer of servlet request") - .publishPercentileHistogram(config.histogram); + .publishPercentileHistogram(config.isHistogram()); if (config.getPercentiles().length > 0) { builder = builder.publishPercentiles(config.getPercentiles()); } @@ -155,11 +163,10 @@ public class WebMvcMetrics { } private LongTaskTimer longTaskTimer(TimerConfig config, HttpServletRequest request, - Object handler) { + Object handler) { return LongTaskTimer.builder(config.getName()) .tags(this.tagsProvider.httpLongRequestTags(request, handler)) - .tags(config.getExtraTags()) - .description("Timer of long servlet request") + .tags(config.getExtraTags()).description("Timer of long servlet request") .register(this.registry); } @@ -182,6 +189,11 @@ public class WebMvcMetrics { if (handler instanceof HandlerMethod) { return timed((HandlerMethod) handler); } + if ((handler == null || handler instanceof ResourceHttpRequestHandler) + && this.autoTimeRequests) { + return Collections.singleton( + new TimerConfig(getServerRequestName(), this.recordAsPercentiles)); + } return Collections.emptySet(); } @@ -198,15 +210,27 @@ public class WebMvcMetrics { } private Set getNonLongTaskAnnotationConfig(AnnotatedElement element) { - return TimedUtils.findTimedAnnotations(element).filter((t) -> !t.longTask()) + return findTimedAnnotations(element).filter((t) -> !t.longTask()) .map(this::fromAnnotation).collect(Collectors.toSet()); } private Set getLongTaskAnnotationConfig(AnnotatedElement element) { - return TimedUtils.findTimedAnnotations(element).filter(Timed::longTask) + return findTimedAnnotations(element).filter(Timed::longTask) .map(this::fromAnnotation).collect(Collectors.toSet()); } + private Stream findTimedAnnotations(AnnotatedElement element) { + Timed timed = AnnotationUtils.findAnnotation(element, Timed.class); + if (timed != null) { + return Stream.of(timed); + } + TimedSet ts = AnnotationUtils.findAnnotation(element, TimedSet.class); + if (ts != null) { + return Arrays.stream(ts.value()); + } + return Stream.empty(); + } + private TimerConfig fromAnnotation(Timed timed) { return new TimerConfig(timed, this::getServerRequestName); } @@ -282,4 +306,5 @@ public class WebMvcMetrics { } } + } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilter.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java similarity index 58% rename from spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilter.java rename to spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java index 9c548865fc..29028e6a05 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilter.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilter.java @@ -35,18 +35,23 @@ import org.springframework.web.servlet.handler.MatchableHandlerMapping; import org.springframework.web.util.NestedServletException; /** - * Intercepts incoming HTTP requests and records metrics about execution time and results. + * Intercepts incoming HTTP requests and records metrics about Spring MVC execution time + * and results. * * @author Jon Schneider * @since 2.0.0 */ @Order(Ordered.HIGHEST_PRECEDENCE) -public class MetricsFilter extends OncePerRequestFilter { +public class WebMvcMetricsFilter extends OncePerRequestFilter { + + private static final Logger logger = LoggerFactory + .getLogger(WebMvcMetricsFilter.class); + private final WebMvcMetrics webMvcMetrics; + private final HandlerMappingIntrospector mappingIntrospector; - private final Logger logger = LoggerFactory.getLogger(MetricsFilter.class); - public MetricsFilter(WebMvcMetrics webMvcMetrics, + public WebMvcMetricsFilter(WebMvcMetrics webMvcMetrics, HandlerMappingIntrospector mappingIntrospector) { this.webMvcMetrics = webMvcMetrics; this.mappingIntrospector = mappingIntrospector; @@ -60,37 +65,44 @@ public class MetricsFilter extends OncePerRequestFilter { @Override protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) - throws ServletException, IOException { - HandlerExecutionChain handler; + throws ServletException, IOException { + HandlerExecutionChain handlerExecutionChain = getHandlerExecutionChain(request); + Object handler = (handlerExecutionChain == null ? null + : handlerExecutionChain.getHandler()); + filterWithMetrics(request, response, filterChain, handler); + } + + private HandlerExecutionChain getHandlerExecutionChain(HttpServletRequest request) { try { MatchableHandlerMapping matchableHandlerMapping = this.mappingIntrospector .getMatchableHandlerMapping(request); - handler = matchableHandlerMapping.getHandler(request); + return (matchableHandlerMapping == null ? null + : matchableHandlerMapping.getHandler(request)); } - catch (Exception e) { - this.logger.debug("Unable to time request", e); - return; + catch (Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Unable to time request", ex); + } + return null; } + } - if (handler != null) { - Object handlerObject = handler.getHandler(); - this.webMvcMetrics.preHandle(request, handlerObject); - try { - filterChain.doFilter(request, response); - - // when an async operation is complete, the whole filter gets called - // again with isAsyncStarted = false - if (!request.isAsyncStarted()) { - this.webMvcMetrics.record(request, response, null); - } - } - catch (NestedServletException e) { - this.webMvcMetrics.record(request, response, e.getCause()); - throw e; - } - } - else { + private void filterWithMetrics(HttpServletRequest request, + HttpServletResponse response, FilterChain filterChain, Object handler) + throws IOException, ServletException, NestedServletException { + this.webMvcMetrics.preHandle(request, handler); + try { filterChain.doFilter(request, response); + // When an async operation is complete, the whole filter gets called again + // with isAsyncStarted = false + if (!request.isAsyncStarted()) { + this.webMvcMetrics.record(request, response, null); + } + } + catch (NestedServletException ex) { + this.webMvcMetrics.record(request, response, ex.getCause()); + throw ex; } } + } diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java index 174fb0b20d..26b74a5e67 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcTags.java @@ -21,6 +21,7 @@ import javax.servlet.http.HttpServletResponse; import io.micrometer.core.instrument.Tag; +import org.springframework.http.HttpStatus; import org.springframework.util.StringUtils; import org.springframework.web.servlet.HandlerMapping; @@ -62,9 +63,19 @@ public final class WebMvcTags { * available, falling back to the request's {@link HttpServletRequest#getPathInfo() * path info} if necessary. * @param request the request + * @param response the response * @return the uri tag derived from the request */ - public static Tag uri(HttpServletRequest request) { + public static Tag uri(HttpServletRequest request, HttpServletResponse response) { + if (response != null) { + HttpStatus status = HttpStatus.valueOf(response.getStatus()); + if (status.is3xxRedirection()) { + return Tag.of("uri", "REDIRECTION"); + } + if (status.equals(HttpStatus.NOT_FOUND)) { + return Tag.of("uri", "NOT_FOUND"); + } + } String uri = (String) request .getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE); if (uri == null) { diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java index 7a7021e2bf..8391ec6156 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointTests.java @@ -28,7 +28,6 @@ import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.Test; -import static io.micrometer.core.instrument.MockClock.clock; import static org.assertj.core.api.Assertions.assertThat; /** @@ -39,7 +38,8 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class MetricsEndpointTests { - private final MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); + private final MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, + new MockClock()); private final MetricsEndpoint endpoint = new MetricsEndpoint(this.registry); @@ -77,8 +77,7 @@ public class MetricsEndpointTests { this.registry.counter("cache", "result", "hit", "host", "1").increment(2); this.registry.counter("cache", "result", "miss", "host", "1").increment(2); this.registry.counter("cache", "result", "hit", "host", "2").increment(2); - clock(registry).add(SimpleConfig.DEFAULT_STEP); - + MockClock.clock(this.registry).add(SimpleConfig.DEFAULT_STEP); MetricsEndpoint.MetricResponse response = this.endpoint.metric("cache", Collections.emptyList()); assertThat(response.getName()).isEqualTo("cache"); @@ -92,8 +91,7 @@ public class MetricsEndpointTests { @Test public void metricWithSpaceInTagValue() { this.registry.counter("counter", "key", "a space").increment(2); - clock(registry).add(SimpleConfig.DEFAULT_STEP); - + MockClock.clock(this.registry).add(SimpleConfig.DEFAULT_STEP); MetricsEndpoint.MetricResponse response = this.endpoint.metric("counter", Collections.singletonList("key:a space")); assertThat(response.getName()).isEqualTo("counter"); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointWebIntegrationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointWebIntegrationTests.java index bd3433d831..94cab37763 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointWebIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/MetricsEndpointWebIntegrationTests.java @@ -29,13 +29,11 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.web.test.WebEndpointRunners; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.test.web.reactive.server.WebTestClient; -import static io.micrometer.core.instrument.MockClock.clock; import static org.assertj.core.api.Assertions.assertThat; /** @@ -46,32 +44,34 @@ import static org.assertj.core.api.Assertions.assertThat; */ @RunWith(WebEndpointRunners.class) public class MetricsEndpointWebIntegrationTests { - private static MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); + + private static MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, + new MockClock()); + private static WebTestClient client; private final ObjectMapper mapper = new ObjectMapper(); - @SuppressWarnings("unchecked") @Test + @SuppressWarnings("unchecked") public void listNames() throws IOException { - String responseBody = client.get() - .uri("/application/metrics").exchange().expectStatus().isOk() - .expectBody(String.class).returnResult().getResponseBody(); + String responseBody = client.get().uri("/application/metrics").exchange() + .expectStatus().isOk().expectBody(String.class).returnResult() + .getResponseBody(); Map> names = this.mapper.readValue(responseBody, Map.class); assertThat(names.get("names")).containsOnlyOnce("jvm.memory.used"); } @Test public void selectByName() throws IOException { - clock(registry).add(SimpleConfig.DEFAULT_STEP); - client.get() - .uri("/application/metrics/jvm.memory.used").exchange().expectStatus() + MockClock.clock(registry).add(SimpleConfig.DEFAULT_STEP); + client.get().uri("/application/metrics/jvm.memory.used").exchange().expectStatus() .isOk().expectBody().jsonPath("$.name").isEqualTo("jvm.memory.used"); } @Test public void selectByTag() { - clock(registry).add(SimpleConfig.DEFAULT_STEP); + MockClock.clock(registry).add(SimpleConfig.DEFAULT_STEP); client.get() .uri("/application/metrics/jvm.memory.used?tag=id:Compressed%20Class%20Space") .exchange().expectStatus().isOk().expectBody().jsonPath("$.name") diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java index 894bb4329a..eb1d72b668 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/client/MetricsRestTemplateCustomizerTests.java @@ -33,8 +33,6 @@ import org.springframework.test.web.client.match.MockRestRequestMatchers; import org.springframework.test.web.client.response.MockRestResponseCreators; import org.springframework.web.client.RestTemplate; -import static io.micrometer.core.instrument.MockClock.clock; -import static java.util.stream.StreamSupport.stream; import static org.assertj.core.api.Assertions.assertThat; /** @@ -46,7 +44,8 @@ public class MetricsRestTemplateCustomizerTests { @Test public void interceptRestTemplate() { - MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); + MeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, + new MockClock()); RestTemplate restTemplate = new RestTemplate(); MetricsRestTemplateCustomizer customizer = new MetricsRestTemplateCustomizer( registry, new DefaultRestTemplateExchangeTagsProvider(), @@ -59,9 +58,11 @@ public class MetricsRestTemplateCustomizerTests { .andRespond(MockRestResponseCreators.withSuccess("OK", MediaType.APPLICATION_JSON)); String result = restTemplate.getForObject("/test/{id}", String.class, 123); - clock(registry).add(SimpleConfig.DEFAULT_STEP); - assertThat(registry.find("http.client.requests").meters()) - .anySatisfy(m -> assertThat(stream(m.getId().getTags().spliterator(), false).map(Tag::getKey)).doesNotContain("bucket")); + MockClock.clock(registry).add(SimpleConfig.DEFAULT_STEP); + assertThat(registry.find("http.client.requests") + .meters()).anySatisfy(m -> assertThat( + StreamSupport.stream(m.getId().getTags().spliterator(), false) + .map(Tag::getKey)).doesNotContain("bucket")); assertThat(registry.find("http.client.requests") .tags("method", "GET", "uri", "/test/{id}", "status", "200") .value(Statistic.Count, 1.0).timer()).isPresent(); diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterAutoTimedTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterAutoTimedTests.java similarity index 60% rename from spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterAutoTimedTests.java rename to spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterAutoTimedTests.java index 8b212a134f..f399c5faae 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterAutoTimedTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterAutoTimedTests.java @@ -24,6 +24,7 @@ import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -49,70 +50,76 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. */ @RunWith(SpringRunner.class) @WebAppConfiguration -public class MetricsFilterAutoTimedTests { +public class WebMvcMetricsFilterAutoTimedTests { + + @Autowired + private MeterRegistry registry; - @Autowired - private MeterRegistry registry; + @Autowired + private MockClock clock; - @Autowired - private MockClock clock; + @Autowired + private WebApplicationContext context; - @Autowired - private WebApplicationContext context; + private MockMvc mvc; - private MockMvc mvc; + @Autowired + private WebMvcMetricsFilter filter; - @Autowired - private MetricsFilter filter; + @Before + public void setupMockMvc() { + this.mvc = MockMvcBuilders.webAppContextSetup(this.context) + .addFilters(this.filter).build(); + } - @Before - public void setupMockMvc() { - this.mvc = MockMvcBuilders.webAppContextSetup(this.context) - .addFilters(filter) - .build(); - } + @Test + public void metricsCanBeAutoTimed() throws Exception { + this.mvc.perform(get("/api/10")).andExpect(status().isOk()); - @Test - public void metricsCanBeAutoTimed() throws Exception { - this.mvc.perform(get("/api/10")).andExpect(status().isOk()); + this.clock.add(SimpleConfig.DEFAULT_STEP); + assertThat( + this.registry.find("http.server.requests").tags("status", "200").timer()) + .hasValueSatisfying((t) -> assertThat(t.count()).isEqualTo(1)); + } - clock.add(SimpleConfig.DEFAULT_STEP); - assertThat(this.registry.find("http.server.requests").tags("status", "200").timer()) - .hasValueSatisfying((t) -> assertThat(t.count()).isEqualTo(1)); - } + @Configuration + @EnableWebMvc + @Import({ Controller.class }) + static class TestConfiguration { - @Configuration - @EnableWebMvc - @Import({Controller.class}) - static class TestConfiguration { - @Bean - MockClock clock() { - return new MockClock(); - } + @Bean + MockClock clock() { + return new MockClock(); + } - @Bean - MeterRegistry meterRegistry(Clock clock) { - return new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); - } + @Bean + MeterRegistry meterRegistry(Clock clock) { + return new SimpleMeterRegistry(SimpleConfig.DEFAULT, clock); + } @Bean public WebMvcMetrics controllerMetrics(MeterRegistry registry) { - return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), "http.server.requests", true, - false); + return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), + "http.server.requests", true, false); } @Bean - public MetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, HandlerMappingIntrospector introspector) { - return new MetricsFilter(controllerMetrics, introspector); + public WebMvcMetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, + HandlerMappingIntrospector introspector) { + return new WebMvcMetricsFilter(controllerMetrics, introspector); } - } - - @RestController - @RequestMapping("/api") - static class Controller { - @GetMapping("/{id}") - public String successful(@PathVariable Long id) { - return id.toString(); - } - } + + } + + @RestController + @RequestMapping("/api") + static class Controller { + + @GetMapping("/{id}") + public String successful(@PathVariable Long id) { + return id.toString(); + } + + } + } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterTests.java similarity index 82% rename from spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterTests.java rename to spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterTests.java index 34d80d4ab7..8b85920722 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/MetricsFilterTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsFilterTests.java @@ -23,7 +23,6 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; -import java.util.stream.StreamSupport; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -33,8 +32,6 @@ import javax.servlet.http.HttpServletResponse; import io.micrometer.core.annotation.Timed; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Statistic; -import io.micrometer.core.instrument.Tag; -import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import io.micrometer.prometheus.PrometheusConfig; import io.micrometer.prometheus.PrometheusMeterRegistry; import org.junit.Before; @@ -42,13 +39,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.SpringBootApplication; -import org.springframework.boot.test.context.SpringBootTest; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.http.HttpStatus; -import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.test.web.servlet.MockMvc; @@ -64,8 +58,6 @@ import org.springframework.web.context.WebApplicationContext; import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.config.annotation.EnableWebMvc; -import org.springframework.web.servlet.config.annotation.InterceptorRegistry; -import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; import static org.assertj.core.api.Assertions.assertThat; @@ -76,13 +68,14 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** - * Tests for {@link MetricsFilter} + * Tests for {@link WebMvcMetricsFilter} * * @author Jon Schneider */ @RunWith(SpringRunner.class) @WebAppConfiguration -public class MetricsFilterTests { +public class WebMvcMetricsFilterTests { + @Autowired private PrometheusMeterRegistry registry; @@ -90,7 +83,7 @@ public class MetricsFilterTests { private WebApplicationContext context; @Autowired - private MetricsFilter filter; + private WebMvcMetricsFilter filter; private MockMvc mvc; @@ -99,16 +92,13 @@ public class MetricsFilterTests { @Before public void setupMockMvc() { - this.mvc = MockMvcBuilders - .webAppContextSetup(this.context) - .addFilters(filter, new RedirectAndNotFoundFilter()) - .build(); + this.mvc = MockMvcBuilders.webAppContextSetup(this.context) + .addFilters(this.filter, new RedirectAndNotFoundFilter()).build(); } @Test public void timedMethod() throws Exception { this.mvc.perform(get("/api/c1/10")).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests") .tags("status", "200", "uri", "/api/c1/{id}", "public", "true") .value(Statistic.Count, 1.0).timer()).isPresent(); @@ -117,7 +107,6 @@ public class MetricsFilterTests { @Test public void subclassedTimedMethod() throws Exception { this.mvc.perform(get("/api/c1/metaTimed/10")).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests") .tags("status", "200", "uri", "/api/c1/metaTimed/{id}") .value(Statistic.Count, 1.0).timer()).isPresent(); @@ -126,7 +115,6 @@ public class MetricsFilterTests { @Test public void untimedMethod() throws Exception { this.mvc.perform(get("/api/c1/untimed/10")).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests") .tags("uri", "/api/c1/untimed/10").timer()).isEmpty(); } @@ -134,39 +122,32 @@ public class MetricsFilterTests { @Test public void timedControllerClass() throws Exception { this.mvc.perform(get("/api/c2/10")).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests").tags("status", "200") - .value(Statistic.Count, 1.0) - .timer()).isPresent(); + .value(Statistic.Count, 1.0).timer()).isPresent(); } @Test public void badClientRequest() throws Exception { this.mvc.perform(get("/api/c1/oops")).andExpect(status().is4xxClientError()); - assertThat(this.registry.find("http.server.requests").tags("status", "400") - .value(Statistic.Count, 1.0) - .timer()).isPresent(); + .value(Statistic.Count, 1.0).timer()).isPresent(); } - @Test public void redirectRequest() throws Exception { this.mvc.perform(get("/api/redirect") - .header(RedirectAndNotFoundFilter.TEST_MISBEHAVE_HEADER, "302")).andExpect(status().is3xxRedirection()); - - assertThat(this.registry.find("http.server.requests") - .tags("uri", "REDIRECTION") + .header(RedirectAndNotFoundFilter.TEST_MISBEHAVE_HEADER, "302")) + .andExpect(status().is3xxRedirection()); + assertThat(this.registry.find("http.server.requests").tags("uri", "REDIRECTION") .tags("status", "302").timer()).isPresent(); } @Test public void notFoundRequest() throws Exception { this.mvc.perform(get("/api/not/found") - .header(RedirectAndNotFoundFilter.TEST_MISBEHAVE_HEADER, "404")).andExpect(status().is4xxClientError()); - - assertThat(this.registry.find("http.server.requests") - .tags("uri", "NOT_FOUND") + .header(RedirectAndNotFoundFilter.TEST_MISBEHAVE_HEADER, "404")) + .andExpect(status().is4xxClientError()); + assertThat(this.registry.find("http.server.requests").tags("uri", "NOT_FOUND") .tags("status", "404").timer()).isPresent(); } @@ -174,8 +155,7 @@ public class MetricsFilterTests { public void unhandledError() throws Exception { assertThatCode(() -> this.mvc.perform(get("/api/c1/unhandledError/10")) .andExpect(status().isOk())) - .hasRootCauseInstanceOf(RuntimeException.class); - + .hasRootCauseInstanceOf(RuntimeException.class); assertThat(this.registry.find("http.server.requests") .tags("exception", "RuntimeException").value(Statistic.Count, 1.0) .timer()).isPresent(); @@ -184,22 +164,16 @@ public class MetricsFilterTests { @Test public void longRunningRequest() throws Exception { MvcResult result = this.mvc.perform(get("/api/c1/long/10")) - .andExpect(request().asyncStarted()) - .andReturn(); - + .andExpect(request().asyncStarted()).andReturn(); // the request is not prematurely recorded as complete - assertThat(this.registry.find("http.server.requests") - .tags("uri", "/api/c1/async").timer()).isNotPresent(); - + assertThat(this.registry.find("http.server.requests").tags("uri", "/api/c1/async") + .timer()).isNotPresent(); // while the mapping is running, it contributes to the activeTasks count assertThat(this.registry.find("my.long.request").tags("region", "test") .value(Statistic.Count, 1.0).longTaskTimer()).isPresent(); - // once the mapping completes, we can gather information about status, etc. - asyncLatch.countDown(); - + this.asyncLatch.countDown(); this.mvc.perform(asyncDispatch(result)).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests").tags("status", "200") .value(Statistic.Count, 1.0).timer()).isPresent(); } @@ -207,7 +181,6 @@ public class MetricsFilterTests { @Test public void endpointThrowsError() throws Exception { this.mvc.perform(get("/api/c1/error/10")).andExpect(status().is4xxClientError()); - assertThat(this.registry.find("http.server.requests").tags("status", "422") .value(Statistic.Count, 1.0).timer()).isPresent(); } @@ -215,7 +188,6 @@ public class MetricsFilterTests { @Test public void regexBasedRequestMapping() throws Exception { this.mvc.perform(get("/api/c1/regex/.abc")).andExpect(status().isOk()); - assertThat(this.registry.find("http.server.requests") .tags("uri", "/api/c1/regex/{id:\\.[a-z]+}").value(Statistic.Count, 1.0) .timer()).isPresent(); @@ -224,7 +196,6 @@ public class MetricsFilterTests { @Test public void recordQuantiles() throws Exception { this.mvc.perform(get("/api/c1/percentiles/10")).andExpect(status().isOk()); - assertThat(this.registry.scrape()).contains("quantile=\"0.5\""); assertThat(this.registry.scrape()).contains("quantile=\"0.95\""); } @@ -232,21 +203,22 @@ public class MetricsFilterTests { @Test public void recordHistogram() throws Exception { this.mvc.perform(get("/api/c1/histogram/10")).andExpect(status().isOk()); - assertThat(this.registry.scrape()).contains("le=\"0.001\""); assertThat(this.registry.scrape()).contains("le=\"30.0\""); } - @Target({ElementType.METHOD}) + @Target({ ElementType.METHOD }) @Retention(RetentionPolicy.RUNTIME) @Timed(percentiles = 0.95) public @interface Timed95 { + } @Configuration @EnableWebMvc - @Import({Controller1.class, Controller2.class}) + @Import({ Controller1.class, Controller2.class }) static class MetricsFilterApp { + @Bean MeterRegistry meterRegistry() { // one of the few registries that support aggregable percentiles @@ -260,39 +232,44 @@ public class MetricsFilterTests { @Bean public WebMvcMetrics controllerMetrics(MeterRegistry registry) { - return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), "http.server.requests", true, - false); + return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), + "http.server.requests", true, false); } @Bean - public MetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, HandlerMappingIntrospector introspector) { - return new MetricsFilter(controllerMetrics, introspector); + public WebMvcMetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, + HandlerMappingIntrospector introspector) { + return new WebMvcMetricsFilter(controllerMetrics, introspector); } + } @RestController @RequestMapping("/api/c1") static class Controller1 { + private final CountDownLatch asyncLatch; - public Controller1(CountDownLatch asyncLatch) { + Controller1(CountDownLatch asyncLatch) { this.asyncLatch = asyncLatch; } - @Timed(extraTags = {"public", "true"}) + @Timed(extraTags = { "public", "true" }) @GetMapping("/{id}") public String successfulWithExtraTags(@PathVariable Long id) { return id.toString(); } @Timed - @Timed(value = "my.long.request", extraTags = {"region", "test"}, longTask = true) + @Timed(value = "my.long.request", extraTags = { "region", + "test" }, longTask = true) @GetMapping("/long/{id}") public Callable takesLongTimeToSatisfy(@PathVariable Long id) { return () -> { try { - asyncLatch.await(); - } catch (InterruptedException e) { + this.asyncLatch.await(); + } + catch (InterruptedException e) { throw new RuntimeException(e); } return id.toString(); @@ -322,7 +299,7 @@ public class MetricsFilterTests { return id; } - @Timed(percentiles = {0.50, 0.95}) + @Timed(percentiles = { 0.50, 0.95 }) @GetMapping("/percentiles/{id}") public String percentiles(@PathVariable String id) { return id; @@ -345,16 +322,19 @@ public class MetricsFilterTests { ModelAndView defaultErrorHandler(HttpServletRequest request, Exception e) { return new ModelAndView("myerror"); } + } @RestController @Timed @RequestMapping("/api/c2") static class Controller2 { + @GetMapping("/{id}") public String successful(@PathVariable Long id) { return id.toString(); } + } static class RedirectAndNotFoundFilter extends OncePerRequestFilter { @@ -362,14 +342,18 @@ public class MetricsFilterTests { static final String TEST_MISBEHAVE_HEADER = "x-test-misbehave-status"; @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { + protected void doFilterInternal(HttpServletRequest request, + HttpServletResponse response, FilterChain filterChain) + throws ServletException, IOException { String misbehave = request.getHeader(TEST_MISBEHAVE_HEADER); if (misbehave != null) { response.setStatus(Integer.parseInt(misbehave)); - } else { + } + else { filterChain.doFilter(request, response); } } + } -} +} diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsIntegrationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsIntegrationTests.java index ff3d7970e7..5edaa6d5bd 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/web/servlet/WebMvcMetricsIntegrationTests.java @@ -36,7 +36,11 @@ import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.context.web.WebAppConfiguration; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; -import org.springframework.web.bind.annotation.*; +import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; @@ -67,39 +71,37 @@ public class WebMvcMetricsIntegrationTests { private MockMvc mvc; @Autowired - private MetricsFilter filter; + private WebMvcMetricsFilter filter; @Before public void setupMockMvc() { this.mvc = MockMvcBuilders.webAppContextSetup(this.context) - .addFilters(filter) - .build(); + .addFilters(this.filter).build(); } @Test public void handledExceptionIsRecordedInMetricTag() throws Exception { this.mvc.perform(get("/api/handledError")).andExpect(status().is5xxServerError()); - - clock.add(SimpleConfig.DEFAULT_STEP); + this.clock.add(SimpleConfig.DEFAULT_STEP); assertThat(this.registry.find("http.server.requests") .tags("exception", "Exception1").value(Statistic.Count, 1.0).timer()) - .isPresent(); + .isPresent(); } @Test public void rethrownExceptionIsRecordedInMetricTag() throws Exception { assertThatCode(() -> this.mvc.perform(get("/api/rethrownError")) .andExpect(status().is5xxServerError())); - - clock.add(SimpleConfig.DEFAULT_STEP); + this.clock.add(SimpleConfig.DEFAULT_STEP); assertThat(this.registry.find("http.server.requests") .tags("exception", "Exception2").value(Statistic.Count, 1.0).timer()) - .isPresent(); + .isPresent(); } @Configuration @EnableWebMvc static class TestConfiguration { + @Bean MockClock clock() { return new MockClock(); @@ -112,13 +114,14 @@ public class WebMvcMetricsIntegrationTests { @Bean public WebMvcMetrics controllerMetrics(MeterRegistry registry) { - return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), "http.server.requests", true, - false); + return new WebMvcMetrics(registry, new DefaultWebMvcTagsProvider(), + "http.server.requests", true, false); } @Bean - public MetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, HandlerMappingIntrospector introspector) { - return new MetricsFilter(controllerMetrics, introspector); + public WebMvcMetricsFilter webMetricsFilter(WebMvcMetrics controllerMetrics, + HandlerMappingIntrospector introspector) { + return new WebMvcMetricsFilter(controllerMetrics, introspector); } @RestController @@ -142,12 +145,15 @@ public class WebMvcMetricsIntegrationTests { } } + } static class Exception1 extends RuntimeException { + } static class Exception2 extends RuntimeException { + } @ControllerAdvice @@ -167,5 +173,7 @@ public class WebMvcMetricsIntegrationTests { ResponseEntity rethrowError(Exception2 ex) throws Throwable { throw ex; } + } -} \ No newline at end of file + +} diff --git a/spring-boot-project/spring-boot-dependencies/pom.xml b/spring-boot-project/spring-boot-dependencies/pom.xml index 42d88f0251..427af16c64 100644 --- a/spring-boot-project/spring-boot-dependencies/pom.xml +++ b/spring-boot-project/spring-boot-dependencies/pom.xml @@ -60,6 +60,7 @@ 2.8.2 1.4.196 1.3 + 2.1.10 3.9 1.2.2 5.2.12.Final @@ -114,7 +115,7 @@ 1.2.3 1.16.18 2.1.2 - 1.0.0-SNAPSHOT + 1.0.0-rc.3 6.2.2.jre8 2.11.0 1.6.0 @@ -2056,6 +2057,11 @@ hamcrest-library ${hamcrest.version} + + org.hdrhistogram + HdrHistogram + ${hdrhistogram.version} + org.hibernate hibernate-c3p0