From 0df278c9afd994fe017a5332fb77a517d437819c Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 29 Dec 2015 10:15:46 +0000 Subject: [PATCH] Export export flag via headers when needed Prevents non-exportable spans from being propagated without knowing their status. --- .../org/springframework/cloud/sleuth/Trace.java | 2 ++ .../integration/TraceChannelInterceptor.java | 7 ++++++- .../sleuth/instrument/web/TraceFilter.java | 14 ++++++++++++-- .../TraceFeignClientAutoConfiguration.java | 3 ++- .../sleuth/log/SleuthLogAutoConfiguration.java | 1 + .../cloud/sleuth/log/Slf4jSpanListener.java | 17 +++++++++-------- .../cloud/sleuth/sampler/IsTracingSampler.java | 2 ++ .../TraceChannelInterceptorTests.java | 15 +++++++++++---- 8 files changed, 45 insertions(+), 16 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java index 48a4258b7..3a2e5d0a6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java @@ -41,6 +41,8 @@ public class Trace { public static final String SPAN_ID_NAME = "X-Span-Id"; + public static final String SPAN_EXPORT_NAME = "X-Span-Export"; + public static final List HEADERS = Arrays.asList(SPAN_ID_NAME, TRACE_ID_NAME, SPAN_NAME_NAME, PARENT_ID_NAME, PROCESS_ID_NAME, NOT_SAMPLED_NAME); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java index adb6612c4..9ec311f4a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java @@ -20,6 +20,7 @@ import org.springframework.cloud.sleuth.MilliSpan; import org.springframework.cloud.sleuth.MilliSpan.MilliSpanBuilder; import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.TraceManager; +import org.springframework.cloud.sleuth.sampler.IsTracingSampler; import org.springframework.integration.channel.AbstractMessageChannel; import org.springframework.integration.context.IntegrationObjectSupport; import org.springframework.messaging.Message; @@ -81,7 +82,11 @@ public class TraceChannelInterceptor extends ChannelInterceptorAdapter { trace = this.traceManager.startSpan(name, span.build()); } else { - trace = this.traceManager.startSpan(name); + if (message.getHeaders().containsKey(Trace.NOT_SAMPLED_NAME)) { + trace = this.traceManager.startSpan(name, IsTracingSampler.INSTANCE, null); + } else { + trace = this.traceManager.startSpan(name); + } } this.traceHolder.set(trace); return SpanMessageHeaders.addSpanHeaders(message, trace.getSpan()); 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 2a5f6c565..eae55ac64 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 @@ -33,6 +33,7 @@ import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.TraceManager; import org.springframework.cloud.sleuth.event.ServerReceivedEvent; import org.springframework.cloud.sleuth.event.ServerSentEvent; +import org.springframework.cloud.sleuth.sampler.IsTracingSampler; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; @@ -132,7 +133,13 @@ public class TraceFilter extends OncePerRequestFilter } else { - trace = this.traceManager.startSpan(name); + if (skip) { + trace = this.traceManager.startSpan(name, IsTracingSampler.INSTANCE, + null); + } + else { + trace = this.traceManager.startSpan(name); + } request.setAttribute(TRACE_REQUEST_ATTR, trace); } @@ -141,6 +148,9 @@ public class TraceFilter extends OncePerRequestFilter trace.getSpan().getTraceId()); addToResponseIfNotPresent(response, Trace.SPAN_ID_NAME, trace.getSpan().getSpanId()); + if (skip) { + addToResponseIfNotPresent(response, Trace.NOT_SAMPLED_NAME, ""); + } try { @@ -215,7 +225,7 @@ public class TraceFilter extends OncePerRequestFilter private String getHeader(HttpServletRequest request, HttpServletResponse response, String name) { String value = request.getHeader(name); - return hasText(value) ? value : response.getHeader(name); + return value!=null ? value : response.getHeader(name); } private void addToResponseIfNotPresent(HttpServletResponse response, String name, diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java index 6b5d04504..ed15803d1 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java @@ -41,8 +41,8 @@ import org.springframework.cloud.sleuth.TraceAccessor; import org.springframework.cloud.sleuth.TraceManager; import org.springframework.cloud.sleuth.event.ClientReceivedEvent; import org.springframework.cloud.sleuth.event.ClientSentEvent; -import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConcurrencyStrategy; import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConcurrencyStrategy; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; @@ -51,6 +51,7 @@ import org.springframework.context.annotation.Primary; import org.springframework.context.annotation.Scope; import com.netflix.hystrix.HystrixCommand; + import feign.Client; import feign.Feign; import feign.FeignException; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java index 23f3c8eba..b15ef69cf 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/SleuthLogAutoConfiguration.java @@ -41,6 +41,7 @@ public class SleuthLogAutoConfiguration { @Bean @ConditionalOnProperty(value = "spring.sleuth.log.slf4j.enabled", matchIfMissing = true) public Slf4jSpanListener slf4jSpanStartedListener() { + // Sets up MDC entries X-Trace-Id and X-Span-Id return new Slf4jSpanListener(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jSpanListener.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jSpanListener.java index e2108a087..c16c8a81a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jSpanListener.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/log/Slf4jSpanListener.java @@ -39,11 +39,11 @@ public class Slf4jSpanListener { public void start(SpanAcquiredEvent event) { Span span = event.getSpan(); MDC.put(Trace.SPAN_ID_NAME, span.getSpanId()); + MDC.put(Trace.SPAN_EXPORT_NAME, String.valueOf(span.isExportable())); MDC.put(Trace.TRACE_ID_NAME, span.getTraceId()); - // TODO: what log level? - log.info("Starting span: {}", span); + log.trace("Starting span: {}", span); if (event.getParent() != null) { - log.info("With parent: {}", event.getParent()); + log.trace("With parent: {}", event.getParent()); } } @@ -53,21 +53,22 @@ public class Slf4jSpanListener { Span span = event.getSpan(); MDC.put(Trace.SPAN_ID_NAME, span.getSpanId()); MDC.put(Trace.TRACE_ID_NAME, span.getTraceId()); - // TODO: what should this log level be? - log.info("Continued span: {}", event.getSpan()); + MDC.put(Trace.SPAN_EXPORT_NAME, String.valueOf(span.isExportable())); + log.trace("Continued span: {}", event.getSpan()); } @EventListener(SpanReleasedEvent.class) @Order(Ordered.LOWEST_PRECEDENCE) public void stop(SpanReleasedEvent event) { - // TODO: what should this log level be? - log.info("Stopped span: {}", event.getSpan()); + log.trace("Stopped span: {}", event.getSpan()); if (event.getParent() != null) { - log.info("With parent: {}", event.getParent()); + log.trace("With parent: {}", event.getParent()); MDC.put(Trace.SPAN_ID_NAME, event.getParent().getSpanId()); + MDC.put(Trace.SPAN_EXPORT_NAME, String.valueOf(event.getParent().isExportable())); } else { MDC.remove(Trace.SPAN_ID_NAME); + MDC.remove(Trace.SPAN_EXPORT_NAME); MDC.remove(Trace.TRACE_ID_NAME); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/IsTracingSampler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/IsTracingSampler.java index 01cb0faa6..590c43f60 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/IsTracingSampler.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/sampler/IsTracingSampler.java @@ -24,6 +24,8 @@ import org.springframework.cloud.sleuth.trace.TraceContextHolder; */ public class IsTracingSampler implements Sampler { + public static IsTracingSampler INSTANCE = new IsTracingSampler(); + @Override public boolean next(Void info) { return TraceContextHolder.isTracing(); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java index 5c033c8a8..ab2e3b1dc 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.instrument.integration; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -28,6 +29,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Trace; import org.springframework.cloud.sleuth.TraceManager; import org.springframework.cloud.sleuth.instrument.integration.TraceChannelInterceptorTests.App; @@ -61,9 +63,12 @@ public class TraceChannelInterceptorTests implements MessageHandler { private Message message; + private Span span; + @Override public void handleMessage(Message message) throws MessagingException { this.message = message; + this.span = TraceContextHolder.getCurrentSpan(); } @Before @@ -78,17 +83,19 @@ public class TraceChannelInterceptorTests implements MessageHandler { } @Test - public void testNoSpanCreation() { + public void nonExportableSpanCreation() { this.channel.send(MessageBuilder.withPayload("hi").setHeader(Trace.NOT_SAMPLED_NAME, "") .build()); assertNotNull("message was null", this.message); String spanId = this.message.getHeaders().get(Trace.SPAN_ID_NAME, String.class); - assertNull("spanId was not null", spanId); + assertNotNull("spanId was null", spanId); + assertNull(TraceContextHolder.getCurrentTrace()); + assertFalse(this.span.isExportable()); } @Test - public void testSpanCreation() { + public void spanCreation() { this.channel.send(MessageBuilder.withPayload("hi").build()); assertNotNull("message was null", this.message); @@ -101,7 +108,7 @@ public class TraceChannelInterceptorTests implements MessageHandler { } @Test - public void testHeaderCreation() { + public void headerCreation() { Trace trace = this.traceManager.startSpan("testSendMessage", new AlwaysSampler(), null); this.channel.send(MessageBuilder.withPayload("hi").build());