diff --git a/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletRequest.java b/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletRequest.java index d471cd311f..90a7be2447 100644 --- a/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletRequest.java +++ b/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletRequest.java @@ -11,9 +11,9 @@ * 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 diff --git a/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletResponse.java b/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletResponse.java index 9ebbdc565e..973f58ab58 100644 --- a/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletResponse.java +++ b/apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/MockHttpServletResponse.java @@ -11,9 +11,9 @@ * 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 diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java index 1d40a9c57e..b90c63d576 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java @@ -389,6 +389,7 @@ public static synchronized void reset() { transformer.reset(instrumentation, RedefinitionStrategy.RETRANSFORMATION); } dynamicClassFileTransformers.clear(); + HelperClassManager.ForDispatcher.clear(); instrumentation = null; } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmInstrumentation.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmInstrumentation.java index c32f812da0..025d600c07 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmInstrumentation.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmInstrumentation.java @@ -11,9 +11,9 @@ * 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 @@ -37,6 +37,8 @@ import javax.annotation.Nullable; import java.security.ProtectionDomain; import java.util.Collection; +import java.util.Collections; +import java.util.List; import static net.bytebuddy.matcher.ElementMatchers.any; @@ -169,6 +171,14 @@ public Advice.OffsetMapping.Factory getOffsetMapping() { return null; } - public void onTypeMatch(TypeDescription typeDescription, ClassLoader classLoader, ProtectionDomain protectionDomain, @Nullable Class classBeingRedefined) { + public List helpers() throws Exception { + return Collections.emptyList(); + } + + public void onTypeMatch(TypeDescription typeDescription, ClassLoader classLoader, ProtectionDomain protectionDomain, @Nullable Class classBeingRedefined) throws Exception { + List helpers = helpers(); + if (!helpers.isEmpty()) { + HelperClassManager.ForDispatcher.inject(classLoader, protectionDomain, helpers); + } } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/HelperClassManager.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/HelperClassManager.java index d06a6ac705..1fd8981b3e 100644 --- a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/HelperClassManager.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/HelperClassManager.java @@ -24,27 +24,46 @@ */ package co.elastic.apm.agent.bci; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.ElasticApmTracer; import com.blogspot.mydailyjava.weaklockfree.WeakConcurrentMap; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.method.MethodList; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.ClassFileLocator; import net.bytebuddy.dynamic.loading.ByteArrayClassLoader; import net.bytebuddy.dynamic.loading.ClassInjector; +import net.bytebuddy.dynamic.loading.MultipleParentClassLoader; import net.bytebuddy.dynamic.loading.PackageDefinitionStrategy; +import net.bytebuddy.pool.TypePool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.annotation.Nullable; import java.io.IOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; import java.lang.ref.WeakReference; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.WeakHashMap; +import java.util.concurrent.ConcurrentMap; + +import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; /** * This class helps to overcome the fact that the agent classes can't access the classes they want to instrument. @@ -142,7 +161,9 @@ public T getForClassLoaderOfClass(Class classOfTargetClassLoader) { *

* * @param the type of the helper class interface + * @deprecated In favor of {@link ForDispatcher} */ + @Deprecated public static class ForSingleClassLoader extends HelperClassManager { @Nullable @@ -199,7 +220,9 @@ public T doGetForClassLoaderOfClass(Class classOfTargetClassLoader) { * startup time and when new applications are deployed). These maps shouldn't grow big as they have an entry per class loader. * * @param + * @deprecated In favor of {@link ForDispatcher} */ + @Deprecated public static class ForAnyClassLoader extends HelperClassManager { // doesn't need to be concurrent - invoked only from a synchronized context @@ -271,7 +294,154 @@ private synchronized T loadAndReferenceHelper(Class classOfTargetClassLoader) } } - static Class injectClass(@Nullable ClassLoader targetClassLoader, @Nullable ProtectionDomain pd, String className, boolean isBootstrapClass) throws IOException, ClassNotFoundException { + public static class ForDispatcher { + + private static final Map>> alreadyInjected = new WeakHashMap>>(); + + /** + * Creates an isolated CL that has two parents: the target class loader and the agent CL. + * The agent class loader is currently the bootstrap CL but in the future it will be an isolated CL that is a child of the bootstrap CL. + *

+ * After the helper CL is created, it registers {@link RegisterMethodHandle}-annotated methods in {@link MethodHandleDispatcher}. + * These method handles are then called from within advices ({@link net.bytebuddy.asm.Advice.OnMethodEnter}/{@link net.bytebuddy.asm.Advice.OnMethodExit}). + * This lets them call the helpers that are loaded from the helper CL. + * The helpers have full access to both the agent API and the types visible to the target CL (such as the servlet API). + *

+ *

+ * See {@link MethodHandleDispatcher} for a diagram. + *

+ */ + public synchronized static void inject(@Nullable ClassLoader targetClassLoader, @Nullable ProtectionDomain protectionDomain, List classesToInject) throws Exception { + classesToInject = new ArrayList<>(classesToInject); + classesToInject.add(MethodHandleRegisterer.class.getName()); + + Set> injectedClasses = getOrCreateInjectedClasses(targetClassLoader); + if (injectedClasses.contains(classesToInject)) { + return; + } + injectedClasses.add(classesToInject); + logger.debug("Creating helper class loader for {} containing {}", targetClassLoader, classesToInject); + + ClassLoader parent = getHelperClassLoaderParent(targetClassLoader); + Map typeDefinitions = getTypeDefinitions(classesToInject); + // child first semantics are important here as the helper CL contains classes that are also present in the agent CL + ClassLoader helperCL = new ByteArrayClassLoader.ChildFirst(parent, true, typeDefinitions); + + registerMethodHandles(classesToInject, helperCL, targetClassLoader, protectionDomain); + } + + private static Set> getOrCreateInjectedClasses(@Nullable ClassLoader targetClassLoader) { + Set> injectedClasses = alreadyInjected.get(targetClassLoader); + if (injectedClasses == null) { + injectedClasses = new HashSet<>(); + alreadyInjected.put(targetClassLoader, injectedClasses); + } + return injectedClasses; + } + + private static ConcurrentMap ensureDispatcherForClassLoaderCreated(@Nullable ClassLoader targetClassLoader, @Nullable ProtectionDomain protectionDomain) throws IOException, ClassNotFoundException, IllegalAccessException, NoSuchFieldException { + ConcurrentMap dispatcherForClassLoader = MethodHandleDispatcher.getDispatcherForClassLoader(targetClassLoader); + ConcurrentMap reflectionDispatcherForClassLoader = MethodHandleDispatcher.getReflectionDispatcherForClassLoader(targetClassLoader); + if (dispatcherForClassLoader == null) { + // there's always a dispatcher for the bootstrap CL + assert targetClassLoader != null; + Class dispatcher = injectClass(targetClassLoader, protectionDomain, MethodHandleDispatcherHolder.class.getName(), true); + dispatcherForClassLoader = (ConcurrentMap) dispatcher.getField("registry").get(null); + reflectionDispatcherForClassLoader = (ConcurrentMap) dispatcher.getField("reflectionRegistry").get(null); + MethodHandleDispatcher.setDispatcherForClassLoader(targetClassLoader, dispatcherForClassLoader, reflectionDispatcherForClassLoader); + } + return dispatcherForClassLoader; + } + + @Nullable + private static ClassLoader getHelperClassLoaderParent(@Nullable ClassLoader targetClassLoader) { + ClassLoader agentClassLoader = HelperClassManager.class.getClassLoader(); + if (agentClassLoader != null && agentClassLoader != ClassLoader.getSystemClassLoader()) { + // future world: when the agent is loaded from an isolated class loader + // the helper class loader has both, the agent class loader and the target class loader as the parent + return new MultipleParentClassLoader(Arrays.asList(agentClassLoader, targetClassLoader)); + } + return targetClassLoader; + } + + /** + * Gets all {@link RegisterMethodHandle} annotated methods and registers them in {@link MethodHandleDispatcher} + */ + private static void registerMethodHandles(List classesToInject, ClassLoader helperCL, @Nullable ClassLoader targetClassLoader, @Nullable ProtectionDomain protectionDomain) throws Exception { + ensureDispatcherForClassLoaderCreated(targetClassLoader, protectionDomain); + ConcurrentMap dispatcherForClassLoader = MethodHandleDispatcher.getDispatcherForClassLoader(targetClassLoader); + ConcurrentMap reflectionDispatcherForClassLoader = MethodHandleDispatcher.getReflectionDispatcherForClassLoader(targetClassLoader); + + Class methodHandleRegistererClass = (Class) Class.forName(MethodHandleRegisterer.class.getName(), true, helperCL); + TypePool typePool = TypePool.Default.of(getAgentClassFileLocator()); + for (String name : classesToInject) { + // check with Byte Buddy matchers, acting on the byte code as opposed to reflection first + // to avoid NoClassDefFoundErrors when the classes refer to optional types + MethodList helperMethods = typePool.describe(name) + .resolve() + .getDeclaredMethods() + .filter(isAnnotatedWith(named(RegisterMethodHandle.class.getName()))); + if (!helperMethods.isEmpty()) { + assertPublic(helperMethods); + + Class clazz = Class.forName(name, true, helperCL); + if (clazz.getClassLoader() != helperCL) { + throw new IllegalStateException("Helper classes not loaded from helper class loader, instead loaded from " + clazz.getClassLoader()); + } + try { + // call in from the context of the created helper class loader so that helperClass.getMethods() doesn't throw NoClassDefFoundError + Method registerStaticMethodHandles = methodHandleRegistererClass.getMethod("registerStaticMethodHandles", ConcurrentMap.class, ConcurrentMap.class, Class.class); + registerStaticMethodHandles.invoke(null, dispatcherForClassLoader, reflectionDispatcherForClassLoader, clazz); + } catch (Exception e) { + logger.error("Exception while trying to register method handles for {}", name); + if (e instanceof InvocationTargetException) { + Throwable targetException = ((InvocationTargetException) e).getTargetException(); + if (targetException instanceof Exception) { + throw (Exception) targetException; + } + } + throw e; + } + } + } + } + + private static void assertPublic(MethodList methods) throws IllegalAccessException { + for (MethodDescription.InDefinedShape privateHelperMethod : methods.filter(not(isPublic()))) { + throw new IllegalAccessException("Helper method is not public: " + privateHelperMethod); + } + } + + /** + * This class is loaded form the helper class loader so that {@link Class#getDeclaredMethods()} doesn't throw a + * {@link NoClassDefFoundError}. + * This would otherwise happen as the methods may reference types not visible to the agent class loader (such as the servlet API). + */ + public static class MethodHandleRegisterer { + + public static void registerStaticMethodHandles(ConcurrentMap dispatcher, ConcurrentMap reflectionDispatcher, Class helperClass) throws ReflectiveOperationException { + for (Method method : helperClass.getMethods()) { + if (Modifier.isStatic(method.getModifiers()) && method.getAnnotation(RegisterMethodHandle.class) != null) { + MethodHandle methodHandle = MethodHandles.lookup().unreflect(method); + String key = helperClass.getName() + "#" + method.getName(); + // intern() to speed up map lookups (short-circuits String::equals via reference equality check) + MethodHandle previousValue = dispatcher.put(key.intern(), methodHandle); + reflectionDispatcher.put(key.intern(), method); + if (previousValue != null) { + throw new IllegalArgumentException("There is already a mapping for '" + key + "'"); + } + } + } + } + } + + public synchronized static void clear() { + alreadyInjected.clear(); + MethodHandleDispatcher.clear(); + } + } + + static Class injectClass(@Nullable ClassLoader targetClassLoader, @Nullable ProtectionDomain pd, String className, boolean isBootstrapClass) throws IOException, ClassNotFoundException { if (targetClassLoader == null) { if (isBootstrapClass) { return Class.forName(className, false, null); @@ -335,15 +505,23 @@ private static Class loadHelperClass(@Nullable ClassLoader targetClassLoa private static Map getTypeDefinitions(List helperClassNames) throws IOException { Map typeDefinitions = new HashMap<>(); + ClassFileLocator agentClassFileLocator = getAgentClassFileLocator(); for (final String helperName : helperClassNames) { - final byte[] classBytes = getAgentClassBytes(helperName); + final byte[] classBytes = agentClassFileLocator.locate(helperName).resolve(); typeDefinitions.put(helperName, classBytes); } return typeDefinitions; } private static byte[] getAgentClassBytes(String className) throws IOException { - final ClassFileLocator locator = ClassFileLocator.ForClassLoader.of(ClassLoader.getSystemClassLoader()); - return locator.locate(className).resolve(); + return getAgentClassFileLocator().locate(className).resolve(); + } + + private static ClassFileLocator getAgentClassFileLocator() { + ClassLoader agentClassLoader = HelperClassManager.class.getClassLoader(); + if (agentClassLoader == null) { + agentClassLoader = ClassLoader.getSystemClassLoader(); + } + return ClassFileLocator.ForClassLoader.of(agentClassLoader); } } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/MethodHandleDispatcherHolder.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/MethodHandleDispatcherHolder.java new file mode 100644 index 0000000000..405c438d96 --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/MethodHandleDispatcherHolder.java @@ -0,0 +1,48 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.bci; + +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; + +import java.lang.invoke.MethodHandle; +import java.lang.reflect.Method; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * A class that holds a static reference to method handler dispatcher for a specific class loader. + * See also {@link MethodHandleDispatcher#dispatcherByClassLoader} + * Used to be loaded by the parent of the class loader that loads the helper + * itself, thus making the helper instance non-GC-eligible as long as the parent class loader is alive. + * NOTE: THIS CLASS SHOULD NEVER BE INSTANTIATED NOR REFERENCED EXPLICITLY, IT SHOULD ONLY BE USED THROUGH REFLECTION + */ +public class MethodHandleDispatcherHolder { + public static final ConcurrentMap registry = new ConcurrentHashMap(); + public static final ConcurrentMap reflectionRegistry = new ConcurrentHashMap(); + + // should never be instanciated + private MethodHandleDispatcherHolder() { + } +} diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/RegisterMethodHandle.java similarity index 60% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java rename to apm-agent-core/src/main/java/co/elastic/apm/agent/bci/RegisterMethodHandle.java index 843a6fb126..cf7c0dbc53 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/InputStreamFactoryHelperImpl.java +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bci/RegisterMethodHandle.java @@ -22,16 +22,18 @@ * under the License. * #L% */ -package co.elastic.apm.agent.servlet.helper; +package co.elastic.apm.agent.bci; -import co.elastic.apm.agent.impl.context.Request; -import co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; -import javax.servlet.ServletInputStream; - -public class InputStreamFactoryHelperImpl implements RequestStreamRecordingInstrumentation.InputStreamWrapperFactory { - @Override - public ServletInputStream wrap(Request request, ServletInputStream servletInputStream) { - return new RecordingServletInputStreamWrapper(request, servletInputStream); - } +/** + * Static methods annotated with this will be registered in {@link co.elastic.apm.agent.bootstrap.MethodHandleDispatcher}. + * This enables advices to call them by looking them up via {@link co.elastic.apm.agent.bootstrap.MethodHandleDispatcher#getMethodHandle(Class, String)} + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface RegisterMethodHandle { } diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/bootstrap/MethodHandleDispatcher.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/bootstrap/MethodHandleDispatcher.java new file mode 100644 index 0000000000..9608b17cbb --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/bootstrap/MethodHandleDispatcher.java @@ -0,0 +1,190 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.bootstrap; + +import co.elastic.apm.agent.annotation.NonnullApi; +import co.elastic.apm.agent.bci.MethodHandleDispatcherHolder; +import com.blogspot.mydailyjava.weaklockfree.WeakConcurrentMap; + +import javax.annotation.Nullable; +import java.lang.invoke.MethodHandle; +import java.lang.ref.WeakReference; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * This class lives in the bootstrap classloader so it's visible to all classes. + * It is used to register method handles in order to call methods of classes that live in another classloader hierarchy. + * This is the only class that needs + */ +@NonnullApi +public class MethodHandleDispatcher { + + /** + * Java 7 has poor support for method handles. + * There are bugs in Java 7 that lead to segfaults after a method handle has been compiled by C2. + * See also https://github.com/elastic/apm-agent-java/issues/458 + */ + public static final boolean USE_REFLECTION = System.getProperty("java.version").startsWith("1.7."); + /** + * If the value was not weakly referenced, + * this would keep the {@link MethodHandle}s alive, + * which would keep the corresponding helper class alive, + * which would keep the helper classloader alive, + * which would keep the application class loader alive. + * This would lead to a class loader leak if the web application is undeployed (all classes of the web app can't be unloaded). + * 
+     *   Bootstrap CL ←─────────────────────────────────── Agent CL
+     *       ↑ └{@link MethodHandleDispatcher}              ↑
+     *       │   └ WeakConcurrentMap>>
+     *     Ext/Platform CL           ╷                      │                          ╷
+     *       ↑                       ╷                      │                          ╷
+     *     System CL                 ╷                      │                          ╷
+     *       ↑                       ╷                      │                          ╷
+     *     Common                    ╷                      │                          ╷
+     *     ↑    ↑                    ╷                      │                          ╷
+     * WebApp1  WebApp2 ←╶╶╶╶╶╶╶╶╶╶╶╶┘                      │                          ╷
+     *          ↑ - {@link MethodHandleDispatcherHolder}    │                          ╷
+     *          │   - ConcurrentMap   │                          ╷
+     *          │          ┌────────────────────┘           │                          ╷
+     *          Helper CL ─┼────────────────────────────────┘                          ╷
+     *           │         ↓                                                           ╷
+     *           └ HelperClass ←╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶╶┘
+     * Legend:
+     *  ╶╶ weak reference
+     *  ── strong reference
+     *
+ */ + private static WeakConcurrentMap>> dispatcherByClassLoader = new WeakConcurrentMap.WithInlinedExpunction<>(); + private static WeakConcurrentMap>> reflectionDispatcherByClassLoader = new WeakConcurrentMap.WithInlinedExpunction<>(); + private static ConcurrentMap bootstrapDispatcher = new ConcurrentHashMap<>(); + private static ConcurrentMap reflectionBootstrapDispatcher = new ConcurrentHashMap<>(); + + /** + * Calls {@link ConcurrentMap#clear()} on all injected {@link MethodHandleDispatcherHolder}s. + *

+ * This should make all helper class loaders eligible for GC. + * That is because the {@link java.lang.invoke.MethodHandle}s registered in the {@link MethodHandleDispatcherHolder} should be the only + * references to the classes loaded by the helper class loader. + *

+ *

+ * As the helper class loaders are (ideally) the only references that keep the Agent CL alive, + * this also makes the Agent CL eligible for CL. + *

+ */ + public synchronized static void clear() { + for (WeakConcurrentMap>> map : Arrays.asList(dispatcherByClassLoader, reflectionDispatcherByClassLoader)) { + for (Map.Entry>> entry : map) { + ConcurrentMap dispatcher = entry.getValue().get(); + if (dispatcher != null) { + dispatcher.clear(); + } + } + map.clear(); + } + bootstrapDispatcher.clear(); + reflectionBootstrapDispatcher.clear(); + } + + /** + * Gets a {@link MethodHandle}, mostly referring to a static helper method called from an advice method. + *

+ * An advice method is a static method that is annotated with {@link net.bytebuddy.asm.Advice.OnMethodEnter} + * or {@link net.bytebuddy.asm.Advice.OnMethodExit}. + *

+ *

+ * Helper methods are classloader specific so that they are able to reference types specific to that class loader. + * An example would be a callback interface like {@code okhttp3.Callback}. + * When there are multiple web applications deployed to the same servlet container, they might all use different versions of OkHttp. + *

+ * + * @param classOfTargetClassLoader + * @param methodHandleName + * @return + */ + public static MethodHandle getMethodHandle(Class classOfTargetClassLoader, String methodHandleName) { + return getMethodHandle(classOfTargetClassLoader.getClassLoader(), methodHandleName); + } + + public static Method getMethod(Class classOfTargetClassLoader, String methodHandleName) { + return getMethod(classOfTargetClassLoader.getClassLoader(), methodHandleName); + } + + public static MethodHandle getMethodHandle(ClassLoader targetClassLoader, String methodHandleName) { + ConcurrentMap dispatcherForClassLoader = getDispatcherForClassLoader(targetClassLoader); + if (dispatcherForClassLoader != null) { + MethodHandle methodHandle = dispatcherForClassLoader.get(methodHandleName); + if (methodHandle != null) { + return methodHandle; + } + } + throw new IllegalArgumentException("No method handle found for " + methodHandleName); + } + + + public static Method getMethod(ClassLoader targetClassLoader, String methodHandleName) { + ConcurrentMap dispatcherForClassLoader = getReflectionDispatcherForClassLoader(targetClassLoader); + if (dispatcherForClassLoader != null) { + Method methodHandle = dispatcherForClassLoader.get(methodHandleName); + if (methodHandle != null) { + return methodHandle; + } + } + throw new IllegalArgumentException("No method found for " + methodHandleName); + } + + public synchronized static void setDispatcherForClassLoader(ClassLoader classLoader, ConcurrentMap dispatcherMap, ConcurrentMap reflectionDispatcherMap) { + dispatcherByClassLoader.put(classLoader, new WeakReference<>(dispatcherMap)); + reflectionDispatcherByClassLoader.put(classLoader, new WeakReference<>(reflectionDispatcherMap)); + } + + @Nullable + public static ConcurrentMap getDispatcherForClassLoader(@Nullable ClassLoader classLoader) { + if (classLoader == null) { + return bootstrapDispatcher; + } + WeakReference> reference = dispatcherByClassLoader.get(classLoader); + if (reference != null) { + return reference.get(); + } + return null; + } + + @Nullable + public static ConcurrentMap getReflectionDispatcherForClassLoader(@Nullable ClassLoader classLoader) { + if (classLoader == null) { + return reflectionBootstrapDispatcher; + } + WeakReference> reference = reflectionDispatcherByClassLoader.get(classLoader); + if (reference != null) { + return reference.get(); + } + return null; + } + +} diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/util/CallDepth.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/CallDepth.java new file mode 100644 index 0000000000..139b8f3e8d --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/CallDepth.java @@ -0,0 +1,72 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.util; + +import java.util.Map; +import java.util.WeakHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * A utility that makes it easy to detect nested method calls. + */ +public class CallDepth { + private static final ThreadLocal, AtomicInteger>> callDepthPerThread = new ThreadLocal, AtomicInteger>>(); + + /** + * Gets and increments the call depth counter. + * Returns {@code 0} if this is the outer-most (non-nested) invocation. + * + * @param clazz the class for which the call depth should be counted. + * Used as a key to distinguish multiple counters for a thread. + * @return the call depth before it has been incremented + */ + public static int increment(Class clazz) { + Map, AtomicInteger> callDepthForCurrentThread = callDepthPerThread.get(); + if (callDepthForCurrentThread == null) { + callDepthForCurrentThread = new WeakHashMap, AtomicInteger>(); + callDepthPerThread.set(callDepthForCurrentThread); + } + AtomicInteger depth = callDepthForCurrentThread.get(clazz); + if (depth == null) { + depth = new AtomicInteger(); + callDepthForCurrentThread.put(clazz, depth); + } + return depth.getAndIncrement(); + } + + /** + * Decrements and gets the call depth counter. + * Returns {@code 0} if this is the outer-most (non-nested) invocation. + * + * @param clazz the class for which the call depth should be counted. + * Used as a key to distinguish multiple counters for a thread. + * @return the call depth after it has been incremented + */ + public static int decrement(Class clazz) { + int depth = callDepthPerThread.get().get(clazz).decrementAndGet(); + assert depth >= 0; + return depth; + } +} diff --git a/apm-agent-core/src/main/java/co/elastic/apm/agent/util/PackageScanner.java b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/PackageScanner.java new file mode 100644 index 0000000000..2752c84155 --- /dev/null +++ b/apm-agent-core/src/main/java/co/elastic/apm/agent/util/PackageScanner.java @@ -0,0 +1,101 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.util; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.List; + +public class PackageScanner { + + /** + * Returns all class names within a package and sub-packages + * + * @param basePackage the package to scan + * @return all class names within a package and sub-packages + * @throws IOException + * @throws URISyntaxException + */ + public static List getClassNames(final String basePackage) throws IOException, URISyntaxException { + String baseFolderResource = basePackage.replace('.', '/'); + final List classNames = new ArrayList<>(); + Enumeration resources = getResourcesFromAgentClassLoader(baseFolderResource); + while (resources.hasMoreElements()) { + URL resource = resources.nextElement(); + URI uri = resource.toURI(); + if (uri.getScheme().equals("jar")) { + // avoids FileSystemAlreadyExistsException + synchronized (PackageScanner.class) { + try (FileSystem fileSystem = FileSystems.newFileSystem(uri, Collections.emptyMap())) { + final Path basePath = fileSystem.getPath(baseFolderResource).toAbsolutePath(); + classNames.addAll(listClassNames(basePackage, basePath)); + } + } + } else { + final Path basePath = Paths.get(uri); + if (basePath.toString().contains("test-classes")) { + continue; + } + classNames.addAll(listClassNames(basePackage, basePath)); + } + } + return classNames; + } + + private static List listClassNames(final String basePackage, final Path basePath) throws IOException { + final List classNames = new ArrayList<>(); + Files.walkFileTree(basePath, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { + if (file.toString().endsWith(".class")) { + classNames.add(basePackage + "." + basePath.relativize(file).toString().replace('/', '.').replace(".class", "")); + } + return FileVisitResult.CONTINUE; + } + }); + return classNames; + } + + private static Enumeration getResourcesFromAgentClassLoader(String baseFolderResource) throws IOException { + ClassLoader agentCL = PackageScanner.class.getClassLoader(); + if (agentCL == null) { + agentCL = ClassLoader.getSystemClassLoader(); + } + return agentCL.getResources(baseFolderResource); + } +} diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/HelperClassManagerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/HelperClassManagerTest.java index 876f6c72c1..4a58c75a78 100644 --- a/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/HelperClassManagerTest.java +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/bci/HelperClassManagerTest.java @@ -11,9 +11,9 @@ * 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 @@ -24,17 +24,23 @@ */ package co.elastic.apm.agent.bci; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.ElasticApmTracer; import org.assertj.core.api.ThrowableAssert; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import java.lang.invoke.WrongMethodTypeException; import java.lang.ref.WeakReference; import java.net.URL; import java.net.URLClassLoader; +import java.util.Collections; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.awaitility.Awaitility.await; import static org.mockito.Mockito.mock; class HelperClassManagerTest { @@ -47,6 +53,11 @@ class HelperClassManagerTest { } } + @AfterEach + void tearDown() { + HelperClassManager.ForDispatcher.clear(); + } + @Test void testLoadHelperSingleClassLoader() { new InnerTestClass().testLoadHelperSingleClassLoader(); @@ -124,6 +135,134 @@ void testCaching() throws ClassNotFoundException, InterruptedException { assertThat(HelperClassManager.ForAnyClassLoader.clId2helperImplListMap.get(targetClassLoader3)).isNotNull(); } + @Test + void testForDispatcher() throws Throwable { + injectForDispatcher(TestHelper.class); + assertThat(MethodHandleDispatcher + .getMethodHandle(HelperClassManagerTest.class, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getParentClassLoader") + .invoke()) + .isEqualTo(HelperClassManagerTest.class.getClassLoader()); + assertThat(MethodHandleDispatcher + .getMethod(HelperClassManagerTest.class, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getParentClassLoader") + .invoke(null)) + .isEqualTo(HelperClassManagerTest.class.getClassLoader()); + } + + @Test + void testHelperClassLoaderUnloading() throws Throwable { + injectForDispatcher(TestHelper.class); + WeakReference helperClassLoader = new WeakReference<>((ClassLoader) MethodHandleDispatcher + .getMethodHandle(HelperClassManagerTest.class, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getClassLoader") + .invoke()); + + MethodHandleDispatcher.clear(); + System.gc(); + + await().untilAsserted(() -> assertThat(helperClassLoader.get()).isNull()); + } + + public static class TestHelper { + @RegisterMethodHandle + public static ClassLoader getClassLoader() { + return TestHelper.class.getClassLoader(); + } + @RegisterMethodHandle + public static ClassLoader getParentClassLoader() { + return TestHelper.class.getClassLoader().getParent(); + } + } + + @Test + void testHelperClassLoaderThreadLocalLeak() throws Throwable { + HelperClassManager.ForDispatcher.inject(HelperClassManagerTest.class.getClassLoader(), HelperClassManagerTest.class.getProtectionDomain(), + List.of("co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelperThreadLocalLeak", + "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelperThreadLocalLeak$1")); + WeakReference helperClassLoader = new WeakReference<>((ClassLoader) MethodHandleDispatcher + .getMethodHandle(HelperClassManagerTest.class, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelperThreadLocalLeak#getClassLoader") + .invoke()); + + MethodHandleDispatcher.clear(); + System.gc(); + Thread.sleep(100); + assertThat(helperClassLoader.get()).isNotNull(); + } + + public static class TestHelperThreadLocalLeak { + private static final ThreadLocal threadLocal = new ThreadLocal() { + // This creates a class loader leak as java.lang.Thread.threadLocals (loaded from bootstrap CL) + // now references a class from the helper CL (this anonymous inner class) + @Override + protected Boolean initialValue() { + return Boolean.TRUE; + } + }; + + @RegisterMethodHandle + public static ClassLoader getClassLoader() { + assertThat(threadLocal.get()).isTrue(); + return TestHelper.class.getClassLoader(); + } + } + + @Test + void testWrongArguments() throws Throwable { + injectForDispatcher(TestHelper.class); + assertThatThrownBy(() -> MethodHandleDispatcher + .getMethodHandle(HelperClassManagerTest.class, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getParentClassLoader") + .invoke(boolean.class)) + .isInstanceOf(WrongMethodTypeException.class); + } + + private void injectForDispatcher(Class helper) throws Exception { + HelperClassManager.ForDispatcher.inject(HelperClassManagerTest.class.getClassLoader(), HelperClassManagerTest.class.getProtectionDomain(), Collections.singletonList(helper.getName())); + } + + @Test + void testForDispatcherOverloadedMethod() { + assertThatThrownBy(() -> injectForDispatcher(OverloadedMethodHelper.class)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("There is already a mapping for"); + } + + public static class OverloadedMethodHelper { + @RegisterMethodHandle + public static void foo() { + } + @RegisterMethodHandle + public static void foo(boolean bar) { + } + } + + @Test + void testForDispatcherPrivateMethod() { + assertThatThrownBy(() -> injectForDispatcher(HelperWithPrivateMethod.class)) + .isInstanceOf(IllegalAccessException.class); + } + + public static class HelperWithPrivateMethod { + @RegisterMethodHandle + private static void foo() { + } + } + + @Test + void testMultipleCL() throws Throwable { + ClassLoader targetClassLoader1 = new URLClassLoader(new URL[]{}, getClass().getClassLoader()); + HelperClassManager.ForDispatcher.inject(targetClassLoader1, getClass().getProtectionDomain(), Collections.singletonList(TestHelper.class.getName())); + assertThat(MethodHandleDispatcher + .getMethodHandle(targetClassLoader1, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getParentClassLoader") + .invoke()) + .isEqualTo(targetClassLoader1); + + ClassLoader targetClassLoader2 = new URLClassLoader(new URL[]{}, getClass().getClassLoader()); + HelperClassManager.ForDispatcher.inject(targetClassLoader2, getClass().getProtectionDomain(), Collections.singletonList(TestHelper.class.getName())); + assertThat(MethodHandleDispatcher + .getMethodHandle(targetClassLoader2, "co.elastic.apm.agent.bci.HelperClassManagerTest$TestHelper#getParentClassLoader") + .invoke()) + .isEqualTo(targetClassLoader2); + } + + private void assertFailLoadingOnlyOnce(HelperClassManager helperClassManager, Class libClass1) { ThrowableAssert.ThrowingCallable throwingCallable = () -> helperClassManager.doGetForClassLoaderOfClass(libClass1); assertThatThrownBy(throwingCallable).isInstanceOf(RuntimeException.class); diff --git a/apm-agent-core/src/test/java/co/elastic/apm/agent/util/PackageScannerTest.java b/apm-agent-core/src/test/java/co/elastic/apm/agent/util/PackageScannerTest.java new file mode 100644 index 0000000000..9286c38d4f --- /dev/null +++ b/apm-agent-core/src/test/java/co/elastic/apm/agent/util/PackageScannerTest.java @@ -0,0 +1,59 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.util; + +import net.bytebuddy.ByteBuddy; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class PackageScannerTest { + + @Test + public void getClassNames() throws Exception { + assertThat(PackageScanner.getClassNames(getClass().getPackageName())) + .contains(PackageScanner.class.getName()); + } + + @Test + public void getDoesNotContainTestClasses() throws Exception { + assertThat(PackageScanner.getClassNames(getClass().getPackageName())) + .doesNotContain(getClass().getName()); + } + + @Test + public void testScanJar() throws Exception { + assertThat(PackageScanner.getClassNames(ByteBuddy.class.getPackageName())) + .contains(ByteBuddy.class.getName()); + // scan again to see verify there's no FileSystemAlreadyExistsException + assertThat(PackageScanner.getClassNames(ByteBuddy.class.getPackageName())) + .contains(ByteBuddy.class.getName()); + } + + @Test + public void getClassNamesOfNonExistentPackage() throws Exception { + assertThat(PackageScanner.getClassNames("foo.bar")).isEmpty(); + } +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java index 244d0b71dc..ff801ecdfb 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/ConnectionInstrumentation.java @@ -24,9 +24,7 @@ */ package co.elastic.apm.agent.jdbc; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.jdbc.helper.JdbcHelper; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -52,24 +50,19 @@ */ public class ConnectionInstrumentation extends JdbcInstrumentation { - public ConnectionInstrumentation(ElasticApmTracer tracer) { - super(tracer); - } - - @VisibleForAdvice @Advice.OnMethodExit(suppress = Throwable.class) - public static void storeSql(@Advice.Return PreparedStatement statement, - @Advice.Argument(0) String sql) { - - if (jdbcHelperManager == null) { - return; + private static void storeSql(@Advice.Origin Class clazz, + @Advice.Return PreparedStatement statement, + @Advice.Argument(0) String sql) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#mapStatementToSql") + .invoke(null, statement, sql); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#mapStatementToSql") + .invoke(statement, sql); } - - JdbcHelper jdbcHelper = jdbcHelperManager.getForClassLoaderOfClass(PreparedStatement.class); - if (jdbcHelper != null) { - jdbcHelper.mapStatementToSql(statement, sql); - } - } @Override diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java index 87ad638219..82944fc406 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/JdbcInstrumentation.java @@ -25,31 +25,19 @@ package co.elastic.apm.agent.jdbc; import co.elastic.apm.agent.bci.ElasticApmInstrumentation; -import co.elastic.apm.agent.bci.HelperClassManager; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.jdbc.helper.JdbcHelper; +import co.elastic.apm.agent.util.PackageScanner; -import javax.annotation.Nullable; import java.util.Collection; import java.util.Collections; +import java.util.List; public abstract class JdbcInstrumentation extends ElasticApmInstrumentation { private static final Collection JDBC_GROUPS = Collections.singleton("jdbc"); - @VisibleForAdvice - @Nullable - public static HelperClassManager jdbcHelperManager = null; - - public JdbcInstrumentation(ElasticApmTracer tracer) { - synchronized (JdbcInstrumentation.class) { - if (jdbcHelperManager == null) { - jdbcHelperManager = HelperClassManager.ForSingleClassLoader.of(tracer, - "co.elastic.apm.agent.jdbc.helper.JdbcHelperImpl", - "co.elastic.apm.agent.jdbc.helper.JdbcHelperImpl$1"); - } - } + @Override + public List helpers() throws Exception { + return PackageScanner.getClassNames(getClass().getPackage().getName()); } @Override diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/StatementInstrumentation.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/StatementInstrumentation.java index 25665ffa6e..b7d0803429 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/StatementInstrumentation.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/StatementInstrumentation.java @@ -25,9 +25,8 @@ package co.elastic.apm.agent.jdbc; import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.transaction.Span; -import co.elastic.apm.agent.jdbc.helper.JdbcHelper; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -55,8 +54,7 @@ public abstract class StatementInstrumentation extends JdbcInstrumentation { private final ElementMatcher methodMatcher; - StatementInstrumentation(ElasticApmTracer tracer, ElementMatcher methodMatcher) { - super(tracer); + StatementInstrumentation(ElementMatcher methodMatcher) { this.methodMatcher = methodMatcher; } @@ -90,8 +88,8 @@ public ElementMatcher getMethodMatcher() { @SuppressWarnings("DuplicatedCode") public static class ExecuteWithQueryInstrumentation extends StatementInstrumentation { - public ExecuteWithQueryInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public ExecuteWithQueryInstrumentation() { + super( named("execute").or(named("executeQuery")) .and(takesArgument(0, String.class)) .and(isPublic()) @@ -101,48 +99,37 @@ public ExecuteWithQueryInstrumentation(ElasticApmTracer tracer) { @Nullable @VisibleForAdvice @Advice.OnMethodEnter(suppress = Throwable.class) - public static Span onBeforeExecute(@Advice.This Statement statement, - @Advice.Argument(0) String sql) { - - if (tracer == null || jdbcHelperManager == null) { - return null; - } - - JdbcHelper helperImpl = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helperImpl == null) { - return null; + private static Span onBeforeExecute(@Advice.Origin Class clazz, + @Advice.This Statement statement, + @Advice.Argument(0) String sql) throws Throwable { + + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Span) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpan") + .invoke(null, sql, statement, false); + } else { + return (Span) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpan") + .invoke(sql, statement, false); } - - return helperImpl.createJdbcSpan(sql, statement, tracer.getActive(), false); } @VisibleForAdvice @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onAfterExecute(@Advice.This Statement statement, - @Advice.Enter @Nullable Span span, - @Advice.Thrown @Nullable Throwable t) { - if (span == null) { - return; + private static void onAfterExecute(@Advice.Origin Class clazz, + @Advice.This Statement statement, + @Advice.Enter @Nullable Span span, + @Advice.Thrown @Nullable Throwable t) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterExecuteQuery") + .invoke(null, statement, span, t); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterExecuteQuery") + .invoke(statement, span, t); } - - if (t == null && jdbcHelperManager != null) { - JdbcHelper helper = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helper != null) { - long count = helper.safeGetUpdateCount(statement); - if (count != Long.MIN_VALUE) { - span.getContext() - .getDb() - .withAffectedRowsCount(count); - } - } - - } - - span.captureException(t) - .deactivate() - .end(); - } } @@ -161,8 +148,8 @@ public static void onAfterExecute(@Advice.This Statement statement, */ public static class ExecuteUpdateWithQueryInstrumentation extends StatementInstrumentation { - public ExecuteUpdateWithQueryInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public ExecuteUpdateWithQueryInstrumentation() { + super( named("executeUpdate").or(named("executeLargeUpdate")) .and(takesArgument(0, String.class)) .and(isPublic()) @@ -172,39 +159,37 @@ public ExecuteUpdateWithQueryInstrumentation(ElasticApmTracer tracer) { @Nullable @VisibleForAdvice @Advice.OnMethodEnter(suppress = Throwable.class) - public static Span onBeforeExecute(@Advice.This Statement statement, - @Advice.Argument(0) String sql) { - if (tracer == null || jdbcHelperManager == null) { - return null; - } - - JdbcHelper helperImpl = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helperImpl == null) { - return null; + private static Span onBeforeExecute(@Advice.Origin Class clazz, + @Advice.This Statement statement, + @Advice.Argument(0) String sql) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Span) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpan") + .invoke(null, sql, statement, false); + } else { + return (Span) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpan") + .invoke(sql, statement, false); } - - return helperImpl.createJdbcSpan(sql, statement, tracer.getActive(), false); } @VisibleForAdvice @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onAfterExecute(@Advice.Enter @Nullable Span span, - @Advice.Thrown @Nullable Throwable t, - @Advice.Return long returnValue /* bytebuddy converts int to long for us here ! */) { - if (span == null) { - return; - } - - if (t == null) { - span.getContext() - .getDb() - .withAffectedRowsCount(returnValue); + private static void onAfterExecute(@Advice.Origin Class clazz, + @Advice.Enter @Nullable Span span, + @Advice.Thrown @Nullable Throwable t, + @Advice.Return long returnValue /* bytebuddy converts int to long for us here ! */) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfter") + .invoke(null, span, t, returnValue); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfter") + .invoke(span, t, returnValue); } - - span.captureException(t) - .deactivate() - .end(); } + } /** @@ -212,8 +197,8 @@ public static void onAfterExecute(@Advice.Enter @Nullable Span span, */ public static class AddBatchInstrumentation extends StatementInstrumentation { - public AddBatchInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public AddBatchInstrumentation() { + super( nameStartsWith("addBatch") .and(takesArgument(0, String.class)) .and(isPublic()) @@ -221,14 +206,15 @@ public AddBatchInstrumentation(ElasticApmTracer tracer) { } @Advice.OnMethodEnter(suppress = Throwable.class) - public static void storeSql(@Advice.This Statement statement, @Advice.Argument(0) String sql) { - if (jdbcHelperManager == null) { - return; - } - - JdbcHelper helperImpl = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helperImpl != null) { - helperImpl.mapStatementToSql(statement, sql); + private static void storeSql(@Advice.Origin Class clazz, @Advice.This Statement statement, @Advice.Argument(0) String sql) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#mapStatementToSql") + .invoke(null, statement, sql); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#mapStatementToSql") + .invoke(statement, sql); } } } @@ -237,14 +223,14 @@ public static void storeSql(@Advice.This Statement statement, @Advice.Argument(0 /** * Instruments: - *
    - *
  • {@link Statement#executeBatch()}
    • - *
  • {@link Statement#executeLargeBatch()} (java8)
    • - *
+ *
    + *
  • {@link Statement#executeBatch()}
    • + *
  • {@link Statement#executeLargeBatch()} (java8)
    • + *
*/ public static class ExecuteBatchInstrumentation extends StatementInstrumentation { - public ExecuteBatchInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public ExecuteBatchInstrumentation() { + super( named("executeBatch").or(named("executeLargeBatch")) .and(takesArguments(0)) .and(isPublic()) @@ -254,66 +240,45 @@ public ExecuteBatchInstrumentation(ElasticApmTracer tracer) { @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - @SuppressWarnings("DuplicatedCode") - public static Span onBeforeExecute(@Advice.This Statement statement) { - if (tracer == null || jdbcHelperManager == null) { - return null; - } - JdbcHelper helper = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helper == null) { - return null; + private static Span onBeforeExecute(@Advice.Origin Class clazz, @Advice.This Statement statement) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Span) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(null, statement, false); + } else { + return (Span) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(statement, false); } - - String sql = helper.retrieveSqlForStatement(statement); - return helper.createJdbcSpan(sql, statement, tracer.getActive(), true); - } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onAfterExecute(@Advice.Enter @Nullable Span span, - @Advice.Thrown Throwable t, - @Advice.Return Object returnValue) { - if (span == null) { - return; - } - - // for 'executeBatch' and 'executeLargeBatch', we have to compute the sum as Statement.getUpdateCount() - // does not seem to return the sum of all elements. As we can use instanceof to check return type - // we do not need to use a separate advice. 'execute' return value is auto-boxed into a Boolean, - // but there is no extra allocation. - long affectedCount = 0; - if (returnValue instanceof int[]) { - int[] array = (int[]) returnValue; - for (int i = 0; i < array.length; i++) { - affectedCount += array[i]; - } - } else if (returnValue instanceof long[]) { - long[] array = (long[]) returnValue; - for (int i = 0; i < array.length; i++) { - affectedCount += array[i]; - } + private static void onAfterExecute(@Advice.Origin Class clazz, + @Advice.Enter @Nullable Span span, + @Advice.Thrown Throwable t, + @Advice.Return Object returnValue) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterExecuteBatch") + .invoke(null, span, t, returnValue); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterExecuteBatch") + .invoke(span, t, returnValue); } - span.getContext() - .getDb() - .withAffectedRowsCount(affectedCount); - - span.captureException(t) - .deactivate() - .end(); } - } /** * Instruments: - *
    - *
  • {@link PreparedStatement#executeUpdate()}
    • - *
  • {@link PreparedStatement#executeLargeUpdate()} ()} (java8)
    • - *
+ *
    + *
  • {@link PreparedStatement#executeUpdate()}
    • + *
  • {@link PreparedStatement#executeLargeUpdate()} ()} (java8)
    • + *
*/ public static class ExecuteUpdateNoQueryInstrumentation extends StatementInstrumentation { - public ExecuteUpdateNoQueryInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public ExecuteUpdateNoQueryInstrumentation() { + super( named("executeUpdate").or(named("executeLargeUpdate")) .and(takesArguments(0)) .and(isPublic()) @@ -322,39 +287,34 @@ public ExecuteUpdateNoQueryInstrumentation(ElasticApmTracer tracer) { @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - @SuppressWarnings("DuplicatedCode") - public static Span onBeforeExecute(@Advice.This Statement statement) { - if (tracer == null || jdbcHelperManager == null) { - return null; + private static Span onBeforeExecute(@Advice.Origin Class clazz, @Advice.This Statement statement) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Span) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(null, statement, true); + } else { + return (Span) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(statement, true); } - - JdbcHelper helperImpl = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helperImpl == null) { - return null; - } - - String sql = helperImpl.retrieveSqlForStatement(statement); - return helperImpl.createJdbcSpan(sql, statement, tracer.getActive(), true); } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onAfterExecute(@Advice.Enter @Nullable Span span, - @Advice.Thrown @Nullable Throwable t, - @Advice.Return long returnValue /* bytebuddy converts int to long for us here ! */) { - if (span == null) { - return; + private static void onAfterExecute(@Advice.Origin Class clazz, + @Advice.Enter @Nullable Span span, + @Advice.Thrown @Nullable Throwable t, + @Advice.Return long returnValue /* bytebuddy converts int to long for us here ! */) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfter") + .invoke(null, span, t, returnValue); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfter") + .invoke(span, t, returnValue); } - - if (t == null) { - span.getContext() - .getDb() - .withAffectedRowsCount(returnValue); - } - - span.captureException(t) - .deactivate() - .end(); } + } /** @@ -365,8 +325,8 @@ public static void onAfterExecute(@Advice.Enter @Nullable Span span, * */ public static class ExecutePreparedStatementInstrumentation extends StatementInstrumentation { - public ExecutePreparedStatementInstrumentation(ElasticApmTracer tracer) { - super(tracer, + public ExecutePreparedStatementInstrumentation() { + super( named("execute").or(named("executeQuery")) .and(takesArguments(0)) .and(isPublic()) @@ -375,42 +335,33 @@ public ExecutePreparedStatementInstrumentation(ElasticApmTracer tracer) { @Nullable @Advice.OnMethodEnter(suppress = Throwable.class) - @SuppressWarnings("DuplicatedCode") - public static Span onBeforeExecute(@Advice.This Statement statement) { - if (tracer != null && jdbcHelperManager != null) { - JdbcHelper helperImpl = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (helperImpl != null) { - @Nullable String sql = helperImpl.retrieveSqlForStatement(statement); - return helperImpl.createJdbcSpan(sql, statement, tracer.getActive(), true); - } + private static Span onBeforeExecute(@Advice.Origin Class clazz, @Advice.This Statement statement) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Span) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(null, statement, true); + } else { + return (Span) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#createJdbcSpanLookupSql") + .invoke(statement, true); } - return null; } @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onAfterExecute(@Advice.This Statement statement, - @Advice.Enter @Nullable Span span, - @Advice.Thrown @Nullable Throwable t) { - - if (span == null) { - return; + private static void onAfterExecute(@Advice.Origin Class clazz, + @Advice.This Statement statement, + @Advice.Enter @Nullable Span span, + @Advice.Thrown @Nullable Throwable t) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterPreparedStatementExecuteQuery") + .invoke(null, statement, span, t); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#onAfterPreparedStatementExecuteQuery") + .invoke(statement, span, t); } - - if (t == null && jdbcHelperManager != null) { - JdbcHelper jdbcHelper = jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (jdbcHelper != null) { - span.getContext() - .getDb() - // getUpdateCount javadoc indicates that this method should be called only once - // however in practice adding this extra call seem to not have noticeable side effects - .withAffectedRowsCount(jdbcHelper.safeGetUpdateCount(statement)); - } - } - - span.captureException(t) - .deactivate() - .end(); } - } + } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/AdviceHelperAdapter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/AdviceHelperAdapter.java new file mode 100644 index 0000000000..4e12746a5e --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/AdviceHelperAdapter.java @@ -0,0 +1,79 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2020 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.jdbc.helper; + +import co.elastic.apm.agent.bci.ElasticApmInstrumentation; +import co.elastic.apm.agent.bci.RegisterMethodHandle; +import co.elastic.apm.agent.impl.transaction.Span; + +import javax.annotation.Nullable; +import java.sql.Statement; + +public class AdviceHelperAdapter { + + private static final JdbcHelper INSTANCE = new JdbcHelper(ElasticApmInstrumentation.tracer); + + @RegisterMethodHandle + public static void mapStatementToSql(Statement statement, String sql) { + INSTANCE.mapStatementToSql(statement, sql); + } + + @RegisterMethodHandle + public static void clearInternalStorage() { + INSTANCE.clearInternalStorage(); + } + + @Nullable + @RegisterMethodHandle + public static Span createJdbcSpanLookupSql(Statement statement, boolean preparedStatement) { + return INSTANCE.createJdbcSpanLookupSql(statement, preparedStatement); + } + + @Nullable + @RegisterMethodHandle + public static Span createJdbcSpan(@Nullable String sql, Statement statement, boolean preparedStatement) { + return INSTANCE.createJdbcSpan(sql, statement, preparedStatement); + } + + @RegisterMethodHandle + public static void onAfterExecuteQuery(Statement statement, @Nullable Span span, @Nullable Throwable t) { + INSTANCE.onAfterExecuteQuery(statement, span, t); + } + + @RegisterMethodHandle + public static void onAfterExecuteBatch(@Nullable Span span, Throwable t, @Nullable Object returnValue) { + INSTANCE.onAfterExecuteBatch(span, t, returnValue); + } + + @RegisterMethodHandle + public static void onAfterPreparedStatementExecuteQuery(Statement statement, @Nullable Span span, @Nullable Throwable t) { + INSTANCE.onAfterPreparedStatementExecuteQuery(statement, span, t); + } + + @RegisterMethodHandle + public static void onAfter(@Nullable Span span, @Nullable Throwable t, long affectedCount) { + INSTANCE.onAfter(span, t, affectedCount); + } +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java index 42f798a965..8f37ed35f9 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java @@ -24,32 +24,75 @@ */ package co.elastic.apm.agent.jdbc.helper; +import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.impl.context.Destination; +import co.elastic.apm.agent.impl.transaction.AbstractSpan; import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.TraceContextHolder; +import co.elastic.apm.agent.jdbc.signature.SignatureParser; import co.elastic.apm.agent.util.DataStructures; import com.blogspot.mydailyjava.weaklockfree.WeakConcurrentMap; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nullable; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.SQLException; +import java.sql.Statement; -public abstract class JdbcHelper { - - private static final WeakConcurrentMap statementSqlMap = DataStructures.createWeakConcurrentMapWithCleanerThread(); +public class JdbcHelper { public static final String DB_SPAN_TYPE = "db"; public static final String DB_SPAN_ACTION = "query"; + private final ThreadLocal SIGNATURE_PARSER_THREAD_LOCAL = new ThreadLocal() { + @Override + protected SignatureParser initialValue() { + return new SignatureParser(); + } + }; + private final WeakConcurrentMap statementSqlMap = DataStructures.createWeakConcurrentMapWithCleanerThread(); + private final Logger logger = LoggerFactory.getLogger(JdbcHelper.class); + private final WeakConcurrentMap metaDataMap = DataStructures.createWeakConcurrentMapWithCleanerThread(); + private final WeakConcurrentMap, Boolean> updateCountSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); + private final WeakConcurrentMap, Boolean> metadataSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); + private final WeakConcurrentMap, Boolean> connectionSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); + private final ElasticApmTracer tracer; + + public JdbcHelper(ElasticApmTracer tracer) { + this.tracer = tracer; + } + /** * Maps the provided sql to the provided Statement object * * @param statement javax.sql.Statement object * @param sql query string */ - public void mapStatementToSql(Object statement, String sql) { + public void mapStatementToSql(Statement statement, String sql) { statementSqlMap.putIfAbsent(statement, sql); } /** - * Returns the SQL statement belonging to provided Statement. + * Clears internal data storage, should only be used for testing + */ + public void clearInternalStorage() { + metaDataMap.clear(); + updateCountSupported.clear(); + metadataSupported.clear(); + connectionSupported.clear(); + } + + + @Nullable + public Span createJdbcSpanLookupSql(Statement statement, boolean preparedStatement) { + return createJdbcSpan(retrieveSqlForStatement(statement), statement, preparedStatement); + } + + + /** + * Looks up the SQL statement belonging to provided Statement. *

* Might return {@code null} when the provided Statement is a wrapper of the actual statement. *

@@ -57,17 +100,146 @@ public void mapStatementToSql(Object statement, String sql) { * @return the SQL statement belonging to provided Statement, or {@code null} */ @Nullable - public String retrieveSqlForStatement(Object statement) { + public String retrieveSqlForStatement(Statement statement) { return statementSqlMap.get(statement); } - /** - * Clears internal data storage, should only be used for testing + @Nullable + public Span createJdbcSpan(@Nullable String sql, Statement statement, boolean preparedStatement) { + TraceContextHolder parent = tracer.getActive(); + if (sql == null || isAlreadyMonitored(parent) || parent == null || !parent.isSampled()) { + return null; + } + + Span span = parent.createSpan().activate(); + if (sql.isEmpty()) { + span.withName("empty query"); + } else { + StringBuilder spanName = span.getAndOverrideName(AbstractSpan.PRIO_DEFAULT); + if (spanName != null) { + SIGNATURE_PARSER_THREAD_LOCAL.get().querySignature(sql, spanName, preparedStatement); + } + } + // setting the type here is important + // getting the meta data can result in another jdbc call + // if that is traced as well -> StackOverflowError + // to work around that, isAlreadyMonitored checks if the parent span is a db span and ignores them + span.withType(DB_SPAN_TYPE); + + // write fields that do not rely on metadata + span.getContext().getDb() + .withStatement(sql.isEmpty() ? "(empty query)" : sql) + .withType("sql"); + + Connection connection = safeGetConnection(statement); + ConnectionMetaData connectionMetaData = connection == null ? null : getConnectionMetaData(connection); + if (connectionMetaData != null) { + span.withSubtype(connectionMetaData.getDbVendor()) + .withAction(DB_SPAN_ACTION); + span.getContext().getDb() + .withUser(connectionMetaData.getUser()); + Destination destination = span.getContext().getDestination() + .withAddress(connectionMetaData.getHost()) + .withPort(connectionMetaData.getPort()); + destination.getService() + .withName(connectionMetaData.getDbVendor()) + .withResource(connectionMetaData.getDbVendor()) + .withType(DB_SPAN_TYPE); + } + + return span; + } + + public void onAfterExecuteQuery(Statement statement, @Nullable Span span, @Nullable Throwable t) { + if (span == null) { + return; + } + onAfter(span, t, t == null ? safeGetUpdateCount(statement) : Long.MIN_VALUE); + } + + public void onAfterExecuteBatch(@Nullable Span span, Throwable t, @Nullable Object returnValue) { + if (span == null) { + return; + } + onAfter(span, t, getAffectedCountForBatch(returnValue)); + } + + public void onAfterPreparedStatementExecuteQuery(Statement statement, @Nullable Span span, @Nullable Throwable t) { + if (span == null) { + return; + } + onAfter(span, t, t == null ? safeGetUpdateCount(statement) : Long.MIN_VALUE); + } + + public void onAfter(@Nullable Span span, @Nullable Throwable t, long affectedCount) { + if (span == null) { + return; + } + if (t == null && affectedCount != Long.MIN_VALUE) { + span.getContext() + .getDb() + .withAffectedRowsCount(affectedCount); + } + + span.captureException(t) + .deactivate() + .end(); + } + + private long getAffectedCountForBatch(@Nullable Object returnValue) { + // for 'executeBatch' and 'executeLargeBatch', we have to compute the sum as Statement.getUpdateCount() + // does not seem to return the sum of all elements. As we can use instanceof to check return type + // we do not need to use a separate advice. 'execute' return value is auto-boxed into a Boolean, + // but there is no extra allocation. + long affectedCount = 0; + if (returnValue instanceof int[]) { + int[] array = (int[]) returnValue; + for (int i = 0; i < array.length; i++) { + affectedCount += array[i]; + } + } else if (returnValue instanceof long[]) { + long[] array = (long[]) returnValue; + for (int i = 0; i < array.length; i++) { + affectedCount += array[i]; + } + } + return affectedCount; + } + + /* + * This makes sure that even when there are wrappers for the statement, + * we only record each JDBC call once. */ - public abstract void clearInternalStorage(); + private boolean isAlreadyMonitored(@Nullable TraceContextHolder parent) { + if (!(parent instanceof Span)) { + return false; + } + Span parentSpan = (Span) parent; + // a db span can't be the child of another db span + // this means the span has already been created for this db call + return parentSpan.getType() != null && parentSpan.getType().equals(DB_SPAN_TYPE); + } @Nullable - public abstract Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable TraceContextHolder parent, boolean preparedStatement); + private Connection safeGetConnection(Statement statement) { + Connection connection = null; + Class type = statement.getClass(); + Boolean supported = isSupported(connectionSupported, type); + if (supported == Boolean.FALSE) { + return null; + } + + try { + connection = statement.getConnection(); + if (supported == null) { + markSupported(connectionSupported, type); + } + } catch (SQLException e) { + markNotSupported(connectionSupported, type, e); + } + + return connection; + } /** * Safely wraps calls to {@link java.sql.Statement#getUpdateCount()} @@ -75,7 +247,76 @@ public String retrieveSqlForStatement(Object statement) { * @param statement {@code java.sql.Statement} instance * @return {@link Long#MIN_VALUE} if statement does not support this feature, returned value otherwise */ - public abstract long safeGetUpdateCount(Object statement); + public long safeGetUpdateCount(Object statement) { + long result = Long.MIN_VALUE; + if (!(statement instanceof Statement)) { + return result; + } + + Class type = statement.getClass(); + Boolean supported = isSupported(updateCountSupported, type); + if (supported == Boolean.FALSE) { + return result; + } + + try { + // getUpdateCount javadoc indicates that this method should be called only once + // however in practice adding this extra call seem to not have noticeable side effects + result = ((Statement) statement).getUpdateCount(); + if (supported == null) { + markSupported(updateCountSupported, type); + } + } catch (SQLException e) { + markNotSupported(updateCountSupported, type, e); + } + + return result; + } + + @Nullable + private Boolean isSupported(WeakConcurrentMap, Boolean> featureMap, Class type) { + return featureMap.get(type); + } + + private void markSupported(WeakConcurrentMap, Boolean> map, Class type) { + map.put(type, Boolean.TRUE); + } + + private void markNotSupported(WeakConcurrentMap, Boolean> map, Class type, SQLException e) { + Boolean previous = map.put(type, Boolean.FALSE); + if (previous == null) { + logger.warn("JDBC feature not supported on class " + type, e); + } + } + + @Nullable + private ConnectionMetaData getConnectionMetaData(Connection connection) { + ConnectionMetaData connectionMetaData = metaDataMap.get(connection); + if (connectionMetaData != null) { + return connectionMetaData; + } + + Class type = connection.getClass(); + Boolean supported = isSupported(metadataSupported, type); + if (supported == Boolean.FALSE) { + return null; + } + + try { + DatabaseMetaData metaData = connection.getMetaData(); + connectionMetaData = ConnectionMetaData.create(metaData.getURL(), metaData.getUserName()); + if (supported == null) { + markSupported(metadataSupported, type); + } + } catch (SQLException e) { + markNotSupported(metadataSupported, type, e); + } + + if (connectionMetaData != null) { + metaDataMap.put(connection, connectionMetaData); + } + return connectionMetaData; + } } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelperImpl.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelperImpl.java deleted file mode 100644 index 783d88c184..0000000000 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelperImpl.java +++ /dev/null @@ -1,228 +0,0 @@ -/*- - * #%L - * Elastic APM Java agent - * %% - * Copyright (C) 2018 - 2020 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.jdbc.helper; - -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.context.Destination; -import co.elastic.apm.agent.impl.transaction.AbstractSpan; -import co.elastic.apm.agent.impl.transaction.Span; -import co.elastic.apm.agent.impl.transaction.TraceContextHolder; -import co.elastic.apm.agent.jdbc.signature.SignatureParser; -import co.elastic.apm.agent.util.DataStructures; -import com.blogspot.mydailyjava.weaklockfree.WeakConcurrentMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.annotation.Nullable; -import java.sql.Connection; -import java.sql.DatabaseMetaData; -import java.sql.SQLException; -import java.sql.Statement; - -@SuppressWarnings("unused") // indirect access to this class provided through HelperClassManager -public class JdbcHelperImpl extends JdbcHelper { - - private static final Logger logger = LoggerFactory.getLogger(JdbcHelperImpl.class); - - // Important implementation note: - // - // because this class is potentially loaded from multiple classloaders, making those fields 'static' will not - // have the expected behavior, thus, any direct reference to `JdbcHelperImpl` should only be obtained from the - // HelperClassManager instance. - private final WeakConcurrentMap metaDataMap = DataStructures.createWeakConcurrentMapWithCleanerThread(); - private final WeakConcurrentMap, Boolean> updateCountSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); - private final WeakConcurrentMap, Boolean> metadataSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); - private final WeakConcurrentMap, Boolean> connectionSupported = new WeakConcurrentMap.WithInlinedExpunction, Boolean>(); - - @VisibleForAdvice - public final ThreadLocal SIGNATURE_PARSER_THREAD_LOCAL = new ThreadLocal() { - @Override - protected SignatureParser initialValue() { - return new SignatureParser(); - } - }; - - @Override - public void clearInternalStorage() { - metaDataMap.clear(); - updateCountSupported.clear(); - metadataSupported.clear(); - connectionSupported.clear(); - } - - @Override - @Nullable - public Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable TraceContextHolder parent, boolean preparedStatement) { - if (!(statement instanceof Statement) || sql == null || isAlreadyMonitored(parent) || parent == null || !parent.isSampled()) { - return null; - } - - Span span = parent.createSpan().activate(); - if (sql.isEmpty()) { - span.withName("empty query"); - } else { - StringBuilder spanName = span.getAndOverrideName(AbstractSpan.PRIO_DEFAULT); - if (spanName != null) { - SIGNATURE_PARSER_THREAD_LOCAL.get().querySignature(sql, spanName, preparedStatement); - } - } - // setting the type here is important - // getting the meta data can result in another jdbc call - // if that is traced as well -> StackOverflowError - // to work around that, isAlreadyMonitored checks if the parent span is a db span and ignores them - span.withType(DB_SPAN_TYPE); - - // write fields that do not rely on metadata - span.getContext().getDb() - .withStatement(sql.isEmpty() ? "(empty query)" : sql) - .withType("sql"); - - Connection connection = safeGetConnection((Statement) statement); - ConnectionMetaData connectionMetaData = connection == null ? null : getConnectionMetaData(connection); - if (connectionMetaData != null) { - span.withSubtype(connectionMetaData.getDbVendor()) - .withAction(DB_SPAN_ACTION); - span.getContext().getDb() - .withUser(connectionMetaData.getUser()); - Destination destination = span.getContext().getDestination() - .withAddress(connectionMetaData.getHost()) - .withPort(connectionMetaData.getPort()); - destination.getService() - .withName(connectionMetaData.getDbVendor()) - .withResource(connectionMetaData.getDbVendor()) - .withType(DB_SPAN_TYPE); - } - - return span; - } - - /* - * This makes sure that even when there are wrappers for the statement, - * we only record each JDBC call once. - */ - private boolean isAlreadyMonitored(@Nullable TraceContextHolder parent) { - if (!(parent instanceof Span)) { - return false; - } - Span parentSpan = (Span) parent; - // a db span can't be the child of another db span - // this means the span has already been created for this db call - return parentSpan.getType() != null && parentSpan.getType().equals(DB_SPAN_TYPE); - } - - @Nullable - private ConnectionMetaData getConnectionMetaData(Connection connection) { - ConnectionMetaData connectionMetaData = metaDataMap.get(connection); - if (connectionMetaData != null) { - return connectionMetaData; - } - - Class type = connection.getClass(); - Boolean supported = isSupported(metadataSupported, type); - if (supported == Boolean.FALSE) { - return null; - } - - try { - DatabaseMetaData metaData = connection.getMetaData(); - connectionMetaData = ConnectionMetaData.create(metaData.getURL(), metaData.getUserName()); - if (supported == null) { - markSupported(metadataSupported, type); - } - } catch (SQLException e) { - markNotSupported(metadataSupported, type, e); - } - - if (connectionMetaData != null) { - metaDataMap.put(connection, connectionMetaData); - } - return connectionMetaData; - } - - @Nullable - private Connection safeGetConnection(Statement statement) { - Connection connection = null; - Class type = statement.getClass(); - Boolean supported = isSupported(connectionSupported, type); - if (supported == Boolean.FALSE) { - return null; - } - - try { - connection = statement.getConnection(); - if (supported == null) { - markSupported(connectionSupported, type); - } - } catch (SQLException e) { - markNotSupported(connectionSupported, type, e); - } - - return connection; - } - - - @Override - public long safeGetUpdateCount(Object statement) { - long result = Long.MIN_VALUE; - if (!(statement instanceof Statement)) { - return result; - } - - Class type = statement.getClass(); - Boolean supported = isSupported(updateCountSupported, type); - if (supported == Boolean.FALSE) { - return result; - } - - try { - result = ((Statement) statement).getUpdateCount(); - if (supported == null) { - markSupported(updateCountSupported, type); - } - } catch (SQLException e) { - markNotSupported(updateCountSupported, type, e); - } - - return result; - } - - @Nullable - private static Boolean isSupported(WeakConcurrentMap, Boolean> featureMap, Class type) { - return featureMap.get(type); - } - - private static void markSupported(WeakConcurrentMap, Boolean> map, Class type) { - map.put(type, Boolean.TRUE); - } - - private static void markNotSupported(WeakConcurrentMap, Boolean> map, Class type, SQLException e) { - Boolean previous = map.put(type, Boolean.FALSE); - if (previous == null) { - logger.warn("JDBC feature not supported on class " + type, e); - } - } - - -} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java index b3390ec7b9..06db2f42bc 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java @@ -25,11 +25,11 @@ package co.elastic.apm.agent.jdbc; import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.context.Db; import co.elastic.apm.agent.impl.context.Destination; import co.elastic.apm.agent.impl.transaction.Span; import co.elastic.apm.agent.impl.transaction.Transaction; -import co.elastic.apm.agent.jdbc.helper.JdbcHelper; import co.elastic.apm.agent.jdbc.signature.SignatureParser; import org.junit.After; import org.junit.Before; @@ -161,11 +161,12 @@ private void executeTest(JdbcTask task) throws SQLException { // clear internal jdbc helper required due to metadata caching and global state about unsupported // JDBC driver features (based on classes instances) - if (JdbcInstrumentation.jdbcHelperManager != null) { - JdbcHelper jdbcHelper = JdbcInstrumentation.jdbcHelperManager.getForClassLoaderOfClass(Statement.class); - if (jdbcHelper != null) { - jdbcHelper.clearInternalStorage(); - } + try { + MethodHandleDispatcher + .getMethodHandle(getClass(), "co.elastic.apm.agent.jdbc.helper.AdviceHelperAdapter#clearInternalStorage") + .invoke(); + } catch (Throwable throwable) { + throwable.printStackTrace(); } } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AbstractServletInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AbstractServletInstrumentation.java index 01deca5a0c..301d7d3c29 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AbstractServletInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AbstractServletInstrumentation.java @@ -26,10 +26,12 @@ import co.elastic.apm.agent.bci.ElasticApmInstrumentation; import co.elastic.apm.agent.bci.bytebuddy.CustomElementMatchers; +import co.elastic.apm.agent.util.PackageScanner; import net.bytebuddy.matcher.ElementMatcher; import java.util.Collection; import java.util.Collections; +import java.util.List; import static co.elastic.apm.agent.servlet.ServletInstrumentation.SERVLET_API; @@ -40,6 +42,11 @@ public Collection getInstrumentationGroupNames() { return Collections.singleton(SERVLET_API); } + @Override + public List helpers() throws Exception { + return PackageScanner.getClassNames(getClass().getPackage().getName()); + } + @Override public final ElementMatcher.Junction getClassLoaderMatcher() { // this class has been introduced in servlet spec 3.0 diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AsyncInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AsyncInstrumentation.java index 2261d60c30..847eac15e9 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AsyncInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/AsyncInstrumentation.java @@ -24,10 +24,10 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.bci.HelperClassManager; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.bci.RegisterMethodHandle; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.servlet.helper.AsyncContextAdviceHelper; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -51,48 +51,28 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -/** - * Only the methods annotated with {@link Advice.OnMethodEnter} and {@link Advice.OnMethodExit} may contain references to - * {@code javax.servlet}, as these are inlined into the matching methods. - * The agent itself does not have access to the Servlet API classes, as they are loaded by a child class loader. - * See https://github.com/raphw/byte-buddy/issues/465 for more information. - * However, the helper class {@link AsyncContextAdviceHelper} has access to the Servlet API, - * as it is loaded by the child classloader of {@link AsyncContext} - * (see {@link StartAsyncInstrumentation.StartAsyncAdvice#onExitStartAsync(AsyncContext)} - * and {@link AsyncContextInstrumentation.AsyncContextStartAdvice#onEnterAsyncContextStart(Runnable)}). - */ public abstract class AsyncInstrumentation extends AbstractServletInstrumentation { private static final String SERVLET_API_ASYNC_GROUP_NAME = "servlet-api-async"; - @Nullable - @VisibleForAdvice - // referring to AsyncContext is legal because of type erasure - public static HelperClassManager> asyncHelperManager; - - public AsyncInstrumentation(ElasticApmTracer tracer) { - synchronized (AsyncInstrumentation.class) { - if (asyncHelperManager == null) { - asyncHelperManager = HelperClassManager.ForSingleClassLoader.of(tracer, - "co.elastic.apm.agent.servlet.helper.AsyncContextAdviceHelperImpl", - "co.elastic.apm.agent.servlet.helper.AsyncContextAdviceHelperImpl$ApmAsyncListenerAllocator", - "co.elastic.apm.agent.servlet.helper.ApmAsyncListener"); - } - - } - } @Override public Collection getInstrumentationGroupNames() { return Arrays.asList(ServletInstrumentation.SERVLET_API, SERVLET_API_ASYNC_GROUP_NAME); } - public interface AsyncContextAdviceHelper { - void onExitStartAsync(T asyncContext); - } - public static class StartAsyncInstrumentation extends AsyncInstrumentation { - public StartAsyncInstrumentation(ElasticApmTracer tracer) { - super(tracer); + + @Advice.OnMethodExit(suppress = Throwable.class) + private static void onExitStartAsync(@Advice.Origin Class clazz, @Advice.Return AsyncContext asyncContext) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$StartAsyncInstrumentation$StartAsyncAdvice#onExitStartAsync") + .invoke(null, asyncContext); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$StartAsyncInstrumentation$StartAsyncAdvice#onExitStartAsync") + .invoke(asyncContext); + } } @Override @@ -128,29 +108,43 @@ public ElementMatcher getMethodMatcher() { ); } - @Override - public Class getAdviceClass() { - return StartAsyncAdvice.class; - } - - @VisibleForAdvice public static class StartAsyncAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - private static void onExitStartAsync(@Advice.Return AsyncContext asyncContext) { - if (tracer != null && asyncHelperManager != null) { - AsyncContextAdviceHelper helperImpl = asyncHelperManager.getForClassLoaderOfClass(AsyncContext.class); - if (helperImpl != null) { - helperImpl.onExitStartAsync(asyncContext); - } - } + private static final AsyncContextAdviceHelper helper = new AsyncContextAdviceHelper(tracer); + + @RegisterMethodHandle + public static void onExitStartAsync(AsyncContext asyncContext) { + helper.onExitStartAsync(asyncContext); } } } public static class AsyncContextInstrumentation extends AsyncInstrumentation { - public AsyncContextInstrumentation(ElasticApmTracer tracer) { - super(tracer); + + @Advice.OnMethodEnter(suppress = Throwable.class) + private static void onEnterAsyncContextStart(@Advice.Origin Class clazz, @Advice.Argument(value = 0, readOnly = false) @Nullable Runnable runnable) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + runnable = (Runnable) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation$AsyncContextStartAdvice#onEnterAsyncContextStart") + .invoke(null, runnable); + } else { + runnable = (Runnable) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation$AsyncContextStartAdvice#onEnterAsyncContextStart") + .invoke(runnable); + } + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Exception.class) + private static void onExitAsyncContextStart(@Advice.Origin Class clazz) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation$AsyncContextStartAdvice#onExitAsyncContextStart") + .invoke(null); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.AsyncInstrumentation$AsyncContextInstrumentation$AsyncContextStartAdvice#onExitAsyncContextStart") + .invoke(); + } } @Override @@ -171,16 +165,11 @@ public ElementMatcher getMethodMatcher() { .and(takesArguments(Runnable.class)); } - @Override - public Class getAdviceClass() { - return AsyncContextStartAdvice.class; - } - - @VisibleForAdvice public static class AsyncContextStartAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - private static void onEnterAsyncContextStart(@Advice.Argument(value = 0, readOnly = false) @Nullable Runnable runnable) { + @Nullable + @RegisterMethodHandle + public static Runnable onEnterAsyncContextStart(@Nullable Runnable runnable) { if (tracer != null && runnable != null && tracer.isWrappingAllowedOnThread()) { final Transaction transaction = tracer.currentTransaction(); if (transaction != null) { @@ -188,10 +177,11 @@ private static void onEnterAsyncContextStart(@Advice.Argument(value = 0, readOnl tracer.avoidWrappingOnThread(); } } + return runnable; } - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Exception.class) - private static void onExitAsyncContextStart() { + @RegisterMethodHandle + public static void onExitAsyncContextStart() { if (tracer != null) { tracer.allowWrappingOnThread(); } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/FilterChainInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/FilterChainInstrumentation.java index 46afd92cef..e2b733894e 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/FilterChainInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/FilterChainInstrumentation.java @@ -24,12 +24,18 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; +import co.elastic.apm.agent.impl.transaction.Transaction; +import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; +import javax.annotation.Nullable; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; + import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.nameContains; @@ -42,10 +48,6 @@ */ public class FilterChainInstrumentation extends AbstractServletInstrumentation { - public FilterChainInstrumentation(ElasticApmTracer tracer) { - ServletApiAdvice.init(tracer); - } - @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("Chain"); @@ -64,9 +66,37 @@ public ElementMatcher getMethodMatcher() { .and(takesArgument(1, named("javax.servlet.ServletResponse"))); } - @Override - public Class getAdviceClass() { - return ServletApiAdvice.class; + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class) + private static Transaction onEnterServletService(@Advice.Origin Class clazz, + @Advice.Argument(0) ServletRequest servletRequest) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Transaction) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onEnterServletService") + .invoke(null, servletRequest); + } else { + return (Transaction) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onEnterServletService") + .invoke(servletRequest); + } + } + + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + private static void onExitServletService(@Advice.Origin Class clazz, + @Advice.Argument(0) ServletRequest servletRequest, + @Advice.Argument(1) ServletResponse servletResponse, + @Advice.Enter @Nullable Transaction transaction, + @Advice.Thrown @Nullable Throwable t, + @Advice.This Object thiz) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onExitServletService") + .invoke(null, servletRequest, servletResponse, transaction, t, thiz); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onExitServletService") + .invoke(servletRequest, servletResponse, transaction, t, thiz); + } } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java index e20846c0b6..a3325ca8b1 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/RequestStreamRecordingInstrumentation.java @@ -24,18 +24,17 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.bci.HelperClassManager; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.impl.context.Request; +import co.elastic.apm.agent.bci.RegisterMethodHandle; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.transaction.Transaction; +import co.elastic.apm.agent.servlet.helper.RecordingServletInputStreamWrapper; +import co.elastic.apm.agent.util.CallDepth; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import javax.annotation.Nullable; import javax.servlet.ServletInputStream; import java.util.Arrays; import java.util.Collection; @@ -50,19 +49,29 @@ public class RequestStreamRecordingInstrumentation extends AbstractServletInstrumentation { - @Nullable - @VisibleForAdvice - // referring to InputStreamWrapperFactory is legal because of type erasure - public static HelperClassManager wrapperHelperClassManager; + @Advice.OnMethodEnter(suppress = Throwable.class) + private static void onReadEnter(@Advice.Origin Class clazz) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation$GetInputStreamAdvice#onReadEnter") + .invoke(null); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation$GetInputStreamAdvice#onReadEnter") + .invoke(); + } + } - public RequestStreamRecordingInstrumentation(ElasticApmTracer tracer) { - ServletApiAdvice.init(tracer); - synchronized (RequestStreamRecordingInstrumentation.class) { - if (wrapperHelperClassManager == null) { - wrapperHelperClassManager = HelperClassManager.ForSingleClassLoader.of(tracer, - "co.elastic.apm.agent.servlet.helper.InputStreamFactoryHelperImpl", - "co.elastic.apm.agent.servlet.helper.RecordingServletInputStreamWrapper"); - } + @Advice.OnMethodExit(suppress = Throwable.class) + private static void afterGetInputStream(@Advice.Origin Class clazz, @Advice.Return(readOnly = false) ServletInputStream inputStream) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + inputStream = (ServletInputStream) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation$GetInputStreamAdvice#afterGetInputStream") + .invoke(null, inputStream); + } else { + inputStream = (ServletInputStream) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.RequestStreamRecordingInstrumentation$GetInputStreamAdvice#afterGetInputStream") + .invoke(inputStream); } } @@ -86,48 +95,25 @@ public Collection getInstrumentationGroupNames() { return Arrays.asList(SERVLET_API, "servlet-input-stream"); } - @Override - public Class getAdviceClass() { - return GetInputStreamAdvice.class; - } - - public interface InputStreamWrapperFactory { - ServletInputStream wrap(Request request, ServletInputStream servletInputStream); - } - public static class GetInputStreamAdvice { - @VisibleForAdvice - public static final ThreadLocal nestedThreadLocal = new ThreadLocal() { - @Override - protected Boolean initialValue() { - return Boolean.FALSE; - } - }; - - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onReadEnter(@Advice.This Object thiz, - @Advice.Local("transaction") Transaction transaction, - @Advice.Local("nested") boolean nested) { - nested = nestedThreadLocal.get(); - nestedThreadLocal.set(Boolean.TRUE); + @RegisterMethodHandle + public static void onReadEnter() { + CallDepth.increment(GetInputStreamAdvice.class); } - @Advice.OnMethodExit(suppress = Throwable.class) - public static void afterGetInputStream(@Advice.Return(readOnly = false) ServletInputStream inputStream, - @Advice.Local("nested") boolean nested) { - if (nested || tracer == null || wrapperHelperClassManager == null) { - return; + @RegisterMethodHandle + public static ServletInputStream afterGetInputStream(ServletInputStream inputStream) { + int callDepth = CallDepth.decrement(GetInputStreamAdvice.class); + if (callDepth > 0 || tracer == null) { + return inputStream; } - try { - final Transaction transaction = tracer.currentTransaction(); - // only wrap if the body buffer has been initialized via ServletTransactionHelper.startCaptureBody - if (transaction != null && transaction.getContext().getRequest().getBodyBuffer() != null) { - inputStream = wrapperHelperClassManager.getForClassLoaderOfClass(inputStream.getClass()).wrap(transaction.getContext().getRequest(), inputStream); - } - } finally { - nestedThreadLocal.set(Boolean.FALSE); + final Transaction transaction = tracer.currentTransaction(); + // only wrap if the body buffer has been initialized via ServletTransactionHelper.startCaptureBody + if (transaction != null && transaction.getContext().getRequest().getBodyBuffer() != null) { + inputStream = new RecordingServletInputStreamWrapper(transaction.getContext().getRequest(), inputStream); } + return inputStream; } } } 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 cc145d521a..188d4b8313 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 @@ -24,17 +24,19 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.bci.RegisterMethodHandle; import co.elastic.apm.agent.configuration.CoreConfiguration; -import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.Scope; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.impl.context.Response; import co.elastic.apm.agent.impl.transaction.Transaction; -import net.bytebuddy.asm.Advice; +import co.elastic.apm.agent.servlet.helper.ServletTransactionCreationHelper; +import co.elastic.apm.agent.util.CallDepth; +import com.blogspot.mydailyjava.weaklockfree.DetachedThreadLocal; import javax.annotation.Nullable; import javax.servlet.DispatcherType; +import javax.servlet.Servlet; import javax.servlet.ServletContext; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -48,55 +50,44 @@ import java.util.List; import java.util.Map; +import static co.elastic.apm.agent.bci.ElasticApmInstrumentation.tracer; import static co.elastic.apm.agent.servlet.ServletTransactionHelper.TRANSACTION_ATTRIBUTE; import static co.elastic.apm.agent.servlet.ServletTransactionHelper.determineServiceName; -/** - * Only the methods annotated with {@link Advice.OnMethodEnter} and {@link Advice.OnMethodExit} may contain references to - * {@code javax.servlet}, as these are inlined into the matching methods. - * The agent itself does not have access to the Servlet API classes, as they are loaded by a child class loader. - * See https://github.com/raphw/byte-buddy/issues/465 for more information. - */ public class ServletApiAdvice { - @Nullable - @VisibleForAdvice - public static ServletTransactionHelper servletTransactionHelper; - - @Nullable - @VisibleForAdvice - public static ElasticApmTracer tracer; + private static final ServletTransactionHelper servletTransactionHelper; + private static final ServletTransactionCreationHelper servletTransactionCreationHelper; - @VisibleForAdvice - public static ThreadLocal excluded = new ThreadLocal() { + private static DetachedThreadLocal excluded = new DetachedThreadLocal(DetachedThreadLocal.Cleaner.INLINE) { @Override - protected Boolean initialValue() { + protected Boolean initialValue(Thread thread) { return Boolean.FALSE; } }; + private static DetachedThreadLocal scopeThreadLocal = new DetachedThreadLocal(DetachedThreadLocal.Cleaner.INLINE); - @VisibleForAdvice - public static final List requestExceptionAttributes = Arrays.asList("javax.servlet.error.exception", "exception", "org.springframework.web.servlet.DispatcherServlet.EXCEPTION", "co.elastic.apm.exception"); + private static final List 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; + static { servletTransactionHelper = new ServletTransactionHelper(tracer); + servletTransactionCreationHelper = new ServletTransactionCreationHelper(tracer); } - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnterServletService(@Advice.Argument(0) ServletRequest servletRequest, - @Advice.Local("transaction") Transaction transaction, - @Advice.Local("scope") Scope scope) { + @Nullable + @RegisterMethodHandle + public static Transaction onEnterServletService(ServletRequest servletRequest) { + int depth = CallDepth.increment(Servlet.class); if (tracer == null) { - return; + return null; } + Transaction transaction = null; // re-activate transactions for async requests final Transaction transactionAttr = (Transaction) servletRequest.getAttribute(TRANSACTION_ATTRIBUTE); - if (tracer.currentTransaction() == null && transactionAttr != null) { - scope = transactionAttr.activateInScope(); + if (depth == 0 && tracer.currentTransaction() == null && transactionAttr != null) { + scopeThreadLocal.set(transactionAttr.activateInScope()); } if (tracer.isRunning() && - servletTransactionHelper != null && servletRequest instanceof HttpServletRequest && servletRequest.getDispatcherType() == DispatcherType.REQUEST && !Boolean.TRUE.equals(excluded.get())) { @@ -108,18 +99,12 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv } final HttpServletRequest request = (HttpServletRequest) servletRequest; - if (ServletInstrumentation.servletTransactionCreationHelperManager != null) { - ServletInstrumentation.ServletTransactionCreationHelper helper = - ServletInstrumentation.servletTransactionCreationHelperManager.getForClassLoaderOfClass(HttpServletRequest.class); - if (helper != null) { - transaction = helper.createAndActivateTransaction(request); - } - } + transaction = servletTransactionCreationHelper.createAndActivateTransaction(request); if (transaction == null) { // if the request is excluded, avoid matching all exclude patterns again on each filter invocation excluded.set(Boolean.TRUE); - return; + return null; } final Request req = transaction.getContext().getRequest(); if (transaction.isSampled() && tracer.getConfig(CoreConfiguration.class).isCaptureHeaders()) { @@ -141,20 +126,23 @@ public static void onEnterServletService(@Advice.Argument(0) ServletRequest serv request.getScheme(), request.getServerName(), request.getServerPort(), request.getRequestURI(), request.getQueryString(), request.getRemoteAddr(), request.getHeader("Content-Type")); } + return transaction; } - @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) - public static void onExitServletService(@Advice.Argument(0) ServletRequest servletRequest, - @Advice.Argument(1) ServletResponse servletResponse, - @Advice.Local("transaction") @Nullable Transaction transaction, - @Advice.Local("scope") @Nullable Scope scope, - @Advice.Thrown @Nullable Throwable t, - @Advice.This Object thiz) { + @RegisterMethodHandle + public static void onExitServletService(ServletRequest servletRequest, + ServletResponse servletResponse, + @Nullable Transaction transaction, + @Nullable Throwable t, + Object thiz) { + int depth = CallDepth.decrement(Servlet.class); if (tracer == null) { return; } excluded.set(Boolean.FALSE); - if (scope != null) { + Scope scope = scopeThreadLocal.get(); + if (depth == 0 && scope != null) { + scopeThreadLocal.clear(); scope.close(); } if (thiz instanceof HttpServlet && servletRequest instanceof HttpServletRequest) { @@ -166,8 +154,7 @@ public static void onExitServletService(@Advice.Argument(0) ServletRequest servl ServletTransactionHelper.setUsernameIfUnset(userPrincipal != null ? userPrincipal.getName() : null, currentTransaction.getContext()); } } - if (servletTransactionHelper != null && - transaction != null && + if (transaction != null && servletRequest instanceof HttpServletRequest && servletResponse instanceof HttpServletResponse) { diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletInstrumentation.java index 960beb15f6..ce86db1aeb 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletInstrumentation.java @@ -24,17 +24,17 @@ */ package co.elastic.apm.agent.servlet; -import co.elastic.apm.agent.bci.HelperClassManager; -import co.elastic.apm.agent.bci.VisibleForAdvice; -import co.elastic.apm.agent.impl.ElasticApmTracer; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import co.elastic.apm.agent.impl.transaction.Transaction; +import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; @@ -57,21 +57,6 @@ public class ServletInstrumentation extends AbstractServletInstrumentation { static final String SERVLET_API = "servlet-api"; - @Nullable - @VisibleForAdvice - // referring to HttpServletRequest is legal because of type erasure - public static HelperClassManager> servletTransactionCreationHelperManager; - - public ServletInstrumentation(ElasticApmTracer tracer) { - ServletApiAdvice.init(tracer); - if (servletTransactionCreationHelperManager == null) { - servletTransactionCreationHelperManager = HelperClassManager.ForSingleClassLoader.of(tracer, - "co.elastic.apm.agent.servlet.helper.ServletTransactionCreationHelperImpl", - "co.elastic.apm.agent.servlet.helper.ServletRequestHeaderGetter" - ); - } - } - @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("Servlet").or(nameContainsIgnoreCase("jsp")); @@ -90,14 +75,36 @@ public ElementMatcher getMethodMatcher() { .and(takesArgument(1, named("javax.servlet.ServletResponse"))); } - @Override - public Class getAdviceClass() { - return ServletApiAdvice.class; + @Nullable + @Advice.OnMethodEnter(suppress = Throwable.class) + private static Transaction onEnterServletService(@Advice.Origin Class clazz, + @Advice.Argument(0) ServletRequest servletRequest) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + return (Transaction) MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onEnterServletService") + .invoke(null, servletRequest); + } else { + return (Transaction) MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onEnterServletService") + .invoke(servletRequest); + } } - @VisibleForAdvice - public interface ServletTransactionCreationHelper { - @Nullable - Transaction createAndActivateTransaction(R request); + @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class) + private static void onExitServletService(@Advice.Origin Class clazz, + @Advice.Argument(0) ServletRequest servletRequest, + @Advice.Argument(1) ServletResponse servletResponse, + @Advice.Enter @Nullable Transaction transaction, + @Advice.Thrown @Nullable Throwable t, + @Advice.This Object thiz) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onExitServletService") + .invoke(null, servletRequest, servletResponse, transaction, t, thiz); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletApiAdvice#onExitServletService") + .invoke(servletRequest, servletResponse, transaction, t, thiz); + } } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java index 21bd241e3a..c79fc384b1 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletTransactionHelper.java @@ -25,7 +25,7 @@ package co.elastic.apm.agent.servlet; import co.elastic.apm.agent.bci.ElasticApmInstrumentation; -import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.bci.RegisterMethodHandle; import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.impl.context.Request; @@ -47,21 +47,13 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import static co.elastic.apm.agent.configuration.CoreConfiguration.EventType.OFF; import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_DEFAULT; import static co.elastic.apm.agent.impl.transaction.AbstractSpan.PRIO_LOW_LEVEL_FRAMEWORK; -import static co.elastic.apm.agent.configuration.CoreConfiguration.EventType.OFF; -/** - * This class must not import classes from {@code javax.servlet} due to class loader issues. - * See https://github.com/raphw/byte-buddy/issues/465 for more information. - */ -@VisibleForAdvice public class ServletTransactionHelper { - @VisibleForAdvice public static final String TRANSACTION_ATTRIBUTE = ServletApiAdvice.class.getName() + ".transaction"; - - @VisibleForAdvice public static final String ASYNC_ATTRIBUTE = ServletApiAdvice.class.getName() + ".async"; private static final String CONTENT_TYPE_FROM_URLENCODED = "application/x-www-form-urlencoded"; @@ -74,18 +66,17 @@ public class ServletTransactionHelper { private final CoreConfiguration coreConfiguration; private final WebConfiguration webConfiguration; - @VisibleForAdvice public ServletTransactionHelper(ElasticApmTracer tracer) { this.coreConfiguration = tracer.getConfig(CoreConfiguration.class); this.webConfiguration = tracer.getConfig(WebConfiguration.class); } - // visible for testing as clearing cache is required between tests execution - static void clearServiceNameCache() { + // needed for testing as clearing cache is required between tests execution + @RegisterMethodHandle + public static void clearServiceNameCache() { nameInitialized.clear(); } - @VisibleForAdvice public static void determineServiceName(@Nullable String servletContextName, ClassLoader servletContextClassLoader, @Nullable String contextPath) { if (ElasticApmInstrumentation.tracer == null || !nameInitialized.add(contextPath == null ? "null" : contextPath)) { return; @@ -109,8 +100,7 @@ public static void determineServiceName(@Nullable String servletContextName, Cla } } - @VisibleForAdvice - public void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, + void fillRequestContext(Transaction transaction, String protocol, String method, boolean secure, String scheme, String serverName, int serverPort, String requestURI, String queryString, String remoteAddr, @Nullable String contentTypeHeader) { @@ -144,15 +134,13 @@ private void startCaptureBody(Transaction transaction, String method, @Nullable } } - @VisibleForAdvice - public static void setUsernameIfUnset(@Nullable String userName, TransactionContext context) { + static void setUsernameIfUnset(@Nullable String userName, TransactionContext context) { // only set username if not manually set if (context.getUser().getUsername() == null) { context.getUser().withUsername(userName); } } - @VisibleForAdvice public void onAfter(Transaction transaction, @Nullable Throwable exception, boolean committed, int status, boolean overrideStatusCodeOnThrowable, String method, @Nullable Map parameterMap, String servletPath, @Nullable String pathInfo, @Nullable String contentTypeHeader, boolean deactivate) { @@ -175,7 +163,7 @@ public void onAfter(Transaction transaction, @Nullable Throwable exception, bool transaction.end(); } - private void doOnAfter(Transaction transaction, @Nullable Throwable exception, boolean committed, int status, + void doOnAfter(Transaction transaction, @Nullable Throwable exception, boolean committed, int status, boolean overrideStatusCodeOnThrowable, String method, @Nullable Map parameterMap, String servletPath, @Nullable String pathInfo, @Nullable String contentTypeHeader) { fillRequestParameters(transaction, method, parameterMap, contentTypeHeader); @@ -232,7 +220,6 @@ private void fillRequestParameters(Transaction transaction, String method, @Null } } - @VisibleForAdvice public boolean captureParameters(String method, @Nullable String contentTypeHeader) { return contentTypeHeader != null && contentTypeHeader.startsWith(CONTENT_TYPE_FROM_URLENCODED) @@ -316,8 +303,7 @@ private String getHttpVersion(String protocol) { } } - @VisibleForAdvice - public static void setTransactionNameByServletClass(@Nullable String method, @Nullable Class servletClass, Transaction transaction) { + static void setTransactionNameByServletClass(@Nullable String method, @Nullable Class servletClass, Transaction transaction) { if (servletClass == null) { return; } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletVersionInstrumentation.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletVersionInstrumentation.java index 11e920502c..1439521352 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletVersionInstrumentation.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/ServletVersionInstrumentation.java @@ -25,7 +25,8 @@ package co.elastic.apm.agent.servlet; import co.elastic.apm.agent.bci.ElasticApmInstrumentation; -import co.elastic.apm.agent.bci.VisibleForAdvice; +import co.elastic.apm.agent.bci.RegisterMethodHandle; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.NamedElement; import net.bytebuddy.description.method.MethodDescription; @@ -60,12 +61,6 @@ */ public abstract class ServletVersionInstrumentation extends ElasticApmInstrumentation { - @VisibleForAdvice - public static final Logger logger = LoggerFactory.getLogger(ServletVersionInstrumentation.class); - - @VisibleForAdvice - public static volatile boolean alreadyLogged = false; - @Override public ElementMatcher getTypeMatcherPreFilter() { return nameContains("Servlet").or(nameContainsIgnoreCase("jsp")); @@ -95,27 +90,15 @@ public ElementMatcher getMethodMatcher() { @Advice.OnMethodEnter(suppress = Throwable.class) @SuppressWarnings("Duplicates") // duplication is fine here as it allows to inline code - private static void onEnter(@Advice.Argument(0) @Nullable ServletConfig servletConfig) { - if (alreadyLogged) { - return; - } - alreadyLogged = true; - - int majorVersion = -1; - int minorVersion = -1; - String serverInfo = null; - if (servletConfig != null) { - ServletContext servletContext = servletConfig.getServletContext(); - if (null != servletContext) { - majorVersion = servletContext.getMajorVersion(); - minorVersion = servletContext.getMinorVersion(); - serverInfo = servletContext.getServerInfo(); - } - } - - logger.info("Servlet container info = {}", serverInfo); - if (majorVersion < 3) { - logger.warn("Unsupported servlet version detected: {}.{}, no Servlet transaction will be created", majorVersion, minorVersion); + private static void onEnter(@Advice.Origin Class clazz, @Advice.Argument(0) @Nullable ServletConfig servletConfig) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletVersionInstrumentation$ServletVersionHelper#warnIfVersionNotSupportedServletConfig") + .invoke(null, servletConfig); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletVersionInstrumentation$ServletVersionHelper#warnIfVersionNotSupportedServletConfig") + .invoke(servletConfig); } } } @@ -134,14 +117,37 @@ public ElementMatcher getMethodMatcher() { @Advice.OnMethodEnter(suppress = Throwable.class) @SuppressWarnings("Duplicates") // duplication is fine here as it allows to inline code - private static void onEnter(@Advice.This Servlet servlet) { + private static void onEnter(@Advice.Origin Class clazz, @Advice.This Servlet servlet) throws Throwable { + if (MethodHandleDispatcher.USE_REFLECTION) { + MethodHandleDispatcher + .getMethod(clazz, "co.elastic.apm.agent.servlet.ServletVersionInstrumentation$ServletVersionHelper#warnIfVersionNotSupportedServlet") + .invoke(null, servlet); + } else { + MethodHandleDispatcher + .getMethodHandle(clazz, "co.elastic.apm.agent.servlet.ServletVersionInstrumentation$ServletVersionHelper#warnIfVersionNotSupportedServlet") + .invoke(servlet); + } + } + } + + public static class ServletVersionHelper { + + private static final Logger logger = LoggerFactory.getLogger(ServletVersionInstrumentation.class); + + private static volatile boolean alreadyLogged = false; + + @RegisterMethodHandle + public static void warnIfVersionNotSupportedServlet(Servlet servlet) { + warnIfVersionNotSupportedServletConfig(servlet.getServletConfig()); + } + + @RegisterMethodHandle + public static void warnIfVersionNotSupportedServletConfig(@Nullable ServletConfig servletConfig) { if (alreadyLogged) { return; } alreadyLogged = true; - ServletConfig servletConfig = servlet.getServletConfig(); - int majorVersion = -1; int minorVersion = -1; String serverInfo = null; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ApmAsyncListener.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ApmAsyncListener.java index 5c2a8c717b..323eb6fa3c 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ApmAsyncListener.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ApmAsyncListener.java @@ -60,16 +60,16 @@ public class ApmAsyncListener implements AsyncListener, Recyclable { private final AtomicBoolean completed = new AtomicBoolean(false); - private final AsyncContextAdviceHelperImpl asyncContextAdviceHelperImpl; + private final AsyncContextAdviceHelper asyncContextAdviceHelper; private final ServletTransactionHelper servletTransactionHelper; @Nullable private volatile Transaction transaction; @Nullable private volatile Throwable throwable; - ApmAsyncListener(AsyncContextAdviceHelperImpl asyncContextAdviceHelperImpl) { - this.asyncContextAdviceHelperImpl = asyncContextAdviceHelperImpl; - this.servletTransactionHelper = asyncContextAdviceHelperImpl.getServletTransactionHelper(); + ApmAsyncListener(AsyncContextAdviceHelper asyncContextAdviceHelper) { + this.asyncContextAdviceHelper = asyncContextAdviceHelper; + this.servletTransactionHelper = asyncContextAdviceHelper.getServletTransactionHelper(); } ApmAsyncListener withTransaction(Transaction transaction) { @@ -141,6 +141,7 @@ public void onStartAsync(AsyncEvent event) { // because only the onExitServletService method may contain references to the servlet API // (see class-level Javadoc) private void endTransaction(AsyncEvent event) { + Transaction transaction = this.transaction; // To ensure transaction is ended only by a single event if (completed.getAndSet(true) || transaction == null) { return; @@ -175,7 +176,7 @@ private void endTransaction(AsyncEvent event) { request.getServletPath(), request.getPathInfo(), contentTypeHeader, false ); } finally { - asyncContextAdviceHelperImpl.recycle(this); + asyncContextAdviceHelper.recycle(this); } } diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelper.java similarity index 93% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelperImpl.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelper.java index f3cc7926cf..7c62e11f56 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelperImpl.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/AsyncContextAdviceHelper.java @@ -11,9 +11,9 @@ * 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 @@ -29,7 +29,6 @@ import co.elastic.apm.agent.objectpool.Allocator; import co.elastic.apm.agent.objectpool.ObjectPool; import co.elastic.apm.agent.objectpool.impl.QueueBasedObjectPool; -import co.elastic.apm.agent.servlet.AsyncInstrumentation; import co.elastic.apm.agent.servlet.ServletApiAdvice; import co.elastic.apm.agent.servlet.ServletTransactionHelper; import org.jctools.queues.atomic.AtomicQueueFactory; @@ -41,7 +40,7 @@ import static co.elastic.apm.agent.servlet.ServletTransactionHelper.TRANSACTION_ATTRIBUTE; import static org.jctools.queues.spec.ConcurrentQueueSpec.createBoundedMpmc; -public class AsyncContextAdviceHelperImpl implements AsyncInstrumentation.AsyncContextAdviceHelper { +public class AsyncContextAdviceHelper { private static final String ASYNC_LISTENER_ADDED = ServletApiAdvice.class.getName() + ".asyncListenerAdded"; private static final int MAX_POOLED_ELEMENTS = 256; @@ -50,7 +49,7 @@ public class AsyncContextAdviceHelperImpl implements AsyncInstrumentation.AsyncC private final ServletTransactionHelper servletTransactionHelper; private final ElasticApmTracer tracer; - public AsyncContextAdviceHelperImpl(ElasticApmTracer tracer) { + public AsyncContextAdviceHelper(ElasticApmTracer tracer) { this.tracer = tracer; servletTransactionHelper = new ServletTransactionHelper(tracer); @@ -67,11 +66,10 @@ ServletTransactionHelper getServletTransactionHelper() { private final class ApmAsyncListenerAllocator implements Allocator { @Override public ApmAsyncListener createInstance() { - return new ApmAsyncListener(AsyncContextAdviceHelperImpl.this); + return new ApmAsyncListener(AsyncContextAdviceHelper.this); } } - @Override public void onExitStartAsync(AsyncContext asyncContext) { final ServletRequest request = asyncContext.getRequest(); if (request.getAttribute(ASYNC_LISTENER_ADDED) != null) { diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java index 5ebb7cba2e..19e21f5b65 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/RecordingServletInputStreamWrapper.java @@ -11,9 +11,9 @@ * 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 @@ -24,7 +24,6 @@ */ package co.elastic.apm.agent.servlet.helper; -import co.elastic.apm.agent.bci.VisibleForAdvice; import co.elastic.apm.agent.impl.context.Request; import co.elastic.apm.agent.util.IOUtils; @@ -34,13 +33,11 @@ import java.nio.CharBuffer; import java.nio.charset.CoderResult; -@VisibleForAdvice public class RecordingServletInputStreamWrapper extends ServletInputStream { private final Request request; private final ServletInputStream servletInputStream; - @VisibleForAdvice public RecordingServletInputStreamWrapper(Request request, ServletInputStream servletInputStream) { this.request = request; this.servletInputStream = servletInputStream; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperImpl.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java similarity index 91% rename from apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperImpl.java rename to apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java index f260be734c..f9198cc839 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelperImpl.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/ServletTransactionCreationHelper.java @@ -29,7 +29,6 @@ import co.elastic.apm.agent.impl.context.web.WebConfiguration; import co.elastic.apm.agent.impl.transaction.Transaction; import co.elastic.apm.agent.matcher.WildcardMatcher; -import co.elastic.apm.agent.servlet.ServletInstrumentation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,21 +37,20 @@ import java.util.Objects; @SuppressWarnings("unused") -public class ServletTransactionCreationHelperImpl implements ServletInstrumentation.ServletTransactionCreationHelper { +public class ServletTransactionCreationHelper { - private static final Logger logger = LoggerFactory.getLogger(ServletTransactionCreationHelperImpl.class); + private static final Logger logger = LoggerFactory.getLogger(ServletTransactionCreationHelper.class); private final ElasticApmTracer tracer; private final CoreConfiguration coreConfiguration; private final WebConfiguration webConfiguration; - public ServletTransactionCreationHelperImpl(ElasticApmTracer tracer) { + public ServletTransactionCreationHelper(ElasticApmTracer tracer) { this.tracer = tracer; coreConfiguration = tracer.getConfig(CoreConfiguration.class); webConfiguration = tracer.getConfig(WebConfiguration.class); } - @Override @Nullable public Transaction createAndActivateTransaction(HttpServletRequest request) { Transaction transaction = null; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/package-info.java b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/package-info.java index 9ca1f1ddae..37c5ddb961 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/package-info.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/main/java/co/elastic/apm/agent/servlet/helper/package-info.java @@ -11,9 +11,9 @@ * 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 @@ -22,46 +22,6 @@ * under the License. * #L% */ -/** - *

- * The classes in this package are loaded by a class loader which is a child of the class loader the Servlet API is loaded from. - * This works around the problem that the agent does not have access to the Servlet API. - * Instead of injecting the helper classes to the same class loader the Servlet API is loaded from, - * which will most likely not be possible anymore as of Java 11 - * (see http://jdk.java.net/11/release-notes#JDK-8193033), - * we create a class loader as the child of the one which loads the Servlet API. - *

- *

- * The classes loaded by this helper class loader can then access the Servlet API. - * However, the agent itself can't directly access the classes from the helper class loader, - * because they are loaded by a child class loader. - *

- *

- * This problem is circumvented by the agent providing an interface, - * which the helper class implements. - * The agent does then not need to know about the implementation type of the interface. - *

- *

- * One thing to be aware of is that the helper class loader needs to implement child first semantics when loading classes. - * Otherwise, it would load the helper implementation from the system or bootstrap classloader (where the agent is loaded from), - * without access to the Servlet API, - * instead of loading them itself so that the helper classes can access the Servlet API. - *

- *

- * Advices have to manually add their required helper classes to {@link co.elastic.apm.agent.bci.HelperClassManager}, - * which takes care of creating the helper class loaders. - *

- * - * 
- *          System/Bootstrap CL (Agent)     provides interface AsyncContextAdviceHelper
- *                     |
- *                     v
- *             App CL (Servlet API)         uses AsyncContextAdviceHelperImpl from Helper CL
- *               /              \
- *              v               v
- * WebApp CL (user code/libs)   Helper CL   loads AsyncContextAdviceHelperImpl and ApmAsyncListener
- *
- */ @NonnullApi package co.elastic.apm.agent.servlet.helper; diff --git a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java index 18a6f3d05c..9ea1ace753 100644 --- a/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java +++ b/apm-agent-plugins/apm-servlet-plugin/src/test/java/co/elastic/apm/agent/servlet/AbstractServletTest.java @@ -25,6 +25,7 @@ package co.elastic.apm.agent.servlet; import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.bootstrap.MethodHandleDispatcher; import okhttp3.OkHttpClient; import okhttp3.Response; import org.eclipse.jetty.server.NetworkConnector; @@ -49,12 +50,7 @@ abstract class AbstractServletTest extends AbstractInstrumentationTest { protected OkHttpClient httpClient; @BeforeEach - void initServerAndClient() throws Exception { - - // because we reuse the same classloader with different servlet context names - // we need to explicitly reset the name cache to make service name detection work as expected - ServletTransactionHelper.clearServiceNameCache(); - + void initServerAndClient() throws Throwable { // server is not reused between tests as handler is provided from subclass // another alternative server = new Server(); @@ -72,6 +68,11 @@ void initServerAndClient() throws Exception { .connectTimeout(10, TimeUnit.SECONDS) .retryOnConnectionFailure(false) .build(); + + // because we reuse the same classloader with different servlet context names + // we need to explicitly reset the name cache to make service name detection work as expected + // we can't call the static method directly because the actual class is within an isolated helper class loader + MethodHandleDispatcher.getMethodHandle(getClass(), "co.elastic.apm.agent.servlet.ServletTransactionHelper#clearServiceNameCache").invoke(); } @AfterEach diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java index d050775cef..6d3f31d859 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/exception/AbstractExceptionHandlerInstrumentationTest.java @@ -75,7 +75,7 @@ public static void setUpAll() { tracer = mockInstrumentationSetup.getTracer(); config = mockInstrumentationSetup.getConfig(); ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install(), - Arrays.asList(new ServletInstrumentation(tracer), new ExceptionHandlerInstrumentation())); + Arrays.asList(new ServletInstrumentation(), new ExceptionHandlerInstrumentation())); } @AfterClass diff --git a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java index dd7f56ef83..94fb6e0b2e 100644 --- a/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java +++ b/apm-agent-plugins/apm-spring-webmvc-plugin/src/test/java/co/elastic/apm/agent/spring/webmvc/template/AbstractViewRenderingInstrumentationTest.java @@ -73,7 +73,7 @@ static void beforeAll() { reporter = mockInstrumentationSetup.getReporter(); config = mockInstrumentationSetup.getConfig(); tracer = mockInstrumentationSetup.getTracer(); - ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install(), Arrays.asList(new ServletInstrumentation(tracer), new ViewRenderInstrumentation())); + ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install(), Arrays.asList(new ServletInstrumentation(), new ViewRenderInstrumentation())); } @AfterAll diff --git a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java index c1d025ee66..7a104cb317 100644 --- a/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java +++ b/integration-tests/application-server-integration-tests/src/test/java/co/elastic/apm/servlet/TomcatIT.java @@ -11,9 +11,9 @@ * 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