From c9945ad173cd66255d106a59228ae447c703c192 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 6 Sep 2017 15:32:20 +0200 Subject: [PATCH 1/7] Added 1.2.x to whitelist --- docs/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pom.xml b/docs/pom.xml index 0a76cded0..a7a0d8bcb 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -14,7 +14,7 @@ spring-cloud-sleuth - 1.0.x,1.1.x + 1.0.x,1.1.x,1.2.x ${basedir}/.. From cfd3a619dd65af646f32b25564f2c8b37fff468b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 14 Sep 2017 17:14:09 +0200 Subject: [PATCH 2/7] Introduced a "shared" field in Span (#698) whenever: - a span or trace id get generated when a request / message arrives - child span is created we set the field to false whenever we find ids in the incoming request / message we set the field to true fixes #696 --- .../springframework/cloud/sleuth/Span.java | 45 +++++++++++--- .../HeaderBasedMessagingExtractor.java | 31 ++++++---- .../web/ZipkinHttpSpanExtractor.java | 10 ++- .../cloud/sleuth/SpanTests.java | 14 +++-- .../cloud/sleuth/assertions/SpanAssert.java | 20 ++++++ .../HeaderBasedMessagingExtractorTests.java | 18 +++--- .../web/HttpServletRequestExtractorTests.java | 62 ++++++++++++++++--- .../stream/ConvertToZipkinSpanList.java | 19 +++--- .../stream/ConvertToZipkinSpanListTests.java | 43 +++++++++++-- .../sleuth/zipkin/ZipkinSpanListener.java | 11 +++- .../zipkin/ZipkinSpanListenerTests.java | 31 ++++++++++ 11 files changed, 240 insertions(+), 64 deletions(-) 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 { From 5b6d0390364bd00adc7f0a747ff6bced80715035 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 14 Sep 2017 17:15:30 +0200 Subject: [PATCH 3/7] Fixed a typo --- .../src/main/java/org/springframework/cloud/sleuth/Span.java | 2 +- .../sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 526530b55..9354644c3 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 @@ -512,7 +512,7 @@ public class Span implements SpanContext { /** * Span and trace id got extracted from a carrier? - * We are adding data to the same span created by a remote client2 + * We are adding data to the same span created by a remote client * * @since 1.3.0 */ 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 15dc2fcb8..861f4580e 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 @@ -242,7 +242,7 @@ public class ConvertToZipkinSpanListTests { .shared(false) .build(); span.stop(); - Spans spans = new Spans(this.host, Collections.singletonList(span)); + Spans spans S= new Spans(this.host, Collections.singletonList(span)); zipkin.Span result = ConvertToZipkinSpanList.convert(spans).get(0); From 5df9c2e37dc1d33fadaa034b5552bad74a64834b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 14 Sep 2017 17:33:43 +0200 Subject: [PATCH 4/7] Fixed a typo --- .../sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 861f4580e..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 @@ -242,7 +242,7 @@ public class ConvertToZipkinSpanListTests { .shared(false) .build(); span.stop(); - Spans spans S= new Spans(this.host, Collections.singletonList(span)); + Spans spans = new Spans(this.host, Collections.singletonList(span)); zipkin.Span result = ConvertToZipkinSpanList.convert(spans).get(0); From cefef8cc9f633fe5444e7f010dfef46fc91e095b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 18 Sep 2017 20:49:55 +0100 Subject: [PATCH 5/7] Fixed return value of the tracer --- .../cloud/sleuth/autoconfig/TraceAutoConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java index e692ab863..875304c5c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java @@ -68,7 +68,7 @@ public class TraceAutoConfiguration { @Bean @ConditionalOnMissingBean(Tracer.class) - public DefaultTracer sleuthTracer(Sampler sampler, Random random, + public Tracer sleuthTracer(Sampler sampler, Random random, SpanNamer spanNamer, SpanLogger spanLogger, SpanReporter spanReporter, TraceKeys traceKeys) { return new DefaultTracer(sampler, random, spanNamer, spanLogger, From 867a49bac8a40f9897b8c193228d051799cbf89a Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 19 Sep 2017 10:59:51 +0100 Subject: [PATCH 6/7] Reusing environment to get address; fixes #701 --- .../stream/ServerPropertiesHostLocator.java | 13 +++++++++++- .../ServerPropertiesEndpointLocator.java | 20 ++++++++++++++++--- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java index 2bc0c1c3b..ce7a21393 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/ServerPropertiesHostLocator.java @@ -25,7 +25,9 @@ import org.springframework.boot.context.embedded.EmbeddedServletContainerInitial import org.springframework.cloud.commons.util.InetUtils; import org.springframework.cloud.commons.util.InetUtilsProperties; import org.springframework.cloud.sleuth.Span; +import org.springframework.context.EnvironmentAware; import org.springframework.context.event.EventListener; +import org.springframework.core.env.Environment; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -43,15 +45,17 @@ import org.springframework.util.StringUtils; * @author Dave Syer * @since 1.0.0 */ -public class ServerPropertiesHostLocator implements HostLocator { +public class ServerPropertiesHostLocator implements HostLocator, EnvironmentAware { private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + private static final String IP_ADDRESS_PROP_NAME = "spring.cloud.client.ipAddress"; private final ServerProperties serverProperties; // Nullable private final String appName; private final InetUtils inetUtils; private final ZipkinProperties zipkinProperties; private Integer port; // Lazy assigned + private Environment environment; @Deprecated public ServerPropertiesHostLocator(ServerProperties serverProperties, String appName) { @@ -103,6 +107,9 @@ public class ServerPropertiesHostLocator implements HostLocator { if (this.serverProperties != null && this.serverProperties.getAddress() != null) { address = this.serverProperties.getAddress().getHostAddress(); } + else if (this.environment != null) { + address = this.environment.getProperty(IP_ADDRESS_PROP_NAME, String.class); + } else { address = this.inetUtils.findFirstNonLoopbackAddress().getHostAddress(); } @@ -125,4 +132,8 @@ public class ServerPropertiesHostLocator implements HostLocator { return serviceName; } + @Override + public void setEnvironment(Environment environment) { + this.environment = environment; + } } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java index abc481d95..2975cef54 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java @@ -16,8 +16,6 @@ package org.springframework.cloud.sleuth.zipkin; -import zipkin.Endpoint; - import java.lang.invoke.MethodHandles; import java.nio.ByteBuffer; @@ -27,8 +25,11 @@ 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.context.EnvironmentAware; import org.springframework.context.event.EventListener; +import org.springframework.core.env.Environment; import org.springframework.util.StringUtils; +import zipkin.Endpoint; /** * {@link EndpointLocator} implementation that: @@ -43,15 +44,18 @@ import org.springframework.util.StringUtils; * @author Dave Syer * @since 1.0.0 */ -public class ServerPropertiesEndpointLocator implements EndpointLocator { +public class ServerPropertiesEndpointLocator implements EndpointLocator, + EnvironmentAware { private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + private static final String IP_ADDRESS_PROP_NAME = "spring.cloud.client.ipAddress"; private final ServerProperties serverProperties; private final String appName; private final InetUtils inetUtils; private final ZipkinProperties zipkinProperties; private Integer port; + private Environment environment; @Deprecated public ServerPropertiesEndpointLocator(ServerProperties serverProperties,String appName) { @@ -108,8 +112,18 @@ public class ServerPropertiesEndpointLocator implements EndpointLocator { return ByteBuffer.wrap(this.serverProperties.getAddress().getAddress()) .getInt(); } + else if (this.environment != null) { + String ipAddress = this.environment + .getProperty(IP_ADDRESS_PROP_NAME, String.class); + return InetUtils.getIpAddressAsInt(ipAddress); + } else { return ByteBuffer.wrap(this.inetUtils.findFirstNonLoopbackAddress().getAddress()).getInt(); } } + + @Override + public void setEnvironment(Environment environment) { + this.environment = environment; + } } From 1b1a1698caf1a478dd4e2b7408eb3db0397ec96f Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 22 Sep 2017 13:21:46 +0200 Subject: [PATCH 7/7] Removed setting of the "fallback span" without this change we created a "fallback span" in the case where a Spring component didn't handle the span. By handling it means that it got processed by anyway (a tag, log was added etc.). Why did we do that? That's because if no component was involved in creation of additional spans then in the Zipkin UI the service that had only the span that has the SR, SS logs wouldn't show up. In other words, if service A called service B and service B had only a single span, then service B wouldn't show up in the UI (there would be no blue bar). All the data would show up however in the client side of that span. with this change we're removing this behaviour. That's because it introduces confusion for our users, that use non Spring related components. Also it seems that the Zipkin UI got fixed and this error no longer is present. If however it turns out that it's still there, it clearly seems that the place to fix this error should be Zipkin and not Sleuth. fixes #708 --- .../sleuth/instrument/web/TraceFilter.java | 19 ------- .../instrument/web/TraceFilterTests.java | 54 +++++++++++++------ 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java index 931bea80d..2cda8c191 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java @@ -171,7 +171,6 @@ public class TraceFilter extends GenericFilterBean { // TODO: how to deal with response annotations and async? return; } - spanFromRequest = createSpanIfRequestNotHandled(request, spanFromRequest, name, skip); detachOrCloseSpans(request, response, spanFromRequest, exception); } } @@ -211,22 +210,6 @@ public class TraceFilter extends GenericFilterBean { } } - // This method is a fallback in case if handler interceptors didn't catch the request. - // In that case we are creating an artificial span so that it can be visible in Zipkin. - private Span createSpanIfRequestNotHandled(HttpServletRequest request, - Span spanFromRequest, String name, boolean skip) { - if (!requestHasAlreadyBeenHandled(request)) { - spanFromRequest = tracer().createSpan(name); - request.setAttribute(TRACE_REQUEST_ATTR, spanFromRequest); - if (log.isDebugEnabled() && !skip) { - log.debug("The request with uri [" + request.getRequestURI() + "] hasn't been handled by any of Sleuth's components. " - + "That means that most likely you're using custom HandlerMappings and didn't add Sleuth's TraceHandlerInterceptor. " - + "Sleuth will create a span to ensure that the graph of calls remains valid in Zipkin"); - } - } - return spanFromRequest; - } - private boolean requestHasAlreadyBeenHandled(HttpServletRequest request) { return request.getAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR) != null; } @@ -239,8 +222,6 @@ public class TraceFilter extends GenericFilterBean { addResponseTagsForSpanWithoutParent(request, response); if (span.hasSavedSpan() && requestHasAlreadyBeenHandled(request)) { recordParentSpan(span.getSavedSpan()); - } else if (!requestHasAlreadyBeenHandled(request)) { - span = tracer().close(span); } recordParentSpan(span); // in case of a response with exception status will close the span when exception dispatch is handled diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java index 31409aa47..5c3ed5dbc 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java @@ -45,6 +45,7 @@ import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.mock.web.MockFilterChain; @@ -135,6 +136,35 @@ public class TraceFilterTests { assertThat(this.span.tags()).containsEntry("http.status_code", HttpStatus.OK.toString()); then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(new ListOfSpans(this.spanReporter.getSpans())) + .hasSize(1) + .hasASpanWithTagEqualTo("http.url", "http://localhost/?foo=bar") + .hasASpanWithTagEqualTo("http.host", "localhost") + .hasASpanWithTagEqualTo("http.path", "/") + .hasASpanWithTagEqualTo("http.method", HttpMethod.GET.toString()) + .hasASpanWithTagEqualTo("http.status_code", HttpStatus.OK.toString()) + .allSpansAreExportable(); + } + + @Test + public void startsNewTraceWithTraceHandlerInterceptor() throws Exception { + final BeanFactory beanFactory = beanFactory(); + TraceFilter filter = new TraceFilter(beanFactory); + filter.doFilter(this.request, this.response, (req, resp) -> { + this.filterChain.doFilter(req, resp); + // Simulate execution of the TraceHandlerInterceptor + request.setAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR, tracer.getCurrentSpan()); + }); + + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(new ListOfSpans(this.spanReporter.getSpans())) + .hasSize(1) + .hasASpanWithTagEqualTo("http.url", "http://localhost/?foo=bar") + .hasASpanWithTagEqualTo("http.host", "localhost") + .hasASpanWithTagEqualTo("http.path", "/") + .hasASpanWithTagEqualTo("http.method", HttpMethod.GET.toString()) + .hasASpanWithTagEqualTo("http.status_code", HttpStatus.OK.toString()) + .allSpansAreExportable(); } @Test @@ -160,16 +190,14 @@ public class TraceFilterTests { TraceFilter filter = new TraceFilter(beanFactory); filter.doFilter(this.request, this.response, this.filterChain); - // this creates a child span which is why we'd expect the parents to include the parent id - // especially important if no handler interceptors have been used. - // We add a child span on the server side to show which controller serviced the request - assertThat(this.span.getParents()).containsOnly(PARENT_ID); - assertThat(parentSpan()) + assertThat(this.span.getSpanId()).isEqualTo(PARENT_ID); + assertThat(this.span) .hasATag("http.url", "http://localhost/?foo=bar") .hasATag("http.host", "localhost") .hasATag("http.path", "/") .hasATag("http.method", "GET"); then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); } private Span parentSpan() { @@ -252,8 +280,8 @@ public class TraceFilterTests { this.request.addHeader("X-Foo", "bar"); filter.doFilter(this.request, this.response, this.filterChain); - assertThat(parentSpan().tags()).contains(entry("http.x-foo", "bar")); - assertThat(parentSpan().tags()).contains(entry("http.x-foo", "bar")); + assertThat(this.span.tags()).contains(entry("http.x-foo", "bar")); + assertThat(this.span.tags()).contains(entry("http.x-foo", "bar")); then(TestSpanContextHolder.getCurrentSpan()).isNull(); } @@ -283,7 +311,7 @@ public class TraceFilterTests { this.request.addHeader("X-Foo", "spam"); filter.doFilter(this.request, this.response, this.filterChain); - assertThat(parentSpan().tags()).contains(entry("http.x-foo", "'bar','spam'")); + assertThat(this.span.tags()).contains(entry("http.x-foo", "'bar','spam'")); then(TestSpanContextHolder.getCurrentSpan()).isNull(); } @@ -426,7 +454,7 @@ public class TraceFilterTests { filter.doFilter(this.request, this.response, this.filterChain); then(new ListOfSpans(this.spanReporter.getSpans())) - .allSpansAreExportable().hasSize(2).hasASpanWithSpanId(Span.hexToId("10")); + .allSpansAreExportable().hasSize(1).hasASpanWithSpanId(Span.hexToId("10")); then(TestSpanContextHolder.getCurrentSpan()).isNull(); then(ExceptionUtils.getLastException()).isNull(); } @@ -479,11 +507,7 @@ public class TraceFilterTests { this.sampler = new NeverSampler(); TraceFilter filter = new TraceFilter(beanFactory()); - filter.doFilter(this.request, this.response, (req, res) -> { - // Simulate the TraceHandlerInterceptor - req.setAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR, span); - this.filterChain.doFilter(req, res); - }); + filter.doFilter(this.request, this.response, this.filterChain); then(new ListOfSpans(this.spanReporter.getSpans())) .doesNotHaveASpanWithName("http:/parent/") @@ -503,7 +527,7 @@ public class TraceFilterTests { * org.springframework.cloud.sleuth.instrument.TraceKeys}. */ public void verifyParentSpanHttpTags(HttpStatus status) { - assertThat(parentSpan().tags()).contains(entry("http.host", "localhost"), + assertThat(this.span.tags()).contains(entry("http.host", "localhost"), entry("http.url", "http://localhost/?foo=bar"), entry("http.path", "/"), entry("http.method", "GET")); verifyCurrentSpanStatusCodeForAContinuedSpan(status);