SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

TraceFilter skipPattern and extra span when X-B3-Flags header is present

Browse Source 
without this change if there's a debug flag we set traceid & spanid for the first request inside the zipkin http extractor. we don't set the parent span flag so an additional span is created.
with this change we set the trace and span id ONLY when the debug flag is set AND the request is malformed. That means - only when the span id is set and there is no trace id.

fixes #665
This commit is contained in:
Marcin Grzejszczak
2017-08-18 16:24:28 +02:00
parent 6b3f238342
commit aa20124088
2 changed files with 44 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ZipkinHttpSpanExtractor.java
View File

@@ -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<String, String> 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<String, String> carrier) {
return traceIdIsMissing(carrier) && spanIdIsPresent(carrier);
}
private boolean traceIdIsMissing(Map<String, String> carrier) {
return carrier.get(Span.TRACE_ID_NAME) == null;
}
private boolean spanIdIsPresent(Map<String, String> carrier) {
return carrier.get(Span.SPAN_ID_NAME) != null;
}
private void generateIdIfMissing(Map<String, String> carrier, String key) {
if (!carrier.containsKey(key)) {
carrier.put(key, Span.idToHex(new Random().nextLong()));

26
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java
View File

@@ -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;
@@ -471,6 +470,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);
}
@@ -510,7 +532,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;