diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptor.java index 556b904e..a533e64f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptor.java @@ -49,6 +49,9 @@ public class MetricsClientHttpRequestInterceptor implements ClientHttpRequestInt @Autowired Collection tagProviders; + @Autowired + ServoMonitorCache servoMonitorCache; + @Value("${netflix.metrics.restClient.metricName:restclient}") String metricName; @@ -75,7 +78,7 @@ public class MetricsClientHttpRequestInterceptor implements ClientHttpRequestInt .builder(metricName); monitorConfigBuilder.withTags(builder); - ServoMonitorCache.getTimer(monitorConfigBuilder.build()).record( + servoMonitorCache.getTimer(monitorConfigBuilder.build()).record( System.nanoTime() - startTime, TimeUnit.NANOSECONDS); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptor.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptor.java index 7ee8dfdc..5db99822 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptor.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptor.java @@ -47,6 +47,9 @@ public class MetricsHandlerInterceptor extends HandlerInterceptorAdapter { @Autowired MonitorRegistry registry; + @Autowired + ServoMonitorCache servoMonitorCache; + @Autowired Collection tagProviders; @@ -89,7 +92,7 @@ public class MetricsHandlerInterceptor extends HandlerInterceptorAdapter { MonitorConfig.Builder monitorConfigBuilder = MonitorConfig.builder(metricName); monitorConfigBuilder.withTags(builder); - ServoMonitorCache.getTimer(monitorConfigBuilder.build()).record( + servoMonitorCache.getTimer(monitorConfigBuilder.build()).record( System.nanoTime() - startTime, TimeUnit.NANOSECONDS); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfiguration.java index 6bd709f0..51b06647 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfiguration.java @@ -60,12 +60,17 @@ public class ServoMetricsAutoConfiguration { @Bean @ConditionalOnMissingBean - public MonitorRegistry monitorRegistry() { - System.setProperty(DefaultMonitorRegistry.class.getCanonicalName() + ".registryClass", servoMetricsConfig() + public MonitorRegistry monitorRegistry(ServoMetricsConfigBean servoMetricsConfig) { + System.setProperty(DefaultMonitorRegistry.class.getCanonicalName() + ".registryClass", servoMetricsConfig .getRegistryClass()); return DefaultMonitorRegistry.getInstance(); } + @Bean + public ServoMonitorCache monitorCache(MonitorRegistry monitorRegistry) { + return new ServoMonitorCache(monitorRegistry); + } + @Bean public MetricReaderPublicMetrics servoPublicMetrics(MonitorRegistry monitorRegistry, ServoMetricNaming servoMetricNaming) { ServoMetricReader reader = new ServoMetricReader(monitorRegistry, servoMetricNaming); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMonitorCache.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMonitorCache.java index ca91e01e..1a0011cc 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMonitorCache.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMonitorCache.java @@ -16,10 +16,8 @@ package org.springframework.cloud.netflix.metrics.servo; import java.util.HashMap; import java.util.Map; -import com.netflix.servo.DefaultMonitorRegistry; import com.netflix.servo.MonitorRegistry; import com.netflix.servo.monitor.BasicTimer; -import com.netflix.servo.monitor.Monitor; import com.netflix.servo.monitor.MonitorConfig; /** @@ -28,32 +26,26 @@ import com.netflix.servo.monitor.MonitorConfig; * @author Jon Schneider */ public class ServoMonitorCache { - private static final Map timerCache = new HashMap<>(); + private final Map timerCache = new HashMap<>(); + private final MonitorRegistry monitorRegistry; + + public ServoMonitorCache(MonitorRegistry monitorRegistry) { + this.monitorRegistry = monitorRegistry; + } /** * @param config contains the name and tags that uniquely identify a timer * @return an already registered timer if it exists, otherwise create/register one and * return it. */ - public synchronized static BasicTimer getTimer(MonitorConfig config) { + public synchronized BasicTimer getTimer(MonitorConfig config) { BasicTimer t = timerCache.get(config); if (t != null) return t; t = new BasicTimer(config); timerCache.put(config, t); - DefaultMonitorRegistry.getInstance().register(t); + monitorRegistry.register(t); return t; } - - /** - * Useful for tests to clear the monitor registry between runs - */ - public static void unregisterAll() { - MonitorRegistry registry = DefaultMonitorRegistry.getInstance(); - for (Monitor monitor : registry.getRegisteredMonitors()) { - registry.unregister(monitor); - } - timerCache.clear(); - } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/AbstractMetricsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/AbstractMetricsTests.java deleted file mode 100644 index f9c9d9d8..00000000 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/AbstractMetricsTests.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * Copyright 2013-2015 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.cloud.netflix.metrics; - -import org.junit.Before; -import org.springframework.cloud.netflix.metrics.servo.ServoMonitorCache; - -/** - * @author Jon Schneider - */ -public class AbstractMetricsTests { - @Before - public void setup() { - ServoMonitorCache.unregisterAll(); - } -} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java index e0b85649..3587f0b9 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsClientHttpRequestInterceptorTests.java @@ -24,6 +24,7 @@ import org.springframework.cloud.netflix.metrics.servo.ServoMetricsAutoConfigura import org.springframework.cloud.netflix.metrics.servo.ServoMonitorCache; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.test.context.ContextConfiguration; @@ -46,10 +47,13 @@ import com.netflix.servo.monitor.MonitorConfig; MetricsRestTemplateTestConfig.class }) @TestPropertySource(properties = { "netflix.metrics.restClient.metricName=metricName", "spring.aop.proxy-target-class=true" }) -public class MetricsClientHttpRequestInterceptorTests extends AbstractMetricsTests { +public class MetricsClientHttpRequestInterceptorTests { @Autowired MonitorRegistry registry; + @Autowired + ServoMonitorCache servoMonitorCache; + @Autowired RestTemplate restTemplate; @@ -67,7 +71,7 @@ public class MetricsClientHttpRequestInterceptorTests extends AbstractMetricsTes .withTag("status", "200") .withTag("clientName", "none"); - BasicTimer timer = ServoMonitorCache.getTimer(builder.build()); + BasicTimer timer = servoMonitorCache.getTimer(builder.build()); Assert.assertEquals(1L, (long) timer.getCount()); mockServer.verify(); @@ -78,6 +82,11 @@ public class MetricsClientHttpRequestInterceptorTests extends AbstractMetricsTes @ImportAutoConfiguration({ ServoMetricsAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, AopAutoConfiguration.class }) class MetricsRestTemplateTestConfig { + @Bean + @Primary + public MonitorRegistry monitorRegistry() { + return new SimpleMonitorRegistry(); + } } @Configuration diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptorIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptorIntegrationTests.java index b055fe40..0ece08d3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptorIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/MetricsHandlerInterceptorIntegrationTests.java @@ -26,6 +26,7 @@ import org.springframework.cloud.netflix.metrics.servo.ServoMetricsAutoConfigura import org.springframework.cloud.netflix.metrics.servo.ServoMonitorCache; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; import org.springframework.http.HttpStatus; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; @@ -60,13 +61,16 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. @WebAppConfiguration @TestPropertySource(properties = "netflix.metrics.rest.metricName=metricName") @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD) -public class MetricsHandlerInterceptorIntegrationTests extends AbstractMetricsTests { +public class MetricsHandlerInterceptorIntegrationTests { @Autowired WebApplicationContext webAppContext; @Autowired MonitorRegistry registry; + @Autowired + ServoMonitorCache servoMonitorCache; + MockMvc mvc; @Test @@ -117,7 +121,7 @@ public class MetricsHandlerInterceptorIntegrationTests extends AbstractMetricsTe if (exceptionType != null) builder = builder.withTag("exception", exceptionType); - BasicTimer timer = ServoMonitorCache.getTimer(builder.build()); + BasicTimer timer = servoMonitorCache.getTimer(builder.build()); Assert.assertEquals(1L, (long) timer.getCount()); } } @@ -131,6 +135,12 @@ class MetricsTestConfig { MetricsTestController testController() { return new MetricsTestController(); } + + @Bean + @Primary + public MonitorRegistry monitorRegistry() { + return new SimpleMonitorRegistry(); + } } @RestController diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/SimpleMonitorRegistry.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/SimpleMonitorRegistry.java new file mode 100644 index 00000000..9e1acb31 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/SimpleMonitorRegistry.java @@ -0,0 +1,52 @@ +/* + * Copyright 2013-2015 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.cloud.netflix.metrics; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import com.netflix.servo.MonitorRegistry; +import com.netflix.servo.monitor.Monitor; + +/** + * @author Jon Schneider + */ +public class SimpleMonitorRegistry implements MonitorRegistry { + List> monitors = new ArrayList<>(); + + @Override + public Collection> getRegisteredMonitors() { + return monitors; + } + + @Override + public void register(Monitor monitor) { + monitors.add(monitor); + } + + @Override + public void unregister(Monitor monitor) { + monitors.remove(monitor); + } + + @Override + public boolean isRegistered(Monitor monitor) { + for (Monitor m : monitors) { + if (m.equals(monitor)) + return true; + } + return false; + } +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricReaderTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricReaderTests.java index 50c372ca..7a99f150 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricReaderTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricReaderTests.java @@ -4,37 +4,24 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import org.junit.Ignore; import org.junit.Test; import org.springframework.boot.actuate.metrics.Metric; -import org.springframework.cloud.netflix.metrics.AbstractMetricsTests; +import org.springframework.cloud.netflix.metrics.SimpleMonitorRegistry; import com.google.common.collect.Lists; -import com.netflix.servo.DefaultMonitorRegistry; -import com.netflix.servo.MonitorRegistry; -import com.netflix.servo.monitor.BasicTimer; -import com.netflix.servo.monitor.Monitor; import com.netflix.servo.monitor.MonitorConfig; import static junit.framework.Assert.assertEquals; -public class ServoMetricReaderTests extends AbstractMetricsTests { +public class ServoMetricReaderTests { @Test - @Ignore public void singleCompositeMonitorYieldsMultipleActuatorMetrics() { - MonitorRegistry registry = DefaultMonitorRegistry.getInstance(); - - // deal with monitors registered in other tests - for (Monitor monitor : registry.getRegisteredMonitors()) { - registry.unregister(monitor); - } - - ServoMetricReader reader = new ServoMetricReader(registry, - new DimensionalServoMetricNaming()); + SimpleMonitorRegistry registry = new SimpleMonitorRegistry(); + ServoMetricReader reader = new ServoMetricReader(registry, new DimensionalServoMetricNaming()); MonitorConfig.Builder builder = new MonitorConfig.Builder("metricName"); - - BasicTimer timer = ServoMonitorCache.getTimer(builder.build()); + ServoMonitorCache servoMonitorCache = new ServoMonitorCache(registry); + servoMonitorCache.getTimer(builder.build()); List> metrics = Lists.newArrayList(reader.findAll()); diff --git a/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsAutoConfiguration.java b/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsAutoConfiguration.java index ca6d6f3d..639df62b 100644 --- a/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsAutoConfiguration.java +++ b/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsAutoConfiguration.java @@ -23,6 +23,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.cloud.netflix.metrics.DefaultMetricsTagProvider; import org.springframework.cloud.netflix.metrics.MetricsInterceptorConfiguration; import org.springframework.cloud.netflix.metrics.MetricsTagProvider; +import org.springframework.cloud.netflix.metrics.servo.ServoMonitorCache; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -66,6 +67,11 @@ public class SpectatorMetricsAutoConfiguration { return new ServoRegistry(); } + @Bean + public ServoMonitorCache monitorCache() { + return new ServoMonitorCache(monitorRegistry()); + } + @Bean @ConditionalOnMissingBean(MetricPoller.class) MetricPoller metricPoller() { diff --git a/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsHandlerInterceptorIntegrationTests.java b/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsHandlerInterceptorIntegrationTests.java index 885bbc5b..dca7d8c3 100644 --- a/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsHandlerInterceptorIntegrationTests.java +++ b/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricsHandlerInterceptorIntegrationTests.java @@ -67,6 +67,9 @@ public class SpectatorMetricsHandlerInterceptorIntegrationTests { @Autowired MonitorRegistry registry; + @Autowired + ServoMonitorCache servoMonitorCache; + MockMvc mvc; @Test @@ -117,7 +120,7 @@ public class SpectatorMetricsHandlerInterceptorIntegrationTests { if (exceptionType != null) builder = builder.withTag("exception", exceptionType); - BasicTimer timer = ServoMonitorCache.getTimer(builder.build()); + BasicTimer timer = servoMonitorCache.getTimer(builder.build()); Assert.assertEquals(1L, (long) timer.getCount()); } }