From d7a0747907f4ab7201f67e7d0c762a324fbe0668 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 2 Mar 2018 10:48:54 +0100 Subject: [PATCH] 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 --- .../sleuth/instrument/web/TraceFilter.java | 18 +++++++------ .../instrument/web/TraceFilterTests.java | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 7 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 5bfbb2342..2645765b9 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 @@ -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(); } 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 d682fc0de..930421992 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 @@ -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 = 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");