From a822e847119aa92d036cbd71438b29fe245e010f Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 18 Oct 2017 14:51:24 -0400 Subject: [PATCH] Update DiscoveryClientHostLocator to use Registration. Leave deprecated constructor with use of deprecated DiscoveryClient.getLocalServiceInstance() see https://github.com/spring-cloud/spring-cloud-commons/issues/265 --- .../stream/DiscoveryClientHostLocator.java | 20 ++++++++---- .../stream/SleuthStreamAutoConfiguration.java | 7 ++-- ...lientEndpointLocatorConfigurationTest.java | 14 ++++---- .../DiscoveryClientHostLocatorTest.java | 32 ++++++++++--------- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocator.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocator.java index 85ed39554..4dce11380 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocator.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocator.java @@ -35,22 +35,28 @@ import org.springframework.util.StringUtils; */ public class DiscoveryClientHostLocator implements HostLocator { - private final DiscoveryClient client; + private final ServiceInstance localServiceInstance; private final ZipkinProperties zipkinProperties; + @Deprecated public DiscoveryClientHostLocator(DiscoveryClient client, ZipkinProperties zipkinProperties) { - this.client = client; - Assert.notNull(this.client, "client"); + Assert.notNull(client, "client"); + this.localServiceInstance = client.getLocalServiceInstance(); + this.zipkinProperties = zipkinProperties; + } + + public DiscoveryClientHostLocator(ServiceInstance localServiceInstance, ZipkinProperties zipkinProperties) { + Assert.notNull(localServiceInstance, "localServiceInstance"); + this.localServiceInstance = localServiceInstance; this.zipkinProperties = zipkinProperties; } @Override public Host locate(Span span) { - ServiceInstance instance = this.client.getLocalServiceInstance(); String serviceId = StringUtils.hasText(this.zipkinProperties.getService().getName()) ? - this.zipkinProperties.getService().getName() : instance.getServiceId(); - return new Host(serviceId, getIpAddress(instance), - instance.getPort()); + this.zipkinProperties.getService().getName() : this.localServiceInstance.getServiceId(); + return new Host(serviceId, getIpAddress(this.localServiceInstance), + this.localServiceInstance.getPort()); } private String getIpAddress(ServiceInstance instance) { diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java index f2643aa72..82bef4969 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfiguration.java @@ -29,6 +29,7 @@ import org.springframework.boot.autoconfigure.integration.IntegrationAutoConfigu import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.client.serviceregistry.Registration; import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.SpanAdjuster; @@ -143,12 +144,12 @@ public class SleuthStreamAutoConfiguration { private Environment environment; @Autowired(required = false) - private DiscoveryClient client; + private Registration registration; @Bean public HostLocator zipkinEndpointLocator() { - if (this.client != null) { - return new DiscoveryClientHostLocator(this.client, this.zipkinProperties); + if (this.registration != null) { + return new DiscoveryClientHostLocator(this.registration, this.zipkinProperties); } return new ServerPropertiesHostLocator(this.serverProperties, this.environment, this.zipkinProperties, this.inetUtils); diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java index 2aeb0b5d1..12969ed31 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java @@ -4,7 +4,7 @@ import org.junit.Test; import org.mockito.Mockito; import org.springframework.boot.SpringApplication; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; -import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.client.serviceregistry.Registration; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -28,7 +28,7 @@ public class DiscoveryClientEndpointLocatorConfigurationTest { @Test public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false", + ConfigurationWithRegistration.class).run("--spring.jmx.enabled=false", "--spring.main.web_environment=false")) { assertThat(ctxt.getBean(HostLocator.class)) .isInstanceOf(ServerPropertiesHostLocator.class); @@ -48,7 +48,7 @@ public class DiscoveryClientEndpointLocatorConfigurationTest { @Test public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class).run("--spring.jmx.enabled=false", + ConfigurationWithRegistration.class).run("--spring.jmx.enabled=false", "--spring.zipkin.locator.discovery.enabled=true", "--spring.main.web_environment=false")) { assertThat(ctxt.getBean(HostLocator.class)) @@ -59,7 +59,7 @@ public class DiscoveryClientEndpointLocatorConfigurationTest { @Test public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { try (ConfigurableApplicationContext ctxt = new SpringApplication( - ConfigurationWithDiscoveryClient.class, + ConfigurationWithRegistration.class, ConfigurationWithCustomLocator.class).run("--spring.jmx.enabled=false", "--spring.zipkin.locator.discovery.enabled=true", "--spring.main.web_environment=false")) { @@ -75,9 +75,9 @@ public class DiscoveryClientEndpointLocatorConfigurationTest { @Configuration @EnableAutoConfiguration - public static class ConfigurationWithDiscoveryClient { - @Bean public DiscoveryClient getDiscoveryClient() { - return Mockito.mock(DiscoveryClient.class); + public static class ConfigurationWithRegistration { + @Bean public Registration registration() { + return Mockito.mock(Registration.class); } } diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocatorTest.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocatorTest.java index fcee652ee..b9d6bc130 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocatorTest.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientHostLocatorTest.java @@ -20,32 +20,35 @@ import java.net.URI; import java.util.Map; import org.junit.Test; -import org.mockito.Mockito; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.client.serviceregistry.Registration; import org.springframework.cloud.commons.util.InetUtils; import static org.assertj.core.api.BDDAssertions.then; -import static org.mockito.BDDMockito.given; /** * @author Marcin Grzejszczak */ public class DiscoveryClientHostLocatorTest { - DiscoveryClient discoveryClient = Mockito.mock(DiscoveryClient.class); - DiscoveryClientHostLocator discoveryClientHostLocator = - new DiscoveryClientHostLocator(this.discoveryClient, new ZipkinProperties()); @Test(expected = IllegalArgumentException.class) - public void should_throw_exception_when_no_discovery_client_is_present() throws Exception { - new DiscoveryClientHostLocator(null, new ZipkinProperties()); + public void should_throw_exception_when_no_registration_is_present() throws Exception { + new DiscoveryClientHostLocator((Registration)null, new ZipkinProperties()); + } + + private DiscoveryClientHostLocator hostLocator(ServiceInstance serviceInstance) { + return hostLocator(serviceInstance, new ZipkinProperties()); + } + + private DiscoveryClientHostLocator hostLocator(ServiceInstance serviceInstance, ZipkinProperties zipkinProperties) { + return new DiscoveryClientHostLocator(serviceInstance, zipkinProperties); } @Test public void should_create_Host_with_0_ip_when_exception_occurs_on_resolving_host() throws Exception { - given(this.discoveryClient.getLocalServiceInstance()).willReturn(serviceInstanceWithInvalidHost()); + DiscoveryClientHostLocator hostLocator = hostLocator(serviceInstanceWithInvalidHost()); - Host host = this.discoveryClientHostLocator.locate(null); + Host host = hostLocator.locate(null); then(host.getServiceName()).isEqualTo("serviceId"); then(host.getPort()).isEqualTo((short)8_000); @@ -54,9 +57,9 @@ public class DiscoveryClientHostLocatorTest { @Test public void should_create_valid_Host_when_proper_host_is_passed() throws Exception { - given(this.discoveryClient.getLocalServiceInstance()).willReturn(serviceInstanceWithValidHost()); + DiscoveryClientHostLocator hostLocator = hostLocator(serviceInstanceWithValidHost()); - Host host = this.discoveryClientHostLocator.locate(null); + Host host = hostLocator.locate(null); then(host.getServiceName()).isEqualTo("serviceId"); then(host.getPort()).isEqualTo((short)8_000); @@ -65,12 +68,11 @@ public class DiscoveryClientHostLocatorTest { @Test public void should_override_the_service_name_from_properties() throws Exception { - given(this.discoveryClient.getLocalServiceInstance()).willReturn(serviceInstanceWithValidHost()); ZipkinProperties zipkinProperties = new ZipkinProperties(); zipkinProperties.getService().setName("foo"); - this.discoveryClientHostLocator = new DiscoveryClientHostLocator(this.discoveryClient, zipkinProperties); + DiscoveryClientHostLocator hostLocator = new DiscoveryClientHostLocator(serviceInstanceWithValidHost(), zipkinProperties); - Host host = this.discoveryClientHostLocator.locate(null); + Host host = hostLocator.locate(null); then(host.getServiceName()).isEqualTo("foo"); then(host.getPort()).isEqualTo((short)8_000);