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 9dfeefc9e..02fa84c4c 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 @@ -251,7 +251,9 @@ public class TraceFilter extends GenericFilterBean { if (log.isDebugEnabled()) { log.debug("Will close span " + span + " since " + (shouldCloseSpan(request) ? "some component marked it for closure" : "response was unsuccessful for the root span")); } - tracer().close(span); + if (!httpStatusSuccessful(response) && !hasErrorController()) { + tracer().close(span); + } if (exception == null || !hasErrorController()) { clearTraceAttribute(request); } @@ -259,7 +261,9 @@ public class TraceFilter extends GenericFilterBean { if (log.isDebugEnabled()) { log.debug("Detaching the span " + span + " since the response was unsuccessful"); } - clearTraceAttribute(request); + if (!httpStatusSuccessful(response) && !hasErrorController()) { + clearTraceAttribute(request); + } if (exception == null || !hasErrorController()) { tracer().detach(span); } 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 d3a552a3a..ab9fa15af 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 @@ -3,7 +3,6 @@ package org.springframework.cloud.sleuth.instrument.web; import java.util.Optional; import java.util.Random; import java.util.concurrent.CompletableFuture; - import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; @@ -39,7 +38,9 @@ import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.request.async.DeferredResult; +import org.springframework.web.util.NestedServletException; +import static org.assertj.core.api.Assertions.fail; 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; @@ -137,12 +138,34 @@ 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_404_exception_was_thrown() throws Exception { Long expectedTraceId = new Random().nextLong(); MvcResult mvcResult = whenSentToNonExistentEndpointWithTraceId(expectedTraceId); then(this.tracer.getCurrentSpan()).isNull(); + then(this.spanAccumulator.getSpans()).hasSize(1); + then(ExceptionUtils.getLastException()).isNull(); + } + + @Test + public void should_log_tracing_information_when_500_exception_was_thrown() throws Exception { + Long expectedTraceId = new Random().nextLong(); + + try { + whenSentToExceptionThrowingEndpoint(expectedTraceId); + fail("Should fail"); + } catch (NestedServletException e) { + then(e).hasRootCauseInstanceOf(RuntimeException.class); + } + + // not null cause TraceFilter has also error dispatch and + // the ErrorController would close the span + then(this.tracer.getCurrentSpan()).isNotNull(); + this.tracer.close(this.tracer.getCurrentSpan()); + then(this.spanAccumulator.getSpans()).hasSize(1); + then(this.spanAccumulator.getSpans().get(0).tags()) + .containsKey("error"); then(ExceptionUtils.getLastException()).isNull(); } @@ -203,6 +226,10 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest { return sendRequestWithTraceId("/exception/nonExistent", Span.TRACE_ID_NAME, passedTraceId, HttpStatus.NOT_FOUND); } + private MvcResult whenSentToExceptionThrowingEndpoint(Long passedTraceId) throws Exception { + return sendRequestWithTraceId("/throwsException", Span.TRACE_ID_NAME, passedTraceId, HttpStatus.INTERNAL_SERVER_ERROR); + } + private MvcResult sendPingWithTraceId(String headerName, Long traceId) throws Exception { return sendRequestWithTraceId("/ping", headerName, traceId);