From c7c191c01ee38a67719d78f09fedb513e41f3e56 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 28 Apr 2016 11:57:00 +0100 Subject: [PATCH] Ensure Servo metrics are exported the same as default metrics Adds an `@ExportMetricReader` which was missing before and led to people losing their metric exports when they had Servo on the classpath. Also allow servo metrics autoconfiguration to be disabled via a flag netflix.metrics.servo.enabled=false (as an alternative to excluding the class in `@EnableAutoConfiguration`). --- .../servo/ServoMetricsAutoConfiguration.java | 26 ++++++--- .../metrics/servo/ServoMetricsConfigBean.java | 24 +++++++- .../NoServoMetricsAutoConfigurationTests.java | 58 +++++++++++++++++++ .../metrics/servo/ServoMetricReaderTests.java | 6 +- .../ServoMetricsAutoConfigurationTests.java | 57 ++++++++++++++++++ .../cloud/netflix/zuul/ZuulProxyTestBase.java | 5 +- 6 files changed, 163 insertions(+), 13 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/NoServoMetricsAutoConfigurationTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfigurationTests.java 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 091c9738..08306afd 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 @@ -1,11 +1,11 @@ /* * Copyright 2013-2014 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. @@ -13,6 +13,7 @@ package org.springframework.cloud.netflix.metrics.servo; +import org.springframework.boot.actuate.autoconfigure.ExportMetricReader; import org.springframework.boot.actuate.autoconfigure.MetricRepositoryAutoConfiguration; import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics; import org.springframework.boot.actuate.metrics.CounterService; @@ -22,6 +23,7 @@ import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.netflix.metrics.DefaultMetricsTagProvider; import org.springframework.cloud.netflix.metrics.MetricsInterceptorConfiguration; import org.springframework.cloud.netflix.metrics.MetricsTagProvider; @@ -45,6 +47,7 @@ import com.netflix.servo.monitor.Monitors; @ConditionalOnMissingClass("com.netflix.spectator.api.Registry") @AutoConfigureBefore(MetricRepositoryAutoConfiguration.class) @Import(MetricsInterceptorConfiguration.class) +@ConditionalOnProperty(name = "spring.metrics.servo.enabled", matchIfMissing = true) public class ServoMetricsAutoConfiguration { @Bean @ConditionalOnMissingBean @@ -61,8 +64,9 @@ public class ServoMetricsAutoConfiguration { @Bean @ConditionalOnMissingBean public MonitorRegistry monitorRegistry(ServoMetricsConfigBean servoMetricsConfig) { - System.setProperty(DefaultMonitorRegistry.class.getCanonicalName() + ".registryClass", servoMetricsConfig - .getRegistryClass()); + System.setProperty( + DefaultMonitorRegistry.class.getCanonicalName() + ".registryClass", + servoMetricsConfig.getRegistryClass()); return DefaultMonitorRegistry.getInstance(); } @@ -72,8 +76,16 @@ public class ServoMetricsAutoConfiguration { } @Bean - public MetricReaderPublicMetrics servoPublicMetrics(MonitorRegistry monitorRegistry, ServoMetricNaming servoMetricNaming) { - ServoMetricReader reader = new ServoMetricReader(monitorRegistry, servoMetricNaming); + @ExportMetricReader + public ServoMetricReader servoMetricReader(MonitorRegistry monitorRegistry, + ServoMetricNaming servoMetricNaming) { + ServoMetricReader reader = new ServoMetricReader(monitorRegistry, + servoMetricNaming); + return reader; + } + + @Bean + public MetricReaderPublicMetrics servoPublicMetrics(ServoMetricReader reader) { return new MetricReaderPublicMetrics(reader); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsConfigBean.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsConfigBean.java index f5eac395..eb69c153 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsConfigBean.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsConfigBean.java @@ -22,9 +22,31 @@ import org.springframework.boot.context.properties.ConfigurationProperties; */ @ConfigurationProperties("netflix.metrics.servo") public class ServoMetricsConfigBean { + + /** + * Enable the Netflix Servo metrics services. If this flag is off Servo can still be + * used by Netflix OSS components, but the Spring Boot metrics collection will be done + * with the default services. + */ + boolean enabled = true; + /** + * Fully qualified class name for monitor registry used by Servo. + */ String registryClass = "com.netflix.servo.BasicMonitorRegistry"; public String getRegistryClass() { - return registryClass; + return this.registryClass; + } + + public boolean getEnabled() { + return this.enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + + public void setRegistryClass(String registryClass) { + this.registryClass = registryClass; } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/NoServoMetricsAutoConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/NoServoMetricsAutoConfigurationTests.java new file mode 100644 index 00000000..fc4a1498 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/NoServoMetricsAutoConfigurationTests.java @@ -0,0 +1,58 @@ +/* + * 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.servo; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.actuate.autoconfigure.ExportMetricReader; +import org.springframework.boot.actuate.metrics.reader.MetricReader; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * @author Dave Syer + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration() +public class NoServoMetricsAutoConfigurationTests { + + @Autowired(required = false) + private ServoMetricNaming naming; + + @Autowired(required = false) + @ExportMetricReader + private MetricReader reader; + + @Test + public void test() { + assertNull(this.naming); + assertNotNull(this.reader); + } + + @EnableAutoConfiguration(exclude = ServoMetricsAutoConfiguration.class) + @Configuration + protected static class TestConfiguration { + + } + +} 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 7a99f150..33561961 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 @@ -11,13 +11,14 @@ import org.springframework.cloud.netflix.metrics.SimpleMonitorRegistry; import com.google.common.collect.Lists; import com.netflix.servo.monitor.MonitorConfig; -import static junit.framework.Assert.assertEquals; +import static org.junit.Assert.assertEquals; public class ServoMetricReaderTests { @Test public void singleCompositeMonitorYieldsMultipleActuatorMetrics() { SimpleMonitorRegistry registry = new SimpleMonitorRegistry(); - ServoMetricReader reader = new ServoMetricReader(registry, new DimensionalServoMetricNaming()); + ServoMetricReader reader = new ServoMetricReader(registry, + new DimensionalServoMetricNaming()); MonitorConfig.Builder builder = new MonitorConfig.Builder("metricName"); ServoMonitorCache servoMonitorCache = new ServoMonitorCache(registry); @@ -42,4 +43,3 @@ public class ServoMetricReaderTests { metricNames.get(3)); } } - diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfigurationTests.java new file mode 100644 index 00000000..1010e8b0 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/metrics/servo/ServoMetricsAutoConfigurationTests.java @@ -0,0 +1,57 @@ +/* + * 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.servo; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.actuate.autoconfigure.ExportMetricReader; +import org.springframework.boot.actuate.metrics.reader.MetricReader; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.junit.Assert.assertNotNull; + +/** + * @author Dave Syer + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration() +public class ServoMetricsAutoConfigurationTests { + + @Autowired(required = false) + private ServoMetricNaming naming; + + @Autowired(required = false) + @ExportMetricReader + private MetricReader reader; + + @Test + public void test() { + assertNotNull(this.naming); + assertNotNull(this.reader); + } + + @EnableAutoConfiguration + @Configuration + protected static class TestConfiguration { + + } + +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java index 3cd32427..4299e157 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyTestBase.java @@ -184,7 +184,8 @@ public abstract class ZuulProxyTestBase { assertEquals("Received {key=[overridden]}", result.getBody()); } - protected static abstract class AbstractZuulProxyApplication extends DelegatingWebMvcConfiguration { + protected static abstract class AbstractZuulProxyApplication + extends DelegatingWebMvcConfiguration { @RequestMapping("/testing123") public String testing123() { @@ -202,7 +203,7 @@ public abstract class ZuulProxyTestBase { } @RequestMapping(value = "/local/{id}", method = RequestMethod.GET) - public ResponseEntity get(@PathVariable String id) { + public ResponseEntity get(@PathVariable String id) { if ("notfound".equalsIgnoreCase(id)) { return ResponseEntity.notFound().build(); }