Catch exception in filter chain and use it to set status in span

When a controller throws an exception the servlet container will
eventually set the response status to 500, but it is still 200
generally when the filter chain finishes, unless we catch the
exception and do something with it.

Fixes gh-57

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,

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;

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;

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);
 	}