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 780cc0454..526530b55 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 @@ -16,12 +16,6 @@ package org.springframework.cloud.sleuth; -import com.fasterxml.jackson.annotation.JsonAutoDetect; -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonInclude; -import org.springframework.util.Assert; -import org.springframework.util.StringUtils; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -34,6 +28,13 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; +import org.springframework.util.Assert; +import org.springframework.util.StringUtils; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonInclude; + /** * Class for gathering and reporting statistics about a block of execution. *

@@ -160,6 +161,13 @@ public class Span implements SpanContext { @JsonIgnore private final Long startNanos; private Long durationMicros; // serialized in json so micros precision isn't lost + /* + Using B3 propagation, it is most typical to share the same span ID across client and + the server. This has backend implications like who owns the timestamp (hint the + client does). When a SpanReporter receives a completed span, it should know if it + is shared or not. + */ + private final boolean shared; @SuppressWarnings("unused") private Span() { @@ -191,17 +199,18 @@ public class Span implements SpanContext { this.durationMicros = current.durationMicros; this.baggage = current.baggage; this.savedSpan = savedSpan; + this.shared = current.shared; } Span(long begin, long end, String name, long traceId, List parents, long spanId, boolean remote, boolean exportable, String processId) { this(begin, end, name, traceId, parents, spanId, remote, exportable, processId, - null); + null, false); } Span(long begin, long end, String name, long traceId, List parents, long spanId, boolean remote, boolean exportable, String processId, - Span savedSpan) { + Span savedSpan, boolean shared) { this(new SpanBuilder() .begin(begin) .end(end) @@ -212,7 +221,8 @@ public class Span implements SpanContext { .remote(remote) .exportable(exportable) .processId(processId) - .savedSpan(savedSpan)); + .savedSpan(savedSpan) + .shared(shared)); } Span(SpanBuilder builder) { @@ -242,6 +252,7 @@ public class Span implements SpanContext { this.logs.addAll(builder.logs); this.baggage = new ConcurrentHashMap<>(); this.baggage.putAll(builder.baggage); + this.shared = builder.shared; } public static SpanBuilder builder() { @@ -499,6 +510,16 @@ public class Span implements SpanContext { return this.exportable; } + /** + * Span and trace id got extracted from a carrier? + * We are adding data to the same span created by a remote client2 + * + * @since 1.3.0 + */ + public boolean isShared() { + return this.shared; + } + /** * Returns the 16 or 32 character hex representation of the span's trace ID * @@ -644,6 +665,7 @@ public class Span implements SpanContext { private final List logs = new ArrayList<>(); private final Map tags = new LinkedHashMap<>(); private final Map baggage = new LinkedHashMap<>(); + private boolean shared; SpanBuilder() { } @@ -748,6 +770,11 @@ public class Span implements SpanContext { return this; } + public Span.SpanBuilder shared(boolean shared) { + this.shared = shared; + return this; + } + /** * Creates a {@link Span.SpanBuilder} from the {@link Span}. */ diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractor.java index 2c02ef5e9..dc464b74a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractor.java @@ -1,12 +1,12 @@ package org.springframework.cloud.sleuth.instrument.messaging; +import java.util.Map; +import java.util.Random; + import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanTextMap; import org.springframework.cloud.sleuth.util.TextMapUtil; -import java.util.Map; -import java.util.Random; - /** * Default implementation for messaging * @@ -18,9 +18,11 @@ public class HeaderBasedMessagingExtractor implements MessagingSpanTextMapExtrac @Override public Span joinTrace(SpanTextMap textMap) { Map carrier = TextMapUtil.asMap(textMap); + boolean spanIdMissing = !hasHeader(carrier, TraceMessageHeaders.SPAN_ID_NAME); + boolean traceIdMissing = !hasHeader(carrier, TraceMessageHeaders.TRACE_ID_NAME); if (Span.SPAN_SAMPLED.equals(carrier.get(TraceMessageHeaders.SPAN_FLAGS_NAME))) { - String traceId = generateTraceIdIfMissing(carrier); - if (!carrier.containsKey(TraceMessageHeaders.SPAN_ID_NAME)) { + String traceId = generateTraceIdIfMissing(carrier, traceIdMissing); + if (spanIdMissing) { carrier.put(TraceMessageHeaders.SPAN_ID_NAME, traceId); } } else if (!hasHeader(carrier, TraceMessageHeaders.SPAN_ID_NAME) @@ -28,28 +30,32 @@ public class HeaderBasedMessagingExtractor implements MessagingSpanTextMapExtrac return null; // TODO: Consider throwing IllegalArgumentException; } - return extractSpanFromHeaders(carrier, Span.builder()); + boolean idMissing = spanIdMissing || traceIdMissing; + return extractSpanFromHeaders(carrier, Span.builder(), idMissing); } - private String generateTraceIdIfMissing(Map carrier) { - if (!hasHeader(carrier, TraceMessageHeaders.TRACE_ID_NAME)) { + private String generateTraceIdIfMissing(Map carrier, + boolean traceIdMissing) { + if (traceIdMissing) { carrier.put(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(new Random().nextLong())); } return carrier.get(TraceMessageHeaders.TRACE_ID_NAME); } - private Span extractSpanFromHeaders(Map carrier, Span.SpanBuilder spanBuilder) { + private Span extractSpanFromHeaders(Map carrier, + Span.SpanBuilder spanBuilder, boolean idMissing) { String traceId = carrier.get(TraceMessageHeaders.TRACE_ID_NAME); spanBuilder = spanBuilder .traceIdHigh(traceId.length() == 32 ? Span.hexToId(traceId, 0) : 0) .traceId(Span.hexToId(traceId)) .spanId(Span.hexToId(carrier.get(TraceMessageHeaders.SPAN_ID_NAME))); String flags = carrier.get(TraceMessageHeaders.SPAN_FLAGS_NAME); - if (Span.SPAN_SAMPLED.equals(flags)) { + boolean debug = Span.SPAN_SAMPLED.equals(flags); + boolean spanSampled = Span.SPAN_SAMPLED.equals(carrier.get(TraceMessageHeaders.SAMPLED_NAME)); + if (debug) { spanBuilder.exportable(true); } else { - spanBuilder.exportable( - Span.SPAN_SAMPLED.equals(carrier.get(TraceMessageHeaders.SAMPLED_NAME))); + spanBuilder.exportable(spanSampled); } String processId = carrier.get(TraceMessageHeaders.PROCESS_ID_NAME); String spanName = carrier.get(TraceMessageHeaders.SPAN_NAME_NAME); @@ -61,6 +67,7 @@ public class HeaderBasedMessagingExtractor implements MessagingSpanTextMapExtrac } setParentIdIfApplicable(carrier, spanBuilder, TraceMessageHeaders.PARENT_ID_NAME); spanBuilder.remote(true); + spanBuilder.shared((debug || spanSampled) && !idMissing); for (Map.Entry entry : carrier.entrySet()) { if (entry.getKey().toLowerCase().startsWith(Span.SPAN_BAGGAGE_HEADER_PREFIX + TraceMessageHeaders.HEADER_DELIMITER)) { spanBuilder.baggage(unprefixedKey(entry.getKey()), entry.getValue()); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java index 91906e711..9881182b9 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java @@ -35,7 +35,8 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { public Span joinTrace(SpanTextMap textMap) { Map carrier = TextMapUtil.asMap(textMap); boolean debug = Span.SPAN_SAMPLED.equals(carrier.get(Span.SPAN_FLAGS)); - if (debug && onlySpanIdIsPresent(carrier)) { + boolean idToBeGenerated = debug && onlySpanIdIsPresent(carrier); + if (idToBeGenerated) { // we're only generating Trace ID since if there's no Span ID will assume // that it's equal to Trace ID - we're trying to fix a malformed request generateIdIfMissing(carrier, Span.TRACE_ID_NAME); @@ -48,7 +49,7 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { boolean skip = this.skipPattern.matcher(uri).matches() || Span.SPAN_NOT_SAMPLED.equals(carrier.get(Span.SAMPLED_NAME)); long spanId = spanId(carrier); - return buildParentSpan(carrier, uri, skip, spanId); + return buildParentSpan(carrier, uri, skip, spanId, idToBeGenerated); } catch (Exception e) { log.error("Exception occurred while trying to extract span from carrier", e); return null; @@ -86,7 +87,8 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { } } - private Span buildParentSpan(Map carrier, String uri, boolean skip, long spanId) { + private Span buildParentSpan(Map carrier, String uri, boolean skip, + long spanId, boolean idToBeGenerated) { String traceId = carrier.get(Span.TRACE_ID_NAME); Span.SpanBuilder span = Span.builder() .traceIdHigh(traceId.length() == 32 ? Span.hexToId(traceId, 0) : 0) @@ -106,6 +108,8 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { span.parent(Span.hexToId(carrier.get(Span.PARENT_ID_NAME))); } span.remote(true); + // trace, span id were retrieved from the headers and span is sampled + span.shared(!(skip || idToBeGenerated)); boolean debug = Span.SPAN_SAMPLED.equals(carrier.get(Span.SPAN_FLAGS)); if (debug) { span.exportable(true); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTests.java index 1fd8ffd52..0484e603c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTests.java @@ -16,15 +16,16 @@ package org.springframework.cloud.sleuth; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import org.junit.Test; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.assertThat; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; import java.io.IOException; import java.util.concurrent.atomic.AtomicLong; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.assertThat; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; +import org.junit.Test; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; /** * @author Marcin Grzejszczak @@ -286,6 +287,7 @@ public class SpanTests { private Span.SpanBuilder builder() { return Span.builder().name("http:name").traceId(1L).spanId(2L).parent(3L) .begin(1L).end(2L).traceId(3L).exportable(true).parent(4L) - .baggage("foo", "bar").remote(true).tag("tag", "tag").log(new Log(System.currentTimeMillis(), "log")); + .baggage("foo", "bar") + .remote(true).shared(true).tag("tag", "tag").log(new Log(System.currentTimeMillis(), "log")); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java index 80639099f..846987bb1 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java @@ -190,6 +190,26 @@ public class SpanAssert extends AbstractAssert { return this; } + public SpanAssert isShared() { + isNotNull(); + if (!this.actual.isShared()) { + String message = "The span is supposed to be shared but it's not!"; + log.error(message); + failWithMessage(message); + } + return this; + } + + public SpanAssert isNotShared() { + isNotNull(); + if (this.actual.isShared()) { + String message = "The span is NOT supposed to be shared but it is!"; + log.error(message); + failWithMessage(message); + } + return this; + } + public SpanAssert isNotExportable() { isNotNull(); if (this.actual.isExportable()) { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractorTests.java index 12cecd440..572b9dfb4 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/HeaderBasedMessagingExtractorTests.java @@ -16,6 +16,8 @@ package org.springframework.cloud.sleuth.instrument.messaging; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; + import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -24,8 +26,6 @@ import org.junit.Test; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanTextMap; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; - /** * @author Marcin Grzejszczak */ @@ -42,7 +42,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isShared(); } @Test @@ -56,7 +56,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isShared(); } @Test @@ -69,7 +69,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isShared(); } @Test @@ -82,7 +82,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isNotExportable(); + then(span).isNotExportable().isNotShared(); } @Test @@ -93,7 +93,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isNotShared(); then(span.traceIdString()).isNotEmpty(); then(span.getSpanId()).isNotNull(); } @@ -107,7 +107,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isNotShared(); then(span.traceIdString()).isNotEmpty(); then(span.getSpanId()).isEqualTo(10L); } @@ -121,7 +121,7 @@ public class HeaderBasedMessagingExtractorTests { Span span = extractor.joinTrace(spanTextMap); - then(span).isExportable(); + then(span).isExportable().isNotShared(); then(span.getTraceId()).isEqualTo(10L); then(span.getSpanId()).isEqualTo(10L); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java index 4a4568044..2a53e7b8c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java @@ -16,6 +16,8 @@ package org.springframework.cloud.sleuth.instrument.web; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; + import javax.servlet.http.HttpServletRequest; import java.util.Arrays; import java.util.Random; @@ -30,8 +32,6 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.cloud.sleuth.Span; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; - @RunWith(MockitoJUnitRunner.class) public class HttpServletRequestExtractorTests { @@ -82,18 +82,60 @@ public class HttpServletRequestExtractorTests { @Test public void should_accept_128bit_trace_id() { - String hex128Bits = "463ac35c9f6413ad48485a3953bb6124"; - String lower64Bits = "48485a3953bb6124"; + String hex128Bits = spanInHeaders(); - BDDMockito.given(this.request.getHeaderNames()) - .willReturn(new Vector<>(Arrays.asList(Span.TRACE_ID_NAME, Span.SPAN_ID_NAME)).elements()); - BDDMockito.given(this.request.getHeader(Span.TRACE_ID_NAME)) - .willReturn(hex128Bits); - BDDMockito.given(this.request.getHeader(Span.SPAN_ID_NAME)) - .willReturn(lower64Bits); Span span = this.extractor.joinTrace(new HttpServletRequestTextMap(this.request)); then(span.traceIdString()).isEqualTo(hex128Bits); } + + @Test + public void should_set_shared_flag_for_sampled_span_in_headers() { + spanInHeaders(); + + Span span = this.extractor.joinTrace(new HttpServletRequestTextMap(this.request)); + + then(span.isShared()).isTrue(); + } + + @Test + public void should_not_set_shared_flag_for_non_sampled_span_in_headers() { + spanInHeaders(); + BDDMockito.given(this.request.getHeader(Span.SAMPLED_NAME)) + .willReturn(Span.SPAN_NOT_SAMPLED); + + Span span = this.extractor.joinTrace(new HttpServletRequestTextMap(this.request)); + + then(span.isShared()).isFalse(); + } + + @Test + public void should_not_set_shared_flag_for_sampled_span_in_headers_without_span_trace_id() { + BDDMockito.given(this.request.getHeaderNames()) + .willReturn(new Vector<>(Arrays.asList(Span.SPAN_FLAGS, Span.SPAN_ID_NAME)).elements()); + BDDMockito.given(this.request.getHeader(Span.SPAN_FLAGS)) + .willReturn("1"); + BDDMockito.given(this.request.getHeader(Span.SPAN_ID_NAME)) + .willReturn("48485a3953bb6124"); + + Span span = this.extractor.joinTrace(new HttpServletRequestTextMap(this.request)); + + then(span.isShared()).isFalse(); + } + + private String spanInHeaders() { + String hex128Bits = "463ac35c9f6413ad48485a3953bb6124"; + String lower64Bits = "48485a3953bb6124"; + + BDDMockito.given(this.request.getHeaderNames()) + .willReturn(new Vector<>(Arrays.asList(Span.TRACE_ID_NAME, Span.SPAN_ID_NAME, Span.SAMPLED_NAME)).elements()); + BDDMockito.given(this.request.getHeader(Span.TRACE_ID_NAME)) + .willReturn(hex128Bits); + BDDMockito.given(this.request.getHeader(Span.SPAN_ID_NAME)) + .willReturn(lower64Bits); + BDDMockito.given(this.request.getHeader(Span.SAMPLED_NAME)) + .willReturn(Span.SPAN_SAMPLED); + return hex128Bits; + } } diff --git a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java index bb9e50465..d2b28a8d7 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java +++ b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java @@ -15,6 +15,10 @@ */ package org.springframework.cloud.sleuth.zipkin.stream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + import org.apache.commons.logging.Log; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.stream.Host; @@ -26,10 +30,6 @@ import zipkin.Constants; import zipkin.Endpoint; import zipkin.Span.Builder; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - /** * This converts sleuth spans to zipkin ones, skipping invalid or unsampled. * @@ -96,9 +96,14 @@ final class ConvertToZipkinSpanList { // rather let the client do that. Worst case we were propagated an unreported ID and // Zipkin backfills timestamp and duration. if (!span.isRemote()) { - zipkinSpan.timestamp(span.getBegin() * 1000); - if (!span.isRunning()) { // duration is authoritative, only write when the span stopped - zipkinSpan.duration(calculateDurationInMicros(span)); + if (Boolean.TRUE.equals(span.isShared())) { + // don't report server-side timestamp on shared spans + zipkinSpan.timestamp(null).duration(null); + } else { + zipkinSpan.timestamp(span.getBegin() * 1000); + if (!span.isRunning()) { // duration is authoritative, only write when the span stopped + zipkinSpan.duration(calculateDurationInMicros(span)); + } } } zipkinSpan.traceIdHigh(span.getTraceIdHigh()); diff --git a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java index 304a1b6eb..15dc2fcb8 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java +++ b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java @@ -15,6 +15,11 @@ */ package org.springframework.cloud.sleuth.zipkin.stream; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Random; + import org.assertj.core.api.Condition; import org.junit.Test; import org.springframework.cloud.sleuth.Span; @@ -23,11 +28,6 @@ import org.springframework.cloud.sleuth.stream.Spans; import zipkin.Constants; import zipkin.Endpoint; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Random; - import static org.assertj.core.api.Assertions.assertThat; public class ConvertToZipkinSpanListTests { @@ -217,6 +217,39 @@ public class ConvertToZipkinSpanListTests { assertThat(result.traceId).isEqualTo(span.getTraceId()); } + @Test + public void shouldRemoveTimestampAndDurationForNonRemoteSharedSpan() { + Span span = Span.builder() + .name("foo") + .exportable(false) + .remote(false) + .shared(true) + .build(); + Spans spans = new Spans(this.host, Collections.singletonList(span)); + + zipkin.Span result = ConvertToZipkinSpanList.convert(spans).get(0); + + assertThat(result.duration).isNull(); + assertThat(result.timestamp).isNull(); + } + + @Test + public void shouldNotRemoveTimestampAndDurationForNonRemoteNonSharedSpan() { + Span span = Span.builder() + .name("foo") + .exportable(false) + .remote(false) + .shared(false) + .build(); + span.stop(); + Spans spans = new Spans(this.host, Collections.singletonList(span)); + + zipkin.Span result = ConvertToZipkinSpanList.convert(spans).get(0); + + assertThat(result.duration).isNotNull(); + assertThat(result.timestamp).isNotNull(); + } + Span span(String name) { return span(name, false); } 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 4332c1168..536d3395c 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 @@ -99,9 +99,14 @@ public class ZipkinSpanListener implements SpanReporter { // rather let the client do that. Worst case we were propagated an unreported ID and // Zipkin backfills timestamp and duration. if (!convertedSpan.isRemote()) { - zipkinSpan.timestamp(convertedSpan.getBegin() * 1000L); - if (!convertedSpan.isRunning()) { // duration is authoritative, only write when the span stopped - zipkinSpan.duration(calculateDurationInMicros(convertedSpan)); + // don't report server-side timestamp on shared spans + if (Boolean.TRUE.equals(convertedSpan.isShared())) { + zipkinSpan.timestamp(null).duration(null); + } else { + zipkinSpan.timestamp(convertedSpan.getBegin() * 1000L); + if (!convertedSpan.isRunning()) { // duration is authoritative, only write when the span stopped + zipkinSpan.duration(calculateDurationInMicros(convertedSpan)); + } } } zipkinSpan.traceIdHigh(convertedSpan.getTraceIdHigh()); 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 878cd5c8b..c4ba712da 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 @@ -319,6 +319,37 @@ public class ZipkinSpanListenerTests { assertThat(result.name).isEqualTo("foo"); } + @Test + public void shouldRemoveTimestampAndDurationForNonRemoteSharedSpan() { + Span span = Span.builder() + .name("foo") + .exportable(false) + .remote(false) + .shared(true) + .build(); + + zipkin.Span result = this.spanListener.convert(span); + + assertThat(result.duration).isNull(); + assertThat(result.timestamp).isNull(); + } + + @Test + public void shouldNotRemoveTimestampAndDurationForNonRemoteNonSharedSpan() { + Span span = Span.builder() + .name("foo") + .exportable(false) + .remote(false) + .shared(false) + .build(); + span.stop(); + + zipkin.Span result = this.spanListener.convert(span); + + assertThat(result.duration).isNotNull(); + assertThat(result.timestamp).isNotNull(); + } + @Configuration @EnableAutoConfiguration protected static class TestConfiguration {