diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/AsyncInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/AsyncInstrumentation.java index df229b5e05..51666f7d49 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/AsyncInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/AsyncInstrumentation.java @@ -23,6 +23,7 @@ import co.elastic.apm.bci.HelperClassManager; import co.elastic.apm.bci.VisibleForAdvice; import co.elastic.apm.impl.ElasticApmTracer; +import co.elastic.apm.impl.transaction.Transaction; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -37,6 +38,7 @@ import java.util.Arrays; import java.util.Collection; +import static co.elastic.apm.servlet.ServletApiAdvice.TRANSACTION_ATTRIBUTE; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -54,7 +56,7 @@ * See https://github.com/raphw/byte-buddy/issues/465 for more information. * However, the helper class {@link StartAsyncAdviceHelper} has access to the Servlet API, * as it is loaded by the child classloader of {@link AsyncContext} - * (see {@link StartAsyncAdvice#onExitStartAsync(AsyncContext)}). + * (see {@link StartAsyncAdvice#onExitStartAsync(HttpServletRequest, AsyncContext)}). */ public class AsyncInstrumentation extends ElasticApmInstrumentation { @@ -122,9 +124,15 @@ public interface StartAsyncAdviceHelper { public static class StartAsyncAdvice { @Advice.OnMethodExit - private static void onExitStartAsync(@Advice.Return AsyncContext asyncContext) { - if (asyncHelper != null) { - asyncHelper.getForClassLoaderOfClass(AsyncContext.class).onExitStartAsync(asyncContext); + private static void onExitStartAsync(@Advice.This HttpServletRequest request, @Advice.Return AsyncContext asyncContext) { + if (tracer != null) { + final Transaction transaction = tracer.currentTransaction(); + if (transaction != null) { + request.setAttribute(TRANSACTION_ATTRIBUTE, transaction); + if (asyncHelper != null) { + asyncHelper.getForClassLoaderOfClass(AsyncContext.class).onExitStartAsync(asyncContext); + } + } } } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/FilterChainInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/FilterChainInstrumentation.java index 84756bacf6..a13e7d646b 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/FilterChainInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/FilterChainInstrumentation.java @@ -20,7 +20,6 @@ package co.elastic.apm.servlet; import co.elastic.apm.bci.ElasticApmInstrumentation; -import co.elastic.apm.bci.VisibleForAdvice; import co.elastic.apm.impl.ElasticApmTracer; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -43,9 +42,6 @@ */ public class FilterChainInstrumentation extends ElasticApmInstrumentation { - @VisibleForAdvice - public static final String EXCLUDE_REQUEST = "elastic.apm.servlet.request.exclude"; - @Override public void init(ElasticApmTracer tracer) { ServletApiAdvice.init(tracer); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletApiAdvice.java index 29a64e8a17..bf4a9d824e 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletApiAdvice.java @@ -55,6 +55,13 @@ public class ServletApiAdvice { @Nullable @VisibleForAdvice public static ElasticApmTracer tracer; + @VisibleForAdvice + public static ThreadLocal excluded = new ThreadLocal() { + @Override + protected Boolean initialValue() { + return Boolean.FALSE; + } + }; static void init(ElasticApmTracer tracer) { ServletApiAdvice.tracer = tracer; @@ -77,7 +84,7 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv if (servletTransactionHelper != null && servletRequest instanceof HttpServletRequest && servletRequest.getDispatcherType() == DispatcherType.REQUEST && - !Boolean.TRUE.equals(servletRequest.getAttribute(FilterChainInstrumentation.EXCLUDE_REQUEST))) { + !Boolean.TRUE.equals(excluded.get())) { final HttpServletRequest request = (HttpServletRequest) servletRequest; transaction = servletTransactionHelper.onBefore( @@ -86,10 +93,8 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv request.getHeader(TraceContext.TRACE_PARENT_HEADER)); if (transaction == null) { // if the request is excluded, avoid matching all exclude patterns again on each filter invocation - request.setAttribute(FilterChainInstrumentation.EXCLUDE_REQUEST, Boolean.TRUE); + excluded.set(Boolean.TRUE); return; - } else { - request.setAttribute(TRANSACTION_ATTRIBUTE, transaction); } final Request req = transaction.getContext().getRequest(); if (transaction.isSampled() && request.getCookies() != null) { @@ -121,6 +126,7 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl if (tracer == null) { return; } + excluded.set(Boolean.FALSE); if (scope != null) { scope.close(); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletTransactionHelper.java index 00ad00bb44..bf7db4f9f2 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/servlet/ServletTransactionHelper.java @@ -39,6 +39,7 @@ import javax.annotation.Nullable; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -185,15 +186,17 @@ private void fillRequestParameters(Transaction transaction, String method, Map ignoreUrls = webConfiguration.getIgnoreUrls(); + boolean excludeUrl = WildcardMatcher.anyMatch(ignoreUrls, servletPath, pathInfo) != null; if (excludeUrl) { logger.debug("Not tracing this request as the URL {} is ignored by one of the matchers", - requestURI, webConfiguration.getIgnoreUrls()); + requestURI, ignoreUrls); } - boolean excludeAgent = userAgentHeader != null && WildcardMatcher.anyMatch(webConfiguration.getIgnoreUserAgents(), userAgentHeader) != null; + final List ignoreUserAgents = webConfiguration.getIgnoreUserAgents(); + boolean excludeAgent = userAgentHeader != null && WildcardMatcher.anyMatch(ignoreUserAgents, userAgentHeader) != null; if (excludeAgent) { logger.debug("Not tracing this request as the User-Agent {} is ignored by one of the matchers", - userAgentHeader, webConfiguration.getIgnoreUserAgents()); + userAgentHeader, ignoreUserAgents); } return excludeUrl || excludeAgent; } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/servlet/ApmFilterTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/servlet/ApmFilterTest.java index 226d925383..dfb5bd8793 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/servlet/ApmFilterTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/servlet/ApmFilterTest.java @@ -49,6 +49,8 @@ import static org.assertj.core.api.Java6Assertions.assertThat; import static org.assertj.core.api.Java6Assertions.assertThatThrownBy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class ApmFilterTest extends AbstractInstrumentationTest { @@ -145,11 +147,14 @@ void testIgnoreUrlStartWithNoMatch() throws IOException, ServletException { @Test void testIgnoreUrlEndWith() throws IOException, ServletException { + filterChain = new MockFilterChain(new HttpServlet() { + }); when(webConfiguration.getIgnoreUrls()).thenReturn(Collections.singletonList(WildcardMatcher.valueOf("*.js"))); final MockHttpServletRequest request = new MockHttpServletRequest(); request.setServletPath("/resources"); request.setPathInfo("test.js"); filterChain.doFilter(request, new MockHttpServletResponse()); + verify(webConfiguration, times(1)).getIgnoreUrls(); assertThat(reporter.getTransactions()).hasSize(0); }