Fixed the way skip pattern and sampled flags are treated by filter

without this change whenever a span was to be skipped, a new trace was generated
with this change:
 - we first check if a trace context is already in the request
 - if sampled flag is set to 0, we reuse the Brave's mechanism of sampling
 - if URI is to be skipped we create an unsampled span BUT we reuse the trace context to build it if it was already there in the request

fixes gh-874
This commit is contained in:
Marcin Grzejszczak
2018-03-02 10:48:54 +01:00
parent 5676f3369a
commit d7a0747907
2 changed files with 36 additions and 7 deletions

View File

@@ -139,8 +139,7 @@ public class TraceFilter extends GenericFilterBean {
HttpServletRequest request = (HttpServletRequest) servletRequest;
HttpServletResponse response = (HttpServletResponse) servletResponse;
String uri = this.urlPathHelper.getPathWithinApplication(request);
boolean skip = this.skipPattern.matcher(uri).matches()
|| SPAN_NOT_SAMPLED.equals(ServletUtils.getHeader(request, response, SAMPLED_NAME));
boolean skip = this.skipPattern.matcher(uri).matches();
Span spanFromRequest = getSpanFromAttribute(request);
Tracer.SpanInScope ws = null;
if (spanFromRequest != null) {
@@ -326,10 +325,12 @@ public class TraceFilter extends GenericFilterBean {
}
return new SpanAndScope(spanFromRequest, ws);
}
TraceContextOrSamplingFlags flags = null;
try {
// TODO: Try to use Brave's mechanism for sampling
flags = httpTracing().tracing()
.propagation().extractor(HttpServletRequest::getHeader).extract(request);
if (skip) {
spanFromRequest = unsampledSpan(name);
spanFromRequest = unsampledSpan(name, flags);
} else {
spanFromRequest = handler().handleReceive(httpTracing().tracing()
.propagation().extractor(HttpServletRequest::getHeader), request);
@@ -345,7 +346,7 @@ public class TraceFilter extends GenericFilterBean {
log.error("Exception occurred while trying to extract tracing context from request. "
+ "Falling back to manual span creation", e);
if (skip) {
spanFromRequest = unsampledSpan(name);
spanFromRequest = unsampledSpan(name, flags);
}
else {
spanFromRequest = httpTracing().tracing().tracer().nextSpan()
@@ -362,9 +363,12 @@ public class TraceFilter extends GenericFilterBean {
.tracer().withSpanInScope(spanFromRequest));
}
private Span unsampledSpan(String name) {
private Span unsampledSpan(String name, TraceContextOrSamplingFlags flags) {
return httpTracing().tracing().tracer()
.nextSpan(TraceContextOrSamplingFlags.create(SamplingFlags.NOT_SAMPLED))
.nextSpan( flags != null ?
flags.sampled(false) :
TraceContextOrSamplingFlags.create(SamplingFlags.NOT_SAMPLED)
)
.kind(Span.Kind.SERVER)
.name(name).start();
}

View File

@@ -16,6 +16,8 @@
package org.springframework.cloud.sleuth.instrument.web;
import java.util.concurrent.atomic.AtomicReference;
import brave.Span;
import brave.Tracer;
import brave.Tracing;
@@ -57,6 +59,7 @@ public class TraceFilterTests {
static final String TRACE_ID_NAME = "X-B3-TraceId";
static final String SPAN_ID_NAME = "X-B3-SpanId";
static final String PARENT_SPAN_ID_NAME = "X-B3-ParentSpanId";
static final String SAMPLED_ID_NAME = "X-B3-Sampled";
static final String SPAN_FLAGS = "X-B3-Flags";
ArrayListSpanReporter reporter = new ArrayListSpanReporter();
@@ -202,6 +205,28 @@ public class TraceFilterTests {
.containsEntry("http.method", HttpMethod.GET.toString());
}
@Test
public void continuesATraceWhenSpanNotSampled() throws Exception {
AtomicReference<Span> span = new AtomicReference<>();
this.request = builder()
.header(SPAN_ID_NAME, PARENT_ID)
.header(TRACE_ID_NAME, SpanUtil.idToHex(2L))
.header(PARENT_SPAN_ID_NAME, SpanUtil.idToHex(3L))
.header(SAMPLED_ID_NAME, 0)
.buildRequest(new MockServletContext());
BeanFactory beanFactory = beanFactory();
TraceFilter filter = new TraceFilter(beanFactory);
filter.doFilter(this.request, this.response, (req, resp) -> {
this.filterChain.doFilter(req, resp);
span.set(this.tracing.tracer().currentSpan());
});
then(Tracing.current().tracer().currentSpan()).isNull();
then(span.get().context().traceIdString())
.isEqualTo(SpanUtil.idToHex(2L));
}
@Test
public void continuesSpanInRequestAttr() throws Exception {
Span span = this.tracer.nextSpan().name("http:foo");