From bfce279d529747412fef6f1d4c39d4ddf5cf4c74 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 7 Jul 2016 10:56:07 +0200 Subject: [PATCH] Re-added missing server side span (#322) * Re-added missing server side span with this change - HandlerInterceptor is responsible for wrapping requests in spans - added Sring Data REST instrumentation - added Zuul instrumentation - it will not wrap error controller related requests with spans - tests have been changed to ensure that ALWAYS there is at least one span on the server side (that way it will show up in Zipkin) - "fallback" mechanism has been added that if a HandlerInterceptor hasn't been used then we are forcing creation of a Span at the server side fixes #321 (cherry picked from commit 4f1ef52e6caa11ba982616cc439196fbee1211cc) --- .travis.yml | 21 +- .../main/asciidoc/spring-cloud-sleuth.adoc | 11 +- spring-cloud-sleuth-core/pom.xml | 26 +++ .../cloud/sleuth/TraceKeys.java | 44 +++- .../autoconfig/TraceAutoConfiguration.java | 8 +- .../sleuth/instrument/web/TraceFilter.java | 86 +++++--- .../web/TraceHandlerInterceptor.java | 186 +++++++++++++++++ .../web/TraceRequestAttributes.java | 42 ++++ .../web/TraceSpringDataBeanPostProcessor.java | 96 +++++++++ .../sleuth/instrument/web/TraceWebAspect.java | 20 -- .../web/TraceWebAutoConfiguration.java | 14 +- .../instrument/web/TraceWebMvcConfigurer.java | 46 +++++ .../instrument/zuul/TracePostZuulFilter.java | 15 +- .../instrument/zuul/TracePreZuulFilter.java | 7 + .../zuul/TraceZuulAutoConfiguration.java | 12 +- ...ceZuulHandlerMappingBeanPostProcessor.java | 114 ++++++++++ .../cloud/sleuth/util/SpanNameUtil.java | 6 +- .../cloud/sleuth/assertions/ListOfSpans.java | 3 +- .../sleuth/assertions/ListOfSpansAssert.java | 77 ++++++- .../cloud/sleuth/assertions/SpanAssert.java | 13 ++ ...stTemplateTraceAspectIntegrationTests.java | 6 + .../web/SpringDataInstrumentationTests.java | 194 ++++++++++++++++++ .../web/TraceFilterIntegrationTests.java | 17 ++ .../instrument/web/TraceFilterTests.java | 6 +- .../web/TraceFilterWebIntegrationTests.java | 119 +++++++++++ ...stTemplateInterceptorIntegrationTests.java | 4 + .../WebClientDiscoveryExceptionTests.java | 3 +- .../instrument/web/client/WebClientTests.java | 21 +- .../feign/FeignClientServerErrorTests.java | 5 + .../client/feign/issues/Issue307Tests.java | 11 + .../common/AbstractMvcIntegrationTest.java | 2 + .../zuul/TraceZuulIntegrationTests.java | 5 + .../cloud/sleuth/util/SpanNameUtilTests.java | 6 + .../src/test/resources/logback.xml | 2 - 34 files changed, 1140 insertions(+), 108 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceRequestAttributes.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java diff --git a/.travis.yml b/.travis.yml index d6bd23957..2d2cc22a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,13 +1,11 @@ language: java jdk: - oraclejdk8 -sudo: false -before_install: - - git config user.name "$GIT_NAME" - - git config user.email "$GIT_EMAIL" - - git config credential.helper "store --file=.git/credentials" - - echo "https://$GH_TOKEN:@github.com" > .git/credentials - - gem install asciidoctor +sudo: required +dist: trusty +cache: + directories: + - $HOME/.m2 install: - | cat < log.log || echo "FAILED!" && grep -v '^.*Download.* http.*$' log.log && exit 1 +- './mvnw -s .settings.xml $MVN_GOAL $MVN_PROFILE -nsu -U --batch-mode -Dmaven.test.redirectTestOutputToFile=true' env: global: - - GIT_NAME="Spencer Gibb" - - GIT_EMAIL=sgibb@pivotal.io - - CI_DEPLOY_USERNAME=sgibb - FEATURE_BRANCH=$(echo ${TRAVIS_BRANCH} | grep -v "master" && echo true || echo false) - SPRING_CLOUD_BUILD=$(echo ${TRAVIS_REPO_SLUG} | grep -q "^spring-cloud/.*$" && echo true || echo false) - MVN_GOAL=$([ "${TRAVIS_PULL_REQUEST}" == "false" -a "${TRAVIS_TAG}" == "" -a "${FEATURE_BRANCH}" == "false" -a "${SPRING_CLOUD_BUILD}" == "true" ] && echo deploy || echo install) - VERSION=$(mvn validate | grep Building | head -1 | sed -e 's/.* //') - MILESTONE=$(echo ${VERSION} | egrep 'M|RC' && echo true || echo false) - MVN_PROFILE=$([ "${MILESTONE}" == "true" ] && echo -P milestone) - - secure: dxec/7oFht2WMaw4GNFNvuWHlnkm1wmFagE3ZrtCcU2SHUW/P2FgG5tiNW9hT2eXHf2H0YUF1ROkL4BrH5qpIlOBtYd0J8Q/67vBqyB112IN2FqoB/F6Erkfp1FKMBBrXXYaGoSvqzmJs6zqS3JRpyr010W2aU8klK0QfqRoJ0g= - TERM=dumb diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 741dfc3b2..e9e7fed8f 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -145,7 +145,7 @@ be low cardinality (e.g. not include identifiers). Since there is a lot of instrumentation going on some of the span names will be artificial like: -- `http:path` when received an http request on a given path +- `controller-method-name` when received by a Controller with a method name `conrollerMethodName` - `async` for asynchronous operations done via wrapped `Callable` and `Runnable`. - `@Scheduled` annotated methods will return the simple name of the class. @@ -428,6 +428,15 @@ Via the `TraceFilter` all sampled incoming requests result in creation of a Span like to skip via the `spring.sleuth.web.skipPattern` property. If you have `ManagementServerProperties` on classpath then its value of `contextPath` gets appended to the provided skip pattern. +==== HandlerInterceptor + +Since we want the span names to be precise we're using a `TraceHandlerInterceptor` that either wraps an + existing `HandlerInterceptor` or is added directly to the list of existing `HandlerInterceptors`. The + `TraceHandlerInterceptor` adds a special request attribute to the given `HttpServletRequest`. If the + the `TraceFilter` doesn't see this attribute set it will create a "fallback" span which is an additional + span created on the server side so that the trace is presented properly in the UI. Seeing that most likely + signifies that there is a missing instrumentation. In that case please file an issue in Spring Cloud Sleuth. + ==== Async Servlet support If your controller returns a `Callable` or a `WebAsyncTask` Spring Cloud Sleuth will continue the existing span instead of creating a new one. diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index f90085dcb..b01d47d4e 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -75,6 +75,12 @@ rxjava true + + + org.springframework.data + spring-data-rest-webmvc + true + com.squareup.okhttp3 okhttp @@ -125,6 +131,26 @@ JUnitParams test + + org.springframework.boot + spring-boot-starter-data-jpa + test + + + org.springframework + spring-orm + test + + + org.springframework.boot + spring-boot-starter-data-rest + test + + + com.h2database + h2 + test + diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java index 5c3ecd999..42468cd0a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceKeys.java @@ -59,6 +59,8 @@ public class TraceKeys { private Async async = new Async(); + private Mvc mvc = new Mvc(); + public Http getHttp() { return this.http; } @@ -75,6 +77,10 @@ public class TraceKeys { return this.async; } + public Mvc getMvc() { + return this.mvc; + } + public void setHttp(Http http) { this.http = http; } @@ -91,6 +97,10 @@ public class TraceKeys { this.async = async; } + public void setMvc(Mvc mvc) { + this.mvc = mvc; + } + public static class Message { private Payload payload = new Payload(); @@ -376,7 +386,6 @@ public class TraceKeys { public void setThreadPoolKey(String threadPoolKey) { this.threadPoolKey = threadPoolKey; } - } /** @@ -442,7 +451,40 @@ public class TraceKeys { public void setMethodNameKey(String methodNameKey) { this.methodNameKey = methodNameKey; } + } + /** + * Trace keys related to MVC controller tags + */ + public static class Mvc { + + /** + * The lower case, hyphen delimited name of the class that processes the request. + * Ex. class named "BookController" will result in "book-controller" tag value. + */ + private String controllerClass = "mvc.controller.class"; + + /** + * The lower case, hyphen delimited name of the class that processes the request. + * Ex. method named "listOfBooks" will result in "list-of-books" tag value. + */ + private String controllerMethod = "mvc.controller.method"; + + public String getControllerClass() { + return this.controllerClass; + } + + public void setControllerClass(String controllerClass) { + this.controllerClass = controllerClass; + } + + public String getControllerMethod() { + return this.controllerMethod; + } + + public void setControllerMethod(String controllerMethod) { + this.controllerMethod = controllerMethod; + } } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java index bb4967dcf..579f81f5b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java @@ -44,7 +44,7 @@ import org.springframework.context.annotation.Configuration; */ @Configuration @ConditionalOnProperty(value="spring.sleuth.enabled", matchIfMissing=true) -@EnableConfigurationProperties +@EnableConfigurationProperties(TraceKeys.class) public class TraceAutoConfiguration { @Bean @@ -68,12 +68,6 @@ public class TraceAutoConfiguration { spanReporter); } - @Bean - @ConditionalOnMissingBean - public TraceKeys traceKeys() { - return new TraceKeys(); - } - @Bean @ConditionalOnMissingBean public SpanNamer spanNamer() { 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 f50b1fea2..30efa50c4 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 @@ -128,26 +128,14 @@ public class TraceFilter extends GenericFilterBean { || Span.SPAN_NOT_SAMPLED.equals(ServletUtils.getHeader(request, response, Span.SAMPLED_NAME)); Span spanFromRequest = getSpanFromAttribute(request); if (spanFromRequest != null) { - this.tracer.continueSpan(spanFromRequest); - if (log.isDebugEnabled()) { - log.debug("There has already been a span in the request " + spanFromRequest + ""); - } + continueSpan(request, spanFromRequest); } if (log.isDebugEnabled()) { log.debug("Received a request to uri [" + uri + "] that should be skipped [" + skip + "]"); } // in case of a response with exception status a exception controller will close the span if (!httpStatusSuccessful(response) && isSpanContinued(request)) { - if (log.isDebugEnabled()) { - log.debug( - "The span was already detached once and we're processing an error"); - } - try { - filterChain.doFilter(request, response); - } finally { - request.setAttribute(TRACE_ERROR_HANDLED_REQUEST_ATTR, true); - this.tracer.close(spanFromRequest); - } + processErrorRequest(filterChain, request, response, spanFromRequest); return; } addToResponseIfNotPresent(response, Span.SAMPLED_NAME, skip ? Span.SPAN_NOT_SAMPLED : Span.SPAN_SAMPLED); @@ -166,33 +154,76 @@ public class TraceFilter extends GenericFilterBean { // Add headers before filter chain in case one of the filters flushes the // response... filterChain.doFilter(request, response); - } - catch (Throwable e) { + } catch (Throwable e) { exception = e; throw e; - } - finally { + } finally { if (isAsyncStarted(request) || request.isAsyncStarted()) { if (log.isDebugEnabled()) { - log.debug("Detaching the span " + spanFromRequest + " since the request is asynchronous"); + log.debug("The span " + spanFromRequest + " will get detached by a HandleInterceptor"); } - this.tracer.detach(spanFromRequest); // TODO: how to deal with response annotations and async? return; } + spanFromRequest = createSpanIfRequestNotHandled(request, spanFromRequest, name, skip); addToResponseIfNotPresent(response, Span.SAMPLED_NAME, skip ? Span.SPAN_NOT_SAMPLED : Span.SPAN_SAMPLED); detachOrCloseSpans(request, response, spanFromRequest, exception); } } + private void processErrorRequest(FilterChain filterChain, HttpServletRequest request, + HttpServletResponse response, Span spanFromRequest) + throws IOException, ServletException { + if (log.isDebugEnabled()) { + log.debug("The span [" + spanFromRequest + "] was already detached once and we're processing an error"); + } + try { + filterChain.doFilter(request, response); + } finally { + request.setAttribute(TRACE_ERROR_HANDLED_REQUEST_ATTR, true); + addResponseTags(response, null); + this.tracer.close(spanFromRequest); + } + } + + private void continueSpan(HttpServletRequest request, Span spanFromRequest) { + this.tracer.continueSpan(spanFromRequest); + request.setAttribute(TraceRequestAttributes.SPAN_CONTINUED_REQUEST_ATTR, "true"); + if (log.isDebugEnabled()) { + log.debug("There has already been a span in the request " + spanFromRequest); + } + } + + // This method is a fallback in case if handler interceptors didn't catch the request. + // In that case we are creating an artificial span so that it can be visible in Zipkin. + private Span createSpanIfRequestNotHandled(HttpServletRequest request, + Span spanFromRequest, String name, boolean skip) { + if (!requestHasAlreadyBeenHandled(request)) { + spanFromRequest = this.tracer.createSpan(name); + request.setAttribute(TRACE_REQUEST_ATTR, spanFromRequest); + if (log.isDebugEnabled() && !skip) { + log.debug("The request with uri [" + request.getRequestURI() + "] hasn't been handled by any of Sleuth's components. " + + "That means that most likely you're using custom HandlerMappings and didn't add Sleuth's TraceHandlerInterceptor. " + + "Sleuth will create a span to ensure that the graph of calls remains valid in Zipkin"); + } + } + return spanFromRequest; + } + + private boolean requestHasAlreadyBeenHandled(HttpServletRequest request) { + return request.getAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR) != null; + } + private void detachOrCloseSpans(HttpServletRequest request, HttpServletResponse response, Span spanFromRequest, Throwable exception) { if (spanFromRequest != null) { addResponseTags(response, exception); - if (spanFromRequest.hasSavedSpan()) { - closeParentSpan(spanFromRequest.getSavedSpan()); + if (spanFromRequest.hasSavedSpan() && requestHasAlreadyBeenHandled(request)) { + recordParentSpan(spanFromRequest.getSavedSpan()); + } else if (!requestHasAlreadyBeenHandled(request)) { + spanFromRequest = this.tracer.close(spanFromRequest); } - closeParentSpan(spanFromRequest); + recordParentSpan(spanFromRequest); // in case of a response with exception status will close the span when exception dispatch is handled if (httpStatusSuccessful(response)) { if (log.isDebugEnabled()) { @@ -213,7 +244,10 @@ public class TraceFilter extends GenericFilterBean { } } - private void closeParentSpan(Span parent) { + private void recordParentSpan(Span parent) { + if (parent == null) { + return; + } if (parent.isRemote()) { if (log.isDebugEnabled()) { log.debug("Sending the parent span " + parent + " to Zipkin"); @@ -247,6 +281,10 @@ public class TraceFilter extends GenericFilterBean { return getSpanFromAttribute(request) != null; } + /** + * In order not to send unnecessary data we're not adding request tags to the server + * side spans. All the tags are there on the client side. + */ private void addRequestTagsForParentSpan(HttpServletRequest request, Span spanFromRequest) { if (spanFromRequest.getName().contains("parent")) { addRequestTags(spanFromRequest, request); 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 new file mode 100644 index 000000000..c9d87e7bc --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceHandlerInterceptor.java @@ -0,0 +1,186 @@ +/* + * 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 javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.lang.invoke.MethodHandles; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.BeanFactory; +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.SpanNameUtil; +import org.springframework.web.method.HandlerMethod; +import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; + +/** + * {@link org.springframework.web.servlet.HandlerInterceptor} that wraps handling of a + * request in a Span. Adds tags related to the class and method name. + * + * The interceptor will not create spans for error controller related paths. + * + * It's important to note that this implementation will set the request attribute + * {@link TraceRequestAttributes#HANDLED_SPAN_REQUEST_ATTR} when the request is processed. + * That way the {@link TraceFilter} will not create the "fallback" span. + * + * @author Marcin Grzejszczak + * @since 1.0.3 + */ +public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { + + private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + + private final BeanFactory beanFactory; + + private Tracer tracer; + private TraceKeys traceKeys; + private ErrorController errorController; + + public TraceHandlerInterceptor(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public boolean preHandle(HttpServletRequest request, HttpServletResponse response, + Object handler) throws Exception { + if (isErrorControllerRelated(request)) { + log.debug("Skipping creation of a span for error controller processing"); + return true; + } + if (isSpanContinued(request)) { + log.debug("Skipping creation of a span since the span is continued"); + return true; + } + String spanName = spanName(handler); + Span span = getTracer().createSpan(spanName); + if (log.isDebugEnabled()) { + log.debug("Created new span " + span + " with name [" + spanName + "]"); + } + addClassMethodTag(handler, span); + addClassNameTag(handler, span); + setSpanInAttribute(request, span); + return true; + } + + private boolean isErrorControllerRelated(HttpServletRequest request) { + return getErrorController() != null && getErrorController().getErrorPath() + .equals(request.getRequestURI()); + } + + private void addClassMethodTag(Object handler, Span span) { + if (handler instanceof HandlerMethod) { + String methodName = SpanNameUtil.toLowerHyphen( + ((HandlerMethod) handler).getMethod().getName()); + getTracer().addTag(getTraceKeys().getMvc().getControllerMethod(), methodName); + if (log.isDebugEnabled()) { + log.debug("Adding a method tag with value [" + methodName + "] to a span " + span); + } + } + } + + private void addClassNameTag(Object handler, Span span) { + String className; + if (handler instanceof HandlerMethod) { + className = SpanNameUtil.toLowerHyphen( + ((HandlerMethod) handler).getBeanType().getSimpleName()); + } else { + className = SpanNameUtil.toLowerHyphen(handler.getClass().getSimpleName()); + } + if (log.isDebugEnabled()) { + log.debug("Adding a class tag with value [" + className + "] to a span " + span); + } + getTracer().addTag(getTraceKeys().getMvc().getControllerClass(), className); + } + + private String spanName(Object handler) { + if (handler instanceof HandlerMethod) { + return SpanNameUtil.toLowerHyphen(((HandlerMethod) handler).getMethod().getName()); + } + return SpanNameUtil.toLowerHyphen(handler.getClass().getSimpleName()); + } + + @Override + public void afterConcurrentHandlingStarted(HttpServletRequest request, + HttpServletResponse response, Object handler) throws Exception { + Span spanFromRequest = getSpanFromAttribute(request); + Span rootSpanFromRequest = getRootSpanFromAttribute(request); + if (log.isDebugEnabled()) { + log.debug("Closing the span " + spanFromRequest + " and detaching its parent " + rootSpanFromRequest + " since the request is asynchronous"); + } + getTracer().close(spanFromRequest); + getTracer().detach(rootSpanFromRequest); + } + + @Override + public void afterCompletion(HttpServletRequest request, HttpServletResponse response, + Object handler, Exception ex) throws Exception { + if (isErrorControllerRelated(request)) { + log.debug("Skipping closing of a span for error controller processing"); + return; + } + if (isSpanContinued(request)) { + log.debug("Skipping closing of a span since it's been continued"); + return; + } + Span span = getSpanFromAttribute(request); + if (log.isDebugEnabled()) { + log.debug("Closing span " + span); + } + getTracer().close(span); + } + + private boolean isSpanContinued(HttpServletRequest request) { + return request.getAttribute(TraceRequestAttributes.SPAN_CONTINUED_REQUEST_ATTR) != null; + } + + private Span getSpanFromAttribute(HttpServletRequest request) { + return (Span) request.getAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR); + } + + private Span getRootSpanFromAttribute(HttpServletRequest request) { + return (Span) request.getAttribute(TraceFilter.TRACE_REQUEST_ATTR); + } + + private void setSpanInAttribute(HttpServletRequest request, Span span) { + request.setAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR, span); + } + + private Tracer getTracer() { + if (this.tracer == null) { + this.tracer = this.beanFactory.getBean(Tracer.class); + } + return this.tracer; + } + + private TraceKeys getTraceKeys() { + if (this.traceKeys == null) { + this.traceKeys = this.beanFactory.getBean(TraceKeys.class); + } + return this.traceKeys; + } + + private ErrorController getErrorController() { + if (this.errorController == null) { + this.errorController = this.beanFactory.getBean(ErrorController.class); + } + return this.errorController; + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceRequestAttributes.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceRequestAttributes.java new file mode 100644 index 000000000..683293807 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceRequestAttributes.java @@ -0,0 +1,42 @@ +/* + * 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; + +/** + * Utility class containing values of {@link javax.servlet.http.HttpServletRequest} attributes + * + * @author Marcin Grzejszczak + * @since 1.0.3 + */ +public final class TraceRequestAttributes { + + /** + * Attribute containing a {@link org.springframework.cloud.sleuth.Span} set on a request when it got handled by a Sleuth component. + * If that attribute is set then {@link TraceFilter} will not create a "fallback" server-side span. + */ + public static final String HANDLED_SPAN_REQUEST_ATTR = TraceRequestAttributes.class.getName() + + ".TRACE_HANDLED"; + + /** + * Attribute set when the {@link org.springframework.cloud.sleuth.Span} got continued in the {@link TraceFilter}. + * The Sleuth tracing components will most likely continue the current Span instead of creating a new one. + */ + public static final String SPAN_CONTINUED_REQUEST_ATTR = TraceRequestAttributes.class.getName() + + ".TRACE_CONTINUED"; + + private TraceRequestAttributes() {} +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java new file mode 100644 index 000000000..98e2f42a6 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceSpringDataBeanPostProcessor.java @@ -0,0 +1,96 @@ +/* + * 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 javax.servlet.http.HttpServletRequest; +import java.lang.invoke.MethodHandles; +import java.util.Collections; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping; +import org.springframework.web.servlet.HandlerExecutionChain; +import org.springframework.web.servlet.HandlerMapping; + +/** + * Bean post processor that wraps Spring Data REST Controllers in named Spans + * + * @author Marcin Grzejszczak + * @since 1.0.3 + */ +class TraceSpringDataBeanPostProcessor implements BeanPostProcessor { + + private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + + private final BeanFactory beanFactory; + + public TraceSpringDataBeanPostProcessor(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) + throws BeansException { + if (bean instanceof DelegatingHandlerMapping && !(bean instanceof TraceDelegatingHandlerMapping)) { + if (log.isDebugEnabled()) { + log.debug("Wrapping bean [" + beanName + "] of type [" + bean.getClass().getSimpleName() + + "] in its trace representation"); + } + return new TraceDelegatingHandlerMapping((DelegatingHandlerMapping) bean, + this.beanFactory); + } + return bean; + } + + @Override + public Object postProcessAfterInitialization(Object bean, String beanName) + throws BeansException { + return bean; + } + + private static class TraceDelegatingHandlerMapping extends DelegatingHandlerMapping { + + private final DelegatingHandlerMapping delegate; + private final BeanFactory beanFactory; + + public TraceDelegatingHandlerMapping(DelegatingHandlerMapping delegate, + BeanFactory beanFactory) { + super(Collections.emptyList()); + this.delegate = delegate; + this.beanFactory = beanFactory; + } + + @Override + public int getOrder() { + return this.delegate.getOrder(); + } + + @Override + public HandlerExecutionChain getHandler(HttpServletRequest request) + throws Exception { + HandlerExecutionChain handlerExecutionChain = this.delegate.getHandler(request); + if (handlerExecutionChain == null) { + return null; + } + handlerExecutionChain.addInterceptor(new TraceHandlerInterceptor(this.beanFactory)); + return handlerExecutionChain; + } + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAspect.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAspect.java index 4a33f04a0..a56ac27a3 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAspect.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAspect.java @@ -24,11 +24,9 @@ import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; import org.aspectj.lang.annotation.Pointcut; -import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanNamer; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.async.TraceContinuingCallable; -import org.springframework.cloud.sleuth.util.SpanNameUtil; import org.springframework.web.context.request.async.WebAsyncTask; /** @@ -85,9 +83,6 @@ public class TraceWebAspect { @Pointcut("@within(org.springframework.stereotype.Controller)") private void anyControllerAnnotated() { } // NOSONAR - @Pointcut("@annotation(org.springframework.web.bind.annotation.RequestMapping)") - private void anyRequestMappingAnnotatedMethod() { } // NOSONAR - @Pointcut("execution(public java.util.concurrent.Callable *(..))") private void anyPublicMethodReturningCallable() { } // NOSONAR @@ -100,21 +95,6 @@ public class TraceWebAspect { @Pointcut("(anyRestControllerAnnotated() || anyControllerAnnotated()) && anyPublicMethodReturningWebAsyncTask()") private void anyControllerOrRestControllerWithPublicWebAsyncTaskMethod() { } // NOSONAR - @Around("(anyRestControllerAnnotated() || anyControllerAnnotated()) && anyRequestMappingAnnotatedMethod()") - @SuppressWarnings("unchecked") - public Object wrapControllerMethodWithCorrelationId(ProceedingJoinPoint pjp) throws Throwable { - String spanName = SpanNameUtil.toLowerHyphen(pjp.getSignature().getName()); - Span span = this.tracer.createSpan(spanName); - if (log.isDebugEnabled()) { - log.debug("Wrapping controller method [" + spanName + "] in a span " + span); - } - try { - return pjp.proceed(); - } finally { - this.tracer.close(span); - } - } - @Around("anyControllerOrRestControllerWithPublicAsyncMethod()") @SuppressWarnings("unchecked") public Object wrapWithCorrelationId(ProceedingJoinPoint pjp) throws Throwable { 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 47c5bee0d..64bb9819a 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,11 +15,11 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import java.util.regex.Pattern; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.util.regex.Pattern; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.actuate.autoconfigure.ManagementServerProperties; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -31,8 +31,8 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.context.embedded.FilterRegistrationBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.SpanExtractor; +import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.SpanNamer; import org.springframework.cloud.sleuth.SpanReporter; import org.springframework.cloud.sleuth.TraceKeys; @@ -40,6 +40,7 @@ import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.util.StringUtils; import static javax.servlet.DispatcherType.ASYNC; @@ -64,6 +65,7 @@ import static javax.servlet.DispatcherType.REQUEST; @ConditionalOnBean(Tracer.class) @AutoConfigureAfter(TraceAutoConfiguration.class) @EnableConfigurationProperties(TraceKeys.class) +@Import(TraceWebMvcConfigurer.class) public class TraceWebAutoConfiguration { /** @@ -77,6 +79,12 @@ public class TraceWebAutoConfiguration { return new TraceWebAspect(tracer, spanNamer); } + @Bean + @ConditionalOnClass(name = "org.springframework.data.rest.webmvc.support.DelegatingHandlerMapping") + public TraceSpringDataBeanPostProcessor traceSpringDataBeanPostProcessor(BeanFactory beanFactory) { + return new TraceSpringDataBeanPostProcessor(beanFactory); + } + @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/TraceWebMvcConfigurer.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java new file mode 100644 index 000000000..35935a39b --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebMvcConfigurer.java @@ -0,0 +1,46 @@ +/* + * 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 org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.servlet.config.annotation.InterceptorRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; + +/** + * MVC Adapter that adds the {@link TraceHandlerInterceptor} + * + * @author Marcin Grzejszczak + * + * @since 1.0.3 + */ +@Configuration +class TraceWebMvcConfigurer extends WebMvcConfigurerAdapter { + @Autowired TraceHandlerInterceptor traceHandlerInterceptor; + + @Bean + public TraceHandlerInterceptor traceHandlerInterceptor(BeanFactory beanFactory) { + return new TraceHandlerInterceptor(beanFactory); + } + + @Override + public void addInterceptors(InterceptorRegistry registry) { + registry.addInterceptor(this.traceHandlerInterceptor); + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java index 3ad574f14..14685fd74 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java @@ -26,7 +26,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; -import org.springframework.http.HttpStatus; /**8 * A post request {@link ZuulFilter} that publishes an event upon start of the filtering @@ -56,7 +55,7 @@ public class TracePostZuulFilter extends ZuulFilter { // TODO: the client sent event should come from the client not the filter! getCurrentSpan().logEvent(Span.CLIENT_RECV); if (log.isDebugEnabled()) { - log.debug("Closing current client span " + getCurrentSpan() + ""); + log.debug("Closing current client span " + getCurrentSpan()); } int httpStatus = RequestContext.getCurrentContext().getResponse().getStatus(); if (httpStatus > 0) { @@ -64,21 +63,9 @@ public class TracePostZuulFilter extends ZuulFilter { String.valueOf(httpStatus)); } this.tracer.close(getCurrentSpan()); - closeParentSpanIfResponseIsNotSuccess(httpStatus); return null; } - private void closeParentSpanIfResponseIsNotSuccess(int httpStatus) { - if (httpStatus > 0 && httpStatusIsNotSuccess(httpStatus)) { - this.tracer.close(getCurrentSpan()); - } - } - - private boolean httpStatusIsNotSuccess(int httpStatus) { - return HttpStatus.valueOf(httpStatus).is4xxClientError() || - HttpStatus.valueOf(httpStatus).is5xxServerError(); - } - @Override public String filterType() { return "post"; 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 12bbf8635..f64224385 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 @@ -30,6 +30,7 @@ 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.instrument.web.TraceRequestAttributes; /** * A pre request {@link ZuulFilter} that sets tracing related headers on the request @@ -73,6 +74,7 @@ public class TracePreZuulFilter extends ZuulFilter { if (log.isDebugEnabled()) { log.debug("Current span is " + span + ""); } + markRequestAsHandled(ctx); Span newSpan = this.tracer.createSpan(span.getName(), span); newSpan.tag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, ZUUL_COMPONENT); this.spanInjector.inject(newSpan, ctx); @@ -94,6 +96,11 @@ public class TracePreZuulFilter extends ZuulFilter { return result; } + // TraceFilter will not create the "fallback" span + private void markRequestAsHandled(RequestContext ctx) { + ctx.getRequest().setAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR, "true"); + } + private Span getCurrentSpan() { return this.tracer.getCurrentSpan(); } 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 52556f521..eb5ecf651 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,10 +15,6 @@ */ 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.apache.http.client.methods.RequestBuilder; import org.springframework.beans.factory.BeanFactory; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -37,6 +33,9 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import okhttp3.Request; +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} @@ -94,4 +93,9 @@ public class TraceZuulAutoConfiguration { return new OkHttpClientRibbonRequestCustomizer(tracer); } + @Bean + public TraceZuulHandlerMappingBeanPostProcessor traceHandlerMappingBeanPostProcessor(BeanFactory beanFactory) { + return new TraceZuulHandlerMappingBeanPostProcessor(beanFactory); + } + } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java new file mode 100644 index 000000000..14eed9913 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulHandlerMappingBeanPostProcessor.java @@ -0,0 +1,114 @@ +/* + * 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.zuul; + +import java.lang.invoke.MethodHandles; +import java.util.List; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.boot.autoconfigure.web.ErrorController; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; +import org.springframework.cloud.netflix.zuul.web.ZuulController; +import org.springframework.cloud.netflix.zuul.web.ZuulHandlerMapping; +import org.springframework.cloud.sleuth.instrument.web.TraceHandlerInterceptor; + +/** + * Bean post processor that wraps {@link ZuulHandlerMapping} in its + * trace representation. + * + * @author Marcin Grzejszczak + * @since 1.0.3 + */ +class TraceZuulHandlerMappingBeanPostProcessor implements BeanPostProcessor { + + private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); + + private final BeanFactory beanFactory; + private RouteLocator routeLocator; + private ZuulController zuul; + private ErrorController errorController; + + public TraceZuulHandlerMappingBeanPostProcessor(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) + throws BeansException { + if (bean instanceof ZuulHandlerMapping && !(bean instanceof TraceZuulHandlerMapping)) { + if (log.isDebugEnabled()) { + log.debug("Wrapping bean [" + beanName + "] of type [" + bean.getClass().getSimpleName() + + "] in its trace representation"); + } + return new TraceZuulHandlerMapping(this.beanFactory, routeLocator(), zuulController(), + errorController()); + } + return bean; + } + + @Override + public Object postProcessAfterInitialization(Object bean, String beanName) + throws BeansException { + return bean; + } + + private static class TraceZuulHandlerMapping extends ZuulHandlerMapping { + + private final BeanFactory beanFactory; + + public TraceZuulHandlerMapping(BeanFactory beanFactory, RouteLocator routeLocator, + ZuulController zuulController, ErrorController errorController) { + super(routeLocator, zuulController); + this.beanFactory = beanFactory; + setErrorController(errorController); + } + + @Override + protected void extendInterceptors(List interceptors) { + interceptors.add(new TraceHandlerInterceptor(this.beanFactory)); + } + } + + private RouteLocator routeLocator() { + if (this.routeLocator == null) { + this.routeLocator = this.beanFactory.getBean(RouteLocator.class); + } + return this.routeLocator; + } + + private ZuulController zuulController() { + if (this.zuul == null) { + this.zuul = this.beanFactory.getBean(ZuulController.class); + } + return this.zuul; + } + + private ErrorController errorController() { + if (this.errorController == null) { + try { + this.errorController = this.beanFactory.getBean(ErrorController.class); + } catch (BeansException b) { + return null; + } + } + return this.errorController; + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java index cc563298b..78ef5ec7d 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/util/SpanNameUtil.java @@ -20,7 +20,6 @@ package org.springframework.cloud.sleuth.util; * Utility class that provides the name in hyphen based notation * * @author Adrian Cole - * * @since 1.0.2 */ public final class SpanNameUtil { @@ -29,8 +28,9 @@ public final class SpanNameUtil { StringBuilder result = new StringBuilder(); for (int i = 0; i < name.length(); i++) { char c = name.charAt(i); - if (c >= 'A' && c <= 'Z') { - result.append('-').append((char) (c + 'a' - 'A')); + if (Character.isUpperCase(c)) { + if (i != 0) result.append('-'); + result.append(Character.toLowerCase(c)); } else { result.append(c); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpans.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpans.java index 697a2b079..cbec93f1c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpans.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpans.java @@ -15,6 +15,7 @@ */ package org.springframework.cloud.sleuth.assertions; +import java.util.ArrayList; import java.util.List; import org.springframework.cloud.sleuth.Span; @@ -27,6 +28,6 @@ public class ListOfSpans { public final List spans; public ListOfSpans(List spans) { - this.spans = spans; + this.spans = new ArrayList<>(spans); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java index 22408ad03..9f460d24f 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java @@ -20,15 +20,16 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.assertj.core.api.AbstractAssert; import org.springframework.cloud.sleuth.Span; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; - import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; @@ -79,12 +80,80 @@ public class ListOfSpansAssert extends AbstractAssert \nto contain at least one span with tag key " + + "equal to <%s>", spansToString(), tagKey); + } + return this; + } + + private boolean spanWithKeyTagExists(String tagKey) { + for (Span span : this.actual.spans) { + if (span.tags().containsKey(tagKey)) { + return true; + } + } + return false; + } + + public ListOfSpansAssert hasASpanWithTagEqualTo(String tagKey, String tagValue) { + isNotNull(); + printSpans(); + List matchingSpans = this.actual.spans.stream() + .filter(span -> tagValue.equals(span.tags().get(tagKey))) + .collect(toList()); + if (matchingSpans.isEmpty()) { + failWithMessage("Expected spans \n <%s> \nto contain at least one span with tag key " + + "equal to <%s> and value equal to <%s>", spansToString(), tagKey, tagValue); + } + return this; + } + + private String spansToString() { + return this.actual.spans.stream().map(span -> "\nSPAN: " + span.toString() + " with name [" + span.getName() + "] " + + "\nwith tags " + span.tags() + "\nwith logs " + span.logs()).collect(Collectors.joining("\n")); + } + + public ListOfSpansAssert doesNotHaveASpanWithName(String name) { + isNotNull(); + printSpans(); + List matchingSpans = findSpansWithName(name); + if (!matchingSpans.isEmpty()) { + failWithMessage("Expected spans \n <%s> \nnot to contain a span with name <%s>", spansToString(), name); + } + return this; + } + + private List findSpansWithName(String name) { + return this.actual.spans.stream() + .filter(span -> span.getName().equals(name)) + .collect(toList()); + } + + public ListOfSpansAssert hasASpanWithName(String name) { + isNotNull(); + printSpans(); + List matchingSpans = findSpansWithName(name); + if (matchingSpans.isEmpty()) { + failWithMessage("Expected spans <%s> to contain a span with name <%s>", spansToString(), name); + } + return this; + } + private void printSpans() { try { - log.info("Stored spans " + this.objectMapper.writeValueAsString(this.actual.spans)); + log.info("Stored spans " + this.objectMapper.writeValueAsString(new ArrayList<>(this.actual.spans))); } catch (JsonProcessingException e) { } } + @Override + protected void failWithMessage(String errorMessage, Object... arguments) { + log.error(errorMessage); + super.failWithMessage(errorMessage, arguments); + } } \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java index 2f53ea20d..3503e64df 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java @@ -99,6 +99,19 @@ public class SpanAssert extends AbstractAssert { return this; } + public SpanAssert hasATagWithKey(String tagKey) { + isNotNull(); + assertThatTagIsPresent(tagKey); + boolean foundTagValue = this.actual.tags().containsKey(tagKey); + if (!foundTagValue) { + String message = String.format("Expected span to have the tag with key <%s>. " + + "Found tags are <%s>", tagKey, this.actual.tags()); + log.error(message); + failWithMessage(message); + } + return this; + } + public SpanAssert matchesATag(String tagKey, String tagRegex) { isNotNull(); assertThatTagIsPresent(tagKey); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/RestTemplateTraceAspectIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/RestTemplateTraceAspectIntegrationTests.java index e401aa213..3a3de25bd 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/RestTemplateTraceAspectIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/RestTemplateTraceAspectIntegrationTests.java @@ -14,6 +14,7 @@ import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.core.env.Environment; @@ -35,6 +36,7 @@ import org.springframework.web.context.request.async.WebAsyncTask; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.BDDAssertions.then; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.request; @@ -67,6 +69,7 @@ public class RestTemplateTraceAspectIntegrationTests { whenARequestIsSentToASyncEndpoint(); thenTraceIdHasBeenSetOnARequestHeader(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -75,6 +78,7 @@ public class RestTemplateTraceAspectIntegrationTests { whenARequestIsSentToAnAsyncRestTemplateEndpoint(); thenTraceIdHasBeenSetOnARequestHeader(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -82,6 +86,7 @@ public class RestTemplateTraceAspectIntegrationTests { throws Exception { whenARequestIsSentToAnAsyncEndpoint("/callablePing"); thenTraceIdHasBeenSetOnARequestHeader(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -89,6 +94,7 @@ public class RestTemplateTraceAspectIntegrationTests { throws Exception { whenARequestIsSentToAnAsyncEndpoint("/webAsyncTaskPing"); thenTraceIdHasBeenSetOnARequestHeader(); + then(ExceptionUtils.getLastException()).isNull(); } private void whenARequestIsSentToAnAsyncRestTemplateEndpoint() throws Exception { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java new file mode 100644 index 000000000..742b382f2 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java @@ -0,0 +1,194 @@ +/* + * 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 javax.annotation.PostConstruct; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import java.util.Collection; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import com.jayway.awaitility.Awaitility; + +import org.assertj.core.api.BDDAssertions; +import org.junit.Before; +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.orm.jpa.EntityScan; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.sleuth.Sampler; +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.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.ParameterizedTypeReference; +import org.springframework.core.env.Environment; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.rest.core.annotation.RepositoryRestResource; +import org.springframework.hateoas.Resources; +import org.springframework.http.HttpMethod; +import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.client.RestTemplate; + +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@WebIntegrationTest({ "server.port=0" }) +@SpringApplicationConfiguration(classes = { ReservationServiceApplication.class }) +@DirtiesContext +public class SpringDataInstrumentationTests { + + @Autowired RestTemplate restTemplate; + @Autowired Environment environment; + @Autowired Tracer tracer; + @Autowired ArrayListSpanAccumulator arrayListSpanAccumulator; + + @Before + public void setup() { + TestSpanContextHolder.removeCurrentSpan(); + } + + @Test + public void should_create_span_instrumented_by_a_handler_interceptor() { + Collection names = names(); + + then(names).isNotEmpty(); + then(this.arrayListSpanAccumulator.getSpans()).isNotEmpty(); + Awaitility.await().until( () -> { + then(new ListOfSpans(this.arrayListSpanAccumulator.getSpans())).hasASpanWithName("http:/reservations") + .hasASpanWithTagKeyEqualTo("mvc.controller.class"); + }); + then(this.tracer.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); + } + + Collection names() { + ParameterizedTypeReference> ptr = + new ParameterizedTypeReference>() { + }; + ResponseEntity> responseEntity = + this.restTemplate.exchange("http://localhost:" + port() + "/reservations", + HttpMethod.GET, + null, + ptr + ); + return responseEntity + .getBody() + .getContent() + .stream() + .map(Reservation::getReservationName) + .collect(Collectors.toList()); + } + + private int port() { + return this.environment.getProperty("local.server.port", Integer.class); + } +} + +@Configuration +@EnableAutoConfiguration +@EntityScan(basePackageClasses = Reservation.class) +class ReservationServiceApplication { + + @Bean RestTemplate restTemplate() { + return new RestTemplate(); + } + + @Bean SampleRecords sampleRecords(ReservationRepository reservationRepository) { + return new SampleRecords(reservationRepository); + } + + @Bean ArrayListSpanAccumulator arrayListSpanAccumulator() { + return new ArrayListSpanAccumulator(); + } + + @Bean Sampler alwaysSampler() { + return new AlwaysSampler(); + } +} + +class SampleRecords { + + private final ReservationRepository reservationRepository; + + @Autowired + public SampleRecords(ReservationRepository reservationRepository) { + this.reservationRepository = reservationRepository; + } + + @PostConstruct + public void create() throws Exception { + Stream.of("Josh", "Jungryeol", "Nosung", "Hyobeom", + "Soeun", "Seunghue", "Peter", "Jooyong") + .forEach(name -> reservationRepository.save(new Reservation(name))); + reservationRepository.findAll().forEach(System.out::println); + } +} + +@RepositoryRestResource +interface ReservationRepository extends JpaRepository { +} + +@Entity +class Reservation { + + @Id + @GeneratedValue + private Long id; // id + + private String reservationName; // reservation_name + + public Long getId() { + return id; + } + + public String getReservationName() { + return reservationName; + } + + @Override + public String toString() { + return "Reservation{" + + "id=" + id + + ", reservationName='" + reservationName + '\'' + + '}'; + } + + Reservation() {// why JPA why??? + } + + public Reservation(String reservationName) { + + this.reservationName = reservationName; + } +} + + diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java index 62fab6cbd..05ee3bb3a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegrationTests.java @@ -22,6 +22,7 @@ import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; import org.springframework.cloud.sleuth.instrument.web.common.AbstractMvcIntegrationTest; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; @@ -66,6 +67,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { .findFirst().get(); then(parentSpan).hasLoggedAnEvent(Span.SERVER_RECV) .hasLoggedAnEvent(Span.SERVER_SEND); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -74,6 +76,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { MvcResult mvcResult = whenSentInfoWithTraceId(new Random().nextLong()); then(notSampledHeaderIsPresent(mvcResult)).isEqualTo(true); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -84,6 +87,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId); then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -93,6 +97,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { whenSentPingWithTraceId(expectedTraceId); then(MDC.getCopyOfContextMap()).isEmpty(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -104,6 +109,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { .andExpect(status().isOk()).andReturn(); then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId); + then(this.tracer.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -119,6 +126,9 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { .filter(span -> span.tags().containsKey("tag")).findFirst(); then(taggedSpan.isPresent()).isTrue(); then(taggedSpan.get()).hasATag("tag", "value"); + then(taggedSpan.get()).hasATag("mvc.controller.method", "deferred"); + then(taggedSpan.get()).hasATag("mvc.controller.class", "test-controller"); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -129,6 +139,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId); then(this.tracer.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -141,6 +152,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { then(this.spanAccumulator.getSpans().stream().filter(span -> span.getSpanId() == span.getTraceId()).findAny().isPresent()).as("a root span exists").isTrue(); then(this.tracer.getCurrentSpan()).isNull(); + then(ExceptionUtils.getLastException()).isNull(); } @Override @@ -239,6 +251,11 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { return "ping"; } + @RequestMapping("/throwsException") + public void throwsException() { + throw new RuntimeException(); + } + @RequestMapping("/deferred") public DeferredResult deferred() { logger.info("deferred"); 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 c36e7957d..28c80214e 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 @@ -158,8 +158,10 @@ public class TraceFilterTests { filter.doFilter(this.request, this.response, this.filterChain); - // this creates a child span which is why we'd expect the parents to include 1L) - assertThat(this.span.getParents()).containsOnly(3L); + // this creates a child span which is why we'd expect the parents to include the parent id + // especially important if no handler interceptors have been used. + // We add a child span on the server side to show which controller serviced the request + assertThat(this.span.getParents()).containsOnly(PARENT_ID); assertThat(parentSpan()) .hasATag("http.url", "http://localhost/?foo=bar") .hasATag("http.host", "localhost") 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 new file mode 100644 index 000000000..8ed1a8ad1 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterWebIntegrationTests.java @@ -0,0 +1,119 @@ +/* + * 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 org.junit.After; +import org.junit.Before; +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.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.sleuth.Sampler; +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.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; +import org.springframework.http.client.ClientHttpResponse; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.DefaultResponseErrorHandler; +import org.springframework.web.client.RestTemplate; + +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@WebIntegrationTest({ "server.port=0" }) +@SpringApplicationConfiguration(classes = { TraceFilterWebIntegrationTests.Config.class }) +public class TraceFilterWebIntegrationTests { + + @Autowired Tracer tracer; + @Autowired ArrayListSpanAccumulator accumulator; + @Autowired RestTemplate restTemplate; + @Autowired Environment environment; + + @Before + @After + public void cleanup() { + ExceptionUtils.setFail(true); + TestSpanContextHolder.removeCurrentSpan(); + } + + @Test + public void should_not_create_a_span_for_error_controller() { + this.restTemplate.getForObject("http://localhost:" + port() + "/", String.class); + + then(this.tracer.getCurrentSpan()).isNull(); + then(new ListOfSpans(this.accumulator.getSpans())) + .doesNotHaveASpanWithName("error") + .hasASpanWithTagEqualTo("http.status_code", "500"); + then(ExceptionUtils.getLastException()).isNull(); + } + + private int port() { + return this.environment.getProperty("local.server.port", Integer.class); + } + + @EnableAutoConfiguration + @Configuration + public static class Config { + + @Bean ExceptionThrowingController controller() { + return new ExceptionThrowingController(); + } + + @Bean ArrayListSpanAccumulator arrayListSpanAccumulator() { + return new ArrayListSpanAccumulator(); + } + + @Bean Sampler alwaysSampler() { + return new AlwaysSampler(); + } + + + @Bean RestTemplate restTemplate() { + RestTemplate restTemplate = new RestTemplate(); + restTemplate.setErrorHandler(new DefaultResponseErrorHandler() { + @Override public void handleError(ClientHttpResponse response) + throws IOException { + } + }); + return restTemplate; + } + } + + @RestController + public static class ExceptionThrowingController { + + @RequestMapping("/") + public void throwException() { + throw new RuntimeException("Throwing exception"); + } + } +} 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 d47951f67..48c4e662e 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 @@ -36,6 +36,7 @@ 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.ExceptionUtils; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; @@ -45,6 +46,8 @@ import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.SocketPolicy; +import static org.assertj.core.api.BDDAssertions.then; + /** * @author Marcin Grzejszczak */ @@ -88,6 +91,7 @@ public class TraceRestTemplateInterceptorIntegrationTests { SleuthAssertions.then(this.tracer.getCurrentSpan()).isEqualTo(span); this.tracer.close(span); + then(ExceptionUtils.getLastException()).isNull(); } private ClientHttpRequestFactory clientHttpRequestFactory() { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java index 10bf439c4..73ff1f476 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientDiscoveryExceptionTests.java @@ -50,6 +50,7 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.client.RestTemplate; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.BDDAssertions.then; @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = { @@ -71,7 +72,6 @@ public class WebClientDiscoveryExceptionTests { @After public void close() { - ExceptionUtils.setFail(false); TestSpanContextHolder.removeCurrentSpan(); } @@ -91,6 +91,7 @@ public class WebClientDiscoveryExceptionTests { SleuthAssertions.then(this.tracer.getCurrentSpan()).isEqualTo(span); this.tracer.close(span); + then(ExceptionUtils.getLastException()).isNull(); } @Test diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java index 5757a78de..3dbae387a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java @@ -18,6 +18,7 @@ package org.springframework.cloud.sleuth.instrument.web.client; import javax.servlet.http.HttpServletRequest; import java.lang.invoke.MethodHandles; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -170,12 +171,17 @@ public class WebClientTests { } private Span spanWithClientEvents() { - return this.listener.getSpans().stream() - .filter(span -> span.logs().stream() - .filter(log -> log.getEvent().contains(Span.CLIENT_RECV) - || log.getEvent().contains(Span.CLIENT_SEND)) - .findFirst().isPresent()) - .findFirst().get(); + List spans = new ArrayList<>(this.listener.getSpans()); + for(Span span : spans) { + boolean present = span.logs().stream() + .filter(log -> log.getEvent().contains(Span.CLIENT_RECV) + || log.getEvent().contains(Span.CLIENT_SEND)) + .findFirst().isPresent(); + if (present) { + return span; + } + } + return null; } Object[] parametersForShouldAttachTraceIdWhenCallingAnotherService() { @@ -220,7 +226,8 @@ public class WebClientTests { Optional storedSpan = this.listener.getSpans().stream() .filter(span -> "404".equals(span.tags().get("http.status_code"))).findFirst(); then(storedSpan.isPresent()).isTrue(); - this.listener.getSpans().stream() + List spans = new ArrayList<>(this.listener.getSpans()); + spans.stream() .forEach(span -> { int initialSize = span.logs().size(); int distinctSize = span.logs().stream().map(Log::getEvent).distinct().collect(Collectors.toList()).size(); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java index 3bfd2ec93..b5277b03a 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignClientServerErrorTests.java @@ -94,6 +94,7 @@ public class FeignClientServerErrorTests { Thread.sleep(100); then(this.capture.toString()) .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -107,6 +108,7 @@ public class FeignClientServerErrorTests { Thread.sleep(100); then(this.capture.toString()) .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -120,6 +122,7 @@ public class FeignClientServerErrorTests { Thread.sleep(100); then(this.capture.toString()) .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -133,6 +136,7 @@ public class FeignClientServerErrorTests { Thread.sleep(100); then(this.capture.toString()) .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -146,6 +150,7 @@ public class FeignClientServerErrorTests { Thread.sleep(100); then(this.capture.toString()) .doesNotContain("Tried to close span but it is not the current span"); + then(ExceptionUtils.getLastException()).isNull(); } @Configuration diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java index c2d491f09..7ba667b43 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java @@ -23,6 +23,7 @@ import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; import org.apache.log4j.Level; import org.apache.log4j.Logger; +import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; @@ -31,6 +32,8 @@ import org.springframework.cloud.client.circuitbreaker.EnableCircuitBreaker; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.stereotype.Component; @@ -40,13 +43,21 @@ import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.RestTemplate; +import static org.assertj.core.api.BDDAssertions.then; + public class Issue307Tests { + @Before + public void setup() { + TestSpanContextHolder.removeCurrentSpan(); + } + @Test public void should_start_context() { try (ConfigurableApplicationContext applicationContext = SpringApplication .run(SleuthSampleApplication.class, "--spring.jmx.enabled=false")) { } + then(ExceptionUtils.getLastException()).isNull(); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcIntegrationTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcIntegrationTest.java index fee7e35f3..af3f80282 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcIntegrationTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/common/AbstractMvcIntegrationTest.java @@ -10,6 +10,7 @@ import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.ApplicationContext; import org.springframework.test.context.web.WebAppConfiguration; @@ -42,6 +43,7 @@ public abstract class AbstractMvcIntegrationTest { @Before public void setup() { ExceptionUtils.setFail(true); + TestSpanContextHolder.removeCurrentSpan(); DefaultMockMvcBuilder mockMvcBuilder = MockMvcBuilders.webAppContextSetup(this.webApplicationContext); configureMockMvcBuilder(mockMvcBuilder); this.mockMvc = mockMvcBuilder.build(); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulIntegrationTests.java index c7810c18d..df168fe7d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulIntegrationTests.java @@ -10,6 +10,7 @@ import com.netflix.zuul.context.RequestContext; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,6 +34,7 @@ import org.springframework.cloud.sleuth.assertions.ListOfSpans; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpEntity; @@ -67,6 +69,7 @@ public class TraceZuulIntegrationTests { @Autowired RestTemplate restTemplate; @Before + @After public void cleanup() { TestSpanContextHolder.removeCurrentSpan(); RequestContext.getCurrentContext().unset(); @@ -92,6 +95,7 @@ public class TraceZuulIntegrationTests { .tag("http.method", "GET") .tag("http.status_code", "200") .tag("http.path", "/simple/foo")); + then(ExceptionUtils.getLastException()).isNull(); } @Test @@ -112,6 +116,7 @@ public class TraceZuulIntegrationTests { .tag("http.method", "GET") .tag("http.status_code", "404") .tag("http.path", "/simple/nonExistentUrl")); + then(ExceptionUtils.getLastException()).isNull(); } private static class TestTag extends HashMap { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java index 35ba6c3f7..517576f2b 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/util/SpanNameUtilTests.java @@ -26,4 +26,10 @@ public class SpanNameUtilTests { SleuthAssertions.then(SpanNameUtil.toLowerHyphen("aMethodNameInCamelCaseNotation")) .isEqualTo("a-method-name-in-camel-case-notation"); } + + @Test + public void should_convert_a_class_name_in_hyphen_based_notation() throws Exception { + SleuthAssertions.then(SpanNameUtil.toLowerHyphen("MySuperClassName")) + .isEqualTo("my-super-class-name"); + } } \ 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 307883eea..fd0c9a35a 100644 --- a/spring-cloud-sleuth-core/src/test/resources/logback.xml +++ b/spring-cloud-sleuth-core/src/test/resources/logback.xml @@ -5,8 +5,6 @@ - -