From 1fc2e870530af15301d3e69b3dd506f606a3c177 Mon Sep 17 00:00:00 2001 From: Lucas Saldanha Date: Wed, 5 Oct 2016 23:50:42 +1300 Subject: [PATCH 1/2] Enable custom Reservoir with Dropwizard metrics Uses the ReservoirFactory to customize the implementation of the Reservoir that will be used when creating Timer and Histogram in the DropwizardMetricServices. Fixes gh-5199 Closes gh-7105 --- .../MetricsDropwizardAutoConfiguration.java | 12 ++- .../dropwizard/DropwizardMetricServices.java | 61 ++++++++++++- .../metrics/dropwizard/ReservoirFactory.java | 39 ++++++++ ...tricsDropwizardAutoConfigurationTests.java | 89 +++++++++++++++++++ .../DropwizardMetricServicesTests.java | 44 +++++++++ 5 files changed, 242 insertions(+), 3 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java index ecd2e99ef0..577b21a87c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java @@ -18,10 +18,12 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.MetricRegistry; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; import org.springframework.boot.actuate.metrics.dropwizard.DropwizardMetricServices; +import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; import org.springframework.boot.actuate.metrics.reader.MetricRegistryMetricReader; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -41,6 +43,9 @@ import org.springframework.context.annotation.Configuration; @AutoConfigureBefore(MetricRepositoryAutoConfiguration.class) public class MetricsDropwizardAutoConfiguration { + @Autowired(required = false) + private ReservoirFactory reservoirFactory; + @Bean @ConditionalOnMissingBean public MetricRegistry metricRegistry() { @@ -52,7 +57,12 @@ public class MetricsDropwizardAutoConfiguration { GaugeService.class }) public DropwizardMetricServices dropwizardMetricServices( MetricRegistry metricRegistry) { - return new DropwizardMetricServices(metricRegistry); + if (this.reservoirFactory == null) { + return new DropwizardMetricServices(metricRegistry); + } + else { + return new DropwizardMetricServices(metricRegistry, this.reservoirFactory); + } } @Bean diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index 93a6a16ee9..846c51ff48 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -24,7 +24,9 @@ import com.codahale.metrics.Counter; import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; import com.codahale.metrics.Meter; +import com.codahale.metrics.Metric; import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Reservoir; import com.codahale.metrics.Timer; import org.springframework.boot.actuate.metrics.CounterService; @@ -53,6 +55,8 @@ public class DropwizardMetricServices implements CounterService, GaugeService { private final MetricRegistry registry; + private ReservoirFactory reservoirFactory; + private final ConcurrentMap gauges = new ConcurrentHashMap(); private final ConcurrentHashMap names = new ConcurrentHashMap(); @@ -65,6 +69,18 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry = registry; } + /** + * Create a new {@link DropwizardMetricServices} instance. + * @param registry the underlying metric registry + * @param reservoirFactory the factory that instantiates the {@link Reservoir} that + * will be used on Timers and Histograms + */ + public DropwizardMetricServices(MetricRegistry registry, + ReservoirFactory reservoirFactory) { + this.registry = registry; + this.reservoirFactory = reservoirFactory; + } + @Override public void increment(String name) { incrementInternal(name, 1L); @@ -91,12 +107,12 @@ public class DropwizardMetricServices implements CounterService, GaugeService { public void submit(String name, double value) { if (name.startsWith("histogram")) { long longValue = (long) value; - Histogram metric = this.registry.histogram(name); + Histogram metric = registerHistogram(name); metric.update(longValue); } else if (name.startsWith("timer")) { long longValue = (long) value; - Timer metric = this.registry.timer(name); + Timer metric = registerTimer(name); metric.update(longValue, TimeUnit.MILLISECONDS); } else { @@ -105,6 +121,43 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } } + private Histogram registerHistogram(String name) { + if (this.reservoirFactory == null) { + return this.registry.histogram(name); + } + else { + Histogram histogram = new Histogram(this.reservoirFactory.getObject()); + return getOrAddMetric(name, histogram); + } + } + + private Timer registerTimer(String name) { + if (this.reservoirFactory == null) { + return this.registry.timer(name); + } + else { + Timer timer = new Timer(this.reservoirFactory.getObject()); + return getOrAddMetric(name, timer); + } + } + + @SuppressWarnings("unchecked") + private T getOrAddMetric(String name, T newMetric) { + Metric metric = this.registry.getMetrics().get(name); + if (metric == null) { + return this.registry.register(name, newMetric); + } + else { + if (metric.getClass().equals(newMetric.getClass())) { + return (T) metric; + } + else { + throw new IllegalArgumentException( + name + " is already used for a different type of metric"); + } + } + } + private void setGaugeValue(String name, double value) { // NOTE: Dropwizard provides no way to do this atomically SimpleGauge gauge = this.gauges.get(name); @@ -148,6 +201,10 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry.remove(name); } + void setReservoirFactory(ReservoirFactory reservoirFactory) { + this.reservoirFactory = reservoirFactory; + } + /** * Simple {@link Gauge} implementation to {@literal double} value. */ diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java new file mode 100644 index 0000000000..b1de0bb628 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java @@ -0,0 +1,39 @@ +/* + * Copyright 2012-2016 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.dropwizard; + +import com.codahale.metrics.Reservoir; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.ObjectFactory; + +/** + * A {@link Reservoir} factory to instantiate the Reservoir that will be set as default + * for the {@link DropwizardMetricServices}. + * The Reservoir instances can't be shared across {@link com.codahale.metrics.Metric}. + * + * @author Lucas Saldanha + */ +public abstract class ReservoirFactory implements ObjectFactory { + + protected abstract Reservoir defaultReservoir(); + + @Override + public Reservoir getObject() throws BeansException { + return defaultReservoir(); + } +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java new file mode 100644 index 0000000000..5f032deea6 --- /dev/null +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2012-2016 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.autoconfigure; + +import com.codahale.metrics.Reservoir; +import com.codahale.metrics.UniformReservoir; + +import org.junit.After; +import org.junit.Test; + +import org.springframework.boot.actuate.metrics.dropwizard.DropwizardMetricServices; +import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import org.springframework.test.util.ReflectionTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link MetricsDropwizardAutoConfiguration}. + * + * @author Lucas Saldanha + */ +public class MetricsDropwizardAutoConfigurationTests { + + private AnnotationConfigApplicationContext context; + + @After + public void after() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void dropwizardWithoutCustomReservoirConfigured() { + this.context = new AnnotationConfigApplicationContext( + MetricsDropwizardAutoConfiguration.class); + + DropwizardMetricServices dropwizardMetricServices = this.context + .getBean(DropwizardMetricServices.class); + + assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) + .isNull(); + } + + @Test + public void dropwizardWithCustomReservoirConfigured() { + this.context = new AnnotationConfigApplicationContext( + MetricsDropwizardAutoConfiguration.class, Config.class); + + DropwizardMetricServices dropwizardMetricServices = this.context + .getBean(DropwizardMetricServices.class); + + assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) + .isNotNull(); + } + + @Configuration + static class Config { + + @Bean + public ReservoirFactory reservoirFactory() { + return new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }; + } + } + +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java index ec745b1646..04f4c1d65a 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java @@ -20,15 +20,22 @@ import java.util.ArrayList; import java.util.List; import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; +import com.codahale.metrics.Reservoir; +import com.codahale.metrics.Timer; +import com.codahale.metrics.UniformReservoir; import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + import static org.assertj.core.api.Assertions.assertThat; /** * Tests for {@link DropwizardMetricServices}. * * @author Dave Syer + * @author Lucas Saldanha */ public class DropwizardMetricServicesTests { @@ -78,6 +85,26 @@ public class DropwizardMetricServicesTests { assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); } + @Test + public void setCustomReservoirTimer() { + this.writer.setReservoirFactory(new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }); + + this.writer.submit("timer.foo", 200); + this.writer.submit("timer.foo", 300); + assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); + + Timer timer = (Timer) this.registry.getMetrics().get("timer.foo"); + Histogram histogram = (Histogram) ReflectionTestUtils + .getField(timer, "histogram"); + assertThat(ReflectionTestUtils.getField(histogram, "reservoir").getClass() + .equals(UniformReservoir.class)).isTrue(); + } + @Test public void setPredefinedHistogram() { this.writer.submit("histogram.foo", 2.1); @@ -85,6 +112,23 @@ public class DropwizardMetricServicesTests { assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2); } + @Test + public void setCustomReservoirHistogram() { + this.writer.setReservoirFactory(new ReservoirFactory() { + @Override + protected Reservoir defaultReservoir() { + return new UniformReservoir(); + } + }); + + this.writer.submit("histogram.foo", 2.1); + this.writer.submit("histogram.foo", 2.3); + assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2); + assertThat(ReflectionTestUtils + .getField(this.registry.getMetrics().get("histogram.foo"), "reservoir") + .getClass().equals(UniformReservoir.class)).isTrue(); + } + /** * Test the case where a given writer is used amongst several threads where each * thread is updating the same set of metrics. This would be an example case of the From 06a7ab0cd5f3036e7ece4edf7c80428874afc362 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 20 Dec 2016 13:52:53 -0800 Subject: [PATCH 2/2] Polish ReservoirFactory support Polish Dropwizrd reservoir support including a refactor of `ReservoirFactory` to allow reservoirs to be created based on a metric name. See gh-5199 See gh-7105 --- .../MetricsDropwizardAutoConfiguration.java | 10 +- .../dropwizard/DropwizardMetricServices.java | 131 ++++++++++++------ .../metrics/dropwizard/ReservoirFactory.java | 36 +++-- ...tricsDropwizardAutoConfigurationTests.java | 34 ++--- .../DropwizardMetricServicesTests.java | 43 +++--- 5 files changed, 161 insertions(+), 93 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java index 577b21a87c..a53bc9db45 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfiguration.java @@ -18,7 +18,7 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.MetricRegistry; -import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; @@ -43,8 +43,12 @@ import org.springframework.context.annotation.Configuration; @AutoConfigureBefore(MetricRepositoryAutoConfiguration.class) public class MetricsDropwizardAutoConfiguration { - @Autowired(required = false) - private ReservoirFactory reservoirFactory; + private final ReservoirFactory reservoirFactory; + + public MetricsDropwizardAutoConfiguration( + ObjectProvider reservoirFactory) { + this.reservoirFactory = reservoirFactory.getIfAvailable(); + } @Bean @ConditionalOnMissingBean diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java index 846c51ff48..98a70ad8cc 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServices.java @@ -31,6 +31,8 @@ import com.codahale.metrics.Timer; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.actuate.metrics.GaugeService; +import org.springframework.core.ResolvableType; +import org.springframework.util.Assert; /** * A {@link GaugeService} and {@link CounterService} that sends data to a Dropwizard @@ -55,7 +57,7 @@ public class DropwizardMetricServices implements CounterService, GaugeService { private final MetricRegistry registry; - private ReservoirFactory reservoirFactory; + private final ReservoirFactory reservoirFactory; private final ConcurrentMap gauges = new ConcurrentHashMap(); @@ -66,19 +68,20 @@ public class DropwizardMetricServices implements CounterService, GaugeService { * @param registry the underlying metric registry */ public DropwizardMetricServices(MetricRegistry registry) { - this.registry = registry; + this(registry, null); } /** * Create a new {@link DropwizardMetricServices} instance. * @param registry the underlying metric registry * @param reservoirFactory the factory that instantiates the {@link Reservoir} that - * will be used on Timers and Histograms + * will be used on Timers and Histograms */ public DropwizardMetricServices(MetricRegistry registry, ReservoirFactory reservoirFactory) { this.registry = registry; - this.reservoirFactory = reservoirFactory; + this.reservoirFactory = (reservoirFactory == null ? ReservoirFactory.NONE + : reservoirFactory); } @Override @@ -106,14 +109,10 @@ public class DropwizardMetricServices implements CounterService, GaugeService { @Override public void submit(String name, double value) { if (name.startsWith("histogram")) { - long longValue = (long) value; - Histogram metric = registerHistogram(name); - metric.update(longValue); + submitHistogram(name, value); } else if (name.startsWith("timer")) { - long longValue = (long) value; - Timer metric = registerTimer(name); - metric.update(longValue, TimeUnit.MILLISECONDS); + submitTimer(name, value); } else { name = wrapGaugeName(name); @@ -121,40 +120,36 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } } - private Histogram registerHistogram(String name) { - if (this.reservoirFactory == null) { - return this.registry.histogram(name); - } - else { - Histogram histogram = new Histogram(this.reservoirFactory.getObject()); - return getOrAddMetric(name, histogram); - } + private void submitTimer(String name, double value) { + long longValue = (long) value; + Timer metric = register(name, new TimerMetricRegistrar()); + metric.update(longValue, TimeUnit.MILLISECONDS); } - private Timer registerTimer(String name) { - if (this.reservoirFactory == null) { - return this.registry.timer(name); - } - else { - Timer timer = new Timer(this.reservoirFactory.getObject()); - return getOrAddMetric(name, timer); - } + private void submitHistogram(String name, double value) { + long longValue = (long) value; + Histogram metric = register(name, new HistogramMetricRegistrar()); + metric.update(longValue); } @SuppressWarnings("unchecked") - private T getOrAddMetric(String name, T newMetric) { + private T register(String name, MetricRegistrar registrar) { + Reservoir reservoir = this.reservoirFactory.getReservoir(name); + if (reservoir == null) { + return registrar.register(this.registry, name); + } Metric metric = this.registry.getMetrics().get(name); - if (metric == null) { - return this.registry.register(name, newMetric); + if (metric != null) { + registrar.checkExisting(metric); + return (T) metric; } - else { - if (metric.getClass().equals(newMetric.getClass())) { - return (T) metric; - } - else { - throw new IllegalArgumentException( - name + " is already used for a different type of metric"); - } + try { + return this.registry.register(name, registrar.createForReservoir(reservoir)); + } + catch (IllegalArgumentException ex) { + Metric added = this.registry.getMetrics().get(name); + registrar.checkExisting(metric); + return (T) added; } } @@ -201,10 +196,6 @@ public class DropwizardMetricServices implements CounterService, GaugeService { this.registry.remove(name); } - void setReservoirFactory(ReservoirFactory reservoirFactory) { - this.reservoirFactory = reservoirFactory; - } - /** * Simple {@link Gauge} implementation to {@literal double} value. */ @@ -227,4 +218,62 @@ public class DropwizardMetricServices implements CounterService, GaugeService { } + /** + * Strategy used to register metrics. + */ + private static abstract class MetricRegistrar { + + private final Class type; + + @SuppressWarnings("unchecked") + MetricRegistrar() { + this.type = (Class) ResolvableType + .forClass(MetricRegistrar.class, getClass()).resolveGeneric(); + } + + public void checkExisting(Metric metric) { + Assert.isInstanceOf(this.type, metric, + "Different metric type already registered"); + } + + protected abstract T register(MetricRegistry registry, String name); + + protected abstract T createForReservoir(Reservoir reservoir); + + } + + /** + * {@link MetricRegistrar} for {@link Timer} metrics. + */ + private static class TimerMetricRegistrar extends MetricRegistrar { + + @Override + protected Timer register(MetricRegistry registry, String name) { + return registry.timer(name); + } + + @Override + protected Timer createForReservoir(Reservoir reservoir) { + return new Timer(reservoir); + } + + } + + /** + * {@link MetricRegistrar} for {@link Histogram} metrics. + */ + private static class HistogramMetricRegistrar extends MetricRegistrar { + + @Override + protected Histogram register(MetricRegistry registry, String name) { + return registry.histogram(name); + } + + @Override + protected Histogram createForReservoir(Reservoir reservoir) { + return new Histogram(reservoir); + } + + } + } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java index b1de0bb628..64396043c5 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/dropwizard/ReservoirFactory.java @@ -18,22 +18,34 @@ package org.springframework.boot.actuate.metrics.dropwizard; import com.codahale.metrics.Reservoir; -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.ObjectFactory; - /** - * A {@link Reservoir} factory to instantiate the Reservoir that will be set as default - * for the {@link DropwizardMetricServices}. - * The Reservoir instances can't be shared across {@link com.codahale.metrics.Metric}. + * Factory interface that can be used by {@link DropwizardMetricServices} to create a + * custom {@link Reservoir}. * * @author Lucas Saldanha + * @author Phillip Webb + * @since 1.5.0 */ -public abstract class ReservoirFactory implements ObjectFactory { +public interface ReservoirFactory { + + /** + * Default empty {@link ReservoirFactory} implementation. + */ + ReservoirFactory NONE = new ReservoirFactory() { + + @Override + public Reservoir getReservoir(String name) { + return null; + } + + }; - protected abstract Reservoir defaultReservoir(); + /** + * Return the {@link Reservoir} instance to use or {@code null} if a custom reservoir + * is not needed. + * @param name the name of the metric + * @return a reservoir instance or {@code null} + */ + Reservoir getReservoir(String name); - @Override - public Reservoir getObject() throws BeansException { - return defaultReservoir(); - } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java index 5f032deea6..f878306b97 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/MetricsDropwizardAutoConfigurationTests.java @@ -18,7 +18,6 @@ package org.springframework.boot.actuate.autoconfigure; import com.codahale.metrics.Reservoir; import com.codahale.metrics.UniformReservoir; - import org.junit.After; import org.junit.Test; @@ -27,7 +26,6 @@ import org.springframework.boot.actuate.metrics.dropwizard.ReservoirFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; - import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -52,24 +50,23 @@ public class MetricsDropwizardAutoConfigurationTests { public void dropwizardWithoutCustomReservoirConfigured() { this.context = new AnnotationConfigApplicationContext( MetricsDropwizardAutoConfiguration.class); - DropwizardMetricServices dropwizardMetricServices = this.context .getBean(DropwizardMetricServices.class); - - assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) - .isNull(); + ReservoirFactory reservoirFactory = (ReservoirFactory) ReflectionTestUtils + .getField(dropwizardMetricServices, "reservoirFactory"); + assertThat(reservoirFactory.getReservoir("test")).isNull(); } @Test public void dropwizardWithCustomReservoirConfigured() { this.context = new AnnotationConfigApplicationContext( MetricsDropwizardAutoConfiguration.class, Config.class); - DropwizardMetricServices dropwizardMetricServices = this.context .getBean(DropwizardMetricServices.class); - - assertThat(ReflectionTestUtils.getField(dropwizardMetricServices, "reservoirFactory")) - .isNotNull(); + ReservoirFactory reservoirFactory = (ReservoirFactory) ReflectionTestUtils + .getField(dropwizardMetricServices, "reservoirFactory"); + assertThat(reservoirFactory.getReservoir("test")) + .isInstanceOf(UniformReservoir.class); } @Configuration @@ -77,13 +74,18 @@ public class MetricsDropwizardAutoConfigurationTests { @Bean public ReservoirFactory reservoirFactory() { - return new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }; + return new UniformReservoirFactory(); } + + } + + private static class UniformReservoirFactory implements ReservoirFactory { + + @Override + public Reservoir getReservoir(String name) { + return new UniformReservoir(); + } + } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java index 04f4c1d65a..535797bc5c 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/dropwizard/DropwizardMetricServicesTests.java @@ -22,14 +22,18 @@ import java.util.List; import com.codahale.metrics.Gauge; import com.codahale.metrics.Histogram; import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.Reservoir; import com.codahale.metrics.Timer; import com.codahale.metrics.UniformReservoir; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.anyString; /** * Tests for {@link DropwizardMetricServices}. @@ -39,10 +43,18 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class DropwizardMetricServicesTests { - private final MetricRegistry registry = new MetricRegistry(); + private MetricRegistry registry = new MetricRegistry(); - private final DropwizardMetricServices writer = new DropwizardMetricServices( - this.registry); + @Mock + private ReservoirFactory reservoirFactory; + + private DropwizardMetricServices writer; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + this.writer = new DropwizardMetricServices(this.registry, this.reservoirFactory); + } @Test public void incrementCounter() { @@ -87,20 +99,14 @@ public class DropwizardMetricServicesTests { @Test public void setCustomReservoirTimer() { - this.writer.setReservoirFactory(new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }); - + given(this.reservoirFactory.getReservoir(anyString())) + .willReturn(new UniformReservoir()); this.writer.submit("timer.foo", 200); this.writer.submit("timer.foo", 300); assertThat(this.registry.timer("timer.foo").getCount()).isEqualTo(2); - Timer timer = (Timer) this.registry.getMetrics().get("timer.foo"); - Histogram histogram = (Histogram) ReflectionTestUtils - .getField(timer, "histogram"); + Histogram histogram = (Histogram) ReflectionTestUtils.getField(timer, + "histogram"); assertThat(ReflectionTestUtils.getField(histogram, "reservoir").getClass() .equals(UniformReservoir.class)).isTrue(); } @@ -114,13 +120,8 @@ public class DropwizardMetricServicesTests { @Test public void setCustomReservoirHistogram() { - this.writer.setReservoirFactory(new ReservoirFactory() { - @Override - protected Reservoir defaultReservoir() { - return new UniformReservoir(); - } - }); - + given(this.reservoirFactory.getReservoir(anyString())) + .willReturn(new UniformReservoir()); this.writer.submit("histogram.foo", 2.1); this.writer.submit("histogram.foo", 2.3); assertThat(this.registry.histogram("histogram.foo").getCount()).isEqualTo(2);