From c951dc88fda1ae8c2c6c2734b4d4518556790d3d Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 24 Aug 2017 14:14:01 +0200 Subject: [PATCH] Improvements of customizing the TraceFilter without this change you have to add a `@Primary` annotation around your custom `TraceFilter` bean to alter the behaviour of the current `TraceFilter` implementation with this change we're marking the `TraceFilter` as conditional on missing bean; also we're ensuring that the registered `SkipPatternProvider` will be reused when no explicit pattern was set. also documentation is added fixes #633 --- .../main/asciidoc/spring-cloud-sleuth.adoc | 15 ++++++++ .../sleuth/instrument/web/TraceFilter.java | 19 ++++++++++- .../web/TraceWebAutoConfiguration.java | 1 + ...ceFilterAlwaysSamplerIntegrationTests.java | 3 ++ .../web/TraceFilterIntegrationTests.java | 34 ++++++++++++++++++- .../instrument/web/TraceFilterTests.java | 3 ++ 6 files changed, 73 insertions(+), 2 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 576d84db0..b0372bf87 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -420,6 +420,21 @@ And you could register them like this: include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java[tags=configuration,indent=0] ---- +=== TraceFilter + +You can also modify the behaviour of the `TraceFilter` - the component that is responsible +for processing the input HTTP request and adding tags basing on the HTTP response. You can customize +the tags, or modify the response headers by registering your own instance of the `TraceFilter` bean. + +In the following example we will register the `TraceFilter` bean and we will add the +`ZIPKIN-TRACE-ID` response header containing the current Span's trace id. Also we will +add to the Span a tag with key `custom` and a value `tag`. + +[source,java] +---- +include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java[tags=response_headers,indent=0] +---- + === Custom SA tag in Zipkin Sometimes you want to create a manual Span that will wrap a call to an external service which is not instrumented. 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 68486dcae..c9b0350c2 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 @@ -31,6 +31,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.ExceptionMessageErrorParser; import org.springframework.cloud.sleuth.Span; @@ -130,7 +131,7 @@ public class TraceFilter extends GenericFilterBean { } public TraceFilter(BeanFactory beanFactory) { - this(beanFactory, Pattern.compile(SleuthWebProperties.DEFAULT_SKIP_PATTERN)); + this(beanFactory, skipPattern(beanFactory)); } public TraceFilter(BeanFactory beanFactory, Pattern skipPattern) { @@ -138,6 +139,22 @@ public class TraceFilter extends GenericFilterBean { this.skipPattern = skipPattern; } + private static Pattern skipPattern(BeanFactory beanFactory) { + try { + TraceWebAutoConfiguration.SkipPatternProvider patternProvider = beanFactory + .getBean(TraceWebAutoConfiguration.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 { 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 f9cffada3..267e5e3c4 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 @@ -95,6 +95,7 @@ public class TraceWebAutoConfiguration { } @Bean + @ConditionalOnMissingBean public TraceFilter traceFilter(BeanFactory beanFactory, SkipPatternProvider skipPatternProvider) { return new TraceFilter(beanFactory, skipPatternProvider.skipPattern()); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterAlwaysSamplerIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterAlwaysSamplerIntegrationTests.java index c36e4db10..08f7f8354 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterAlwaysSamplerIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterAlwaysSamplerIntegrationTests.java @@ -9,6 +9,7 @@ import org.junit.runner.RunWith; import org.mockito.BDDMockito; import org.mockito.Mockito; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.sleuth.NoOpSpanReporter; @@ -68,6 +69,8 @@ public class TraceFilterAlwaysSamplerIntegrationTests extends AbstractMvcIntegra private BeanFactory beanFactory() { BeanFactory beanFactory = Mockito.mock(BeanFactory.class); + BDDMockito.given(beanFactory.getBean(TraceWebAutoConfiguration.SkipPatternProvider.class)) + .willThrow(new NoSuchBeanDefinitionException("foo")); 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); 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 c8f470790..d3a552a3a 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 @@ -4,6 +4,8 @@ import java.util.Optional; import java.util.Random; import java.util.concurrent.CompletableFuture; +import javax.servlet.http.HttpServletResponse; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.After; @@ -11,6 +13,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.slf4j.MDC; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.autoconfigure.ManagementServerProperties; import org.springframework.boot.test.context.SpringBootTest; @@ -59,7 +62,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { } @Test - public void should_create_and_return_trace_in_HTTP_header() throws Exception { + public void should_create_a_trace() throws Exception { whenSentPingWithoutTracingData(); then(this.spanAccumulator.getSpans()).hasSize(1); @@ -157,6 +160,17 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { then(new ListOfSpans(this.spanAccumulator.getSpans())).hasServerSideSpansInProperOrder(); } + @Test + public void should_return_custom_response_headers_when_custom_trace_filter_gets_registered() throws Exception { + Long expectedTraceId = new Random().nextLong(); + + MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId); + + then(ExceptionUtils.getLastException()).isNull(); + then(mvcResult.getResponse().getHeader("ZIPKIN-TRACE-ID")).isEqualTo(Span.idToHex(expectedTraceId)); + then(new ListOfSpans(this.spanAccumulator.getSpans())).hasASpanWithTagEqualTo("custom", "tag"); + } + @Override protected void configureMockMvcBuilder(DefaultMockMvcBuilder mockMvcBuilder) { mockMvcBuilder.addFilters(this.traceFilter); @@ -288,5 +302,23 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { Sampler alwaysSampler() { return new AlwaysSampler(); } + + //tag::response_headers[] + @Bean + TraceFilter myTraceFilter(BeanFactory beanFactory, final Tracer tracer) { + return new TraceFilter(beanFactory) { + @Override protected void addResponseTags(HttpServletResponse response, + Throwable e) { + // execute the default behaviour + super.addResponseTags(response, e); + // for readability we're returning trace id in a hex form + response.addHeader("ZIPKIN-TRACE-ID", + Span.idToHex(tracer.getCurrentSpan().getTraceId())); + // we can also add some custom tags + tracer.addTag("custom", "tag"); + } + }; + } + //end::response_headers[] } } 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 a4eaa9d4b..5ff090a89 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 @@ -28,6 +28,7 @@ import org.mockito.BDDMockito; import org.mockito.Mock; import org.mockito.Mockito; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.sleuth.DefaultSpanNamer; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.ExceptionMessageErrorParser; @@ -506,6 +507,8 @@ public class TraceFilterTests { private BeanFactory beanFactory() { BeanFactory beanFactory = Mockito.mock(BeanFactory.class); + BDDMockito.given(beanFactory.getBean(TraceWebAutoConfiguration.SkipPatternProvider.class)) + .willThrow(new NoSuchBeanDefinitionException("foo")); 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);