Adding SA only if peer tag is present

without this change Zipkin doesn't properly visualize the span on the server side.
with this change we're setting SA only if peer tag is set.

fixes #481
This commit is contained in:
Marcin Grzejszczak
2016-12-19 13:26:04 +01:00
parent b24713c943
commit ad700ea664
4 changed files with 42 additions and 12 deletions

View File

@@ -130,11 +130,12 @@ final class ConvertToZipkinSpanList {
private static void ensureServerAddr(Span span, Builder zipkinSpan,
Endpoint ep) {
String serviceName = span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)
? span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME) : ep.serviceName;
Endpoint endpoint = ep.toBuilder().serviceName(serviceName).build();
zipkinSpan.addBinaryAnnotation(
BinaryAnnotation.address(Constants.SERVER_ADDR, endpoint));
if (span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)) {
Endpoint endpoint = ep.toBuilder().serviceName(span.tags().get(
Span.SPAN_PEER_SERVICE_TAG_NAME)).build();
zipkinSpan.addBinaryAnnotation(
BinaryAnnotation.address(Constants.SERVER_ADDR, endpoint));
}
}
private static boolean notClientOrServer(Span span) {

View File

@@ -74,9 +74,10 @@ public class ConvertToZipkinSpanListTests {
}
@Test
public void appendsServerAddressTagIfClientLogIsPresent() {
public void appendServerAddressTagIfClientLogIsPresentWhenPeerServiceIsPresent() {
Span span = span("foo");
span.logEvent(Constants.CLIENT_SEND);
span.tag(Span.SPAN_PEER_SERVICE_TAG_NAME, "myservice");
Spans spans = new Spans(this.host, Collections.singletonList(span));
List<zipkin.Span> result = ConvertToZipkinSpanList.convert(spans);
@@ -89,6 +90,21 @@ public class ConvertToZipkinSpanListTests {
.contains("myservice");
}
@Test
public void doesNotAppendServerAddressTagIfClientLogIsPresent() {
Span span = span("foo");
span.logEvent(Constants.CLIENT_SEND);
Spans spans = new Spans(this.host, Collections.singletonList(span));
List<zipkin.Span> result = ConvertToZipkinSpanList.convert(spans);
assertThat(result)
.hasSize(1)
.flatExtracting(input1 -> input1.binaryAnnotations)
.filteredOn("key", Constants.SERVER_ADDR)
.isEmpty();
}
@Test
public void shouldReuseServerAddressTag() {
Span span = span("foo");

View File

@@ -127,10 +127,11 @@ public class ZipkinSpanListener implements SpanReporter {
}
private void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan) {
String serviceName = span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME) ?
span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME) : this.endpointLocator.local().serviceName;
zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR,
this.endpointLocator.local().toBuilder().serviceName(serviceName).build()));
if (span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)) {
zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR,
this.endpointLocator.local().toBuilder().serviceName(
span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME)).build()));
}
}
private boolean notClientOrServer(Span span) {

View File

@@ -17,7 +17,6 @@
package org.springframework.cloud.sleuth.zipkin;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import javax.annotation.PostConstruct;
@@ -204,8 +203,9 @@ public class ZipkinSpanListenerTests {
}
@Test
public void appendsServerAddressTagIfClientLogIsPresent() {
public void appendServerAddressTagIfClientLogIsPresentWhenPeerServiceIsPresent() {
this.parent.logEvent(Constants.CLIENT_SEND);
this.parent.tag(Span.SPAN_PEER_SERVICE_TAG_NAME, "fooservice");
this.parent.stop();
zipkin.Span result = this.spanReporter.convert(this.parent);
@@ -215,6 +215,18 @@ public class ZipkinSpanListenerTests {
.isNotEmpty();
}
@Test
public void doesNotAppendServerAddressTagIfClientLogIsPresent() {
this.parent.logEvent(Constants.CLIENT_SEND);
this.parent.stop();
zipkin.Span result = this.spanReporter.convert(this.parent);
assertThat(result.binaryAnnotations)
.filteredOn("key", Constants.SERVER_ADDR)
.isEmpty();
}
@Test
public void converts128BitTraceId() {
Span span = Span.builder().traceIdHigh(1L).traceId(2L).spanId(3L).name("foo").build();