diff --git a/benchmarks/pom.xml b/benchmarks/pom.xml index 3c6bd6c2d..a0c924bd2 100644 --- a/benchmarks/pom.xml +++ b/benchmarks/pom.xml @@ -17,7 +17,7 @@ Benchmarks Benchmarks (JMH) org.springframework.cloud - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT benchmarks diff --git a/docs/pom.xml b/docs/pom.xml index f46f8eac7..0927eea06 100644 --- a/docs/pom.xml +++ b/docs/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT spring-cloud-sleuth-docs pom @@ -14,7 +14,7 @@ spring-cloud-sleuth - 1.0.x + 1.0.x,1.1.x ${basedir}/.. diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 1f59207b8..984ed90bf 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -3,6 +3,7 @@ :github-raw: http://raw.github.com/{github-repo}/{github-tag} :github-code: http://github.com/{github-repo}/tree/{github-tag} :toc: left +:toclevels: 8 :nofooter: Spring Cloud Sleuth @@ -37,7 +38,9 @@ fixed fraction of spans. NOTE: the `PercentageBasedSampler` is the default if you are using `spring-cloud-sleuth-zipkin` or `spring-cloud-sleuth-stream`. You can -configure the exports using `spring.sleuth.sampler.percentage`. +configure the exports using `spring.sleuth.sampler.percentage`. The passed +value needs to be a double from `0.0` to `1.0` so it's not a percentage. +For backwards compatibility reasons we're not changing the property name. A sampler can be installed just by creating a bean definition, e.g: @@ -227,8 +230,7 @@ to your bean definition. === HTTP -For HTTP these are the beans responsible for creation of a Span from a `HttpServletRequest` - and filling in the `HttpServletResponse` with tracing information. +For HTTP these are the beans responsible for creation of a Span from a `HttpServletRequest`. [source,java] ---- @@ -236,11 +238,6 @@ For HTTP these are the beans responsible for creation of a Span from a `HttpServ public SpanExtractor httpServletRequestSpanExtractor() { ... } - -@Bean -public SpanInjector httpServletResponseSpanInjector() { - ... -} ---- You can override them by providing your own implementation and by adding a `@Primary` annotation @@ -261,20 +258,27 @@ This is a an example of a `SpanExtractor` include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java[tags=extractor,indent=0] ---- -The following `SpanInjector` could be created - -[source,java] ----- -include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java[tags=injector,indent=0] ----- - -And you could register them like this: +And you could register it like this: [source,java] ---- include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java[tags=configuration,indent=0] ---- +Spring Cloud Sleuth does not add trace/span related headers to the Http Response for security reasons. If you need the headers then a custom `SpanInjector` +that injects the headers into the Http Response and a Servlet filter which makes use of this can be added the following way: + +[source,java] +---- +include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java[tags=injector,indent=0] +---- + +And you could register them like this: +[source,java] +---- +include::../../../..//spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java[tags=configuration,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/pom.xml b/pom.xml index 2c07235b6..529abe4d1 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ 4.0.0 spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT pom Spring Cloud Sleuth Spring Cloud Sleuth diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index e58f7e09b..9b3aba544 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -11,7 +11,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java index 4a152355d..e44e6d44a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java @@ -90,6 +90,7 @@ public class Span { public static final String SPAN_NOT_SAMPLED = "0"; public static final String SPAN_LOCAL_COMPONENT_TAG_NAME = "lc"; + public static final String SPAN_ERROR_TAG_NAME = "error"; /** * cr - Client Receive. Signifies the end of the span. The client has diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceExecutorBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceExecutorBeanPostProcessor.java index 66f97a16b..04e8ea0ef 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceExecutorBeanPostProcessor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/async/TraceExecutorBeanPostProcessor.java @@ -43,8 +43,7 @@ class TraceExecutorBeanPostProcessor implements BeanPostProcessor { if (bean instanceof ThreadPoolTaskExecutor && !(bean instanceof TaskScheduler) && !(bean instanceof LazyTraceThreadPoolTaskExecutor)) { return new LazyTraceThreadPoolTaskExecutor(this.beanFactory, (ThreadPoolTaskExecutor) bean); - } - if (bean instanceof Executor && !(bean instanceof TaskScheduler) && !(bean instanceof LazyTraceExecutor)) { + } else if (bean instanceof Executor && !(bean instanceof TaskScheduler) && !(bean instanceof LazyTraceExecutor)) { return new LazyTraceExecutor(this.beanFactory, (Executor) bean); } return bean; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java index 6f594ea4b..df22ff6e8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/AbstractTraceChannelInterceptor.java @@ -1,5 +1,9 @@ package org.springframework.cloud.sleuth.instrument.messaging; +import java.lang.invoke.MethodHandles; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanExtractor; import org.springframework.cloud.sleuth.SpanInjector; @@ -22,6 +26,8 @@ import org.springframework.util.ClassUtils; abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter implements ExecutorChannelInterceptor { + private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + /** * If a span comes from messaging components then it will have this value as a prefix * to its name. @@ -63,7 +69,12 @@ abstract class AbstractTraceChannelInterceptor extends ChannelInterceptorAdapter * missing. */ protected Span buildSpan(Message message) { - return this.spanExtractor.joinTrace(message); + try { + return this.spanExtractor.joinTrace(message); + } catch (Exception e) { + log.error("Exception occurred while trying to extract span from carrier", e); + return null; + } } String getChannelName(MessageChannel channel) { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptor.java index a290b5a27..171ab01ad 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptor.java @@ -23,6 +23,7 @@ import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.sampler.NeverSampler; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.MessageHandler; @@ -53,6 +54,7 @@ public class TraceChannelInterceptor extends AbstractTraceChannelInterceptor { } else if (spanFromHeader != null) { spanFromHeader.logEvent(Span.CLIENT_RECV); } + addErrorTag(ex); getTracer().close(spanFromHeader); } @@ -120,10 +122,17 @@ public class TraceChannelInterceptor extends AbstractTraceChannelInterceptor { Span spanFromHeader = getSpanFromHeader(message); if (spanFromHeader!= null) { spanFromHeader.logEvent(Span.SERVER_SEND); + addErrorTag(ex); } getTracer().detach(spanFromHeader); } + private void addErrorTag(Exception ex) { + if (ex != null) { + getTracer().addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(ex)); + } + } + private Span getSpanFromHeader(Message message) { if (message == null) { return null; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractor.java index 8081b1104..2a682bfe6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractor.java @@ -54,13 +54,18 @@ class HttpServletRequestExtractor implements SpanExtractor { // can't build a Span without trace id return null; } - String uri = this.urlPathHelper.getPathWithinApplication(carrier); - boolean skip = this.skipPattern.matcher(uri).matches() - || Span.SPAN_NOT_SAMPLED.equals(carrier.getHeader(Span.SAMPLED_NAME)); - long traceId = Span - .hexToId(carrier.getHeader(Span.TRACE_ID_NAME)); - long spanId = spanId(carrier, traceId); - return buildParentSpan(carrier, uri, skip, traceId, spanId); + try { + String uri = this.urlPathHelper.getPathWithinApplication(carrier); + boolean skip = this.skipPattern.matcher(uri).matches() + || Span.SPAN_NOT_SAMPLED.equals(carrier.getHeader(Span.SAMPLED_NAME)); + long traceId = Span + .hexToId(carrier.getHeader(Span.TRACE_ID_NAME)); + long spanId = spanId(carrier, traceId); + return buildParentSpan(carrier, uri, skip, traceId, spanId); + } catch (Exception e) { + log.error("Exception occurred while trying to extract span from carrier", e); + return null; + } } private long spanId(HttpServletRequest carrier, long traceId) { 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 e5222e2cc..96ed118a7 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 @@ -15,18 +15,18 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.regex.Pattern; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -36,6 +36,7 @@ import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.sampler.NeverSampler; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; @@ -134,19 +135,13 @@ public class TraceFilter extends GenericFilterBean { return; } String name = HTTP_COMPONENT + ":" + uri; - try { - spanFromRequest = createSpan(request, skip, spanFromRequest, name); - } catch (IllegalArgumentException e) { - filterChain.doFilter(request, response); - response.sendError(HttpStatus.BAD_REQUEST.value(), - "Exception tracing request [" + e.getMessage() + "]"); - return; - } Throwable exception = null; try { + spanFromRequest = createSpan(request, skip, spanFromRequest, name); filterChain.doFilter(request, response); } catch (Throwable e) { exception = e; + this.tracer.addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(e)); throw e; } finally { if (isAsyncStarted(request) || request.isAsyncStarted()) { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java index 58bc9a4b1..538195de3 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java @@ -27,6 +27,7 @@ import org.springframework.boot.autoconfigure.web.ErrorController; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.cloud.sleuth.util.SpanNameUtil; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; @@ -150,6 +151,9 @@ public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { if (log.isDebugEnabled()) { log.debug("Closing span " + span); } + if (ex != null) { + getTracer().addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(ex)); + } getTracer().close(span); } 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 39cd582f2..43117dae3 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 @@ -15,8 +15,8 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; +import java.util.regex.Pattern; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Value; @@ -83,6 +83,12 @@ public class TraceWebAutoConfiguration { return new TraceSpringDataBeanPostProcessor(beanFactory); } + @Bean + @ConditionalOnMissingBean + public HttpTraceKeysInjector httpTraceKeysInjector(Tracer tracer, TraceKeys traceKeys) { + return new HttpTraceKeysInjector(tracer, traceKeys); + } + @Bean public FilterRegistrationBean traceWebFilter(Tracer tracer, TraceKeys traceKeys, SkipPatternProvider skipPatternProvider, SpanReporter spanReporter, diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthWebClientEnabled.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthWebClientEnabled.java new file mode 100644 index 000000000..3c127edee --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthWebClientEnabled.java @@ -0,0 +1,18 @@ +package org.springframework.cloud.sleuth.instrument.web.client; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; + +import java.lang.annotation.*; + +/** + * Helper annotation to enable Sleuth web client + * + * @author Marcin Grzejszczak + * @since 1.0.11 + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.METHOD}) +@Documented +@ConditionalOnProperty(value = "spring.sleuth.web.client.enabled", matchIfMissing = true) +@interface SleuthWebClientEnabled { +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java index 8eda2fbc4..ce71beb84 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptor.java @@ -18,9 +18,11 @@ package org.springframework.cloud.sleuth.instrument.web.client; import java.io.IOException; +import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.http.HttpRequest; import org.springframework.http.client.ClientHttpRequestExecution; import org.springframework.http.client.ClientHttpRequestInterceptor; @@ -59,6 +61,7 @@ public class TraceRestTemplateInterceptor extends AbstractTraceHttpRequestInterc if (log.isDebugEnabled()) { log.debug("Exception occurred while trying to execute the request. Will close the span [" + currentSpan() + "]", e); } + this.tracer.addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(e)); this.tracer.close(currentSpan()); throw e; } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java index 67c32afc2..d5fb8f2e6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java @@ -22,11 +22,10 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; -import org.springframework.cloud.sleuth.SpanAccessor; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.task.AsyncListenableTaskExecutor; @@ -46,21 +45,18 @@ import org.springframework.web.client.AsyncRestTemplate; * @since 1.0.0 */ @Configuration +@SleuthWebClientEnabled @ConditionalOnProperty(value = "spring.sleuth.web.async.client.enabled", matchIfMissing = true) @ConditionalOnClass(AsyncRestTemplate.class) -@ConditionalOnBean(SpanAccessor.class) -@AutoConfigureAfter(TraceAutoConfiguration.class) +@ConditionalOnBean(HttpTraceKeysInjector.class) +@AutoConfigureAfter(TraceWebAutoConfiguration.class) public class TraceWebAsyncClientAutoConfiguration { @Autowired Tracer tracer; - @Autowired - private HttpTraceKeysInjector httpTraceKeysInjector; - @Autowired - private SpanInjector spanInjector; - @Autowired(required = false) - private ClientHttpRequestFactory clientHttpRequestFactory; - @Autowired(required = false) - private AsyncClientHttpRequestFactory asyncClientHttpRequestFactory; + @Autowired private HttpTraceKeysInjector httpTraceKeysInjector; + @Autowired private SpanInjector spanInjector; + @Autowired(required = false) private ClientHttpRequestFactory clientHttpRequestFactory; + @Autowired(required = false) private AsyncClientHttpRequestFactory asyncClientHttpRequestFactory; private TraceAsyncClientHttpRequestFactoryWrapper traceAsyncClientHttpRequestFactory() { ClientHttpRequestFactory clientFactory = this.clientHttpRequestFactory; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java index d07efb11c..6585b0b13 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java @@ -26,12 +26,10 @@ import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; -import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.sleuth.SpanInjector; -import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpRequest; @@ -47,10 +45,10 @@ import org.springframework.web.client.RestTemplate; * @since 1.0.0 */ @Configuration -@ConditionalOnProperty(value = "spring.sleuth.web.client.enabled", matchIfMissing = true) +@SleuthWebClientEnabled @ConditionalOnClass(RestTemplate.class) -@ConditionalOnBean(Tracer.class) -@AutoConfigureAfter(TraceAutoConfiguration.class) +@ConditionalOnBean(HttpTraceKeysInjector.class) +@AutoConfigureAfter(TraceWebAutoConfiguration.class) public class TraceWebClientAutoConfiguration { @Bean @@ -66,12 +64,6 @@ public class TraceWebClientAutoConfiguration { return new HttpRequestInjector(); } - @Bean - @ConditionalOnMissingBean - public HttpTraceKeysInjector httpTraceKeysInjector(Tracer tracer, TraceKeys traceKeys) { - return new HttpTraceKeysInjector(tracer, traceKeys); - } - @Configuration protected static class TraceInterceptorConfiguration { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java index 1c66c0108..33acce88b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java @@ -31,6 +31,7 @@ import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import feign.Client; import feign.Request; import feign.Response; +import org.springframework.cloud.sleuth.util.ExceptionUtils; /** * A Feign Client that closes a Span if there is no response body. In other cases Span @@ -134,11 +135,7 @@ class TraceFeignClient implements Client { private void logError(Exception e) { Span span = getTracer().getCurrentSpan(); if (span != null) { - String message = e.getMessage() != null ? e.getMessage() : e.toString(); - if (log.isDebugEnabled()) { - log.debug("Appending exception [" + message + "] to span " + span); - } - getTracer().addTag("error", message); + getTracer().addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(e)); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java index 6595fefa2..664f57bc8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java @@ -26,6 +26,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration; +import org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Scope; @@ -45,7 +46,7 @@ import feign.Feign; @ConditionalOnClass(Client.class) @ConditionalOnBean(Tracer.class) @AutoConfigureBefore(FeignAutoConfiguration.class) -@AutoConfigureAfter(SleuthHystrixAutoConfiguration.class) +@AutoConfigureAfter({SleuthHystrixAutoConfiguration.class, TraceWebAutoConfiguration.class}) public class TraceFeignClientAutoConfiguration { @Bean diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java index efe3c9113..f940093ed 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java @@ -16,14 +16,10 @@ package org.springframework.cloud.sleuth.instrument.zuul; -import java.lang.invoke.MethodHandles; -import java.net.URI; - import com.netflix.zuul.ExecutionStatus; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.ZuulFilterResult; import com.netflix.zuul.context.RequestContext; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; @@ -32,6 +28,9 @@ import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import org.springframework.cloud.sleuth.instrument.web.TraceRequestAttributes; +import java.lang.invoke.MethodHandles; +import java.net.URI; + /** * A pre request {@link ZuulFilter} that sets tracing related headers on the request * from the current span. We're doing so to ensure tracing propagates to the next hop. diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java index 65dc9a89e..546d24596 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java @@ -15,6 +15,10 @@ */ package org.springframework.cloud.sleuth.instrument.zuul; +import com.netflix.client.http.HttpRequest; +import com.netflix.zuul.ZuulFilter; +import com.netflix.zuul.context.RequestContext; + import org.springframework.beans.factory.BeanFactory; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -26,15 +30,11 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import com.netflix.client.http.HttpRequest; -import com.netflix.zuul.ZuulFilter; -import com.netflix.zuul.context.RequestContext; - /** * {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration Auto-configuration} * enables span information propagation when using Zuul. @@ -47,7 +47,7 @@ import com.netflix.zuul.context.RequestContext; @ConditionalOnWebApplication @ConditionalOnClass(ZuulFilter.class) @ConditionalOnBean(Tracer.class) -@AutoConfigureAfter(TraceAutoConfiguration.class) +@AutoConfigureAfter(TraceWebAutoConfiguration.class) public class TraceZuulAutoConfiguration { @Bean diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/ExceptionUtils.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/ExceptionUtils.java index 7cabe94a8..2c0182a20 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/ExceptionUtils.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/ExceptionUtils.java @@ -55,4 +55,8 @@ public final class ExceptionUtils { ExceptionUtils.fail = fail; ExceptionUtils.lastException = null; } + + public static String getExceptionMessage(Throwable e) { + return e.getMessage() != null ? e.getMessage() : e.toString(); + } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/issues/issue410/Issue410Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/issues/issue410/Issue410Tests.java index 982e11b00..dd3c17a08 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/issues/issue410/Issue410Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/async/issues/issue410/Issue410Tests.java @@ -16,6 +16,8 @@ package org.springframework.cloud.sleuth.instrument.async.issues.issue410; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; @@ -23,7 +25,9 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.boot.autoconfigure.SpringBootApplication; import org.springframework.boot.autoconfigure.data.web.SpringDataWebAutoConfiguration; import org.springframework.boot.test.SpringApplicationConfiguration; @@ -31,6 +35,7 @@ import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.instrument.async.LazyTraceExecutor; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -47,7 +52,7 @@ import org.springframework.web.client.RestTemplate; import com.jayway.awaitility.Awaitility; -import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; /** * @author Marcin Grzejszczak @@ -66,27 +71,71 @@ public class Issue410Tests { @Test public void should_pass_tracing_info_for_tasks_running_without_a_pool() { Span span = this.tracer.createSpan("foo"); + try { + String response = this.restTemplate.getForObject("http://localhost:" + port() + "/without_pool", String.class); - String response = this.restTemplate.getForObject("http://localhost:" + port() + "/without_pool", String.class); - - then(response).isEqualTo(Span.idToHex(span.getTraceId())); - Awaitility.await().until(() -> { - then(this.asyncTask.getSpan().get()).isNotNull(); - then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); - }); + then(response).isEqualTo(Span.idToHex(span.getTraceId())); + Awaitility.await().until(() -> { + then(this.asyncTask.getSpan().get()).isNotNull(); + then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); + }); + } finally { + this.tracer.close(span); + } } @Test public void should_pass_tracing_info_for_tasks_running_with_a_pool() { Span span = this.tracer.createSpan("foo"); + try { + String response = this.restTemplate.getForObject("http://localhost:" + port() + "/with_pool", String.class); - String response = this.restTemplate.getForObject("http://localhost:" + port() + "/with_pool", String.class); + then(response).isEqualTo(Span.idToHex(span.getTraceId())); + Awaitility.await().until(() -> { + then(this.asyncTask.getSpan().get()).isNotNull(); + then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); + }); + } finally { + this.tracer.close(span); + } + } - then(response).isEqualTo(Span.idToHex(span.getTraceId())); - Awaitility.await().until(() -> { - then(this.asyncTask.getSpan().get()).isNotNull(); - then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); - }); + /** + * Related to issue #423 + */ + @Test + public void should_pass_tracing_info_for_completable_futures_with_executor() { + Span span = this.tracer.createSpan("foo"); + try { + String response = this.restTemplate.getForObject("http://localhost:" + port() + "/completable", String.class); + + then(response).isEqualTo(Span.idToHex(span.getTraceId())); + Awaitility.await().until(() -> { + then(this.asyncTask.getSpan().get()).isNotNull(); + then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); + }); + } finally { + this.tracer.close(span); + } + } + + /** + * Related to issue #423 + */ + @Test + public void should_pass_tracing_info_for_completable_futures_with_task_scheduler() { + Span span = this.tracer.createSpan("foo"); + try { + String response = this.restTemplate.getForObject("http://localhost:" + port() + "/taskScheduler", String.class); + + then(response).isEqualTo(Span.idToHex(span.getTraceId())); + Awaitility.await().until(() -> { + then(this.asyncTask.getSpan().get()).isNotNull(); + then(this.asyncTask.getSpan().get().getTraceId()).isEqualTo(span.getTraceId()); + }); + } finally { + this.tracer.close(span); + } } private int port() { @@ -123,6 +172,9 @@ class AsyncTask { private AtomicReference span = new AtomicReference<>(); @Autowired Tracer tracer; + @Autowired @Qualifier("poolTaskExecutor") Executor executor; + @Autowired @Qualifier("taskScheduler") Executor taskScheduler; + @Autowired BeanFactory beanFactory; @Async("poolTaskExecutor") public void runWithPool() { @@ -136,6 +188,58 @@ class AsyncTask { this.span.set(this.tracer.getCurrentSpan()); } + public Span completableFutures() throws ExecutionException, InterruptedException { + log.info("This task is running with completable future"); + CompletableFuture span1 = CompletableFuture + .supplyAsync(() -> { + AsyncTask.log.info("First completable future"); + return AsyncTask.this.tracer.getCurrentSpan(); + }, AsyncTask.this.executor); + CompletableFuture span2 = CompletableFuture + .supplyAsync(() -> { + AsyncTask.log.info("Second completable future"); + return AsyncTask.this.tracer.getCurrentSpan(); + }, AsyncTask.this.executor); + CompletableFuture response = CompletableFuture.allOf(span1, span2) + .thenApply(ignoredVoid -> { + AsyncTask.log.info("Third completable future"); + Span joinedSpan1 = span1.join(); + Span joinedSpan2 = span2.join(); + then(joinedSpan2).isNotNull(); + then(joinedSpan1).hasTraceIdEqualTo(joinedSpan2.getTraceId()); + AsyncTask.log.info("TraceIds are correct"); + return joinedSpan2; + }); + this.span.set(response.get()); + return this.span.get(); + } + + public Span taskScheduler() throws ExecutionException, InterruptedException { + log.info("This task is running with completable future"); + CompletableFuture span1 = CompletableFuture + .supplyAsync(() -> { + AsyncTask.log.info("First completable future"); + return AsyncTask.this.tracer.getCurrentSpan(); + }, new LazyTraceExecutor(AsyncTask.this.beanFactory, AsyncTask.this.taskScheduler)); + CompletableFuture span2 = CompletableFuture + .supplyAsync(() -> { + AsyncTask.log.info("Second completable future"); + return AsyncTask.this.tracer.getCurrentSpan(); + }, new LazyTraceExecutor(AsyncTask.this.beanFactory, AsyncTask.this.taskScheduler)); + CompletableFuture response = CompletableFuture.allOf(span1, span2) + .thenApply(ignoredVoid -> { + AsyncTask.log.info("Third completable future"); + Span joinedSpan1 = span1.join(); + Span joinedSpan2 = span2.join(); + then(joinedSpan2).isNotNull(); + then(joinedSpan1).hasTraceIdEqualTo(joinedSpan2.getTraceId()); + AsyncTask.log.info("TraceIds are correct"); + return joinedSpan2; + }); + this.span.set(response.get()); + return this.span.get(); + } + public AtomicReference getSpan() { return span; } @@ -165,4 +269,16 @@ class Application { return Span.idToHex(this.tracer.getCurrentSpan().getTraceId()); } + @RequestMapping("/completable") + public String completable() throws ExecutionException, InterruptedException { + log.info("Executing completable"); + return Span.idToHex(this.asyncTask.completableFutures().getTraceId()); + } + + @RequestMapping("/taskScheduler") + public String taskScheduler() throws ExecutionException, InterruptedException { + log.info("Executing completable via task scheduler"); + return Span.idToHex(this.asyncTask.taskScheduler().getTraceId()); + } + } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java index 567f90e39..d86eab7fd 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/messaging/TraceChannelInterceptorTests.java @@ -31,6 +31,7 @@ import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.assertions.SleuthAssertions; import org.springframework.cloud.sleuth.instrument.messaging.TraceChannelInterceptorTests.App; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; @@ -87,7 +88,7 @@ public class TraceChannelInterceptorTests implements MessageHandler { this.message = message; this.span = TestSpanContextHolder.getCurrentSpan(); if (message.getHeaders().containsKey("THROW_EXCEPTION")) { - throw new RuntimeException(); + throw new RuntimeException("A terrible exception has occurred"); } } @@ -268,6 +269,9 @@ public class TraceChannelInterceptorTests implements MessageHandler { then(this.message).isNotNull(); this.tracer.close(span); then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(new ListOfSpans(this.accumulator.getSpans())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "A terrible exception has occurred"); } @Test @@ -281,7 +285,7 @@ public class TraceChannelInterceptorTests implements MessageHandler { String traceId = this.message.getHeaders().get(Span.TRACE_ID_NAME, String.class); then(traceId).isNull(); - then(accumulator.getSpans()).isEmpty(); + then(this.accumulator.getSpans()).isEmpty(); then(TestSpanContextHolder.getCurrentSpan()).isNull(); } @@ -299,6 +303,18 @@ public class TraceChannelInterceptorTests implements MessageHandler { then(traceId).isEqualTo(Span.hexToId(lower64Bits)); } + @Test + public void shouldNotBreakWhenInvalidHeadersAreSent() { + this.tracedChannel.send(MessageBuilder.withPayload("hi") + .setHeader(TraceMessageHeaders.PARENT_ID_NAME, "-") + .setHeader(TraceMessageHeaders.TRACE_ID_NAME, Span.idToHex(10L)) + .setHeader(TraceMessageHeaders.SPAN_ID_NAME, Span.idToHex(20L)).build()); + + then(this.message).isNotNull(); + then(this.accumulator.getSpans()).isNotEmpty(); + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + } + @Configuration @EnableAutoConfiguration static class App { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java index fe1afc5df..6cc85427f 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/HttpServletRequestExtractorTests.java @@ -16,9 +16,9 @@ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.http.HttpServletRequest; import java.util.Random; import java.util.regex.Pattern; +import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Test; @@ -28,7 +28,6 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.springframework.cloud.sleuth.Span; -import static org.assertj.core.api.Assertions.fail; import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; @RunWith(MockitoJUnitRunner.class) @@ -54,12 +53,7 @@ public class HttpServletRequestExtractorTests { BDDMockito.given(this.request.getHeader(Span.TRACE_ID_NAME)) .willReturn("invalid"); - try { - this.extractor.joinTrace(this.request); - fail("should throw an exception"); - } catch (IllegalArgumentException e) { - then(e).hasMessageContaining("Malformed id"); - } + then(this.extractor.joinTrace(this.request)).isNull(); } @Test @@ -69,12 +63,7 @@ public class HttpServletRequestExtractorTests { BDDMockito.given(this.request.getHeader(Span.SPAN_ID_NAME)) .willReturn("invalid"); - try { - this.extractor.joinTrace(this.request); - fail("should throw an exception"); - } catch (IllegalArgumentException e) { - then(e).hasMessageContaining("Malformed id"); - } + then(this.extractor.joinTrace(this.request)).isNull(); } @Test @@ -86,12 +75,7 @@ public class HttpServletRequestExtractorTests { BDDMockito.given(this.request.getHeader(Span.PARENT_ID_NAME)) .willReturn("invalid"); - try { - this.extractor.joinTrace(this.request); - fail("should throw an exception"); - } catch (IllegalArgumentException e) { - then(e).hasMessageContaining("Malformed id"); - } + then(this.extractor.joinTrace(this.request)).isNull(); } @Test diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java new file mode 100644 index 000000000..1783bbfce --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceCustomFilterResponseInjectorTests.java @@ -0,0 +1,159 @@ +/* + * Copyright 2013-2016 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 java.io.IOException; +import java.net.URI; +import java.util.HashMap; +import java.util.Map; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletResponse; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.SpanInjector; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.context.ApplicationListener; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpHeaders; +import org.springframework.http.RequestEntity; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestHeader; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestTemplate; +import org.springframework.web.filter.GenericFilterBean; + +import static org.assertj.core.api.BDDAssertions.then; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(TraceCustomFilterResponseInjectorTests.Config.class) +@WebIntegrationTest(randomPort = true) +@DirtiesContext +public class TraceCustomFilterResponseInjectorTests { + @Autowired RestTemplate restTemplate; + @Autowired Config config; + @Autowired CustomRestController customRestController; + + + @Test + @SuppressWarnings("unchecked") + public void should_inject_trace_and_span_ids_in_response_headers() { + RequestEntity requestEntity = RequestEntity + .get(URI.create("http://localhost:" + this.config.port + "/headers")) + .build(); + + @SuppressWarnings("rawtypes") + ResponseEntity responseEntity = this.restTemplate.exchange(requestEntity, Map.class); + + then(responseEntity.getHeaders()) + .containsKeys(Span.TRACE_ID_NAME, Span.SPAN_ID_NAME) + .as("Trace headers must be present in response headers"); + } + + @Configuration + @EnableAutoConfiguration + static class Config + implements ApplicationListener { + int port; + + // tag::configuration[] + @Bean + SpanInjector customHttpServletResponseSpanInjector() { + return new CustomHttpServletResponseSpanInjector(); + } + + @Bean + HttpResponseInjectingTraceFilter responseInjectingTraceFilter(Tracer tracer) { + return new HttpResponseInjectingTraceFilter(tracer, customHttpServletResponseSpanInjector()); + } + // end::configuration[] + + @Override + public void onApplicationEvent(EmbeddedServletContainerInitializedEvent event) { + this.port = event.getEmbeddedServletContainer().getPort(); + } + + @Bean + public RestTemplate restTemplate() { + return new RestTemplate(); + } + + @Bean + CustomRestController customRestController() { + return new CustomRestController(); + } + + + } + + // tag::injector[] + static class CustomHttpServletResponseSpanInjector + implements SpanInjector { + + @Override + public void inject(Span span, HttpServletResponse carrier) { + carrier.addHeader(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); + carrier.addHeader(Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); + } + } + + static class HttpResponseInjectingTraceFilter extends GenericFilterBean { + + private final Tracer tracer; + private final SpanInjector spanInjector; + + public HttpResponseInjectingTraceFilter(Tracer tracer, SpanInjector spanInjector) { + this.tracer = tracer; + this.spanInjector = spanInjector; + } + + @Override + public void doFilter(ServletRequest request, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { + HttpServletResponse response = (HttpServletResponse) servletResponse; + Span currentSpan = this.tracer.getCurrentSpan(); + this.spanInjector.inject(currentSpan, response); + filterChain.doFilter(request, response); + } + } + // end::injector[] + + @RestController + static class CustomRestController { + + @RequestMapping("/headers") + public Map headers(@RequestHeader HttpHeaders headers) { + Map map = new HashMap<>(); + for (String key : headers.keySet()) { + map.put(key, headers.getFirst(key)); + } + return map; + } + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java index 652d961fa..708a5f4a0 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterCustomExtractorTests.java @@ -16,12 +16,11 @@ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.net.URI; import java.util.HashMap; import java.util.Map; import java.util.Random; +import javax.servlet.http.HttpServletRequest; import org.junit.Before; import org.junit.Test; @@ -34,7 +33,6 @@ import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanExtractor; -import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; @@ -83,13 +81,13 @@ public class TraceFilterCustomExtractorTests { .header("mySpanId", Span.idToHex(spanId)).build(); @SuppressWarnings("rawtypes") - ResponseEntity responseHeaders = this.restTemplate.exchange(requestEntity, + ResponseEntity responseEntity = this.restTemplate.exchange(requestEntity, Map.class); await().until(() -> then(this.accumulator.getSpans().stream().filter( span -> span.getSpanId() == spanId).findFirst().get()) .hasTraceIdEqualTo(traceId)); - then(responseHeaders.getBody()) + then(responseEntity.getBody()) .containsEntry("correlationid", Span.idToHex(traceId)) .containsEntry("myspanid", Span.idToHex(spanId)) .as("input request headers"); @@ -107,12 +105,6 @@ public class TraceFilterCustomExtractorTests { SpanExtractor customHttpServletRequestSpanExtractor() { return new CustomHttpServletRequestSpanExtractor(); } - - @Bean - @Primary - SpanInjector customHttpServletResponseSpanInjector() { - return new CustomHttpServletResponseSpanInjector(); - } // end::configuration[] @Override @@ -157,19 +149,6 @@ public class TraceFilterCustomExtractorTests { } // end::extractor[] - // tag::injector[] - static class CustomHttpServletResponseSpanInjector - implements SpanInjector { - - @Override - public void inject(Span span, HttpServletResponse carrier) { - carrier.addHeader("correlationId", Span.idToHex(span.getTraceId())); - carrier.addHeader("mySpanId", Span.idToHex(span.getSpanId())); - // inject the rest of Span values to the header - } - } - // end::injector[] - @RestController static class CustomRestController { 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 4787beef3..db7a6a213 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 @@ -16,10 +16,11 @@ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.http.HttpServletRequest; +import java.util.ArrayList; import java.util.Optional; import java.util.Random; import java.util.regex.Pattern; +import javax.servlet.http.HttpServletRequest; import org.junit.After; import org.junit.Before; @@ -32,12 +33,14 @@ import org.springframework.cloud.sleuth.SpanExtractor; import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.log.SpanLogger; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.sampler.NeverSampler; import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.mock.web.MockFilterChain; @@ -285,7 +288,7 @@ public class TraceFilterTests { } @Test - public void catchesException() throws Exception { + public void shouldAnnotateSpanWithErrorWhenExceptionIsThrown() throws Exception { this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID) .header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, this.spanReporter, @@ -296,7 +299,7 @@ public class TraceFilterTests { javax.servlet.ServletResponse response) throws java.io.IOException, javax.servlet.ServletException { throw new RuntimeException("Planned"); - }; + } }; try { filter.doFilter(this.request, this.response, this.filterChain); @@ -307,6 +310,8 @@ public class TraceFilterTests { verifyParentSpanHttpTags(HttpStatus.INTERNAL_SERVER_ERROR); then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(new ListOfSpans(this.spanReporter.getSpans())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, "Planned"); } @Test @@ -323,7 +328,7 @@ public class TraceFilterTests { } @Test - public void returns400IfSpanIsMalformed() throws Exception { + public void returns400IfSpanIsMalformedAndCreatesANewSpan() throws Exception { this.request = builder().header(Span.SPAN_ID_NAME, "asd") .header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, this.spanReporter, @@ -331,8 +336,26 @@ public class TraceFilterTests { filter.doFilter(this.request, this.response, this.filterChain); + then(new ArrayList<>(this.spanReporter.getSpans())).isNotEmpty(); then(TestSpanContextHolder.getCurrentSpan()).isNull(); - then(this.response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + then(ExceptionUtils.getLastException()).isNull(); + then(this.response.getStatus()).isEqualTo(HttpStatus.OK.value()); + } + + @Test + public void returns200IfSpanParentIsMalformedAndCreatesANewSpan() throws Exception { + this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID) + .header(Span.PARENT_ID_NAME, "-") + .header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); + TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, this.spanReporter, + this.spanExtractor, this.httpTraceKeysInjector); + + filter.doFilter(this.request, this.response, this.filterChain); + + then(new ArrayList<>(this.spanReporter.getSpans())).isNotEmpty(); + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); + then(this.response.getStatus()).isEqualTo(HttpStatus.OK.value()); } public void verifyParentSpanHttpTags() { 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 8ed1a8ad1..662f1567a 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 @@ -27,6 +27,7 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.sleuth.Sampler; +import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; @@ -74,6 +75,9 @@ public class TraceFilterWebIntegrationTests { .doesNotHaveASpanWithName("error") .hasASpanWithTagEqualTo("http.status_code", "500"); then(ExceptionUtils.getLastException()).isNull(); + then(new ListOfSpans(this.accumulator.getSpans())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "Request processing failed; nested exception is java.lang.RuntimeException: Throwing exception"); } private int port() { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebDisabledTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebDisabledTests.java new file mode 100644 index 000000000..73919b566 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceWebDisabledTests.java @@ -0,0 +1,27 @@ +package org.springframework.cloud.sleuth.instrument.web; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.IntegrationTest; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@IntegrationTest({ "spring.sleuth.web.enabled=true", "spring.sleuth.web.client.enabled=false"}) +@SpringApplicationConfiguration(classes = { TraceWebDisabledTests.Config.class }) +public class TraceWebDisabledTests { + + @Test + public void should_load_context() { + + } + + @Configuration + @EnableAutoConfiguration + public static class Config {} +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorIntegrationTests.java index 48c4e662e..73a363ecc 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/TraceRestTemplateInterceptorIntegrationTests.java @@ -27,15 +27,16 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.springframework.cloud.sleuth.DefaultSpanNamer; -import org.springframework.cloud.sleuth.NoOpSpanReporter; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.TraceKeys; +import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.assertions.SleuthAssertions; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import org.springframework.cloud.sleuth.log.NoOpSpanLogger; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpRequestInterceptor; @@ -59,10 +60,12 @@ public class TraceRestTemplateInterceptorIntegrationTests { private DefaultTracer tracer; + private ArrayListSpanAccumulator spanAccumulator = new ArrayListSpanAccumulator(); + @Before public void setup() { this.tracer = new DefaultTracer(new AlwaysSampler(), new Random(), - new DefaultSpanNamer(), new NoOpSpanLogger(), new NoOpSpanReporter()); + new DefaultSpanNamer(), new NoOpSpanLogger(), this.spanAccumulator); this.template.setInterceptors(Arrays.asList( new TraceRestTemplateInterceptor(this.tracer, new HttpRequestInjector(), new HttpTraceKeysInjector(this.tracer, new TraceKeys())))); @@ -91,6 +94,9 @@ public class TraceRestTemplateInterceptorIntegrationTests { SleuthAssertions.then(this.tracer.getCurrentSpan()).isEqualTo(span); this.tracer.close(span); + SleuthAssertions.then(new ListOfSpans(this.spanAccumulator.getSpans())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "Read timed out"); then(ExceptionUtils.getLastException()).isNull(); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java index 1477bd2ce..ce8f7ea21 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientTests.java @@ -99,7 +99,7 @@ public class TraceFeignClientTests { then(this.tracer.getCurrentSpan()).isEqualTo(span); then(this.spanAccumulator.getSpans().get(0)) .hasNotLoggedAnEvent(Span.CLIENT_RECV) - .hasATag("error", "exception has occurred"); + .hasATag(Span.SPAN_ERROR_TAG_NAME, "exception has occurred"); } } \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java index 67512f8cb..5cd501607 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/servererrors/FeignClientServerErrorTests.java @@ -20,11 +20,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import com.netflix.hystrix.exception.HystrixRuntimeException; -import com.netflix.loadbalancer.BaseLoadBalancer; -import com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.Server; - import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -44,6 +39,7 @@ import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.annotation.Bean; @@ -58,10 +54,16 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.RestTemplate; +import com.jayway.awaitility.Awaitility; +import com.netflix.hystrix.exception.HystrixRuntimeException; +import com.netflix.loadbalancer.BaseLoadBalancer; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; + import feign.codec.Decoder; import feign.codec.ErrorDecoder; -import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; /** * Related to https://github.com/spring-cloud/spring-cloud-sleuth/issues/257 @@ -76,10 +78,12 @@ public class FeignClientServerErrorTests { @Autowired TestFeignInterface feignInterface; @Autowired TestFeignWithCustomConfInterface customConfFeignInterface; + @Autowired Listener listener; @Rule public OutputCapture capture = new OutputCapture(); @Before public void setup() { + this.listener.clear(); ExceptionUtils.setFail(true); } @@ -90,11 +94,17 @@ public class FeignClientServerErrorTests { } catch (HystrixRuntimeException e) { } - // ugly :/ waiting for rx thread to complete - Thread.sleep(100); - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); - then(ExceptionUtils.getLastException()).isNull(); + Awaitility.await().until(() -> { + then(this.capture.toString()) + .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); + then(new ListOfSpans(this.listener.getEvents())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "Request processing failed; nested exception is java.lang.RuntimeException: Internal Error"); + then(new ListOfSpans(this.listener.getEvents())) + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "Internal Error"); + }); } @Test @@ -104,11 +114,11 @@ public class FeignClientServerErrorTests { } catch (HystrixRuntimeException e) { } - // ugly :/ waiting for rx thread to complete - Thread.sleep(100); - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); - then(ExceptionUtils.getLastException()).isNull(); + Awaitility.await().until(() -> { + then(this.capture.toString()) + .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); + }); } @Test @@ -118,11 +128,10 @@ public class FeignClientServerErrorTests { } catch (HystrixRuntimeException e) { } - // ugly :/ waiting for rx thread to complete - Thread.sleep(100); - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); - then(ExceptionUtils.getLastException()).isNull(); + Awaitility.await().until(() -> { + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); + }); } @Test @@ -132,11 +141,10 @@ public class FeignClientServerErrorTests { } catch (HystrixRuntimeException e) { } - // ugly :/ waiting for rx thread to complete - Thread.sleep(100); - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); - then(ExceptionUtils.getLastException()).isNull(); + Awaitility.await().until(() -> { + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); + }); } @Test @@ -146,11 +154,10 @@ public class FeignClientServerErrorTests { } catch (HystrixRuntimeException e) { } - // ugly :/ waiting for rx thread to complete - Thread.sleep(100); - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); - then(ExceptionUtils.getLastException()).isNull(); + Awaitility.await().until(() -> { + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); + }); } @Configuration @@ -226,7 +233,11 @@ public class FeignClientServerErrorTests { private List events = new ArrayList<>(); public List getEvents() { - return this.events; + return new ArrayList<>(this.events); + } + + public void clear() { + this.events.clear(); } @Override @@ -246,8 +257,7 @@ public class FeignClientServerErrorTests { @RequestHeader(Span.TRACE_ID_NAME) String traceId, @RequestHeader(Span.SPAN_ID_NAME) String spanId, @RequestHeader(Span.PARENT_ID_NAME) String parentId) { - return new ResponseEntity<>("internal error", - HttpStatus.INTERNAL_SERVER_ERROR); + throw new RuntimeException("Internal Error"); } @RequestMapping("/notfound") diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/ExceptionUtilsTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/ExceptionUtilsTest.java index 7820dd83f..13da88cc6 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/ExceptionUtilsTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/ExceptionUtilsTest.java @@ -50,4 +50,22 @@ public class ExceptionUtilsTest { then(e).isInstanceOf(IllegalStateException.class); } } + + @Test + public void should_print_error_message_when_there_is_one() throws Exception { + Throwable e = new RuntimeException("Foo"); + + String message = ExceptionUtils.getExceptionMessage(e); + + then(message).isEqualTo("Foo"); + } + + @Test + public void should_print_to_string_when_there_is_no_error() throws Exception { + Throwable e = new RuntimeException(); + + String message = ExceptionUtils.getExceptionMessage(e); + + then(message).isEqualTo("java.lang.RuntimeException"); + } } \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/resources/logback.xml b/spring-cloud-sleuth-core/src/test/resources/logback.xml index fd0c9a35a..4d584ab3f 100644 --- a/spring-cloud-sleuth-core/src/test/resources/logback.xml +++ b/spring-cloud-sleuth-core/src/test/resources/logback.xml @@ -2,6 +2,7 @@ + diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index ae173aca4..7bf2ee6e7 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -9,7 +9,7 @@ spring-cloud-sleuth-dependencies - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT pom spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index afe391ff7..d87a6b0e9 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml index 213148634..bcb5206e4 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-feign/pom.xml @@ -20,7 +20,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml index 3683d93be..2cbd44cbe 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml index 21887c6c2..71fd2e1df 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-ribbon/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-stream/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-stream/pom.xml index d7b593c57..da1c6d34a 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-stream/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-stream/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml index 4502c5635..47c6dfe61 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-test-core/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml index b39ee5f9d..117a3a912 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-websocket/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/pom.xml index 2d08fe3a6..67fb0764b 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml index be217be4f..202c6c482 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin/pom.xml @@ -27,7 +27,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml index b041f3406..6d0a2a375 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth-samples - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-sleuth-stream/pom.xml b/spring-cloud-sleuth-stream/pom.xml index 6fe1dce35..9a756a4ca 100644 --- a/spring-cloud-sleuth-stream/pom.xml +++ b/spring-cloud-sleuth-stream/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT diff --git a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/StreamSpanReporter.java b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/StreamSpanReporter.java index 935b789be..3ddcbeade 100644 --- a/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/StreamSpanReporter.java +++ b/spring-cloud-sleuth-stream/src/main/java/org/springframework/cloud/sleuth/stream/StreamSpanReporter.java @@ -83,7 +83,14 @@ public class StreamSpanReporter implements SpanReporter { @Override public void report(Span span) { if (span.isExportable()) { - this.queue.add(span); + try { + this.queue.add(span); + } catch (Exception e) { + this.spanMetricReporter.incrementDroppedSpans(1); + if (log.isDebugEnabled()) { + log.debug("The span " + span + " will not be sent to Zipkin due to [" + e + "]"); + } + } } else { if (log.isDebugEnabled()) { log.debug("The span " + span + " will not be sent to Zipkin due to sampling"); diff --git a/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanReporterTests.java b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanReporterTests.java new file mode 100644 index 000000000..49c5402bf --- /dev/null +++ b/spring-cloud-sleuth-stream/src/test/java/org/springframework/cloud/sleuth/stream/StreamSpanReporterTests.java @@ -0,0 +1,52 @@ +/* + * Copyright 2013-2016 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.stream; + +import java.util.concurrent.ArrayBlockingQueue; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.metric.SpanMetricReporter; + +import static org.mockito.BDDMockito.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(MockitoJUnitRunner.class) +public class StreamSpanReporterTests { + + @Mock HostLocator endpointLocator; + @Mock SpanMetricReporter spanMetricReporter; + @InjectMocks StreamSpanReporter reporter; + + @Test + public void should_not_throw_an_exception_when_queue_size_is_exceeded() throws Exception { + ArrayBlockingQueue queue = new ArrayBlockingQueue<>(1); + queue.add(Span.builder().name("foo").build()); + this.reporter.setQueue(queue); + + this.reporter.report(Span.builder().name("bar").exportable(true).build()); + + then(spanMetricReporter).should().incrementDroppedSpans(1); + } + +} \ No newline at end of file diff --git a/spring-cloud-sleuth-zipkin-stream/pom.xml b/spring-cloud-sleuth-zipkin-stream/pom.xml index 4c1832b9c..26a937f06 100644 --- a/spring-cloud-sleuth-zipkin-stream/pom.xml +++ b/spring-cloud-sleuth-zipkin-stream/pom.xml @@ -12,7 +12,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT diff --git a/spring-cloud-sleuth-zipkin/pom.xml b/spring-cloud-sleuth-zipkin/pom.xml index 1a6f7676c..82afa1f08 100644 --- a/spring-cloud-sleuth-zipkin/pom.xml +++ b/spring-cloud-sleuth-zipkin/pom.xml @@ -28,7 +28,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. diff --git a/spring-cloud-starter-sleuth/pom.xml b/spring-cloud-starter-sleuth/pom.xml index 7a71b4433..5b6d58794 100644 --- a/spring-cloud-starter-sleuth/pom.xml +++ b/spring-cloud-starter-sleuth/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. spring-cloud-starter-sleuth diff --git a/spring-cloud-starter-zipkin/pom.xml b/spring-cloud-starter-zipkin/pom.xml index fb72cfe01..eb9236a63 100644 --- a/spring-cloud-starter-zipkin/pom.xml +++ b/spring-cloud-starter-zipkin/pom.xml @@ -5,7 +5,7 @@ org.springframework.cloud spring-cloud-sleuth - 1.0.10.BUILD-SNAPSHOT + 1.0.11.BUILD-SNAPSHOT .. spring-cloud-starter-zipkin