improve servlet error capture

CHANGELOG.md

@@ -9,6 +9,8 @@
 ## Features
  * Supporting OpenTracing version 0.33 
  * Added annotation and meta-annotation matching support for `trace_methods`
+ * Improved servlet exception capture via attributes: `javax.servlet.error.exception`, `exception`, `org.springframework.web.servlet.DispatcherServlet.EXCEPTION`, `co.elastic.apm.exception`.
+   Added instrumentation for DispatcherServlet#processHandlerException.
 
 ## Bug Fixes
  * A warning in logs saying APM server is not available when using 1.8 with APM server 6.x

apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java

@@ -44,7 +44,9 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.security.Principal;
+import java.util.Arrays;
 import java.util.Enumeration;
+import java.util.List;
 import java.util.Map;
 
 import static co.elastic.apm.agent.servlet.ServletTransactionHelper.determineServiceName;
@@ -72,6 +74,9 @@ protected Boolean initialValue() {
         }
     };
 
+    @VisibleForAdvice
+    public static final List<String> requestExceptionAttributes = Arrays.asList("javax.servlet.error.exception", "exception", "org.springframework.web.servlet.DispatcherServlet.EXCEPTION", "co.elastic.apm.exception");
+
     static void init(ElasticApmTracer tracer) {
         ServletApiAdvice.tracer = tracer;
         servletTransactionHelper = new ServletTransactionHelper(tracer);
@@ -185,7 +190,19 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl
                 } else {
                     parameterMap = null;
                 }
-                servletTransactionHelper.onAfter(transaction, t, response.isCommitted(), response.getStatus(), request.getMethod(),
+
+                Throwable t2 = null;
+                if (t == null) {
+                    for (String attributeName : requestExceptionAttributes) {
+                        Object throwable = request.getAttribute(attributeName);
+                        if (throwable != null && throwable instanceof Throwable) {
+                            t2 = (Throwable) throwable;
+                            break;
+                        }
+                    }
+                }
+
+                servletTransactionHelper.onAfter(transaction, t == null ? t2 : t, response.isCommitted(), response.getStatus(), request.getMethod(),
                     parameterMap, request.getServletPath(), request.getPathInfo(), contentTypeHeader, true);
             }
         }

apm-agent-plugins/apm-spring-webmvc-plugin/src/main/java/co/elastic/apm/agent/spring/webmvc/ExceptionHandlerInstrumentation.java

@@ -0,0 +1,78 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2019 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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.
+ * #L%
+ */
+package co.elastic.apm.agent.spring.webmvc;
+
+import co.elastic.apm.agent.bci.ElasticApmInstrumentation;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+import javax.servlet.http.HttpServletRequest;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.returns;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+public class ExceptionHandlerInstrumentation extends ElasticApmInstrumentation {
+
+    @Override
+    public Class<?> getAdviceClass() {
+        return ExceptionHandlerAdviceService.class;
+    }
+
+    public static class ExceptionHandlerAdviceService {
+
+        @Advice.OnMethodEnter(suppress = Throwable.class)
+        public static void captureException(@Advice.Argument(0) HttpServletRequest request,
+                                            @Advice.Argument(3) Exception e) {
+            if (request != null) {
+                request.setAttribute("co.elastic.apm.exception", e);
+            }
+        }
+    }
+
+    @Override
+    public ElementMatcher<? super TypeDescription> getTypeMatcher() {
+        return named("org.springframework.web.servlet.DispatcherServlet");
+    }
+
+    @Override
+    public ElementMatcher<? super MethodDescription> getMethodMatcher() {
+        return named("processHandlerException")
+            .and(takesArgument(0, named("javax.servlet.http.HttpServletRequest")))
+            .and(takesArgument(1, named("javax.servlet.http.HttpServletResponse")))
+            .and(takesArgument(2, named("java.lang.Object")))
+            .and(takesArgument(3, named("java.lang.Exception")))
+            .and(returns(named("org.springframework.web.servlet.ModelAndView")));
+    }
+
+    @Override
+    public Collection<String> getInstrumentationGroupNames() {
+        return Arrays.asList("exception-handler");
+    }
+}

apm-agent-plugins/apm-spring-webmvc-plugin/src/main/resources/META-INF/services/co.elastic.apm.agent.bci.ElasticApmInstrumentation

@@ -1,3 +1,4 @@
 co.elastic.apm.agent.spring.webmvc.SpringTransactionNameInstrumentation
 co.elastic.apm.agent.spring.webmvc.DispatcherServletRenderInstrumentation
 co.elastic.apm.agent.spring.webmvc.SpringServiceNameInstrumentation
+co.elastic.apm.agent.spring.webmvc.ExceptionHandlerInstrumentation

apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java

