diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSampler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSampler.java new file mode 100644 index 000000000..598b51dad --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSampler.java @@ -0,0 +1,46 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.util.regex.Pattern; + +import brave.http.HttpAdapter; +import brave.http.HttpSampler; + +/** + * Doesn't sample a span if skip pattern is matched + * + * @author Marcin Grzejszczak + * @since 2.0.0 + */ +class SleuthHttpSampler extends HttpSampler { + + private final Pattern pattern; + + SleuthHttpSampler(SkipPatternProvider provider) { + this.pattern = provider.skipPattern(); + } + + @Override public Boolean trySample(HttpAdapter adapter, Req request) { + String url = adapter.path(request); + boolean shouldSkip = this.pattern.matcher(url).matches(); + if (shouldSkip) { + return false; + } + return null; + } +} 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 2645765b9..202fa089e 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 @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web; import java.io.IOException; -import java.util.regex.Pattern; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; @@ -28,8 +27,6 @@ import brave.Span; import brave.Tracer; import brave.http.HttpServerHandler; import brave.http.HttpTracing; -import brave.propagation.SamplingFlags; -import brave.propagation.TraceContextOrSamplingFlags; import brave.servlet.HttpServletAdapter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -92,12 +89,8 @@ public class TraceFilter extends GenericFilterBean { private static final String TRACE_EXCEPTION_REQUEST_ATTR = TraceFilter.class.getName() + ".EXCEPTION"; - private static final String SAMPLED_NAME = "X-B3-Sampled"; - private static final String SPAN_NOT_SAMPLED = "0"; - private HttpTracing tracing; private TraceKeys traceKeys; - private final Pattern skipPattern; private final BeanFactory beanFactory; private HttpServerHandler handler; private Boolean hasErrorController; @@ -105,30 +98,8 @@ public class TraceFilter extends GenericFilterBean { private final UrlPathHelper urlPathHelper = new UrlPathHelper(); public TraceFilter(BeanFactory beanFactory) { - this(beanFactory, skipPattern(beanFactory)); - } - - public TraceFilter(BeanFactory beanFactory, Pattern skipPattern) { this.beanFactory = beanFactory; - this.skipPattern = skipPattern; } - - private static Pattern skipPattern(BeanFactory beanFactory) { - try { - SkipPatternProvider patternProvider = beanFactory - .getBean(SkipPatternProvider.class); - // the null value will not happen on production but might happen in tests - if (patternProvider != null) { - return patternProvider.skipPattern(); - } - } catch (NoSuchBeanDefinitionException e) { - if (log.isDebugEnabled()) { - log.debug("The default SkipPatternProvider implementation is missing, will fallback to a default value of patterns"); - } - } - return Pattern.compile(SleuthWebProperties.DEFAULT_SKIP_PATTERN); - } - @Override public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { @@ -139,14 +110,13 @@ 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 spanFromRequest = getSpanFromAttribute(request); Tracer.SpanInScope ws = null; if (spanFromRequest != null) { ws = continueSpan(request, spanFromRequest); } if (log.isDebugEnabled()) { - log.debug("Received a request to uri [" + uri + "] that should not be sampled [" + skip + "]"); + log.debug("Received a request to uri [" + uri + "]"); } // in case of a response with exception status a exception controller will close the span if (!httpStatusSuccessful(response) && isSpanContinued(request)) { @@ -157,7 +127,7 @@ public class TraceFilter extends GenericFilterBean { SpanAndScope spanAndScope = new SpanAndScope(); Throwable exception = null; try { - spanAndScope = createSpan(request, skip, spanFromRequest, name, ws); + spanAndScope = createSpan(request, spanFromRequest, name, ws); filterChain.doFilter(request, response); } catch (Throwable e) { exception = e; @@ -236,7 +206,7 @@ public class TraceFilter extends GenericFilterBean { } } else if ((shouldCloseSpan(request) || isRootSpan(span)) && stillTracingCurrentSpan(span)) { if (log.isDebugEnabled()) { - log.debug("Will close span " + span + " since " + (shouldCloseSpan(request) ? "some component marked it for closure" : "response was unsuccessful for the root span")); + log.debug("Will handle sent for span " + span); } handler().handleSend(response, exception, span); if (shouldCloseSpan(request)) { @@ -318,61 +288,26 @@ public class TraceFilter extends GenericFilterBean { * Creates a span and appends it as the current request's attribute */ private SpanAndScope createSpan(HttpServletRequest request, - boolean skip, Span spanFromRequest, String name, Tracer.SpanInScope ws) { + Span spanFromRequest, String name, Tracer.SpanInScope ws) { if (spanFromRequest != null) { if (log.isDebugEnabled()) { log.debug("Span has already been created - continuing with the previous one"); } return new SpanAndScope(spanFromRequest, ws); } - TraceContextOrSamplingFlags flags = null; - try { - flags = httpTracing().tracing() - .propagation().extractor(HttpServletRequest::getHeader).extract(request); - if (skip) { - spanFromRequest = unsampledSpan(name, flags); - } else { - spanFromRequest = handler().handleReceive(httpTracing().tracing() - .propagation().extractor(HttpServletRequest::getHeader), request); - } - if (log.isDebugEnabled()) { - log.debug("Found a parent span " + spanFromRequest.context() + " in the request"); - } - request.setAttribute(TRACE_REQUEST_ATTR, spanFromRequest); - if (log.isDebugEnabled()) { - log.debug("Parent span is " + spanFromRequest + ""); - } - } catch (Exception e) { - log.error("Exception occurred while trying to extract tracing context from request. " - + "Falling back to manual span creation", e); - if (skip) { - spanFromRequest = unsampledSpan(name, flags); - } - else { - spanFromRequest = httpTracing().tracing().tracer().nextSpan() - .kind(Span.Kind.SERVER) - .name(name).start(); - request.setAttribute(TRACE_SPAN_WITHOUT_PARENT, spanFromRequest); - } - request.setAttribute(TRACE_REQUEST_ATTR, spanFromRequest); - if (log.isDebugEnabled()) { - log.debug("No parent span present - creating a new span"); - } + spanFromRequest = handler().handleReceive(httpTracing().tracing() + .propagation().extractor(HttpServletRequest::getHeader), request); + if (log.isDebugEnabled()) { + log.debug("Found a parent span " + spanFromRequest.context() + " in the request"); + } + request.setAttribute(TRACE_REQUEST_ATTR, spanFromRequest); + if (log.isDebugEnabled()) { + log.debug("Parent span is " + spanFromRequest + ""); } return new SpanAndScope(spanFromRequest, httpTracing().tracing() .tracer().withSpanInScope(spanFromRequest)); } - private Span unsampledSpan(String name, TraceContextOrSamplingFlags flags) { - return httpTracing().tracing().tracer() - .nextSpan( flags != null ? - flags.sampled(false) : - TraceContextOrSamplingFlags.create(SamplingFlags.NOT_SAMPLED) - ) - .kind(Span.Kind.SERVER) - .name(name).start(); - } - class SpanAndScope { final Span span; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java index baad251d9..c8b492da3 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHttpAutoConfiguration.java @@ -24,7 +24,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.TraceKeys; -import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -38,23 +37,28 @@ import org.springframework.context.annotation.Configuration; @Configuration @ConditionalOnBean(Tracing.class) @ConditionalOnProperty(name = "spring.sleuth.http.enabled", havingValue = "true", matchIfMissing = true) -@AutoConfigureAfter(TraceAutoConfiguration.class) +@AutoConfigureAfter(TraceWebAutoConfiguration.class) public class TraceHttpAutoConfiguration { @Bean @ConditionalOnMissingBean @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", havingValue = "false", matchIfMissing = true) - HttpTracing sleuthHttpTracing(Tracing tracing) { - return HttpTracing.create(tracing); + HttpTracing sleuthHttpTracing(Tracing tracing, SkipPatternProvider provider) { + return HttpTracing + .newBuilder(tracing) + .serverSampler(new SleuthHttpSampler(provider)) + .build(); } @Bean @ConditionalOnMissingBean @ConditionalOnProperty(name = "spring.sleuth.http.legacy.enabled", havingValue = "true") - HttpTracing legacySleuthHttpTracing(Tracing tracing, TraceKeys traceKeys, ErrorParser errorParser) { + HttpTracing legacySleuthHttpTracing(Tracing tracing, TraceKeys traceKeys, + ErrorParser errorParser, SkipPatternProvider provider) { return HttpTracing.newBuilder(tracing) .clientParser(new SleuthHttpClientParser(traceKeys)) .serverParser(new SleuthHttpServerParser(traceKeys, errorParser)) + .serverSampler(new SleuthHttpSampler(provider)) .build(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java index 4277dfda8..230c1c350 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java @@ -25,8 +25,8 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.util.StringUtils; @@ -41,16 +41,14 @@ import org.springframework.util.StringUtils; */ @Configuration @ConditionalOnProperty(value = "spring.sleuth.web.enabled", matchIfMissing = true) -@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.ANY) @ConditionalOnBean(Tracing.class) -@AutoConfigureAfter(TraceHttpAutoConfiguration.class) +@AutoConfigureAfter(TraceAutoConfiguration.class) @EnableConfigurationProperties(SleuthWebProperties.class) public class TraceWebAutoConfiguration { @Configuration @ConditionalOnClass(ManagementServerProperties.class) - @ConditionalOnMissingBean( - SkipPatternProvider.class) + @ConditionalOnMissingBean(SkipPatternProvider.class) @EnableConfigurationProperties(SleuthWebProperties.class) protected static class SkipPatternProviderConfig { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java index 0efbfed9f..5528f80ab 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebServletAutoConfiguration.java @@ -87,8 +87,7 @@ public class TraceWebServletAutoConfiguration { @Bean @ConditionalOnMissingBean - public TraceFilter traceFilter(BeanFactory beanFactory, - SkipPatternProvider skipPatternProvider) { - return new TraceFilter(beanFactory, skipPatternProvider.skipPattern()); + public TraceFilter traceFilter(BeanFactory beanFactory) { + return new TraceFilter(beanFactory); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSamplerTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSamplerTests.java new file mode 100644 index 000000000..bb2548496 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SleuthHttpSamplerTests.java @@ -0,0 +1,55 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.util.regex.Pattern; + +import brave.http.HttpAdapter; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.BDDMockito; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.assertj.core.api.BDDAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(MockitoJUnitRunner.class) +public class SleuthHttpSamplerTests { + + @Mock HttpAdapter adapter; + + @Test + public void should_delegate_sampling_decision_if_pattern_is_not_matched() { + SkipPatternProvider provider = () -> Pattern.compile("foo"); + BDDMockito.given(this.adapter.path(BDDMockito.any())).willReturn("url"); + SleuthHttpSampler sampler = new SleuthHttpSampler(provider); + + then(sampler.trySample(this.adapter, new Object())).isNull(); + } + + @Test + public void should_not_sample_if_pattern_is_matched() { + SkipPatternProvider provider = () -> Pattern.compile(".*"); + BDDMockito.given(this.adapter.path(BDDMockito.any())).willReturn("url"); + SleuthHttpSampler sampler = new SleuthHttpSampler(provider); + + then(sampler.trySample(this.adapter, new Object())).isFalse(); + } +} \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java index bb8b5ead8..bd13bb07f 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java @@ -357,9 +357,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { } @Bean - TraceFilter myTraceFilter(BeanFactory beanFactory, - SkipPatternProvider skipPatternProvider) { - return new TraceFilter(beanFactory, skipPatternProvider.skipPattern()) { + TraceFilter myTraceFilter(BeanFactory beanFactory) { + return new TraceFilter(beanFactory) { @Override void abandonSpan(Span span) { log.info("Simulating Error Controller"); span.finish(); 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 930421992..afffead58 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 @@ -17,6 +17,7 @@ package org.springframework.cloud.sleuth.instrument.web; import java.util.concurrent.atomic.AtomicReference; +import java.util.regex.Pattern; import brave.Span; import brave.Tracer; @@ -73,6 +74,7 @@ public class TraceFilterTests { .clientParser(new SleuthHttpClientParser(this.traceKeys)) .serverParser(new SleuthHttpServerParser(this.traceKeys, new ExceptionMessageErrorParser())) + .serverSampler(new SleuthHttpSampler(() -> Pattern.compile(""))) .build(); SleuthProperties properties = new SleuthProperties(); @@ -124,6 +126,7 @@ public class TraceFilterTests { .clientParser(new SleuthHttpClientParser(this.traceKeys)) .serverParser(new SleuthHttpServerParser(this.traceKeys, new ExceptionMessageErrorParser())) + .serverSampler(new SleuthHttpSampler(() -> Pattern.compile(""))) .build(); BeanFactory beanFactory = beanFactory(); BDDMockito.given(beanFactory.getBean(HttpTracing.class)).willReturn(httpTracing);