Ensure servo metrics contribute to health indicator

Without this change the health status is always UNKNOWN because it
never finds the servo metrics for the health indicator. It's also
a general spruce up of the servo and eureka metrics and health
infrastructure which was looking a little more complicated than
it needed to be (no need for a separate MetricReader).

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java

@@ -70,8 +70,7 @@ public class EurekaClientAutoConfiguration {
 	@Bean
 	@ConditionalOnMissingBean
 	public EurekaHealthIndicator eurekaHealthIndicator(EurekaInstanceConfig config) {
-		return new EurekaHealthIndicator(discoveryClient, new ServoMetricReader(),
-				config);
+		return new EurekaHealthIndicator(discoveryClient, new ServoMetricReader(), config);
 	}
 
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/eureka/EurekaHealthIndicator.java

@@ -66,7 +66,7 @@ public class EurekaHealthIndicator implements HealthIndicator {
 				"Remote status from Eureka server");
 		@SuppressWarnings("unchecked")
 		Metric<Number> value = (Metric<Number>) metrics
-				.findOne("counter.servo.DiscoveryClient_Failed");
+				.findOne("counter.servo.discoveryclient_failed");
 		if (value != null) {
 			int renewalPeriod = instanceConfig.getLeaseRenewalIntervalInSeconds();
 			int latest = value.getValue().intValue();

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignConfiguration.java

@@ -1,17 +1,21 @@
 package org.springframework.cloud.netflix.feign;
 
-import feign.*;
+import java.net.URI;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.cloud.netflix.archaius.ConfigurableEnvironmentConfiguration;
+import org.springframework.context.annotation.Configuration;
+
+import feign.Client;
+import feign.Contract;
+import feign.Feign;
+import feign.Logger;
+import feign.Request;
+import feign.Retryer;
 import feign.codec.Decoder;
 import feign.codec.Encoder;
 import feign.codec.ErrorDecoder;
 import feign.ribbon.LoadBalancingTarget;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.cloud.netflix.ribbon.RibbonClientPreprocessor;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.cloud.netflix.archaius.ConfigurableEnvironmentConfiguration;
-
-import javax.inject.Inject;
-import java.net.URI;
 
 /**
  * @author Spencer Gibb
@@ -21,9 +25,6 @@ public class FeignConfiguration {
     @Autowired
     ConfigurableEnvironmentConfiguration envConfig; //FIXME: howto enforce this?
 
-    @Autowired
-    RibbonClientPreprocessor ribbonClientPreprocessor;
-
     @Autowired
     Decoder decoder;
 
@@ -78,9 +79,6 @@ public class FeignConfiguration {
 
     protected <T> T loadBalance(Feign.Builder builder, Class<T> type, String schemeName) {
         String name = URI.create(schemeName).getHost();
-        // TODO: This should be transparent
-        ribbonClientPreprocessor.preprocess(name);
-
         if(ribbonClient != null) {
             return builder.client(ribbonClient).target(type, schemeName);
         } else {

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java

@@ -58,19 +58,4 @@ public class RibbonAutoConfiguration {
 		return new RibbonInterceptor(loadBalancerClient);
 	}
 
-	@Configuration
-	protected static class DefaultRibbonClientPreprocessor {
-
-		@Bean
-		@ConditionalOnMissingBean(RibbonClientPreprocessor.class)
-		public RibbonClientPreprocessor ribbonClientPreprocessor() {
-			return new RibbonClientPreprocessor() {
-				@Override
-				public void preprocess(String serviceId) {
-					// no-op
-				}
-			};
-		}
-
-	}
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java

@@ -15,9 +15,6 @@
  */
 package org.springframework.cloud.netflix.ribbon;
 
-import javax.annotation.PostConstruct;
-
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
@@ -52,19 +49,9 @@ public class RibbonClientConfiguration {
 	@Value("${ribbon.eureka.approximateZoneFromHostname:false}")
 	private boolean approximateZoneFromHostname = false;
 
-	@Autowired(required = false)
-	private RibbonClientPreprocessor preprocessor;
-
 	// TODO: maybe re-instate autowired load balancers: identified by name they could be
 	// associated with ribbon clients
 	
-	@PostConstruct
-	public void init() {
-		if (preprocessor!=null) {			
-			preprocessor.preprocess(name);
-		}
-	}
-
 	@Bean
 	@ConditionalOnMissingBean
 	public IClientConfig ribbonClientConfig() {

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationRegistrar.java

@@ -41,6 +41,10 @@ public class RibbonClientConfigurationRegistrar implements ImportBeanDefinitionR
 						client.get("configuration"));
 			}
 		}
+		if (attrs != null && attrs.containsKey("defaultConfiguration")) {
+			registerClientConfiguration(registry, "default." + metadata.getEnclosingClassName(),
+					attrs.get("defaultConfiguration"));
+		}
 		Map<String, Object> client = metadata.getAnnotationAttributes(
 				RibbonClient.class.getName(), true);
 		if (client != null && client.containsKey("name")) {
@@ -55,7 +59,7 @@ public class RibbonClientConfigurationRegistrar implements ImportBeanDefinitionR
 				.genericBeanDefinition(RibbonClientSpecification.class);
 		builder.addConstructorArgValue(name);
 		builder.addConstructorArgValue(configuration);
-		registry.registerBeanDefinition(name + "RibbonClientSpecification",
+		registry.registerBeanDefinition(name + ".RibbonClientSpecification",
 				builder.getBeanDefinition());
 	}
 

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientPreprocessor.java

@@ -1,10 +0,0 @@
-package org.springframework.cloud.netflix.ribbon;
-
-/**
- * Allows different service discovery implementations to configure ribbon prior to usage.
- * TODO: this could potentially be done via AOP
- * @author Spencer Gibb
- */
-public interface RibbonClientPreprocessor {
-    public void preprocess(String serviceId);
-}

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClients.java

@@ -40,4 +40,6 @@ public @interface RibbonClients {
 
 	RibbonClient[] value() default {};
 
+	Class<?>[] defaultConfiguration() default {};
+
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/SpringClientFactory.java

@@ -4,6 +4,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.springframework.beans.BeanUtils;
@@ -98,6 +99,13 @@ public class SpringClientFactory implements DisposableBean, ApplicationContextAw
 				context.register(configuration);
 			}
 		}
+		for (Entry<String, RibbonClientSpecification> entry : configurations.entrySet()) {
+			if (entry.getKey().startsWith("default.")) {
+				for (Class<?> configuration : entry.getValue().getConfiguration()) {
+					context.register(configuration);
+				}	
+			}
+		}
 		context.register(PropertyPlaceholderAutoConfiguration.class,
 				RibbonClientConfiguration.class);
 		context.getEnvironment()

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfiguration.java

@@ -6,7 +6,11 @@ import static com.netflix.client.config.CommonClientConfigKey.NFLoadBalancerRule
 import static com.netflix.client.config.CommonClientConfigKey.NIWSServerListClassName;
 import static com.netflix.client.config.CommonClientConfigKey.NIWSServerListFilterClassName;
 
-import org.springframework.cloud.netflix.ribbon.RibbonClientPreprocessor;
+import javax.annotation.PostConstruct;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.Configuration;
 
 import com.netflix.config.ConfigurationManager;
 import com.netflix.config.DeploymentContext.ContextKey;
@@ -24,19 +28,29 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList;
  * @author Spencer Gibb
  * @author Dave Syer
  */
-public class EurekaRibbonClientPreprocessor implements RibbonClientPreprocessor {
+@Configuration
+public class EurekaRibbonClientConfiguration {
+
+	@Value("${ribbon.client.name}")
+	private String serviceId = "client";
 
 	protected static final String VALUE_NOT_SET = "__not__set__";
 	protected static final String DEFAULT_NAMESPACE = "ribbon";
 
+	@Autowired(required = false)
 	private EurekaClientConfig clientConfig;
 
-	public EurekaRibbonClientPreprocessor(EurekaClientConfig clientConfig) {
-		this.clientConfig = clientConfig;
+	public EurekaRibbonClientConfiguration() {
 	}
 
-	@Override
-	public void preprocess(String serviceId) {
+	public EurekaRibbonClientConfiguration(EurekaClientConfig clientConfig,
+			String serviceId) {
+		this.clientConfig = clientConfig;
+		this.serviceId = serviceId;
+	}
+
+	@PostConstruct
+	public void preprocess() {
 		if (clientConfig != null
 				&& ConfigurationManager.getDeploymentContext().getValue(ContextKey.zone) == null) {
 			String[] zones = clientConfig.getAvailabilityZones(clientConfig.getRegion());
@@ -49,14 +63,12 @@ public class EurekaRibbonClientPreprocessor implements RibbonClientPreprocessor
 			}
 		}
 		// TODO: should this look more like hibernate spring boot props?
-		// TODO: only set the property if it hasn't already been set?
 		setProp(serviceId, NIWSServerListClassName.key(),
 				DiscoveryEnabledNIWSServerList.class.getName());
 		// FIXME: what should this be?
 		setProp(serviceId, DeploymentContextBasedVipAddresses.key(), serviceId);
 		setProp(serviceId, NFLoadBalancerRuleClassName.key(),
 				ZoneAvoidanceRule.class.getName());
-		// TODO: use bean name indirection to get this filter to be a @Bean
 		setProp(serviceId, NIWSServerListFilterClassName.key(),
 				ZonePreferenceServerListFilter.class.getName());
 		setProp(serviceId, EnableZoneAffinity.key(), "true");

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/eureka/RibbonEurekaAutoConfiguration.java

@@ -15,19 +15,16 @@
  */
 package org.springframework.cloud.netflix.ribbon.eureka;
 
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration;
-import org.springframework.cloud.netflix.ribbon.RibbonClientPreprocessor;
+import org.springframework.cloud.netflix.ribbon.RibbonClients;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
-import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
-import com.netflix.discovery.EurekaClientConfig;
 import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList;
 
 /**
@@ -40,13 +37,6 @@ import com.netflix.niws.loadbalancer.DiscoveryEnabledNIWSServerList;
 @ConditionalOnBean(SpringClientFactory.class)
 @ConditionalOnExpression("${ribbon.eureka.enabled:true}")
 @AutoConfigureAfter(RibbonAutoConfiguration.class)
+@RibbonClients(defaultConfiguration = EurekaRibbonClientConfiguration.class)
 public class RibbonEurekaAutoConfiguration {
-	
-	@Autowired(required=false)
-	private EurekaClientConfig clientConfig;
-
-    @Bean
-    public RibbonClientPreprocessor ribbonClientPreprocessor() {
-        return new EurekaRibbonClientPreprocessor(clientConfig);
-    }
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/servo/ServoMetricsAutoConfiguration.java

@@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.servo;
 
 import org.springframework.boot.actuate.autoconfigure.EndpointAutoConfiguration;
 import org.springframework.boot.actuate.autoconfigure.MetricRepositoryAutoConfiguration;
+import org.springframework.boot.actuate.endpoint.MetricReaderPublicMetrics;
 import org.springframework.boot.actuate.metrics.reader.MetricReader;
 import org.springframework.boot.autoconfigure.AutoConfigureAfter;
 import org.springframework.boot.autoconfigure.AutoConfigureBefore;
@@ -41,10 +42,10 @@ import com.netflix.servo.monitor.Monitors;
 @AutoConfigureBefore(EndpointAutoConfiguration.class)
 @AutoConfigureAfter({MetricRepositoryAutoConfiguration.class})
 public class ServoMetricsAutoConfiguration {
-
+	
 	@Bean
 	@ConditionalOnMissingBean
-	public ServoPublicMetrics servoPublicMetrics(MetricReader reader) {
-		return new ServoPublicMetrics(reader);
+	public MetricReaderPublicMetrics servoPublicMetrics() {
+		return new MetricReaderPublicMetrics(new ServoMetricReader());
 	}
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/servo/ServoPublicMetrics.java

@@ -1,52 +0,0 @@
-/*
- * 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.
- */
-
-package org.springframework.cloud.netflix.servo;
-
-import java.util.Collection;
-
-import org.springframework.boot.actuate.endpoint.PublicMetrics;
-import org.springframework.boot.actuate.endpoint.VanillaPublicMetrics;
-import org.springframework.boot.actuate.metrics.Metric;
-import org.springframework.boot.actuate.metrics.reader.MetricReader;
-
-/**
- * {@link PublicMetrics} implementation for Servo metrics.
- * 
- * @author Dave Syer
- * @author Christian Dupuis
- */
-public class ServoPublicMetrics extends VanillaPublicMetrics {
-
-	private final ServoMetricReader servo;
-
-	public ServoPublicMetrics(MetricReader reader) {
-		super(reader);
-		this.servo = new ServoMetricReader();
-	}
-
-	@Override
-	public Collection<Metric<?>> metrics() {
-		Collection<Metric<?>> metrics = super.metrics();
-		if (servo != null) {
-			for (Metric<?> metric : servo.findAll()) {
-				metrics.add(metric);
-			}
-		}
-		return metrics;
-	}
-
-}

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java

@@ -9,7 +9,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.context.config.annotation.RefreshScope;
-import org.springframework.cloud.netflix.ribbon.RibbonClientPreprocessor;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
 import org.springframework.cloud.netflix.zuul.filters.post.SendErrorFilter;
 import org.springframework.cloud.netflix.zuul.filters.post.SendResponseFilter;
@@ -36,9 +35,6 @@ public class ZuulConfiguration {
 	@Autowired(required = false)
 	private TraceRepository traces;
 
-	@Autowired
-	private RibbonClientPreprocessor preprocessor;
-	
 	@Autowired
 	private SpringClientFactory clientFactory;
 
@@ -101,7 +97,7 @@ public class ZuulConfiguration {
 	// route filters
 	@Bean
 	public RibbonRoutingFilter ribbonRoutingFilter() {
-		RibbonRoutingFilter filter = new RibbonRoutingFilter(preprocessor, clientFactory);
+		RibbonRoutingFilter filter = new RibbonRoutingFilter(clientFactory);
 		if (traces != null) {
 			filter.setTraces(traces);
 		}

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java

@@ -17,7 +17,6 @@ import org.apache.commons.io.IOUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.actuate.trace.TraceRepository;
-import org.springframework.cloud.netflix.ribbon.RibbonClientPreprocessor;
 import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
 import org.springframework.util.StringUtils;
 
@@ -40,13 +39,9 @@ public class RibbonRoutingFilter extends ZuulFilter {
 
 	private TraceRepository traces;
 
-	private RibbonClientPreprocessor preprocessor;
-
 	private SpringClientFactory clientFactory;
 
-	public RibbonRoutingFilter(RibbonClientPreprocessor preprocessor,
-			SpringClientFactory clientFactory) {
-		this.preprocessor = preprocessor;
+	public RibbonRoutingFilter(SpringClientFactory clientFactory) {
 		this.clientFactory = clientFactory;
 	}
 

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java

@@ -30,9 +30,6 @@ import com.netflix.loadbalancer.ServerStats;
  */
 public class RibbonLoadBalancerClientTests {
 
-    @Mock
-    RibbonClientPreprocessor preprocessor;
-
     @Mock
     SpringClientFactory clientFactory;
 

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/eureka/EurekaRibbonClientConfigurationTests.java

@@ -18,7 +18,7 @@ package org.springframework.cloud.netflix.ribbon.eureka;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.springframework.cloud.netflix.ribbon.eureka.EurekaRibbonClientPreprocessor.VALUE_NOT_SET;
+import static org.springframework.cloud.netflix.ribbon.eureka.EurekaRibbonClientConfiguration.VALUE_NOT_SET;
 
 import org.junit.After;
 import org.junit.Test;
@@ -36,7 +36,7 @@ import com.netflix.loadbalancer.ZoneAwareLoadBalancer;
  * @author Dave Syer
  *
  */
-public class EurekaRibbonClientPreprocessorTests {
+public class EurekaRibbonClientConfigurationTests {
 
 	@After
 	public void close() {
@@ -48,9 +48,9 @@ public class EurekaRibbonClientPreprocessorTests {
 		EurekaClientConfigBean client = new EurekaClientConfigBean();
 		client.getAvailabilityZones().put(client.getRegion(), "foo");
 		SpringClientFactory clientFactory = new SpringClientFactory();
-		EurekaRibbonClientPreprocessor clientPreprocessor = new EurekaRibbonClientPreprocessor(
-				client);
-		clientPreprocessor.preprocess("service");
+		EurekaRibbonClientConfiguration clientPreprocessor = new EurekaRibbonClientConfiguration(
+				client, "service");
+		clientPreprocessor.preprocess();
 		ILoadBalancer balancer = clientFactory.getLoadBalancer("service");
 		assertNotNull(balancer);
 		@SuppressWarnings("unchecked")
@@ -63,8 +63,8 @@ public class EurekaRibbonClientPreprocessorTests {
 	@Test
 	public void testSetProp() {
 		EurekaClientConfigBean client = new EurekaClientConfigBean();
-		EurekaRibbonClientPreprocessor preprocessor = new EurekaRibbonClientPreprocessor(
-				client);
+		EurekaRibbonClientConfiguration preprocessor = new EurekaRibbonClientConfiguration(
+				client, "myService");
 
 		String serviceId = "myService";
 		String suffix = "mySuffix";