@@ -0,0 +1,103 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2019 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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.
+ * #L%
+ */
+package co.elastic.apm.agent.spring.webmvc.exception;
+
+import co.elastic.apm.agent.MockReporter;
+import co.elastic.apm.agent.bci.ElasticApmAgent;
+import co.elastic.apm.agent.configuration.SpyConfiguration;
+import co.elastic.apm.agent.impl.ElasticApmTracer;
+import co.elastic.apm.agent.impl.ElasticApmTracerBuilder;
+import co.elastic.apm.agent.servlet.ServletInstrumentation;
+import co.elastic.apm.agent.spring.webmvc.ExceptionHandlerInstrumentation;
+import net.bytebuddy.agent.ByteBuddyAgent;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.TestConfiguration;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.setup.MockMvcBuilders;
+import org.springframework.web.context.WebApplicationContext;
+
+import java.io.UnsupportedEncodingException;
+import java.util.Arrays;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(value = SpringJUnit4ClassRunner.class)
+@WebAppConfiguration
+@TestConfiguration
+public abstract class AbstractExceptionHandlerInstrumentationTest {
+
+    protected static MockReporter reporter;
+    protected static ElasticApmTracer tracer;
+    protected MockMvc mockMvc;
+
+    @Autowired
+    WebApplicationContext wac;
+
+    @BeforeClass
+    @BeforeAll
+    public static void setUpAll() {
+        reporter = new MockReporter();
+        tracer = new ElasticApmTracerBuilder()
+            .configurationRegistry(SpyConfiguration.createSpyConfig())
+            .reporter(reporter)
+            .build();
+        ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install(),
+            Arrays.asList(new ServletInstrumentation(tracer), new ExceptionHandlerInstrumentation()));
+    }
+
+    @AfterClass
+    @AfterAll
+    public static void afterAll() {
+        ElasticApmAgent.reset();
+    }
+
+    @Before
+    @BeforeEach
+    public void setup() {
+        this.mockMvc =
+            MockMvcBuilders.webAppContextSetup(wac).build();
+    }
+
+    protected void assertExceptionCapture(Class exceptionClazz, MockHttpServletResponse response, int statusCode, String responseContent, String exceptionMessage) throws UnsupportedEncodingException {
+        assertEquals(1, reporter.getTransactions().size());
+        assertEquals(0, reporter.getSpans().size());
+        assertEquals(1, reporter.getErrors().size());
+        assertEquals(exceptionMessage, reporter.getErrors().get(0).getException().getMessage());
+        assertEquals(exceptionClazz, reporter.getErrors().get(0).getException().getClass());
+        assertEquals(statusCode, response.getStatus());
+        assertEquals(responseContent, response.getContentAsString());
+    }
+
+}

apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/ExceptionHandlerInstrumentationWithExceptionHandlerTest.java

@@ -0,0 +1,49 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2019 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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.
+ * #L%
+ */
+package co.elastic.apm.agent.spring.webmvc.exception;
+
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.exception_handler.ExceptionHandlerRuntimeException;
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.exception_handler.ExceptionHandlerController;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+
+@ContextConfiguration(classes = {
+    ExceptionHandlerController.class})
+public class ExceptionHandlerInstrumentationWithExceptionHandlerTest extends AbstractExceptionHandlerInstrumentationTest {
+
+    @Test
+    public void testExceptionCapture() throws Exception {
+        ResultActions resultActions = this.mockMvc.perform(get("/exception-handler/throw-exception"));
+        MvcResult result = resultActions.andReturn();
+        MockHttpServletResponse response = result.getResponse();
+
+        assertExceptionCapture(ExceptionHandlerRuntimeException.class, response, 409, "exception-handler runtime exception occured", "runtime exception occured");
+    }
+}

apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/ExceptionHandlerInstrumentationWithExceptionResolverTest.java

@@ -0,0 +1,54 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2019 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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.
+ * #L%
+ */
+package co.elastic.apm.agent.spring.webmvc.exception;
+
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.exception_resolver.ExceptionResolverController;
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.exception_resolver.ExceptionResolverRuntimeException;
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.exception_resolver.RestResponseStatusExceptionResolver;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+import static org.junit.Assert.assertEquals;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+
+@ContextConfiguration(classes = {
+    ExceptionResolverController.class,
+    RestResponseStatusExceptionResolver.class})
+public class ExceptionHandlerInstrumentationWithExceptionResolverTest extends AbstractExceptionHandlerInstrumentationTest {
+
+    @Test
+    public void testCallApiWithExceptionThrown() throws Exception {
+        ResultActions resultActions = this.mockMvc.perform(get("/exception-resolver/throw-exception"));
+        MvcResult result = resultActions.andReturn();
+        MockHttpServletResponse response = result.getResponse();
+
+        assertExceptionCapture(ExceptionResolverRuntimeException.class, response, 200, "", "runtime exception occured");
+        assertEquals("error-page", response.getForwardedUrl());
+        assertEquals("runtime exception occured", result.getModelAndView().getModel().get("message"));
+    }
+}

apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/ExceptionHandlerInstrumentationWithGlobalAdviceTest.java

@@ -0,0 +1,53 @@
+/*-
+ * #%L
+ * Elastic APM Java agent
+ * %%
+ * Copyright (C) 2018 - 2019 Elastic and contributors
+ * %%
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you 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.
+ * #L%
+ */
+package co.elastic.apm.agent.spring.webmvc.exception;
+
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.controller_advice.ControllerAdviceController;
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.controller_advice.ControllerAdviceRuntimeException;
+import co.elastic.apm.agent.spring.webmvc.exception.testapp.controller_advice.GlobalExceptionHandler;
+import org.junit.Test;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+
+@ContextConfiguration(classes = {
+    ControllerAdviceController.class,
+    GlobalExceptionHandler.class
+})
+public class ExceptionHandlerInstrumentationWithGlobalAdviceTest extends AbstractExceptionHandlerInstrumentationTest {
+
+    @Test
+    public void testExceptionCaptureWithGlobalControllerAdvice() throws Exception {
+
+        ResultActions resultActions = this.mockMvc.perform(get("/controller-advice/throw-exception"));
+        MvcResult result = resultActions.andReturn();
+        MockHttpServletResponse response = result.getResponse();
+
+        assertExceptionCapture(ControllerAdviceRuntimeException.class, response, 409, "controller-advice runtime exception occured", "runtime exception occured");
+    }
+}