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 864d1d454..45e01d6ed 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 @@ -143,12 +143,17 @@ public class TraceFilter extends OncePerRequestFilter request.setAttribute(TRACE_REQUEST_ATTR, trace); } + Throwable exception = null; try { addRequestAnnotations(request); filterChain.doFilter(request, response); } + catch (Throwable e) { + exception = e; + throw e; + } finally { if (isAsyncStarted(request) || request.isAsyncStarted()) { // TODO: how to deal with response annotations and async? @@ -158,8 +163,8 @@ public class TraceFilter extends OncePerRequestFilter addToResponseIfNotPresent(response, Trace.NOT_SAMPLED_NAME, ""); } if (trace != null) { + addResponseAnnotations(response, exception); addResponseHeaders(response, trace.getSpan()); - addResponseAnnotations(response); if (trace.getSavedTrace() != null) { publish(new ServerSentEvent(this, trace.getSavedTrace().getSpan(), trace.getSpan())); @@ -204,9 +209,17 @@ public class TraceFilter extends OncePerRequestFilter } } - private void addResponseAnnotations(HttpServletResponse response) { - this.traceManager.addAnnotation("/http/response/status_code", - String.valueOf(response.getStatus())); + private void addResponseAnnotations(HttpServletResponse response, Throwable e) { + if (response.getStatus() == HttpServletResponse.SC_OK && e != null) { + // Filter chain threw exception but the response status may not have been set + // yet, so we have to guess. + this.traceManager.addAnnotation("/http/response/status_code", + String.valueOf(HttpServletResponse.SC_INTERNAL_SERVER_ERROR)); + } + else { + this.traceManager.addAnnotation("/http/response/status_code", + String.valueOf(response.getStatus())); + } for (String name : response.getHeaderNames()) { for (String value : response.getHeaders(name)) { @@ -219,7 +232,7 @@ public class TraceFilter extends OncePerRequestFilter private String getHeader(HttpServletRequest request, HttpServletResponse response, String name) { String value = request.getHeader(name); - return value!=null ? value : response.getHeader(name); + return value != null ? value : response.getHeader(name); } private void addToResponseIfNotPresent(HttpServletResponse response, String name, diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncIntegrationTests.java similarity index 96% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncIntegrationTests.java index 896c2854e..8111f6f33 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncIntegrationTests.java @@ -24,8 +24,8 @@ import com.jayway.awaitility.Awaitility; @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = { - TraceAsyncITest.TraceAsyncITestConfiguration.class }) -public class TraceAsyncITest { + TraceAsyncIntegrationTests.TraceAsyncITestConfiguration.class }) +public class TraceAsyncIntegrationTests { @Autowired ClassPerformingAsyncLogic classPerformingAsyncLogic; @Autowired TraceManager traceManager; diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegartionTests.java similarity index 95% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterITest.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegartionTests.java index 45fb1ea1f..8e77edf22 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterIntegartionTests.java @@ -19,9 +19,9 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.setup.DefaultMockMvcBuilder; @RunWith(SpringJUnit4ClassRunner.class) -@SpringApplicationConfiguration(TraceFilterITest.class) +@SpringApplicationConfiguration(TraceFilterIntegartionTests.class) @DefaultTestAutoConfiguration -public class TraceFilterITest extends MvcITest { +public class TraceFilterIntegartionTests extends MvcITest { @Autowired TraceManager traceManager; 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 1a10466a9..1ae00dc8c 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 @@ -70,7 +70,7 @@ public class TraceFilterTests { new JdkIdGenerator(), this.publisher) { @Override protected Trace createTrace(Trace trace, Span span) { - TraceFilterTests.this.span= span; + TraceFilterTests.this.span = span; return super.createTrace(trace, span); } }; @@ -135,7 +135,33 @@ public class TraceFilterTests { assertNull(TraceContextHolder.getCurrentTrace()); } + @Test + public void catchesException() throws Exception { + TraceFilter filter = new TraceFilter(this.traceManager); + this.filterChain = new MockFilterChain() { + @Override + public void doFilter(javax.servlet.ServletRequest request, + javax.servlet.ServletResponse response) + throws java.io.IOException, javax.servlet.ServletException { + throw new RuntimeException("Planned"); + }; + }; + try { + filter.doFilter(this.request, this.response, this.filterChain); + } + catch (RuntimeException e) { + assertEquals("Planned", e.getMessage()); + } + verifyHttpAnnotations(HttpStatus.INTERNAL_SERVER_ERROR); + + assertNull(TraceContextHolder.getCurrentTrace()); + } + public void verifyHttpAnnotations() { + verifyHttpAnnotations(HttpStatus.OK); + } + + public void verifyHttpAnnotations(HttpStatus status) { hasAnnotation(this.span, "/http/request/uri", "http://localhost/"); hasAnnotation(this.span, "/http/request/endpoint", "/"); hasAnnotation(this.span, "/http/request/method", "GET"); @@ -143,8 +169,7 @@ public class TraceFilterTests { MediaType.APPLICATION_JSON_VALUE); hasAnnotation(this.span, "/http/request/headers/user-agent", "MockMvc"); - hasAnnotation(this.span, "/http/response/status_code", - HttpStatus.OK.toString()); + hasAnnotation(this.span, "/http/response/status_code", status.toString()); hasAnnotation(this.span, "/http/response/headers/content-type", MediaType.APPLICATION_JSON_VALUE); }