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 8b891bad3..8e38085f1 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 @@ -220,7 +220,7 @@ public class TraceFilter extends GenericFilterBean { log.debug( "Won't detach the span " + span + " since error has already been handled"); } - } else { + } else if (this.tracer.isTracing()) { if (log.isDebugEnabled()) { log.debug("Detaching the span " + span + " since the response was unsuccessful"); } 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 538195de3..ff5a144f8 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 @@ -62,26 +62,18 @@ public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { - if (isErrorControllerRelated(request)) { - if (log.isDebugEnabled()) { - log.debug("Skipping creation of a span for error controller processing"); - } - return true; - } - if (isSpanContinued(request)) { - if (log.isDebugEnabled()) { - log.debug("Skipping creation of a span since the span is continued"); - } - return true; - } String spanName = spanName(handler); - Span span = getTracer().createSpan(spanName); + boolean continueSpan = getRootSpanFromAttribute(request) != null; + Span span = continueSpan ? getRootSpanFromAttribute(request) : getTracer().createSpan(spanName); if (log.isDebugEnabled()) { log.debug("Created new span " + span + " with name [" + spanName + "]"); } addClassMethodTag(handler, span); addClassNameTag(handler, span); setSpanInAttribute(request, span); + if (!continueSpan) { + setNewSpanCreatedAttribute(request, span); + } return true; } @@ -123,7 +115,7 @@ public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { @Override public void afterConcurrentHandlingStarted(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { - Span spanFromRequest = getSpanFromAttribute(request); + Span spanFromRequest = getNewSpanFromAttribute(request); Span rootSpanFromRequest = getRootSpanFromAttribute(request); if (log.isDebugEnabled()) { log.debug("Closing the span " + spanFromRequest + " and detaching its parent " + rootSpanFromRequest + " since the request is asynchronous"); @@ -141,28 +133,27 @@ public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { } return; } - if (isSpanContinued(request)) { - if (log.isDebugEnabled()) { - 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); - } + Span span = getRootSpanFromAttribute(request); if (ex != null) { - getTracer().addTag(Span.SPAN_ERROR_TAG_NAME, ExceptionUtils.getExceptionMessage(ex)); + String errorMsg = ExceptionUtils.getExceptionMessage(ex); + if (log.isDebugEnabled()) { + log.debug("Adding an error tag [" + errorMsg + "] to span " + span + ""); + } + getTracer().addTag(Span.SPAN_ERROR_TAG_NAME, errorMsg); + } + if (getNewSpanFromAttribute(request) != null) { + if (log.isDebugEnabled()) { + log.debug("Closing span " + span); + } + Span newSpan = getNewSpanFromAttribute(request); + getTracer().continueSpan(newSpan); + getTracer().close(newSpan); + clearNewSpanCreatedAttribute(request); } - 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 getNewSpanFromAttribute(HttpServletRequest request) { + return (Span) request.getAttribute(TraceRequestAttributes.NEW_SPAN_REQUEST_ATTR); } private Span getRootSpanFromAttribute(HttpServletRequest request) { @@ -173,6 +164,14 @@ public class TraceHandlerInterceptor extends HandlerInterceptorAdapter { request.setAttribute(TraceRequestAttributes.HANDLED_SPAN_REQUEST_ATTR, span); } + private void setNewSpanCreatedAttribute(HttpServletRequest request, Span span) { + request.setAttribute(TraceRequestAttributes.NEW_SPAN_REQUEST_ATTR, span); + } + + private void clearNewSpanCreatedAttribute(HttpServletRequest request) { + request.removeAttribute(TraceRequestAttributes.NEW_SPAN_REQUEST_ATTR); + } + private Tracer getTracer() { if (this.tracer == null) { this.tracer = this.beanFactory.getBean(Tracer.class); 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 index 683293807..1b3ccfbb0 100644 --- 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 @@ -31,6 +31,12 @@ public final class TraceRequestAttributes { public static final String HANDLED_SPAN_REQUEST_ATTR = TraceRequestAttributes.class.getName() + ".TRACE_HANDLED"; + /** + * Set if Handler interceptor has executed some logic + */ + public static final String NEW_SPAN_REQUEST_ATTR = TraceRequestAttributes.class.getName() + + ".TRACE_HANDLED_NEW_SPAN"; + /** * 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. 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 a81a5cd14..658dfc36b 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 @@ -16,6 +16,9 @@ package org.springframework.cloud.sleuth.assertions; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -156,7 +159,7 @@ public class ListOfSpansAssert extends AbstractAssert \nto contain at least one span with tag key " - + "equal to <%s> and value equal to <%s>", spansToString(), tagKey, tagValue); + + "equal to <%s> and value equal to <%s>.\n\n", spansToString(), tagKey, tagValue); } return this; } 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 2e4685510..6efb641cc 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 @@ -1,10 +1,5 @@ package org.springframework.cloud.sleuth.instrument.web; -import static org.assertj.core.api.BDDAssertions.then; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; - import java.util.Optional; import java.util.Random; import java.util.concurrent.CompletableFuture; @@ -22,6 +17,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanReporter; +import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; import org.springframework.cloud.sleuth.instrument.web.common.AbstractMvcIntegrationTest; @@ -40,16 +36,19 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.request.async.DeferredResult; +import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + @RunWith(SpringRunner.class) @SpringBootTest(classes = TraceFilterIntegrationTests.Config.class) public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { private static Log logger = LogFactory.getLog(TraceFilterIntegrationTests.class); - @Autowired - TraceFilter traceFilter; - @Autowired - ArrayListSpanAccumulator spanAccumulator; + @Autowired TraceFilter traceFilter; + @Autowired ArrayListSpanAccumulator spanAccumulator; private static Span span; @@ -63,10 +62,11 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { public void should_create_and_return_trace_in_HTTP_header() throws Exception { whenSentPingWithoutTracingData(); - Span parentSpan = this.spanAccumulator.getSpans().stream().filter(span -> span - .getSpanId() == TraceFilterIntegrationTests.span.getParents().get(0)) - .findFirst().get(); - then(parentSpan).hasLoggedAnEvent(Span.SERVER_RECV) + then(this.spanAccumulator.getSpans()).hasSize(1); + Span span = this.spanAccumulator.getSpans().get(0); + then(span).hasLoggedAnEvent(Span.SERVER_RECV) + .hasATagWithKey(new TraceKeys().getMvc().getControllerClass()) + .hasATagWithKey(new TraceKeys().getMvc().getControllerMethod()) .hasLoggedAnEvent(Span.SERVER_SEND); then(ExceptionUtils.getLastException()).isNull(); } @@ -87,7 +87,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { throws Exception { Long expectedTraceId = new Random().nextLong(); - whenSentPingWithTraceId(expectedTraceId); + MvcResult mvcResult = whenSentPingWithTraceId(expectedTraceId); then(ExceptionUtils.getLastException()).isNull(); } @@ -107,21 +107,20 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { Long expectedTraceId = new Random().nextLong(); MvcResult mvcResult = whenSentFutureWithTraceId(expectedTraceId); - this.mockMvc.perform(asyncDispatch(mvcResult)).andExpect(status().isOk()) - .andReturn(); + this.mockMvc.perform(asyncDispatch(mvcResult)) + .andExpect(status().isOk()).andReturn(); then(this.tracer.getCurrentSpan()).isNull(); then(ExceptionUtils.getLastException()).isNull(); } @Test - public void should_add_a_custom_tag_to_the_span_created_in_controller() - throws Exception { + public void should_add_a_custom_tag_to_the_span_created_in_controller() throws Exception { Long expectedTraceId = new Random().nextLong(); MvcResult mvcResult = whenSentDeferredWithTraceId(expectedTraceId); - this.mockMvc.perform(asyncDispatch(mvcResult)).andExpect(status().isOk()) - .andReturn(); + this.mockMvc.perform(asyncDispatch(mvcResult)) + .andExpect(status().isOk()).andReturn(); Optional taggedSpan = this.spanAccumulator.getSpans().stream() .filter(span -> span.tags().containsKey("tag")).findFirst(); @@ -133,27 +132,24 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { } @Test - public void should_log_tracing_information_when_exception_was_thrown() - throws Exception { + public void should_log_tracing_information_when_exception_was_thrown() throws Exception { Long expectedTraceId = new Random().nextLong(); - whenSentToNonExistentEndpointWithTraceId(expectedTraceId); + MvcResult mvcResult = whenSentToNonExistentEndpointWithTraceId(expectedTraceId); then(this.tracer.getCurrentSpan()).isNull(); then(ExceptionUtils.getLastException()).isNull(); } @Test - public void should_assume_that_a_request_without_span_and_with_trace_is_a_root_span() - throws Exception { + public void should_assume_that_a_request_without_span_and_with_trace_is_a_root_span() throws Exception { Long expectedTraceId = new Random().nextLong(); whenSentRequestWithTraceIdAndNoSpanId(expectedTraceId); whenSentRequestWithTraceIdAndNoSpanId(expectedTraceId); - then(this.spanAccumulator.getSpans().stream() - .filter(span -> span.getSpanId() == span.getTraceId()).findAny() - .isPresent()).as("a root span exists").isTrue(); + 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(); } @@ -186,10 +182,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { return sendDeferredWithTraceId(Span.TRACE_ID_NAME, passedTraceId); } - private MvcResult whenSentToNonExistentEndpointWithTraceId(Long passedTraceId) - throws Exception { - return sendRequestWithTraceId("/exception/nonExistent", Span.TRACE_ID_NAME, - passedTraceId, HttpStatus.NOT_FOUND); + private MvcResult whenSentToNonExistentEndpointWithTraceId(Long passedTraceId) throws Exception { + return sendRequestWithTraceId("/exception/nonExistent", Span.TRACE_ID_NAME, passedTraceId, HttpStatus.NOT_FOUND); } private MvcResult sendPingWithTraceId(String headerName, Long traceId) @@ -206,8 +200,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { throws Exception { return this.mockMvc .perform(MockMvcRequestBuilders.get(path).accept(MediaType.TEXT_PLAIN) - .header(headerName, Span.idToHex(traceId)).header( - Span.SPAN_ID_NAME, Span.idToHex(new Random().nextLong()))) + .header(headerName, Span.idToHex(traceId)) + .header(Span.SPAN_ID_NAME, Span.idToHex(new Random().nextLong()))) .andReturn(); } @@ -219,13 +213,14 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { .andReturn(); } - private MvcResult sendRequestWithTraceId(String path, String headerName, Long traceId, - HttpStatus status) throws Exception { + private MvcResult sendRequestWithTraceId(String path, String headerName, Long traceId, HttpStatus status) + throws Exception { return this.mockMvc .perform(MockMvcRequestBuilders.get(path).accept(MediaType.TEXT_PLAIN) - .header(headerName, Span.idToHex(traceId)).header( - Span.SPAN_ID_NAME, Span.idToHex(new Random().nextLong()))) - .andExpect(status().is(status.value())).andReturn(); + .header(headerName, Span.idToHex(traceId)) + .header(Span.SPAN_ID_NAME, Span.idToHex(new Random().nextLong()))) + .andExpect(status().is(status.value())) + .andReturn(); } private boolean notSampledHeaderIsPresent(MvcResult mvcResult) { 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 eb5375814..019b79c72 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 @@ -16,9 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign.servererrors; -import static org.assertj.core.api.BDDAssertions.then; -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -66,6 +63,9 @@ 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,14 +76,10 @@ import feign.codec.ErrorDecoder; @TestPropertySource(properties = { "spring.application.name=fooservice" }) public class FeignClientServerErrorTests { - @Autowired - TestFeignInterface feignInterface; - @Autowired - TestFeignWithCustomConfInterface customConfFeignInterface; - @Autowired - Listener listener; - @Rule - public OutputCapture capture = new OutputCapture(); + @Autowired TestFeignInterface feignInterface; + @Autowired TestFeignWithCustomConfInterface customConfFeignInterface; + @Autowired Listener listener; + @Rule public OutputCapture capture = new OutputCapture(); @Before public void setup() { @@ -95,19 +91,16 @@ public class FeignClientServerErrorTests { public void shouldCloseSpanOnInternalServerError() throws InterruptedException { try { this.feignInterface.internalError(); - } - catch (HystrixRuntimeException e) { + } catch (HystrixRuntimeException e) { } 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"); + .hasASpanWithTagEqualTo(Span.SPAN_ERROR_TAG_NAME, + "Request processing failed; nested exception is java.lang.RuntimeException: Internal Error"); }); } @@ -115,8 +108,7 @@ public class FeignClientServerErrorTests { public void shouldCloseSpanOnNotFound() throws InterruptedException { try { this.feignInterface.notFound(); - } - catch (HystrixRuntimeException e) { + } catch (HystrixRuntimeException e) { } Awaitility.await().until(() -> { @@ -130,45 +122,37 @@ public class FeignClientServerErrorTests { public void shouldCloseSpanOnOk() throws InterruptedException { try { this.feignInterface.ok(); - } - catch (HystrixRuntimeException e) { + } catch (HystrixRuntimeException e) { } Awaitility.await().until(() -> { - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); then(ExceptionUtils.getLastException()).isNull(); }); } @Test - public void shouldCloseSpanOnOkWithCustomFeignConfiguration() - throws InterruptedException { + public void shouldCloseSpanOnOkWithCustomFeignConfiguration() throws InterruptedException { try { this.customConfFeignInterface.ok(); - } - catch (HystrixRuntimeException e) { + } catch (HystrixRuntimeException e) { } Awaitility.await().until(() -> { - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); then(ExceptionUtils.getLastException()).isNull(); }); } @Test - public void shouldCloseSpanOnNotFoundWithCustomFeignConfiguration() - throws InterruptedException { + public void shouldCloseSpanOnNotFoundWithCustomFeignConfiguration() throws InterruptedException { try { this.customConfFeignInterface.notFound(); - } - catch (HystrixRuntimeException e) { + } catch (HystrixRuntimeException e) { } Awaitility.await().until(() -> { - then(this.capture.toString()) - .doesNotContain("Tried to close span but it is not the current span"); + then(this.capture.toString()).doesNotContain("Tried to close span but it is not the current span"); then(ExceptionUtils.getLastException()).isNull(); }); } @@ -176,9 +160,10 @@ public class FeignClientServerErrorTests { @Configuration @EnableAutoConfiguration @EnableFeignClients - @RibbonClients({ - @RibbonClient(value = "fooservice", configuration = SimpleRibbonClientConfiguration.class), - @RibbonClient(value = "customConfFooService", configuration = SimpleRibbonClientConfiguration.class) }) + @RibbonClients({@RibbonClient(value = "fooservice", + configuration = SimpleRibbonClientConfiguration.class), + @RibbonClient(value = "customConfFooService", + configuration = SimpleRibbonClientConfiguration.class)}) public static class TestConfiguration { @Bean @@ -197,8 +182,7 @@ public class FeignClientServerErrorTests { return new RestTemplate(); } - @Bean - Sampler testSampler() { + @Bean Sampler testSampler() { return new AlwaysSampler(); } @@ -227,6 +211,7 @@ public class FeignClientServerErrorTests { ResponseEntity ok(); } + @Configuration public static class CustomFeignClientConfiguration { @Bean diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java index b2fea095e..882488d0d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java @@ -115,7 +115,8 @@ public class WebClientTests { then(getHeader(response, Span.SPAN_ID_NAME)).isNull(); then(this.listener.getSpans()).isNotEmpty(); Optional noTraceSpan = new ArrayList<>(this.listener.getSpans()).stream().filter(span -> - "http:/notrace".equals(span.getName()) && !span.tags().isEmpty()).findFirst(); + "http:/notrace".equals(span.getName()) && !span.tags().isEmpty() + && span.tags().containsKey("http.path")).findFirst(); then(noTraceSpan.isPresent()).isTrue(); // TODO: matches cause there is an issue with Feign not providing the full URL at the interceptor level then(noTraceSpan.get()).matchesATag("http.url", ".*/notrace") diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469.java new file mode 100644 index 000000000..e73f7fdad --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469.java @@ -0,0 +1,29 @@ +/* + * 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.view; + +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.web.servlet.config.annotation.ViewControllerRegistry; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; + +@EnableAutoConfiguration +public class Issue469 extends WebMvcConfigurerAdapter { + + @Override public void addViewControllers(ViewControllerRegistry registry) { + registry.addViewController("/welcome").setViewName("welcome"); + } +} \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469Tests.java new file mode 100644 index 000000000..1cfbd65d5 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/view/Issue469Tests.java @@ -0,0 +1,68 @@ +/* + * 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.view; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.core.env.Environment; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.client.RestTemplate; + +import static org.assertj.core.api.BDDAssertions.then; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = Issue469.class) +@TestPropertySource(properties = {"spring.mvc.view.prefix=/WEB-INF/jsp/", + "spring.mvc.view.suffix=.jsp"}) +@WebIntegrationTest({ "server.port=0" }) +public class Issue469Tests { + + @Autowired Tracer tracer; + @Autowired Environment environment; + RestTemplate restTemplate = new RestTemplate(); + + @Before + public void setup() { + ExceptionUtils.setFail(true); + } + + @Test + public void should_not_result_in_tracing_exceptions_when_using_view_controllers() throws Exception { + try { + this.restTemplate + .getForObject("http://localhost:" + port() + "/welcome", String.class); + } catch (Exception e) { + // JSPs are not rendered + then(e).hasMessage("404 Not Found"); + } + + then(ExceptionUtils.getLastException()).isNull(); + then(this.tracer.getCurrentSpan()).isNull(); + } + + private int port() { + return this.environment.getProperty("local.server.port", Integer.class); + } + +} \ No newline at end of file diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java index afcebbb02..aff5ca52a 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java @@ -113,10 +113,10 @@ public class MessagingApplicationTests extends AbstractIntegrationTest { Optional eventSentSpan = findSpanWithAnnotation(Constants.SERVER_SEND); Optional eventReceivedSpan = findSpanWithAnnotation(Constants.CLIENT_RECV); Optional lastHttpSpansParent = findLastHttpSpansParent(); - // "http:/parent/" -> "home" -> "message:messages" -> "http:/foo" (CS + CR) -> "http:/foo" (SS) -> "foo" + // "http:/parent/" -> "message:messages" -> "http:/foo" (CS + CR) -> "http:/foo" (SS) Collections.sort(this.integrationTestSpanCollector.hashedSpans); thenAllSpansArePresent(firstHttpSpan, eventSpans, lastHttpSpansParent, eventSentSpan, eventReceivedSpan); - then(this.integrationTestSpanCollector.hashedSpans).as("There were 6 spans").hasSize(6); + then(this.integrationTestSpanCollector.hashedSpans).as("There were 4 spans").hasSize(4); log.info("Checking the parent child structure"); List> parentChild = this.integrationTestSpanCollector.hashedSpans.stream() .filter(span -> span.parentId != null) @@ -147,7 +147,8 @@ public class MessagingApplicationTests extends AbstractIntegrationTest { private Optional findFirstHttpRequestSpan() { return this.integrationTestSpanCollector.hashedSpans.stream() // home is the name of the method - .filter(span -> "home".equals(span.name)).findFirst(); + .filter(span -> span.binaryAnnotations.stream() + .anyMatch(binaryAnnotation -> new String(binaryAnnotation.value).equals("home"))).findFirst(); } private void thenAllSpansArePresent(Optional firstHttpSpan,