diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java index 445e26b12..6abd557ae 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java @@ -64,6 +64,10 @@ public class Span { public static final String SPAN_ID_NAME = "X-Span-Id"; public static final String SPAN_EXPORT_NAME = "X-Span-Export"; public static final String SPAN_LOCAL_COMPONENT_TAG_NAME = "lc"; + /** + * As in Open Tracing + */ + public static final String SPAN_PEER_SERVICE_TAG_NAME = "peer.service"; private final long begin; private long end = 0; diff --git a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIterator.java b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIterator.java index 7e6ff577a..99e5c7e3c 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIterator.java +++ b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIterator.java @@ -15,7 +15,9 @@ */ package org.springframework.cloud.sleuth.zipkin.stream; +import java.util.Arrays; import java.util.Iterator; +import java.util.List; import java.util.NoSuchElementException; import org.apache.commons.logging.Log; @@ -24,6 +26,7 @@ import org.springframework.cloud.sleuth.stream.Host; import org.springframework.cloud.sleuth.stream.SleuthSink; import org.springframework.cloud.sleuth.stream.Spans; import org.springframework.util.StringUtils; + import zipkin.BinaryAnnotation; import zipkin.Constants; import zipkin.Endpoint; @@ -38,6 +41,9 @@ import zipkin.Span.Builder; * @since 1.0.0 */ final class SamplingZipkinSpanIterator implements Iterator { + private static final List ZIPKIN_START_EVENTS = Arrays.asList( + Constants.CLIENT_RECV, Constants.SERVER_RECV + ); private static final Log log = org.apache.commons.logging.LogFactory .getLog(SamplingZipkinSpanIterator.class); @@ -99,6 +105,10 @@ final class SamplingZipkinSpanIterator implements Iterator { *
  • Create timeline annotations based on data from Span object. *
  • Create binary annotations based on data from Span object. * + * + * When logging {@link Constants#CLIENT_SEND}, instrumentation should also log the {@link Constants#SERVER_ADDR} + * Check Zipkin code + * for more information */ // VisibleForTesting static zipkin.Span convert(Span span, Host host) { @@ -109,18 +119,14 @@ final class SamplingZipkinSpanIterator implements Iterator { // A zipkin span without any annotations cannot be queried, add special "lc" to // avoid that. - if (span.logs().isEmpty() && span.tags().isEmpty()) { - String processId = span.getProcessId() != null - ? span.getProcessId().toLowerCase() - : ZipkinMessageListener.UNKNOWN_PROCESS_ID; - zipkinSpan.addBinaryAnnotation( - BinaryAnnotation.create(Constants.LOCAL_COMPONENT, processId, ep)); + if (notClientOrServer(span)) { + ensureLocalComponent(span, zipkinSpan, ep); } - else { - ZipkinMessageListener.addZipkinAnnotations(zipkinSpan, span, ep); - ZipkinMessageListener.addZipkinBinaryAnnotations(zipkinSpan, span, ep); + ZipkinMessageListener.addZipkinAnnotations(zipkinSpan, span, ep); + ZipkinMessageListener.addZipkinBinaryAnnotations(zipkinSpan, span, ep); + if (hasClientSend(span)) { + ensureServerAddr(span, zipkinSpan, ep); } - zipkinSpan.timestamp(span.getBegin() * 1000); zipkinSpan.duration(span.getAccumulatedMillis() * 1000); zipkinSpan.traceId(span.getTraceId()); @@ -138,4 +144,43 @@ final class SamplingZipkinSpanIterator implements Iterator { } return zipkinSpan.build(); } + + private static void ensureLocalComponent(Span span, Builder zipkinSpan, + Endpoint ep) { + if (span.tags().containsKey(Constants.LOCAL_COMPONENT)) { + return; + } + String processId = span.getProcessId() != null + ? span.getProcessId().toLowerCase() + : ZipkinMessageListener.UNKNOWN_PROCESS_ID; + zipkinSpan.addBinaryAnnotation( + BinaryAnnotation.create(Constants.LOCAL_COMPONENT, processId, ep)); + } + + private static void ensureServerAddr(Span span, zipkin.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; + zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR, + Endpoint.create(serviceName, ep.ipv4, ep.port))); + } + + private static boolean notClientOrServer(Span span) { + for (org.springframework.cloud.sleuth.Log log : span.logs()) { + if (ZIPKIN_START_EVENTS.contains(log.getEvent())) { + return false; + } + } + return true; + } + + private static boolean hasClientSend(Span span) { + for (org.springframework.cloud.sleuth.Log log : span.logs()) { + if (Constants.CLIENT_SEND.equals(log.getEvent())) { + return !span.tags().containsKey(Constants.SERVER_ADDR); + } + } + return false; + } + } \ No newline at end of file diff --git a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIteratorTests.java b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIteratorTests.java index 624c01e9a..0976fabdc 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIteratorTests.java +++ b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/SamplingZipkinSpanIteratorTests.java @@ -25,6 +25,8 @@ import org.junit.Test; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.stream.Host; import org.springframework.cloud.sleuth.stream.Spans; + +import zipkin.Constants; import zipkin.Sampler; import static org.assertj.core.api.Assertions.assertThat; @@ -76,6 +78,54 @@ public class SamplingZipkinSpanIteratorTests { "message:foo", "message:baz"); } + @Test + public void appendsLocalComponentTagIfNoZipkinLogIsPresent() { + Spans spans = new Spans(this.host, Collections.singletonList(span("foo"))); + + Iterator result = new SamplingZipkinSpanIterator( + Sampler.create(1.0f), spans); + + assertThat(result) + .flatExtracting(s -> s.binaryAnnotations) + .extracting(input -> input.key) + .contains(Constants.LOCAL_COMPONENT); + } + + @Test + public void appendsServerAddressTagIfClientLogIsPresent() { + Span span = span("foo"); + span.logEvent(Constants.CLIENT_SEND); + Spans spans = new Spans(this.host, Collections.singletonList(span)); + + Iterator result = new SamplingZipkinSpanIterator( + Sampler.create(1.0f), spans); + + assertThat(result) + .hasSize(1) + .flatExtracting(input1 -> input1.binaryAnnotations) + .filteredOn("key", Constants.SERVER_ADDR) + .extracting(input -> input.endpoint.serviceName) + .contains("myservice"); + } + + @Test + public void shouldReuseServerAddressTag() { + Span span = span("foo"); + span.logEvent(Constants.CLIENT_SEND); + span.tag(Span.SPAN_PEER_SERVICE_TAG_NAME, "barservice"); + Spans spans = new Spans(this.host, Collections.singletonList(span)); + + Iterator result = new SamplingZipkinSpanIterator( + Sampler.create(1.0f), spans); + + assertThat(result) + .hasSize(1) + .flatExtracting(input1 -> input1.binaryAnnotations) + .filteredOn("key", Constants.SERVER_ADDR) + .extracting(input -> input.endpoint.serviceName) + .contains("barservice"); + } + Span span(String name) { Long id = new Random().nextLong(); return new Span(1, 3, "message:" + name, id, Collections.emptyList(), id, true, true, 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 9b7aeef00..dd5e610bd 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 @@ -17,6 +17,8 @@ package org.springframework.cloud.sleuth.zipkin; import java.nio.charset.Charset; +import java.util.Arrays; +import java.util.List; import java.util.Map; import org.springframework.cloud.sleuth.Log; @@ -44,6 +46,10 @@ import zipkin.Endpoint; * @since 1.0.0 */ public class ZipkinSpanListener { + private static final List ZIPKIN_START_EVENTS = Arrays.asList( + Constants.CLIENT_RECV, Constants.SERVER_RECV + ); + private static final org.apache.commons.logging.Log log = org.apache.commons.logging.LogFactory .getLog(ZipkinSpanListener.class); private static final Charset UTF_8 = Charset.forName("UTF-8"); @@ -122,27 +128,24 @@ public class ZipkinSpanListener { *
  • Create timeline annotations based on data from Span object. *
  • Create binary annotations based on data from Span object. * + * + * When logging {@link Constants#CLIENT_SEND}, instrumentation should also log the {@link Constants#SERVER_ADDR} + * Check Zipkin code + * for more information */ // Visible for testing zipkin.Span convert(Span span) { zipkin.Span.Builder zipkinSpan = new zipkin.Span.Builder(); // A zipkin span without any annotations cannot be queried, add special "lc" to avoid that. - if (span.logs().isEmpty() && span.tags().isEmpty()) { - byte[] processId = span.getProcessId() != null - ? span.getProcessId().toLowerCase().getBytes(UTF_8) - : UNKNOWN_BYTES; - BinaryAnnotation component = new BinaryAnnotation.Builder() - .type(BinaryAnnotation.Type.STRING) - .key("lc") // LOCAL_COMPONENT - .value(processId) - .endpoint(this.localEndpoint).build(); - zipkinSpan.addBinaryAnnotation(component); - } else { - addZipkinAnnotations(zipkinSpan, span, this.localEndpoint); - addZipkinBinaryAnnotations(zipkinSpan, span, this.localEndpoint); + if (notClientOrServer(span)) { + ensureLocalComponent(span, zipkinSpan); + } + addZipkinAnnotations(zipkinSpan, span, this.localEndpoint); + addZipkinBinaryAnnotations(zipkinSpan, span, this.localEndpoint); + if (hasClientSend(span)) { + ensureServerAddr(span, zipkinSpan); } - zipkinSpan.timestamp(span.getBegin() * 1000L); zipkinSpan.duration(span.getAccumulatedMillis() * 1000L); zipkinSpan.traceId(span.getTraceId()); @@ -160,6 +163,46 @@ public class ZipkinSpanListener { return zipkinSpan.build(); } + private void ensureLocalComponent(Span span, zipkin.Span.Builder zipkinSpan) { + if (span.tags().containsKey(Constants.LOCAL_COMPONENT)) { + return; + } + byte[] processId = span.getProcessId() != null + ? span.getProcessId().toLowerCase().getBytes(UTF_8) + : UNKNOWN_BYTES; + BinaryAnnotation component = new BinaryAnnotation.Builder() + .type(BinaryAnnotation.Type.STRING) + .key("lc") // LOCAL_COMPONENT + .value(processId) + .endpoint(this.localEndpoint).build(); + zipkinSpan.addBinaryAnnotation(component); + } + + 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.localEndpoint.serviceName; + zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR, + Endpoint.create(serviceName, this.localEndpoint.ipv4, this.localEndpoint.port))); + } + + private boolean notClientOrServer(Span span) { + for (Log log : span.logs()) { + if (ZIPKIN_START_EVENTS.contains(log.getEvent())) { + return false; + } + } + return true; + } + + private boolean hasClientSend(Span span) { + for (org.springframework.cloud.sleuth.Log log : span.logs()) { + if (Constants.CLIENT_SEND.equals(log.getEvent())) { + return !span.tags().containsKey(Constants.SERVER_ADDR); + } + } + return false; + } + /** * Add annotations from the sleuth Span. */ @@ -178,13 +221,13 @@ public class ZipkinSpanListener { * Adds binary annotation from the sleuth Span */ private void addZipkinBinaryAnnotations(zipkin.Span.Builder zipkinSpan, - Span span, Endpoint endpoint) { + Span span, Endpoint ep) { for (Map.Entry e : span.tags().entrySet()) { BinaryAnnotation binaryAnn = new BinaryAnnotation.Builder() .type(BinaryAnnotation.Type.STRING) .key(e.getKey()) .value(e.getValue().getBytes(UTF_8)) - .endpoint(endpoint).build(); + .endpoint(ep).build(); zipkinSpan.addBinaryAnnotation(binaryAnn); } } diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java index d61bbce99..fe3055554 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java @@ -16,10 +16,11 @@ package org.springframework.cloud.sleuth.zipkin; -import javax.annotation.PostConstruct; import java.util.ArrayList; import java.util.List; +import javax.annotation.PostConstruct; + import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -41,6 +42,8 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import zipkin.Constants; + import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -135,6 +138,45 @@ public class ZipkinSpanListenerTests { assertEquals(2, this.test.spans.size()); } + @Test + public void appendsLocalComponentTagIfNoZipkinLogIsPresent() { + this.parent.logEvent("hystrix/retry"); + this.parent.stop(); + + zipkin.Span result = this.listener.convert(this.parent); + + assertThat(result.binaryAnnotations) + .extracting(input -> input.key) + .contains(Constants.LOCAL_COMPONENT); + } + + @Test + public void appendsServerAddressTagIfClientLogIsPresent() { + this.parent.logEvent(Constants.CLIENT_SEND); + this.parent.stop(); + + zipkin.Span result = this.listener.convert(this.parent); + + assertThat(result.binaryAnnotations) + .filteredOn("key", Constants.SERVER_ADDR) + .extracting(input -> input.endpoint.serviceName) + .containsOnly("unknown"); + } + + @Test + public void shouldReuseServerAddressTag() { + this.parent.logEvent(Constants.CLIENT_SEND); + this.parent.tag(Span.SPAN_PEER_SERVICE_TAG_NAME, "fooservice"); + this.parent.stop(); + + zipkin.Span result = this.listener.convert(this.parent); + + assertThat(result.binaryAnnotations) + .filteredOn("key", Constants.SERVER_ADDR) + .extracting(input -> input.endpoint.serviceName) + .containsOnly("fooservice"); + } + @Configuration @Import({ ZipkinTestConfiguration.class, ZipkinAutoConfiguration.class, TraceAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class })