Fixes missing zipkin service name and polishes span converters.
Missing service name: Zipkin service names were logged as null, which is invalid and led to them showing up as "unknown" in the zipkin ui. This was due to a wiring bug, and a special-case, which this change fixes. The special-case was when a sleuth span had no annotations. Since zipkin service names are attached to annotations, they are only queryable when annotations exist. When there are no annotations, we add the "lc" binary annotation, which makes that span attached to the correct service in zipkin. Polishing: Zipkin timestamps were not always set as microseconds. This fixes that. The de-facto label in zipkin for unknown service is "unknown". This fixes the code, which formerly fell back to "application". This also removes complexity in assigning timestamp and duration as we no longer need to make pseudo-annotations "acquire" and "release". Finally, this adds tests about above consistently to both scs-zipkin and scs-zipkin-stream.
This commit is contained in:
@@ -16,7 +16,6 @@
|
||||
|
||||
package org.springframework.cloud.sleuth.stream;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.boot.autoconfigure.web.ServerProperties;
|
||||
import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent;
|
||||
import org.springframework.cloud.sleuth.Span;
|
||||
@@ -28,15 +27,14 @@ import org.springframework.context.event.EventListener;
|
||||
*/
|
||||
public class ServerPropertiesHostLocator implements HostLocator {
|
||||
|
||||
@Value("${spring.application.name:application}")
|
||||
private String appName;
|
||||
|
||||
private ServerProperties serverProperties;
|
||||
|
||||
private final ServerProperties serverProperties;
|
||||
private final String appName;
|
||||
private Integer port;
|
||||
|
||||
public ServerPropertiesHostLocator(ServerProperties serverProperties) {
|
||||
public ServerPropertiesHostLocator(ServerProperties serverProperties,
|
||||
String appName) {
|
||||
this.serverProperties = serverProperties;
|
||||
this.appName = appName;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -80,7 +78,7 @@ public class ServerPropertiesHostLocator implements HostLocator {
|
||||
|
||||
private String getServiceName(Span span) {
|
||||
String serviceName;
|
||||
if (span.getProcessId() != null) {
|
||||
if (span.getProcessId() != null) { // TODO: javadocs say this isn't nullable!
|
||||
serviceName = span.getProcessId().toLowerCase();
|
||||
}
|
||||
else {
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
package org.springframework.cloud.sleuth.stream;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
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.ConditionalOnMissingClass;
|
||||
@@ -77,9 +78,12 @@ public class SleuthStreamAutoConfiguration {
|
||||
@Autowired(required = false)
|
||||
private ServerProperties serverProperties;
|
||||
|
||||
@Value("${spring.application.name:unknown}")
|
||||
private String appName;
|
||||
|
||||
@Bean
|
||||
public HostLocator zipkinEndpointLocator() {
|
||||
return new ServerPropertiesHostLocator(this.serverProperties);
|
||||
return new ServerPropertiesHostLocator(this.serverProperties, this.appName);
|
||||
}
|
||||
|
||||
}
|
||||
@@ -91,6 +95,9 @@ public class SleuthStreamAutoConfiguration {
|
||||
@Autowired(required = false)
|
||||
private ServerProperties serverProperties;
|
||||
|
||||
@Value("${spring.application.name:unknown}")
|
||||
private String appName;
|
||||
|
||||
@Autowired(required = false)
|
||||
private DiscoveryClient client;
|
||||
|
||||
@@ -99,7 +106,7 @@ public class SleuthStreamAutoConfiguration {
|
||||
if (this.client != null) {
|
||||
return new DiscoveryClientHostLocator(this.client);
|
||||
}
|
||||
return new ServerPropertiesHostLocator(this.serverProperties);
|
||||
return new ServerPropertiesHostLocator(this.serverProperties, this.appName);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
/*
|
||||
* Copyright 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.sleuth.stream;
|
||||
|
||||
import java.net.InetAddress;
|
||||
import java.net.UnknownHostException;
|
||||
import java.util.Collections;
|
||||
import org.junit.Test;
|
||||
import org.springframework.boot.autoconfigure.web.ServerProperties;
|
||||
import org.springframework.cloud.sleuth.MilliSpan;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
public class ServerPropertiesHostLocatorTests {
|
||||
MilliSpan span = new MilliSpan(1, 3, "name", "traceId", Collections.<String>emptyList(), "spanId", true, true, "processId");
|
||||
|
||||
@Test
|
||||
public void portDefaultsTo8080() {
|
||||
ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(new ServerProperties(), "unknown");
|
||||
|
||||
assertThat(locator.locate(span).getPort()).isEqualTo((short) 8080);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void portFromServerProperties() {
|
||||
ServerProperties properties = new ServerProperties();
|
||||
properties.setPort(1234);
|
||||
|
||||
ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(properties, "unknown");
|
||||
|
||||
assertThat(locator.locate(span).getPort()).isEqualTo((short) 1234);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void portDefaultsToLocalhost() {
|
||||
ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(new ServerProperties(), "unknown");
|
||||
|
||||
assertThat(locator.locate(span).getAddress())
|
||||
.isEqualTo("127.0.0.1");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void hostFromServerPropertiesIp() throws UnknownHostException {
|
||||
ServerProperties properties = new ServerProperties();
|
||||
properties.setAddress(InetAddress.getByAddress(new byte[]{1, 2, 3, 4}));
|
||||
|
||||
ServerPropertiesHostLocator locator = new ServerPropertiesHostLocator(properties, "unknown");
|
||||
|
||||
assertThat(locator.locate(span).getAddress())
|
||||
.isEqualTo("1.2.3.4");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user