diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 446e24ca1..6f07f6a07 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -305,6 +305,21 @@ By default Sleuth assumes that when you send a span to Zipkin, you want the span spring.zipkin.service.name: foo ---- +=== Host locator + +In order to define the host that is corresponding to a particular span we need to resolve the host name +and port. The default approach is to take it from server properties. If those for some reason are not set +then we're trying to retrieve the host name from the network interfaces. + +If you have the discovery client enabled and prefer to retrieve the host address from the registered +instance in a service registry then you have to set the property (it's applicable for both HTTP and +Stream based span reporting). + +[source,yaml] +---- +spring.zipkin.locator.discovery.enabled: true +---- + == Span Data as Messages You can accumulate and send span data over diff --git a/spring-cloud-sleuth-stream/pom.xml b/spring-cloud-sleuth-stream/pom.xml index 48aa8ed39..1082b1a7a 100644 --- a/spring-cloud-sleuth-stream/pom.xml +++ b/spring-cloud-sleuth-stream/pom.xml @@ -27,7 +27,6 @@ org.springframework.cloud spring-cloud-commons - true org.springframework.boot 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 86b618f6e..172c80eb0 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 @@ -52,8 +52,8 @@ public class DiscoveryClientHostLocator implements HostLocator { @Override public Host locate(Span span) { ServiceInstance instance = this.client.getLocalServiceInstance(); - String serviceId = StringUtils.hasText(this.zipkinProperties.getName()) ? - this.zipkinProperties.getName() : instance.getServiceId(); + String serviceId = StringUtils.hasText(this.zipkinProperties.getService().getName()) ? + this.zipkinProperties.getService().getName() : instance.getServiceId(); return new Host(serviceId, getIpAddress(instance), instance.getPort()); } diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java index 37e33a582..1a9c90f4d 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java @@ -16,8 +16,14 @@ package org.springframework.cloud.sleuth.stream; +import java.lang.invoke.MethodHandles; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.cloud.sleuth.Span; import org.springframework.context.event.EventListener; import org.springframework.util.Assert; @@ -32,30 +38,37 @@ import org.springframework.util.StringUtils; *
  • port - from lazily assigned port or {@link ServerProperties}
    • * * - * You can override the value of service id by {@link ZipkinProperties#setName(String)} + * You can override the value of service id by {@link ZipkinProperties#getService()} * * @author Dave Syer * @since 1.0.0 */ public class ServerPropertiesHostLocator implements HostLocator { + private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + private final ServerProperties serverProperties; // Nullable private final String appName; + private final InetUtils inetUtils; private final ZipkinProperties zipkinProperties; private Integer port; // Lazy assigned @Deprecated - public ServerPropertiesHostLocator(ServerProperties serverProperties, - String appName) { - this(serverProperties, appName, new ZipkinProperties()); + public ServerPropertiesHostLocator(ServerProperties serverProperties, String appName) { + this(serverProperties, appName, new ZipkinProperties(),null); } - public ServerPropertiesHostLocator(ServerProperties serverProperties, - String appName, ZipkinProperties zipkinProperties) { + public ServerPropertiesHostLocator(ServerProperties serverProperties, String appName, + ZipkinProperties zipkinProperties, InetUtils inetUtils) { this.serverProperties = serverProperties; this.appName = appName; Assert.notNull(this.appName, "appName"); this.zipkinProperties = zipkinProperties; + if (inetUtils == null) { + this.inetUtils = new InetUtils(new InetUtilsProperties()); + } else { + this.inetUtils = inetUtils; + } } @Override @@ -91,21 +104,24 @@ public class ServerPropertiesHostLocator implements HostLocator { address = this.serverProperties.getAddress().getHostAddress(); } else { - address = "127.0.0.1"; + address = this.inetUtils.findFirstNonLoopbackAddress().getHostAddress(); } return address; } private String getServiceName(Span span) { String serviceName; - if (StringUtils.hasText(this.zipkinProperties.getName())) { - serviceName = this.zipkinProperties.getName(); + if (StringUtils.hasText(this.zipkinProperties.getService().getName())) { + serviceName = this.zipkinProperties.getService().getName(); } else if (span.getProcessId() != null) { serviceName = span.getProcessId(); } else { serviceName = this.appName; } + if (log.isDebugEnabled()) { + log.debug("Span will contain serviceName [" + serviceName + "]"); + } return serviceName; } 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 cd71a167a..eb027ff00 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 @@ -22,11 +22,11 @@ import org.springframework.boot.autoconfigure.AutoConfigureAfter; 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.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.metric.SpanMetricReporter; import org.springframework.cloud.sleuth.metric.TraceMetricsAutoConfiguration; @@ -91,7 +91,8 @@ public class SleuthStreamAutoConfiguration { } @Configuration - @ConditionalOnMissingClass("org.springframework.cloud.client.discovery.DiscoveryClient") + @ConditionalOnMissingBean(HostLocator.class) + @ConditionalOnProperty(value = "spring.zipkin.locator.discovery.enabled", havingValue = "false", matchIfMissing = true) protected static class DefaultEndpointLocatorConfiguration { @Autowired(required = false) @@ -100,18 +101,24 @@ public class SleuthStreamAutoConfiguration { @Autowired private ZipkinProperties zipkinProperties; + @Autowired + private InetUtils inetUtils; + @Value("${spring.application.name:unknown}") private String appName; @Bean public HostLocator zipkinEndpointLocator() { - return new ServerPropertiesHostLocator(this.serverProperties, this.appName, this.zipkinProperties); + return new ServerPropertiesHostLocator(this.serverProperties, this.appName, this.zipkinProperties, + this.inetUtils); } } @Configuration @ConditionalOnClass(DiscoveryClient.class) + @ConditionalOnMissingBean(HostLocator.class) + @ConditionalOnProperty(value = "spring.zipkin.locator.discovery.enabled", havingValue = "true") protected static class DiscoveryClientEndpointLocatorConfiguration { @Autowired(required = false) @@ -120,6 +127,9 @@ public class SleuthStreamAutoConfiguration { @Autowired private ZipkinProperties zipkinProperties; + @Autowired(required = false) + private InetUtils inetUtils; + @Value("${spring.application.name:unknown}") private String appName; @@ -131,7 +141,8 @@ public class SleuthStreamAutoConfiguration { if (this.client != null) { return new DiscoveryClientHostLocator(this.client, this.zipkinProperties); } - return new ServerPropertiesHostLocator(this.serverProperties, this.appName, this.zipkinProperties); + return new ServerPropertiesHostLocator(this.serverProperties, this.appName, this.zipkinProperties, + this.inetUtils); } } diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ZipkinProperties.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ZipkinProperties.java index 700ee1023..908de229c 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ZipkinProperties.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ZipkinProperties.java @@ -24,17 +24,66 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * @author Marcin Grzejszczak * @since 1.0.12 */ -@ConfigurationProperties("spring.zipkin.service") +@ConfigurationProperties("spring.zipkin") public class ZipkinProperties { - /** The name of the service, from which the Span was sent via Stream, that should appear in Zipkin */ - private String name; + private Service service = new Service(); - public String getName() { - return this.name; + private Locator locator = new Locator(); + + public Service getService() { + return this.service; } - public void setName(String name) { - this.name = name; + public void setService(Service service) { + this.service = service; + } + + public Locator getLocator() { + return this.locator; + } + + public void setLocator(Locator locator) { + this.locator = locator; + } + + public static class Service { + /** The name of the service, from which the Span was sent via Stream, that should appear in Zipkin */ + private String name; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + } + + public static class Locator { + + private Discovery discovery; + + public Discovery getDiscovery() { + return this.discovery; + } + + public void setDiscovery(Discovery discovery) { + this.discovery = discovery; + } + + public static class Discovery { + + /** Enabling of locating the host name via service discovery */ + private boolean enabled; + + public boolean isEnabled() { + return this.enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + } } } 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 new file mode 100644 index 000000000..c01dd1e16 --- /dev/null +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/DiscoveryClientEndpointLocatorConfigurationTest.java @@ -0,0 +1,94 @@ +package org.springframework.cloud.sleuth.stream; + +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.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Matcin Wielgus + */ +public class DiscoveryClientEndpointLocatorConfigurationTest { + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + EmptyConfiguration.class).run("--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(ServerPropertiesHostLocator.class); + } + } + + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class) + .run("--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(ServerPropertiesHostLocator.class); + } + } + + @Test + public void endpointLocatorShouldRespectExistingEndpointLocator() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithCustomLocator.class) + .run("--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + } + } + + @Test + public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run( + "--spring.zipkin.locator.discovery.enabled=true", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isInstanceOf(DiscoveryClientHostLocator.class); + } + } + + @Test + public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { + try (ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class, + ConfigurationWithCustomLocator.class).run( + "--spring.zipkin.locator.discovery.enabled=true", + "--spring.main.web_environment=false")) { + assertThat(ctxt.getBean(HostLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + } + } + + @Configuration + @EnableAutoConfiguration + public static class EmptyConfiguration { + } + + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithDiscoveryClient { + @Bean + public DiscoveryClient getDiscoveryClient() { + return Mockito.mock(DiscoveryClient.class); + } + } + + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithCustomLocator { + static HostLocator locator = Mockito.mock(HostLocator.class); + + @Bean + public HostLocator getEndpointLocator() { + return locator; + } + } +} \ No newline at end of file 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 46f4bf72d..b596eed16 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 @@ -67,7 +67,7 @@ public class DiscoveryClientHostLocatorTest { public void should_override_the_service_name_from_properties() throws Exception { given(this.discoveryClient.getLocalServiceInstance()).willReturn(serviceInstanceWithValidHost()); ZipkinProperties zipkinProperties = new ZipkinProperties(); - zipkinProperties.setName("foo"); + zipkinProperties.getService().setName("foo"); this.discoveryClientHostLocator = new DiscoveryClientHostLocator(this.discoveryClient, zipkinProperties); Host host = this.discoveryClientHostLocator.locate(null); diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocatorTests.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocatorTests.java index a63c02268..22ca4ea5c 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocatorTests.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocatorTests.java @@ -21,49 +21,57 @@ import java.net.UnknownHostException; import java.util.Collections; import org.junit.Test; +import org.mockito.Mockito; import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.cloud.sleuth.Span; import static org.assertj.core.api.Assertions.assertThat; public class ServerPropertiesHostLocatorTests { + + public static final byte[] ADR1234 = { 1, 2, 3, 4 }; + Span span = new Span(1, 3, "http:name", 1L, Collections.emptyList(), 2L, true, true, "process"); @Test - public void portDefaultsTo8080() { + public void portDefaultsTo8080() throws UnknownHostException { ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator( - new ServerProperties(), "unknown", new ZipkinProperties()); + new ServerProperties(), "unknown", new ZipkinProperties(), + localAddress(ADR1234)); assertThat(locator.locate(this.span).getPort()).isEqualTo((short) 8080); } @Test - public void portFromServerProperties() { + public void portFromServerProperties() throws UnknownHostException { ServerProperties properties = new ServerProperties(); properties.setPort(1234); ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(properties, - "unknown", new ZipkinProperties()); + "unknown", new ZipkinProperties(),localAddress(ADR1234)); assertThat(locator.locate(this.span).getPort()).isEqualTo((short) 1234); } @Test - public void portDefaultsToLocalhost() { + public void portDefaultsToLocalhost() throws UnknownHostException { ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator( - new ServerProperties(), "unknown", new ZipkinProperties()); + new ServerProperties(), "unknown", new ZipkinProperties(), + localAddress(ADR1234)); - assertThat(locator.locate(this.span).getAddress()).isEqualTo("127.0.0.1"); + assertThat(locator.locate(this.span).getAddress()).isEqualTo("1.2.3.4"); } @Test public void hostFromServerPropertiesIp() throws UnknownHostException { ServerProperties properties = new ServerProperties(); - properties.setAddress(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 })); + properties.setAddress(InetAddress.getByAddress(ADR1234)); ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(properties, - "unknown", new ZipkinProperties()); + "unknown", new ZipkinProperties(),localAddress(new byte[] { 1, 1, 1, 1 })); assertThat(locator.locate(this.span).getAddress()).isEqualTo("1.2.3.4"); } @@ -73,11 +81,18 @@ public class ServerPropertiesHostLocatorTests { ServerProperties properties = new ServerProperties(); properties.setAddress(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 })); ZipkinProperties zipkinProperties = new ZipkinProperties(); - zipkinProperties.setName("foo"); + zipkinProperties.getService().setName("foo"); ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(properties, - "unknown", zipkinProperties); + "unknown", zipkinProperties,localAddress(ADR1234)); assertThat(locator.locate(this.span).getServiceName()).isEqualTo("foo"); } + + private InetUtils localAddress(byte[] address) throws UnknownHostException { + InetUtils mocked = Mockito.spy(new InetUtils(new InetUtilsProperties())); + Mockito.when(mocked.findFirstNonLoopbackAddress()) + .thenReturn(InetAddress.getByAddress(address)); + return mocked; + } } diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfigurationTest.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfigurationTest.java index 3e0d02bf1..51cfab3bf 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfigurationTest.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/SleuthStreamAutoConfigurationTest.java @@ -10,6 +10,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.test.EnvironmentTestUtils; +import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.cloud.sleuth.log.NoOpSpanLogger; import org.springframework.cloud.sleuth.log.SpanLogger; @@ -120,7 +121,7 @@ public class SleuthStreamAutoConfigurationTest { @Import({ SleuthStreamAutoConfiguration.class, TraceMetricsAutoConfiguration.class, TestSupportBinderAutoConfiguration.class, ChannelBindingAutoConfiguration.class, TraceAutoConfiguration.class, - PropertyPlaceholderAutoConfiguration.class }) + PropertyPlaceholderAutoConfiguration.class, UtilAutoConfiguration.class }) public static class BaseConfiguration { @Bean SpanLogger spanLogger() { diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanListenerTests.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanListenerTests.java index c99df54f0..f26bb6399 100644 --- a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanListenerTests.java +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanListenerTests.java @@ -33,6 +33,7 @@ import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.metrics.CounterService; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanReporter; @@ -158,7 +159,7 @@ public class StreamSpanListenerTests { @Import({ ZipkinTestConfiguration.class, SleuthStreamAutoConfiguration.class, TraceMetricsAutoConfiguration.class, TestSupportBinderAutoConfiguration.class, ChannelBindingAutoConfiguration.class, TraceAutoConfiguration.class, - PropertyPlaceholderAutoConfiguration.class }) + PropertyPlaceholderAutoConfiguration.class, UtilAutoConfiguration.class }) protected static class TestConfiguration { } diff --git a/spring-cloud-sleuth-zipkin/pom.xml b/spring-cloud-sleuth-zipkin/pom.xml index 6750fef8c..9e6ebd7c5 100644 --- a/spring-cloud-sleuth-zipkin/pom.xml +++ b/spring-cloud-sleuth-zipkin/pom.xml @@ -40,7 +40,6 @@ org.springframework.cloud spring-cloud-commons - true org.springframework.boot diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/FallbackHavingEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/FallbackHavingEndpointLocator.java index 5ac4ca5d2..e9c58805d 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/FallbackHavingEndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/FallbackHavingEndpointLocator.java @@ -1,7 +1,5 @@ package org.springframework.cloud.sleuth.zipkin; -import java.util.concurrent.atomic.AtomicReference; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import zipkin.Endpoint; @@ -14,8 +12,6 @@ import zipkin.Endpoint; */ public class FallbackHavingEndpointLocator implements EndpointLocator { - private final AtomicReference cachedEndpoint = new AtomicReference<>(); - private static final Log log = LogFactory.getLog(FallbackHavingEndpointLocator.class); private final DiscoveryClientEndpointLocator discoveryClientEndpointLocator; @@ -29,8 +25,7 @@ public class FallbackHavingEndpointLocator implements EndpointLocator { @Override public Endpoint local() { - this.cachedEndpoint.compareAndSet(null, endpoint()); - return this.cachedEndpoint.get(); + return endpoint(); } private Endpoint endpoint() { diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java index b148b5ff9..8749914a9 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java @@ -17,12 +17,14 @@ package org.springframework.cloud.sleuth.zipkin; import java.lang.invoke.MethodHandles; +import java.nio.ByteBuffer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent; import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.context.event.EventListener; import org.springframework.util.StringUtils; @@ -47,20 +49,25 @@ public class ServerPropertiesEndpointLocator implements EndpointLocator { private final ServerProperties serverProperties; private final String appName; + private final InetUtils inetUtils; private final ZipkinProperties zipkinProperties; private Integer port; @Deprecated - public ServerPropertiesEndpointLocator(ServerProperties serverProperties, - String appName) { - this(serverProperties, appName, new ZipkinProperties()); + public ServerPropertiesEndpointLocator(ServerProperties serverProperties,String appName) { + this(serverProperties,appName,new ZipkinProperties(), null); } public ServerPropertiesEndpointLocator(ServerProperties serverProperties, - String appName, ZipkinProperties zipkinProperties) { + String appName, ZipkinProperties zipkinProperties, InetUtils inetUtils) { this.serverProperties = serverProperties; this.appName = appName; this.zipkinProperties = zipkinProperties; + if (inetUtils == null) { + this.inetUtils = new InetUtils(new InetUtilsProperties()); + } else { + this.inetUtils = inetUtils; + } } @Override @@ -97,11 +104,12 @@ public class ServerPropertiesEndpointLocator implements EndpointLocator { } private int getAddress() { - if (this.serverProperties!=null && this.serverProperties.getAddress() != null) { - return InetUtils.getIpAddressAsInt(this.serverProperties.getAddress().getHostAddress()); + if (this.serverProperties != null && this.serverProperties.getAddress() != null) { + return ByteBuffer.wrap(this.serverProperties.getAddress().getAddress()) + .getInt(); } else { - return 127 << 24 | 1; + return ByteBuffer.wrap(this.inetUtils.findFirstNonLoopbackAddress().getAddress()).getInt(); } } } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java index 829f7aa33..01082bdb1 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java @@ -21,11 +21,11 @@ import org.springframework.beans.factory.annotation.Value; 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.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.discovery.DiscoveryClient; +import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; @@ -70,7 +70,8 @@ public class ZipkinAutoConfiguration { } @Configuration - @ConditionalOnMissingClass("org.springframework.cloud.client.discovery.DiscoveryClient") + @ConditionalOnMissingBean(EndpointLocator.class) + @ConditionalOnProperty(value = "spring.zipkin.locator.discovery.enabled", havingValue = "false", matchIfMissing = true) protected static class DefaultEndpointLocatorConfiguration { @Autowired(required=false) @@ -79,19 +80,24 @@ public class ZipkinAutoConfiguration { @Autowired private ZipkinProperties zipkinProperties; + @Autowired(required=false) + private InetUtils inetUtils; + @Value("${spring.application.name:unknown}") private String appName; @Bean public EndpointLocator zipkinEndpointLocator() { return new ServerPropertiesEndpointLocator(this.serverProperties, this.appName, - this.zipkinProperties); + this.zipkinProperties, this.inetUtils); } } @Configuration @ConditionalOnClass(DiscoveryClient.class) + @ConditionalOnMissingBean(EndpointLocator.class) + @ConditionalOnProperty(value = "spring.zipkin.locator.discovery.enabled", havingValue = "true") protected static class DiscoveryClientEndpointLocatorConfiguration { @Autowired(required=false) @@ -100,6 +106,9 @@ public class ZipkinAutoConfiguration { @Autowired private ZipkinProperties zipkinProperties; + @Autowired(required=false) + private InetUtils inetUtils; + @Value("${spring.application.name:unknown}") private String appName; @@ -110,7 +119,7 @@ public class ZipkinAutoConfiguration { public EndpointLocator zipkinEndpointLocator() { return new FallbackHavingEndpointLocator(discoveryClientEndpointLocator(), new ServerPropertiesEndpointLocator(this.serverProperties, this.appName, - this.zipkinProperties)); + this.zipkinProperties, this.inetUtils)); } private DiscoveryClientEndpointLocator discoveryClientEndpointLocator() { diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java index 1a27fd047..1144d0d0c 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java @@ -34,6 +34,12 @@ public class ZipkinProperties { private Service service = new Service(); + private Locator locator = new Locator(); + + public Locator getLocator() { + return this.locator; + } + public String getBaseUrl() { return this.baseUrl; } @@ -74,6 +80,10 @@ public class ZipkinProperties { this.service = service; } + public void setLocator(Locator locator) { + this.locator = locator; + } + /** When enabled, spans are gzipped before sent to the zipkin server */ public static class Compression { @@ -102,4 +112,31 @@ public class ZipkinProperties { this.name = name; } } + + public static class Locator { + + private Discovery discovery; + + public Discovery getDiscovery() { + return this.discovery; + } + + public void setDiscovery(Discovery discovery) { + this.discovery = discovery; + } + + public static class Discovery { + + /** Enabling of locating the host name via service discovery */ + private boolean enabled; + + public boolean isEnabled() { + return this.enabled; + } + + public void setEnabled(boolean enabled) { + this.enabled = enabled; + } + } + } } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java index f00aa22d3..4b33fdf91 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java @@ -119,7 +119,7 @@ public class ZipkinSpanListener implements SpanReporter { return zipkinSpan.build(); } - private void ensureLocalComponent(Span span, zipkin.Span.Builder zipkinSpan) { + private void ensureLocalComponent(Span span, zipkin.Span.Builder zipkinSpan, Endpoint localEndpoint) { if (span.tags().containsKey(Constants.LOCAL_COMPONENT)) { return; } @@ -130,14 +130,14 @@ public class ZipkinSpanListener implements SpanReporter { .type(BinaryAnnotation.Type.STRING) .key("lc") // LOCAL_COMPONENT .value(processId) - .endpoint(this.endpointLocator.local()).build(); + .endpoint(localEndpoint).build(); zipkinSpan.addBinaryAnnotation(component); } - private void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan) { + private void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan, Endpoint localEndpoint) { if (span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)) { zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR, - this.endpointLocator.local().toBuilder().serviceName( + localEndpoint.toBuilder().serviceName( span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME)).build())); } } @@ -160,10 +160,10 @@ public class ZipkinSpanListener implements SpanReporter { } if (notClientOrServer) { // A zipkin span without any annotations cannot be queried, add special "lc" to avoid that. - ensureLocalComponent(span, zipkinSpan); + ensureLocalComponent(span, zipkinSpan, endpoint); } if (hasClientSend) { - ensureServerAddr(span, zipkinSpan); + ensureServerAddr(span, zipkinSpan, endpoint); } if (instanceIdToTag && this.environment != null) { setInstanceIdIfPresent(zipkinSpan, endpoint, Span.INSTANCEID); diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java new file mode 100644 index 000000000..510d18899 --- /dev/null +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocatorConfigurationTest.java @@ -0,0 +1,89 @@ +package org.springframework.cloud.sleuth.zipkin; + +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.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Matcin Wielgus + */ +public class DiscoveryClientEndpointLocatorConfigurationTest { + + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocator() { + ConfigurableApplicationContext ctxt = new SpringApplication( + EmptyConfiguration.class).run(); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(ServerPropertiesEndpointLocator.class); + ctxt.close(); + } + + @Test + public void endpointLocatorShouldDefaultToServerPropertiesEndpointLocatorEvenWhenDiscoveryClientPresent() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run(); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(ServerPropertiesEndpointLocator.class); + ctxt.close(); + } + + @Test + public void endpointLocatorShouldRespectExistingEndpointLocator() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithCustomLocator.class).run(); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + ctxt.close(); + } + + @Test + public void endpointLocatorShouldBeFallbackHavingEndpointLocatorWhenAskedTo() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class).run("--spring.zipkin.locator.discovery.enabled=true"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isInstanceOf(FallbackHavingEndpointLocator.class); + ctxt.close(); + } + + @Test + public void endpointLocatorShouldRespectExistingEndpointLocatorEvenWhenAskedToBeDiscovery() { + ConfigurableApplicationContext ctxt = new SpringApplication( + ConfigurationWithDiscoveryClient.class,ConfigurationWithCustomLocator.class).run("--spring.zipkin.locator.discovery.enabled=true"); + assertThat(ctxt.getBean(EndpointLocator.class)) + .isSameAs(ConfigurationWithCustomLocator.locator); + ctxt.close(); + } + + @Configuration + @EnableAutoConfiguration + public static class EmptyConfiguration { + } + + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithDiscoveryClient { + @Bean + public DiscoveryClient getDiscoveryClient() { + return Mockito.mock(DiscoveryClient.class); + } + } + + @Configuration + @EnableAutoConfiguration + public static class ConfigurationWithCustomLocator { + static EndpointLocator locator = Mockito.mock(EndpointLocator.class); + + @Bean + public EndpointLocator getEndpointLocator() { + return locator; + } + } + +} \ No newline at end of file diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/HttpZipkinSpanReporterTest.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/HttpZipkinSpanReporterTest.java index e5b0a9b1e..a7b9ddb82 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/HttpZipkinSpanReporterTest.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/HttpZipkinSpanReporterTest.java @@ -6,6 +6,8 @@ import java.util.concurrent.atomic.AtomicReference; import org.junit.Rule; import org.junit.Test; import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.cloud.sleuth.DefaultSpanNamer; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.log.NoOpSpanLogger; @@ -130,7 +132,7 @@ public class HttpZipkinSpanReporterTest { Tracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), new DefaultSpanNamer(), new NoOpSpanLogger(), new ZipkinSpanListener(receivedSpan::set, new ServerPropertiesEndpointLocator(new ServerProperties(), "foo", - new ZipkinProperties()))); + new ZipkinProperties(), new InetUtils(new InetUtilsProperties())))); // tag::service_name[] org.springframework.cloud.sleuth.Span newSpan = tracer.createSpan("redis"); try { diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocatorTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocatorTests.java index 2614388f5..92304542c 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocatorTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocatorTests.java @@ -20,46 +20,53 @@ import java.net.InetAddress; import java.net.UnknownHostException; import org.junit.Test; +import org.mockito.Mockito; import org.springframework.boot.autoconfigure.web.ServerProperties; +import org.springframework.cloud.commons.util.InetUtils; +import org.springframework.cloud.commons.util.InetUtilsProperties; import static org.assertj.core.api.Assertions.assertThat; public class ServerPropertiesEndpointLocatorTests { + public static final byte[] ADDRESS1234 = { 1, 2, 3, 4 }; + @Test - public void portDefaultsTo8080() { + public void portDefaultsTo8080() throws UnknownHostException { ServerPropertiesEndpointLocator locator = new ServerPropertiesEndpointLocator( - new ServerProperties(), "unknown", new ZipkinProperties()); + new ServerProperties(), "unknown", new ZipkinProperties(), + localAddress(ADDRESS1234)); assertThat(locator.local().port).isEqualTo((short) 8080); } @Test - public void portFromServerProperties() { + public void portFromServerProperties() throws UnknownHostException { ServerProperties properties = new ServerProperties(); properties.setPort(1234); ServerPropertiesEndpointLocator locator = new ServerPropertiesEndpointLocator( - properties, "unknown", new ZipkinProperties()); + properties, "unknown", new ZipkinProperties(),localAddress(ADDRESS1234)); assertThat(locator.local().port).isEqualTo((short) 1234); } @Test - public void portDefaultsToLocalhost() { + public void portDefaultsToLocalhost() throws UnknownHostException { ServerPropertiesEndpointLocator locator = new ServerPropertiesEndpointLocator( - new ServerProperties(), "unknown", new ZipkinProperties()); + new ServerProperties(), "unknown", new ZipkinProperties(), localAddress(ADDRESS1234)); - assertThat(locator.local().ipv4).isEqualTo(127 << 24 | 1); + assertThat(locator.local().ipv4).isEqualTo(1 << 24 | 2 << 16 | 3 << 8 | 4); } @Test public void hostFromServerPropertiesIp() throws UnknownHostException { ServerProperties properties = new ServerProperties(); - properties.setAddress(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 })); + properties.setAddress(InetAddress.getByAddress(ADDRESS1234)); ServerPropertiesEndpointLocator locator = new ServerPropertiesEndpointLocator( - properties, "unknown", new ZipkinProperties()); + properties, "unknown", new ZipkinProperties(), + localAddress(new byte[] { 4, 4, 4, 4 })); assertThat(locator.local().ipv4).isEqualTo(1 << 24 | 2 << 16 | 3 << 8 | 4); } @@ -71,8 +78,15 @@ public class ServerPropertiesEndpointLocatorTests { zipkinProperties.getService().setName("foo"); ServerPropertiesEndpointLocator locator = new ServerPropertiesEndpointLocator( - properties, "unknown", zipkinProperties); + properties, "unknown", zipkinProperties,localAddress(ADDRESS1234)); assertThat(locator.local().serviceName).isEqualTo("foo"); } + + private InetUtils localAddress(byte[] address) throws UnknownHostException { + InetUtils mocked = Mockito.spy(new InetUtils(new InetUtilsProperties())); + Mockito.when(mocked.findFirstNonLoopbackAddress()) + .thenReturn(InetAddress.getByAddress(address)); + return mocked; + } }