From d14f33ec6c5bd82107e471ec2f29949624b09a99 Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Thu, 3 Feb 2022 10:27:04 +0100 Subject: [PATCH 1/3] Added Instrumentation for `ServletContextListener#contextInitialized` --- .../InitServiceNameInstrumentation.java | 37 ++++++++++--------- .../apm/agent/servlet/ServletApiAdvice.java | 8 ++-- .../adapter/JakartaServletApiAdapter.java | 8 +++- .../adapter/JavaxServletApiAdapter.java | 8 +++- .../agent/servlet/adapter/ServletAdapter.java | 4 +- .../servlet/adapter/ServletApiAdapter.java | 4 +- .../InitServiceNameInstrumentationTest.java | 26 +++++++++++++ 7 files changed, 69 insertions(+), 26 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java index 4d9ef40f39..b41a2ca386 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java @@ -54,19 +54,18 @@ public abstract class InitServiceNameInstrumentation extends AbstractServletInst @Override public ElementMatcher getTypeMatcherPreFilter() { - return nameContains("Filter").or(nameContains("Servlet")); + return nameContains("Filter").or(nameContains("Servlet")).or(nameContains("Listener")); } @Override public ElementMatcher getTypeMatcher() { - return not(isInterface()).and(hasSuperType(namedOneOf("javax.servlet.Filter", "javax.servlet.Servlet", "jakarta.servlet.Filter", "jakarta.servlet.Servlet"))); + return not(isInterface()).and(hasSuperType(namedOneOf("javax.servlet.ServletContextListener", "javax.servlet.Filter", "jakarta.servlet.ServletContextListener", "javax.servlet.Servlet", "jakarta.servlet.Filter", "jakarta.servlet.Servlet"))); } @Override public ElementMatcher getMethodMatcher() { - return named("init") - .and(takesArguments(1)) - .and(takesArgument(0, nameEndsWith("Config"))); + return named("init").and(takesArguments(1).and(takesArgument(0, nameEndsWith("Config")))) + .or(named("contextInitialized").and(takesArguments(1).and(takesArgument(0, nameEndsWith("ServletContextEvent"))))); } public static class JavaxInitServiceNameInstrumentation extends InitServiceNameInstrumentation { @@ -80,15 +79,17 @@ public String rootClassNameThatClassloaderCanLoad() { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) - public static void onEnter(@Advice.Argument(0) @Nullable Object config) { - if (config == null) { + public static void onEnter(@Advice.Argument(0) @Nullable Object arg) { + if (arg == null) { return; } javax.servlet.ServletContext servletContext; - if (config instanceof javax.servlet.FilterConfig) { - servletContext = adapter.getServletContextFromFilterConfig((javax.servlet.FilterConfig) config); - } else if (config instanceof javax.servlet.ServletConfig) { - servletContext = adapter.getServletContextFromServletConfig((javax.servlet.ServletConfig) config); + if (arg instanceof javax.servlet.FilterConfig) { + servletContext = adapter.getServletContextFromFilterConfig((javax.servlet.FilterConfig) arg); + } else if (arg instanceof javax.servlet.ServletConfig) { + servletContext = adapter.getServletContextFromServletConfig((javax.servlet.ServletConfig) arg); + } else if (arg instanceof javax.servlet.ServletContextEvent) { + servletContext = adapter.getServletContextFromServletContextEvent((javax.servlet.ServletContextEvent) arg); } else { return; } @@ -109,15 +110,17 @@ public String rootClassNameThatClassloaderCanLoad() { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) - public static void onEnter(@Advice.Argument(0) @Nullable Object config) { - if (config == null) { + public static void onEnter(@Advice.Argument(0) @Nullable Object arg) { + if (arg == null) { return; } jakarta.servlet.ServletContext servletContext; - if (config instanceof jakarta.servlet.FilterConfig) { - servletContext = adapter.getServletContextFromFilterConfig((jakarta.servlet.FilterConfig) config); - } else if (config instanceof jakarta.servlet.ServletConfig) { - servletContext = adapter.getServletContextFromServletConfig((jakarta.servlet.ServletConfig) config); + if (arg instanceof jakarta.servlet.FilterConfig) { + servletContext = adapter.getServletContextFromFilterConfig((jakarta.servlet.FilterConfig) arg); + } else if (arg instanceof jakarta.servlet.ServletConfig) { + servletContext = adapter.getServletContextFromServletConfig((jakarta.servlet.ServletConfig) arg); + } else if (arg instanceof jakarta.servlet.ServletContextEvent) { + servletContext = adapter.getServletContextFromServletContextEvent((jakarta.servlet.ServletContextEvent) arg); } else { return; } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java index 7af1b387be..74d76223a3 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletApiAdvice.java @@ -58,8 +58,8 @@ public abstract class ServletApiAdvice { private static final List requestExceptionAttributes = Arrays.asList("javax.servlet.error.exception", "jakarta.servlet.error.exception", "exception", "org.springframework.web.servlet.DispatcherServlet.EXCEPTION", "co.elastic.apm.exception"); @Nullable - public static Object onServletEnter( - ServletApiAdapter adapter, + public static Object onServletEnter( + ServletApiAdapter adapter, Object servletRequest) { ElasticApmTracer tracer = GlobalTracer.getTracerImpl(); @@ -159,8 +159,8 @@ public static void onExitServlet( - ServletApiAdapter adapter, + public static void onExitServlet( + ServletApiAdapter adapter, Object servletRequest, Object servletResponse, @Nullable Object transactionOrScopeOrSpan, diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java index 976b5409ef..c2e8369675 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JakartaServletApiAdapter.java @@ -26,6 +26,7 @@ import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletConfig; import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletContextEvent; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; @@ -38,7 +39,7 @@ import java.util.Enumeration; import java.util.Map; -public class JakartaServletApiAdapter implements ServletApiAdapter { +public class JakartaServletApiAdapter implements ServletApiAdapter { public static final JakartaServletApiAdapter INSTANCE = new JakartaServletApiAdapter(); @@ -214,6 +215,11 @@ public boolean isInstanceOfHttpServlet(Object object) { return object instanceof HttpServlet; } + @Override + public ServletContext getServletContextFromServletContextEvent(ServletContextEvent servletContextEvent) { + return servletContextEvent.getServletContext(); + } + @Override public ServletContext getServletContextFromServletConfig(ServletConfig filterConfig) { return filterConfig.getServletContext(); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java index 9dbb19faaa..877e267683 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/JavaxServletApiAdapter.java @@ -28,6 +28,7 @@ import javax.servlet.RequestDispatcher; import javax.servlet.ServletConfig; import javax.servlet.ServletContext; +import javax.servlet.ServletContextEvent; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -38,7 +39,7 @@ import java.util.Enumeration; import java.util.Map; -public class JavaxServletApiAdapter implements ServletApiAdapter { +public class JavaxServletApiAdapter implements ServletApiAdapter { private static final JavaxServletApiAdapter INSTANCE = new JavaxServletApiAdapter(); @@ -213,6 +214,11 @@ public boolean isInstanceOfHttpServlet(Object object) { return object instanceof HttpServlet; } + @Override + public ServletContext getServletContextFromServletContextEvent(ServletContextEvent servletContextEvent) { + return servletContextEvent.getServletContext(); + } + @Override public ServletContext getServletContextFromServletConfig(ServletConfig filterConfig) { return filterConfig.getServletContext(); diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java index 0f5a2ae074..9e1e39148d 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletAdapter.java @@ -21,10 +21,12 @@ import co.elastic.apm.agent.sdk.state.GlobalState; @GlobalState -public interface ServletAdapter { +public interface ServletAdapter { boolean isInstanceOfHttpServlet(Object object); + ServletContext getServletContextFromServletContextEvent(ServletContextEvent servletContextEvent); + ServletContext getServletContextFromServletConfig(ServletConfig filterConfig); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java index e8bdbe6d6f..5f085536a5 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/adapter/ServletApiAdapter.java @@ -21,10 +21,10 @@ import co.elastic.apm.agent.sdk.state.GlobalState; @GlobalState -public interface ServletApiAdapter extends +public interface ServletApiAdapter extends ServletRequestResponseAdapter, ServletContextAdapter, - ServletAdapter, + ServletAdapter, FilterAdapter { } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java index e143bf806e..d70add640d 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentationTest.java @@ -23,11 +23,14 @@ import org.junit.jupiter.api.Test; import org.springframework.mock.web.MockFilterConfig; import org.springframework.mock.web.MockServletConfig; +import org.springframework.mock.web.MockServletContext; import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.Servlet; +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -38,6 +41,19 @@ class InitServiceNameInstrumentationTest extends AbstractInstrumentationTest { + @Test + void testContextInitialized() { + ServletContextListener servletContextListener = new NoopServletContextListener(); + + CustomManifestLoader cl = new CustomManifestLoader(() -> getClass().getResourceAsStream("/TEST-MANIFEST.MF")); + CustomManifestLoader.withThreadContextClassLoader(cl, () -> { + servletContextListener.contextInitialized(new ServletContextEvent(new MockServletContext())); + tracer.startRootTransaction(cl).end(); + }); + + assertServiceInfo(); + } + @Test void testServletInit() { Servlet servlet = new HttpServlet() { @@ -71,6 +87,16 @@ private void assertServiceInfo() { assertThat(traceContext.getServiceVersion()).isEqualTo("1.42.0"); } + private static class NoopServletContextListener implements ServletContextListener { + @Override + public void contextInitialized(ServletContextEvent servletContextEvent) { + } + + @Override + public void contextDestroyed(ServletContextEvent servletContextEvent) { + } + } + private static class NoopFilter implements Filter { @Override public void init(FilterConfig filterConfig) { From 2f8cd6f42f616420919b6660923afceab3b529e5 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 3 Feb 2022 13:56:59 +0100 Subject: [PATCH 2/3] Minor formatting and cleanup --- .../InitServiceNameInstrumentation.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java index b41a2ca386..a0bcf04828 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java @@ -59,13 +59,19 @@ public ElementMatcher getTypeMatcherPreFilter() { @Override public ElementMatcher getTypeMatcher() { - return not(isInterface()).and(hasSuperType(namedOneOf("javax.servlet.ServletContextListener", "javax.servlet.Filter", "jakarta.servlet.ServletContextListener", "javax.servlet.Servlet", "jakarta.servlet.Filter", "jakarta.servlet.Servlet"))); + return not(isInterface()).and(hasSuperType(namedOneOf( + "javax.servlet.ServletContextListener", "javax.servlet.Filter", "javax.servlet.Servlet", + "jakarta.servlet.ServletContextListener", "jakarta.servlet.Filter", "jakarta.servlet.Servlet"))); } @Override public ElementMatcher getMethodMatcher() { - return named("init").and(takesArguments(1).and(takesArgument(0, nameEndsWith("Config")))) - .or(named("contextInitialized").and(takesArguments(1).and(takesArgument(0, nameEndsWith("ServletContextEvent"))))); + return named("init") + .and(takesArguments(1)) + .and(takesArgument(0, nameEndsWith("Config"))) + .or(named("contextInitialized") + .and(takesArguments(1)) + .and(takesArgument(0, nameEndsWith("ServletContextEvent")))); } public static class JavaxInitServiceNameInstrumentation extends InitServiceNameInstrumentation { @@ -80,9 +86,6 @@ public String rootClassNameThatClassloaderCanLoad() { public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void onEnter(@Advice.Argument(0) @Nullable Object arg) { - if (arg == null) { - return; - } javax.servlet.ServletContext servletContext; if (arg instanceof javax.servlet.FilterConfig) { servletContext = adapter.getServletContextFromFilterConfig((javax.servlet.FilterConfig) arg); @@ -111,9 +114,6 @@ public static class AdviceClass { @Advice.OnMethodEnter(suppress = Throwable.class, inline = false) public static void onEnter(@Advice.Argument(0) @Nullable Object arg) { - if (arg == null) { - return; - } jakarta.servlet.ServletContext servletContext; if (arg instanceof jakarta.servlet.FilterConfig) { servletContext = adapter.getServletContextFromFilterConfig((jakarta.servlet.FilterConfig) arg); From fc5fbe4bec229572dc518ad985b70c339adc90a7 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 3 Feb 2022 13:58:17 +0100 Subject: [PATCH 3/3] Amend javadoc --- .../apm/agent/servlet/InitServiceNameInstrumentation.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java index a0bcf04828..7238e21be0 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/InitServiceNameInstrumentation.java @@ -45,6 +45,8 @@ *
  • {@link jakarta.servlet.Filter#init(jakarta.servlet.FilterConfig)}
    • *
  • {@link javax.servlet.Servlet#init(javax.servlet.ServletConfig)}
    • *
  • {@link jakarta.servlet.Servlet#init(jakarta.servlet.ServletConfig)}
    • + *
  • {@link javax.servlet.ServletContextListener#contextInitialized(javax.servlet.ServletContextEvent)}
    • + *
  • {@link jakarta.servlet.ServletContextListener#contextInitialized(jakarta.servlet.ServletContextEvent)}
    • * * * Determines the service name based on the webapp's {@code META-INF/MANIFEST.MF} file early in the startup process.