From 9f6801aa0592202567b1b2bea6c48b898fe1b0bc Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 7 Mar 2018 21:54:30 -0500 Subject: [PATCH] New span parser (#891) --- .../sleuth/annotation/DefaultSpanCreator.java | 26 +++++------- .../{SpanCreator.java => NewSpanParser.java} | 17 ++++---- .../annotation/SleuthAdvisorConfig.java | 40 +++++++++---------- .../SleuthAnnotationAutoConfiguration.java | 4 +- .../annotation/SpanTagAnnotationHandler.java | 23 ++++------- .../web/SleuthTraceHandlerInterceptor.java | 25 +++++------- ...hNewSpanParserAnnotationDisableTests.java} | 6 +-- ...NewSpanParserAnnotationNoSleuthTests.java} | 6 +-- 8 files changed, 61 insertions(+), 86 deletions(-) rename spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/{SpanCreator.java => NewSpanParser.java} (67%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/{SleuthSpanCreatorAnnotationDisableTests.java => SleuthNewSpanParserAnnotationDisableTests.java} (88%) rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/{SleuthSpanCreatorAnnotationNoSleuthTests.java => SleuthNewSpanParserAnnotationNoSleuthTests.java} (89%) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/DefaultSpanCreator.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/DefaultSpanCreator.java index 36ae41a90..e6cff02ec 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/DefaultSpanCreator.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/DefaultSpanCreator.java @@ -15,8 +15,7 @@ */ package org.springframework.cloud.sleuth.annotation; -import brave.Span; -import brave.Tracing; +import brave.SpanCustomizer; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -24,31 +23,26 @@ import org.springframework.cloud.sleuth.util.SpanNameUtil; import org.springframework.util.StringUtils; /** - * Default implementation of the {@link SpanCreator} that creates - * a new span around the annotated method. + * Default implementation of the {@link NewSpanParser} that parses only the + * span name. * * @author Christian Schwerdtfeger * @since 1.2.0 */ -class DefaultSpanCreator implements SpanCreator { +class DefaultNewSpanParser implements NewSpanParser { - private static final Log log = LogFactory.getLog(DefaultSpanCreator.class); + private static final Log log = LogFactory.getLog(DefaultNewSpanParser.class); - private final Tracing tracer; - - DefaultSpanCreator(Tracing tracer) { - this.tracer = tracer; - } - - @Override public Span createSpan(MethodInvocation pjp, NewSpan newSpanAnnotation) { - String name = StringUtils.isEmpty(newSpanAnnotation.name()) ? - pjp.getMethod().getName() : newSpanAnnotation.name(); + @Override + public void parse(MethodInvocation pjp, NewSpan newSpan, SpanCustomizer span) { + String name = StringUtils.isEmpty(newSpan.name()) ? + pjp.getMethod().getName() : newSpan.name(); String changedName = SpanNameUtil.toLowerHyphen(name); if (log.isDebugEnabled()) { log.debug("For the class [" + pjp.getThis().getClass() + "] method " + "[" + pjp.getMethod().getName() + "] will name the span [" + changedName + "]"); } - return this.tracer.tracer().nextSpan().name(changedName).start(); + span.name(changedName); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanCreator.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/NewSpanParser.java similarity index 67% rename from spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanCreator.java rename to spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/NewSpanParser.java index e1dba87cf..91a31a60b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanCreator.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/NewSpanParser.java @@ -16,20 +16,17 @@ package org.springframework.cloud.sleuth.annotation; -import brave.Span; +import brave.SpanCustomizer; import org.aopalliance.intercept.MethodInvocation; /** - * A contract for creating a new span for a given join point - * and the {@link NewSpan} annotation. + * Parses data for a span created via a {@link NewSpan} annotation. * - * @author Christian Schwerdtfeger - * @since 1.2.0 + * @author Adrian Cole + * @since 2.0.0 */ -public interface SpanCreator { +public interface NewSpanParser { - /** - * Returns a new {@link Span} for the join point and {@link NewSpan} - */ - Span createSpan(MethodInvocation methodInvocation, NewSpan newSpan); + /** Override to control the name and tags on an annotation-based span */ + void parse(MethodInvocation methodInvocation, NewSpan newSpan, SpanCustomizer span); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAdvisorConfig.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAdvisorConfig.java index 7454ebb69..53183904f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAdvisorConfig.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAdvisorConfig.java @@ -23,7 +23,6 @@ import javax.annotation.PostConstruct; import brave.Span; import brave.Tracer; -import brave.Tracing; import org.aopalliance.aop.Advice; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; @@ -175,8 +174,8 @@ class SleuthInterceptor implements IntroductionInterceptor, BeanFactoryAware { private static final String METHOD_KEY = "method"; private BeanFactory beanFactory; - private SpanCreator spanCreator; - private Tracing tracing; + private NewSpanParser newSpanParser; + private Tracer tracer; private SpanTagAnnotationHandler spanTagAnnotationHandler; private ErrorParser errorParser; @@ -193,13 +192,14 @@ class SleuthInterceptor implements IntroductionInterceptor, BeanFactoryAware { if (newSpan == null && continueSpan == null) { return invocation.proceed(); } - Span span = tracing().tracer().currentSpan(); + Span span = tracer().currentSpan(); if (newSpan != null || span == null) { - span = spanCreator().createSpan(invocation, newSpan); + span = tracer().nextSpan().start(); + newSpanParser().parse(invocation, newSpan, span); } String log = log(continueSpan); boolean hasLog = StringUtils.hasText(log); - try (Tracer.SpanInScope ws = tracing().tracer().withSpanInScope(span)) { + try (Tracer.SpanInScope ws = tracer().withSpanInScope(span)) { if (hasLog) { logEvent(span, log + ".before"); } @@ -216,13 +216,11 @@ class SleuthInterceptor implements IntroductionInterceptor, BeanFactoryAware { errorParser().parseErrorTags(span, e); throw e; } finally { - if (span != null) { - if (hasLog) { - logEvent(span, log + ".after"); - } - if (newSpan != null) { - span.finish(); - } + if (hasLog) { + logEvent(span, log + ".after"); + } + if (newSpan != null) { + span.finish(); } } } @@ -249,18 +247,18 @@ class SleuthInterceptor implements IntroductionInterceptor, BeanFactoryAware { return ""; } - private Tracing tracing() { - if (this.tracing == null) { - this.tracing = this.beanFactory.getBean(Tracing.class); + private Tracer tracer() { + if (this.tracer == null) { + this.tracer = this.beanFactory.getBean(Tracer.class); } - return this.tracing; + return this.tracer; } - private SpanCreator spanCreator() { - if (this.spanCreator == null) { - this.spanCreator = this.beanFactory.getBean(SpanCreator.class); + private NewSpanParser newSpanParser() { + if (this.newSpanParser == null) { + this.newSpanParser = this.beanFactory.getBean(NewSpanParser.class); } - return this.spanCreator; + return this.newSpanParser; } private SpanTagAnnotationHandler spanTagAnnotationHandler() { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAnnotationAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAnnotationAutoConfiguration.java index 624658035..62a783918 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAnnotationAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SleuthAnnotationAutoConfiguration.java @@ -46,8 +46,8 @@ import org.springframework.context.annotation.Role; public class SleuthAnnotationAutoConfiguration { @Bean - @ConditionalOnMissingBean SpanCreator spanCreator(Tracing tracing) { - return new DefaultSpanCreator(tracing); + @ConditionalOnMissingBean NewSpanParser newSpanParser() { + return new DefaultNewSpanParser(); } @Bean diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanTagAnnotationHandler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanTagAnnotationHandler.java index 86a381895..31e73c475 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanTagAnnotationHandler.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/annotation/SpanTagAnnotationHandler.java @@ -20,8 +20,7 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; -import brave.Span; -import brave.Tracing; +import brave.SpanCustomizer; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -46,7 +45,7 @@ class SpanTagAnnotationHandler { private static final Log log = LogFactory.getLog(SpanTagAnnotationHandler.class); private final BeanFactory beanFactory; - private Tracing tracing; + private SpanCustomizer spanCustomizer; SpanTagAnnotationHandler(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -126,14 +125,14 @@ class SpanTagAnnotationHandler { } } - private Span span() { - Span span = tracing().tracer().currentSpan(); - if (span != null) { - return span; + private SpanCustomizer span() { + if (this.spanCustomizer == null) { + this.spanCustomizer = this.beanFactory.getBean(SpanCustomizer.class); } - return tracing().tracer().nextSpan(); + return this.spanCustomizer; } + private String resolveTagKey( SleuthAnnotatedParameter container) { return StringUtils.hasText(container.annotation.value()) ? @@ -153,12 +152,4 @@ class SpanTagAnnotationHandler { } return argument.toString(); } - - private Tracing tracing() { - if (this.tracing == null) { - this.tracing = this.beanFactory.getBean(Tracing.class); - } - return this.tracing; - } - } 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 index 713f9d607..64e11f825 100644 --- 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 @@ -19,8 +19,7 @@ package org.springframework.cloud.sleuth.instrument.web; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import brave.Span; -import brave.http.HttpTracing; +import brave.SpanCustomizer; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; @@ -41,7 +40,7 @@ class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { private static final Log log = LogFactory.getLog(SleuthTraceHandlerInterceptor.class); private final BeanFactory beanFactory; - private HttpTracing tracing; + private SpanCustomizer spanCustomizer; private TraceKeys traceKeys; private ErrorParser errorParser; @@ -52,11 +51,7 @@ class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { - Span span = httpTracing().tracing() - .tracer().currentSpan(); - if (span == null) { - return true; - } + SpanCustomizer span = spanCustomizer(); if (log.isDebugEnabled()) { log.debug("Adding tags to span " + span); } @@ -65,7 +60,7 @@ class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { return true; } - private void addClassMethodTag(Object handler, Span span) { + private void addClassMethodTag(Object handler, SpanCustomizer span) { if (handler instanceof HandlerMethod) { String methodName = ((HandlerMethod) handler).getMethod().getName(); span.tag(traceKeys().getMvc().getControllerMethod(), methodName); @@ -75,7 +70,7 @@ class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { } } - private void addClassNameTag(Object handler, Span span) { + private void addClassNameTag(Object handler, SpanCustomizer span) { String className; if (handler instanceof HandlerMethod) { className = ((HandlerMethod) handler).getBeanType().getSimpleName(); @@ -97,17 +92,17 @@ class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter { @Override public void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex) { - Span span = httpTracing().tracing().tracer().currentSpan(); + SpanCustomizer span = spanCustomizer(); if (ex != null && span != null) { errorParser().parseErrorTags(span, ex); } } - private HttpTracing httpTracing() { - if (this.tracing == null) { - this.tracing = this.beanFactory.getBean(HttpTracing.class); + private SpanCustomizer spanCustomizer() { + if (this.spanCustomizer == null) { + this.spanCustomizer = this.beanFactory.getBean(SpanCustomizer.class); } - return this.tracing; + return this.spanCustomizer; } private TraceKeys traceKeys() { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationDisableTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationDisableTests.java similarity index 88% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationDisableTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationDisableTests.java index 6e9296943..dff55c6d8 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationDisableTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationDisableTests.java @@ -27,12 +27,12 @@ import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) @SpringBootTest(classes = SleuthAnnotationAutoConfiguration.class, properties = "spring.sleuth.annotation.enabled=false") -public class SleuthSpanCreatorAnnotationDisableTests { +public class SleuthNewSpanParserAnnotationDisableTests { - @Autowired(required = false) SpanCreator spanCreator; + @Autowired(required = false) NewSpanParser newSpanParser; @Test public void shouldNotAutowireBecauseConfigIsDisabled() { - assertThat(this.spanCreator).isNull(); + assertThat(this.newSpanParser).isNull(); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationNoSleuthTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationNoSleuthTests.java similarity index 89% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationNoSleuthTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationNoSleuthTests.java index 4954491d9..dc7f4204a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthSpanCreatorAnnotationNoSleuthTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/annotation/SleuthNewSpanParserAnnotationNoSleuthTests.java @@ -28,14 +28,14 @@ import static org.assertj.core.api.Assertions.assertThat; @RunWith(SpringRunner.class) @SpringBootTest(classes = SleuthAnnotationAutoConfiguration.class, properties = "spring.sleuth.enabled=false") -public class SleuthSpanCreatorAnnotationNoSleuthTests { +public class SleuthNewSpanParserAnnotationNoSleuthTests { - @Autowired(required = false) SpanCreator spanCreator; + @Autowired(required = false) NewSpanParser newSpanParser; @Autowired(required = false) Tracing tracing; @Test public void shouldNotAutowireBecauseConfigIsDisabled() { - assertThat(this.spanCreator).isNull(); + assertThat(this.newSpanParser).isNull(); assertThat(this.tracing).isNull(); } }