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 5cfe870ea..91906e711 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 @@ -1,16 +1,16 @@ package org.springframework.cloud.sleuth.instrument.web; +import java.lang.invoke.MethodHandles; +import java.util.Map; +import java.util.Random; +import java.util.regex.Pattern; + import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanTextMap; import org.springframework.cloud.sleuth.util.TextMapUtil; import org.springframework.util.StringUtils; -import java.lang.invoke.MethodHandles; -import java.util.Map; -import java.util.Random; -import java.util.regex.Pattern; - /** * Default implementation, compatible with Zipkin propagation. * @@ -35,11 +35,11 @@ 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) { + if (debug && onlySpanIdIsPresent(carrier)) { // we're only generating Trace ID since if there's no Span ID will assume - // that it's equal to Trace ID + // that it's equal to Trace ID - we're trying to fix a malformed request generateIdIfMissing(carrier, Span.TRACE_ID_NAME); - } else if (carrier.get(Span.TRACE_ID_NAME) == null) { + } else if (traceIdIsMissing(carrier)) { // can't build a Span without trace id return null; } @@ -55,6 +55,18 @@ public class ZipkinHttpSpanExtractor implements HttpSpanExtractor { } } + private boolean onlySpanIdIsPresent(Map carrier) { + return traceIdIsMissing(carrier) && spanIdIsPresent(carrier); + } + + private boolean traceIdIsMissing(Map carrier) { + return carrier.get(Span.TRACE_ID_NAME) == null; + } + + private boolean spanIdIsPresent(Map carrier) { + return carrier.get(Span.SPAN_ID_NAME) != null; + } + private void generateIdIfMissing(Map carrier, String key) { if (!carrier.containsKey(key)) { carrier.put(key, Span.idToHex(new Random().nextLong())); 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 50cee09dc..a4eaa9d4b 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 @@ -31,7 +31,6 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.cloud.sleuth.DefaultSpanNamer; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.ExceptionMessageErrorParser; -import org.springframework.cloud.sleuth.NoOpSpanReporter; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanReporter; @@ -447,6 +446,29 @@ public class TraceFilterTests { then(ExceptionUtils.getLastException()).isNull(); } + @Test + public void samplesASpanDebugFlagWithInterceptor() throws Exception { + this.request = builder() + .header(Span.SPAN_FLAGS, 1) + .buildRequest(new MockServletContext()); + 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); + }); + + then(new ListOfSpans(this.spanReporter.getSpans())) + .doesNotHaveASpanWithName("http:/parent/") + .hasASpanWithName("http:/") + .hasSize(1) + .allSpansAreExportable(); + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); + } + public void verifyParentSpanHttpTags() { verifyParentSpanHttpTags(HttpStatus.OK); } @@ -487,7 +509,7 @@ public class TraceFilterTests { BDDMockito.given(beanFactory.getBean(Tracer.class)).willReturn(this.tracer); BDDMockito.given(beanFactory.getBean(TraceKeys.class)).willReturn(this.traceKeys); BDDMockito.given(beanFactory.getBean(HttpSpanExtractor.class)).willReturn(this.spanExtractor); - BDDMockito.given(beanFactory.getBean(SpanReporter.class)).willReturn(new NoOpSpanReporter()); + BDDMockito.given(beanFactory.getBean(SpanReporter.class)).willReturn(this.spanReporter); BDDMockito.given(beanFactory.getBean(HttpTraceKeysInjector.class)).willReturn(this.httpTraceKeysInjector); BDDMockito.given(beanFactory.getBean(ErrorParser.class)).willReturn(new ExceptionMessageErrorParser()); return beanFactory;