diff --git a/pom.xml b/pom.xml index bf62ea1f7..221166ee1 100644 --- a/pom.xml +++ b/pom.xml @@ -272,7 +272,7 @@ Elmhurst.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT - 4.17.2 + 4.17.3-SNAPSHOT 2.5.1 2.0.0.RELEASE diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java index ad6a6d573..c9cc31bc1 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java @@ -47,8 +47,11 @@ import org.springframework.boot.context.properties.ConfigurationProperties; * what's you are storing. * * @since 1.0.0 + * + * @deprecated the Brave's defaults are suggested to be used */ @ConfigurationProperties("spring.sleuth.keys") +@Deprecated public class TraceKeys { private Http http = new Http(); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthTraceHandlerInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthTraceHandlerInterceptor.java deleted file mode 100644 index 64e11f825..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/SleuthTraceHandlerInterceptor.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * 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 javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import brave.SpanCustomizer; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.beans.factory.BeanFactory; -import org.springframework.cloud.sleuth.ErrorParser; -import org.springframework.cloud.sleuth.TraceKeys; -import org.springframework.web.method.HandlerMethod; -import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; - -/** - * {@link org.springframework.web.servlet.HandlerInterceptor} that wraps handling of a - * adds tags related to the class and method name. - * - * @author Marcin Grzejszczak - * @since 1.0.3 - */ -class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { - - private static final Log log = LogFactory.getLog(SleuthTraceHandlerInterceptor.class); - - private final BeanFactory beanFactory; - private SpanCustomizer spanCustomizer; - private TraceKeys traceKeys; - private ErrorParser errorParser; - - public SleuthTraceHandlerInterceptor(BeanFactory beanFactory) { - this.beanFactory = beanFactory; - } - - @Override - public boolean preHandle(HttpServletRequest request, HttpServletResponse response, - Object handler) { - SpanCustomizer span = spanCustomizer(); - if (log.isDebugEnabled()) { - log.debug("Adding tags to span " + span); - } - addClassMethodTag(handler, span); - addClassNameTag(handler, span); - return true; - } - - private void addClassMethodTag(Object handler, SpanCustomizer span) { - if (handler instanceof HandlerMethod) { - String methodName = ((HandlerMethod) handler).getMethod().getName(); - span.tag(traceKeys().getMvc().getControllerMethod(), methodName); - if (log.isDebugEnabled()) { - log.debug("Adding a method tag with value [" + methodName + "] to a span " + span); - } - } - } - - private void addClassNameTag(Object handler, SpanCustomizer span) { - String className; - if (handler instanceof HandlerMethod) { - className = ((HandlerMethod) handler).getBeanType().getSimpleName(); - } else { - className = handler.getClass().getSimpleName(); - } - if (log.isDebugEnabled()) { - log.debug("Adding a class tag with value [" + className + "] to a span " + span); - } - span.tag(traceKeys().getMvc().getControllerClass(), className); - } - - @Override - public void afterConcurrentHandlingStarted(HttpServletRequest request, - HttpServletResponse response, Object handler) { - - } - - @Override - public void afterCompletion(HttpServletRequest request, HttpServletResponse response, - Object handler, Exception ex) { - SpanCustomizer span = spanCustomizer(); - if (ex != null && span != null) { - errorParser().parseErrorTags(span, ex); - } - } - - private SpanCustomizer spanCustomizer() { - if (this.spanCustomizer == null) { - this.spanCustomizer = this.beanFactory.getBean(SpanCustomizer.class); - } - return this.spanCustomizer; - } - - private TraceKeys traceKeys() { - if (this.traceKeys == null) { - this.traceKeys = this.beanFactory.getBean(TraceKeys.class); - } - return this.traceKeys; - } - - private ErrorParser errorParser() { - if (this.errorParser == null) { - this.errorParser = this.beanFactory.getBean(ErrorParser.class); - } - return this.errorParser; - } - -} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java index 0d364aa30..fcc3d679b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java @@ -19,7 +19,7 @@ package org.springframework.cloud.sleuth.instrument.web; import java.util.Collections; import javax.servlet.http.HttpServletRequest; -import brave.spring.webmvc.TracingHandlerInterceptor; +import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; @@ -89,12 +89,7 @@ class TraceSpringDataBeanPostProcessor implements BeanPostProcessor { if (handlerExecutionChain == null) { return null; } - handlerExecutionChain.addInterceptor(this.applicationContext.getBean(TracingHandlerInterceptor.class)); - String legacyEnabled = this.applicationContext.getEnvironment() - .getProperty("spring.sleuth.http.legacy.enabled", "false"); - if (Boolean.parseBoolean(legacyEnabled)) { - handlerExecutionChain.addInterceptor(this.applicationContext.getBean(SleuthTraceHandlerInterceptor.class)); - } + handlerExecutionChain.addInterceptor(this.applicationContext.getBean(SpanCustomizingAsyncHandlerInterceptor.class)); return handlerExecutionChain; } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java index 682589104..d55993e6c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java @@ -16,46 +16,28 @@ package org.springframework.cloud.sleuth.instrument.web; -import brave.http.HttpTracing; -import brave.spring.webmvc.TracingHandlerInterceptor; -import org.springframework.beans.factory.BeanFactory; +import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.ApplicationContext; -import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; /** - * MVC Adapter that adds the {@link TracingHandlerInterceptor} + * MVC Adapter that adds the {@link SpanCustomizingAsyncHandlerInterceptor} * * @author Marcin Grzejszczak * * @since 1.0.3 */ @Configuration +@Import(SpanCustomizingAsyncHandlerInterceptor.class) class TraceWebMvcConfigurer implements WebMvcConfigurer { @Autowired ApplicationContext applicationContext; - @Bean - public TracingHandlerInterceptor tracingHandlerInterceptor(HttpTracing tracing) { - return (TracingHandlerInterceptor) TracingHandlerInterceptor.create(tracing); - } - - @Bean - @ConditionalOnProperty("spring.sleuth.http.legacy.enabled") - public SleuthTraceHandlerInterceptor legacySleuthTraceHandlerInterceptor(BeanFactory beanFactory) { - return new SleuthTraceHandlerInterceptor(beanFactory); - } - @Override public void addInterceptors(InterceptorRegistry registry) { - registry.addInterceptor(this.applicationContext.getBean(TracingHandlerInterceptor.class)); - String legacyEnabled = this.applicationContext.getEnvironment() - .getProperty("spring.sleuth.http.legacy.enabled", "false"); - if (Boolean.parseBoolean(legacyEnabled)) { - registry.addInterceptor(this.applicationContext.getBean(SleuthTraceHandlerInterceptor.class)); - } + registry.addInterceptor(this.applicationContext.getBean(SpanCustomizingAsyncHandlerInterceptor.class)); } } 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 29e50be80..051a5ee62 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 @@ -19,6 +19,7 @@ package org.springframework.cloud.sleuth.instrument.web; import brave.Tracer; import brave.http.HttpTracing; import brave.servlet.TracingFilter; +import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -54,6 +55,7 @@ import static javax.servlet.DispatcherType.REQUEST; @ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET) @ConditionalOnBean(HttpTracing.class) @AutoConfigureAfter(TraceHttpAutoConfiguration.class) +@Import(SpanCustomizingAsyncHandlerInterceptor.class) public class TraceWebServletAutoConfiguration { public static final int TRACING_FILTER_ORDER = Ordered.HIGHEST_PRECEDENCE + 5; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java index 735f52720..7aa53fa71 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java @@ -18,7 +18,7 @@ package org.springframework.cloud.sleuth.instrument.zuul; import java.lang.invoke.MethodHandles; -import brave.spring.webmvc.TracingHandlerInterceptor; +import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.BeansException; @@ -58,7 +58,7 @@ class TraceZuulHandlerMappingBeanPostProcessor implements BeanPostProcessor { } ZuulHandlerMapping zuulHandlerMapping = (ZuulHandlerMapping) bean; zuulHandlerMapping.setInterceptors( - this.beanFactory.getBean(TracingHandlerInterceptor.class)); + this.beanFactory.getBean(SpanCustomizingAsyncHandlerInterceptor.class)); } return bean; } 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 561a34eba..526764c7e 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 @@ -96,7 +96,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { public void should_create_a_trace() throws Exception { whenSentPingWithoutTracingData(); - then(this.reporter.getSpans()).hasSize(2); + then(this.reporter.getSpans()).hasSize(1); zipkin2.Span span = this.reporter.getSpans().get(0); then(span.tags()) .containsKey(new TraceKeys().getMvc().getControllerClass()) @@ -175,14 +175,11 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { whenSentToNonExistentEndpointWithTraceId(expectedTraceId); // it's a span with the same ids - then(this.reporter.getSpans()).hasSize(2); + then(this.reporter.getSpans()).hasSize(1); zipkin2.Span serverSpan = this.reporter.getSpans().get(0); then(serverSpan.tags()) .containsEntry("custom", "tag") .containsEntry("http.status_code", "404"); - zipkin2.Span handlerSpan = this.reporter.getSpans().get(0); - then(handlerSpan.tags()) - .containsEntry("http.status_code", "404"); then(this.tracer.currentSpan()).isNull(); } @@ -199,12 +196,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { // we need to dump the span cause it's not in TraceFilter since TF // has also error dispatch and the ErrorController would report the span - then(this.reporter.getSpans()).hasSize(2); - // server + then(this.reporter.getSpans()).hasSize(1); then(this.reporter.getSpans().get(0).tags()) - .containsEntry("error", "java.lang.RuntimeException"); - // handler - then(this.reporter.getSpans().get(1).tags()) .containsEntry("error", "Request processing failed; nested exception is java.lang.RuntimeException"); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java index a41c0942a..b3970917c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java @@ -84,19 +84,15 @@ public class TraceFilterWebIntegrationTests { } then(Tracing.current().tracer().currentSpan()).isNull(); - then(this.accumulator.getSpans()).hasSize(2); + then(this.accumulator.getSpans()).hasSize(1); Span fromFirstTraceFilterFlow = this.accumulator.getSpans().get(0); then(fromFirstTraceFilterFlow.tags()) .containsEntry("http.status_code", "500") .containsEntry("http.method", "GET") - .containsEntry("error", "Throwing exception") - .containsEntry("mvc.controller.class", "ExceptionThrowingController"); - Span fromErrorController = this.accumulator.getSpans().get(1); - then(fromErrorController.tags()) - .containsEntry("http.status_code", "500") + .containsEntry("mvc.controller.class", "ExceptionThrowingController") .containsEntry("error", "Request processing failed; nested exception is java.lang.RuntimeException: Throwing exception"); // issue#714 - String hex = fromErrorController.traceId(); + String hex = fromFirstTraceFilterFlow.traceId(); String[] split = capture.toString().split("\n"); List list = Arrays.stream(split).filter(s -> s.contains( "Uncaught exception thrown")) @@ -114,9 +110,10 @@ public class TraceFilterWebIntegrationTests { } then(Tracing.current().tracer().currentSpan()).isNull(); - then(this.accumulator.getSpans()).hasSize(2).as("spans with same id, one from server, one from handler"); + then(this.accumulator.getSpans()).hasSize(1); then(this.accumulator.getSpans().get(0).kind().ordinal()).isEqualTo(Span.Kind.SERVER.ordinal()); then(this.accumulator.getSpans().get(0).tags()).containsEntry("http.status_code", "400"); + then(this.accumulator.getSpans().get(0).tags()).containsEntry("http.path", "/test_bad_request"); } @Test