New span parser (#891)

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);
 	}
 
 }

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);
 }

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() {

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

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;
-	}
-
 }

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() {

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();
 	}
 }

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();
 	}
 }