Due to possible performance problems when DiscoveryClient gives nonexistent DNS name as local address Local endpoint is created from server properties by default.

Local Ip determined from spring.commons InetUtils.
    To enable local endpoint resolution via service discovery, DiscoveryClient must be present and 'spring.zipkin.service.locator.discovery' should be set to true.
    FallbackHavingEndpointLocator was caching and recreating EndpointLocator in the same time - removed the caching part.

    Fixes gh-403
This commit is contained in:
Marcin Grzejszczak
2017-01-11 12:31:18 +01:00
parent e4aa135253
commit 9785bcae15
20 changed files with 437 additions and 70 deletions

View File

@@ -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());
}

View File

@@ -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,24 +38,37 @@ import org.springframework.util.StringUtils;
* <li><b>port</b> - from lazily assigned port or {@link ServerProperties}</li>
* </ul>
*
* 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
public ServerPropertiesHostLocator(ServerProperties serverProperties,
String appName, ZipkinProperties zipkinProperties) {
@Deprecated
public ServerPropertiesHostLocator(ServerProperties serverProperties, String appName) {
this(serverProperties, appName, new ZipkinProperties(),null);
}
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
@@ -85,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;
}

View File

@@ -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);
}
}

View File

@@ -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;
}
}
}
}

View File

@@ -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;
}
}
}

View File

@@ -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);

View File

@@ -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.<Long>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;
}
}

View File

@@ -9,6 +9,7 @@ import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;
import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
import org.springframework.cloud.commons.util.UtilAutoConfiguration;
import org.springframework.boot.test.util.EnvironmentTestUtils;
import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration;
import org.springframework.cloud.sleuth.log.NoOpSpanLogger;
@@ -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() {

View File

@@ -35,6 +35,7 @@ import org.springframework.boot.actuate.metrics.CounterService;
import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
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;
@@ -159,7 +160,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 {
}