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..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
@@ -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 client
+ *
+ * @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/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,
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/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java
index 25d08ba25..0f91366e2 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/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 6f48f175a..6451f9f29 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
@@ -94,18 +94,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-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 c5fb54d35..13a94b511 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);
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 fff984a5a..1fa005540 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
@@ -23,7 +23,9 @@ import org.springframework.boot.web.servlet.context.ServletWebServerInitializedE
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 java.lang.invoke.MethodHandles;
* @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;
public ServerPropertiesHostLocator(ServerProperties serverProperties, String appName,
ZipkinProperties zipkinProperties, InetUtils inetUtils) {
@@ -98,6 +102,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();
}
@@ -120,4 +127,8 @@ public class ServerPropertiesHostLocator implements HostLocator {
return serviceName;
}
+ @Override
+ public void setEnvironment(Environment environment) {
+ this.environment = environment;
+ }
}
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/ServerPropertiesEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java
index 2c49cd37b..a76a7ee99 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
@@ -22,7 +22,9 @@ import org.springframework.boot.autoconfigure.web.ServerProperties;
import org.springframework.boot.web.servlet.context.ServletWebServerInitializedEvent;
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;
@@ -42,15 +44,18 @@ import java.nio.ByteBuffer;
* @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;
public ServerPropertiesEndpointLocator(ServerProperties serverProperties,
String appName, ZipkinProperties zipkinProperties, InetUtils inetUtils) {
@@ -102,8 +107,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;
+ }
}
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 {