Indy plugins

apm-agent-core/pom.xml

@@ -62,9 +62,14 @@
         </dependency>
         <dependency>
             <groupId>net.bytebuddy</groupId>
-            <artifactId>byte-buddy</artifactId>
+            <artifactId>byte-buddy-dep</artifactId>
             <version>${version.byte-buddy}</version>
         </dependency>
+        <dependency>
+            <groupId>org.ow2.asm</groupId>
+            <artifactId>asm-tree</artifactId>
+            <version>8.0.1</version>
+        </dependency>
         <dependency>
             <groupId>org.hdrhistogram</groupId>
             <artifactId>HdrHistogram</artifactId>
@@ -139,6 +144,18 @@
             <version>${version.cucumber}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-math3</artifactId>
+            <version>3.2</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-pool2</artifactId>
+            <version>2.6.2</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/AgentMain.java

@@ -98,7 +98,7 @@ public synchronized static void init(String agentArguments, Instrumentation inst
                 .getMethod("initialize", String.class, Instrumentation.class, File.class, boolean.class)
                 .invoke(null, agentArguments, instrumentation, agentJarFile, premain);
             System.setProperty("ElasticApm.attached", Boolean.TRUE.toString());
-        } catch (Exception e) {
+        } catch (Exception | LinkageError e) {
             System.err.println("Failed to start agent");
             e.printStackTrace();
         }
@@ -127,31 +127,41 @@ static boolean isJavaVersionSupported(String version, String vmName) {
         if (major < '7') {
             // given code is compiled with java 7, this one is unlikely in practice
             return false;
-        } else if (major == '7' || major > '8') {
-            return true;
         } else if (!vmName.contains("HotSpot(TM)")) {
             // non-hotspot JVMs are not concerned (yet)
             return true;
-        } else {
+        } else if (major == '7') {
+            // HotSpot 7
+            // versions prior to that have unreliable invoke dynamic support according to
+            // https://groovy-lang.org/indy.html
+            return isUpdateVersionAtLeast(version, 60);
+        } else if (major == '8' ){
             // HotSpot 8
-            int updateIndex = version.lastIndexOf("_");
-            if (updateIndex <= 0) {
-                // GA release '1.8.0'
-                return false;
+            return isUpdateVersionAtLeast(version, 40);
+        } else {
+            // > 8
+            return true;
+        }
+    }
+
+    private static boolean isUpdateVersionAtLeast(String version, int minimumUpdateVersion) {
+        int updateIndex = version.lastIndexOf("_");
+        if (updateIndex <= 0) {
+            // GA release '1.8.0'
+            return false;
+        } else {
+            int versionSuffixIndex = version.indexOf('-', updateIndex + 1);
+            String updateVersion;
+            if (versionSuffixIndex <= 0) {
+                updateVersion = version.substring(updateIndex + 1);
             } else {
-                int versionSuffixIndex = version.indexOf('-', updateIndex + 1);
-                String updateVersion;
-                if (versionSuffixIndex <= 0) {
-                    updateVersion = version.substring(updateIndex + 1);
-                } else {
-                    updateVersion = version.substring(updateIndex + 1, versionSuffixIndex);
-                }
-                try {
-                    return Integer.parseInt(updateVersion) >= 40;
-                } catch (NumberFormatException e) {
-                    // in case of unknown format, we just support by default
-                    return true;
-                }
+                updateVersion = version.substring(updateIndex + 1, versionSuffixIndex);
+            }
+            try {
+                return Integer.parseInt(updateVersion) >= minimumUpdateVersion;
+            } catch (NumberFormatException e) {
+                // in case of unknown format, we just support by default
+                return true;
             }
         }
     }

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java

@@ -29,9 +29,11 @@
 import co.elastic.apm.agent.bci.bytebuddy.FailSafeDeclaredMethodsCompiler;
 import co.elastic.apm.agent.bci.bytebuddy.MatcherTimer;
 import co.elastic.apm.agent.bci.bytebuddy.MinimumClassFileVersionValidator;
+import co.elastic.apm.agent.bci.bytebuddy.PatchBytecodeVersionTo51Transformer;
 import co.elastic.apm.agent.bci.bytebuddy.RootPackageCustomLocator;
 import co.elastic.apm.agent.bci.bytebuddy.SimpleMethodSignatureOffsetMappingFactory;
 import co.elastic.apm.agent.bci.bytebuddy.SoftlyReferencingTypePoolCache;
+import co.elastic.apm.agent.bci.bytebuddy.postprocessor.AssignToPostProcessorFactory;
 import co.elastic.apm.agent.bci.methodmatching.MethodMatcher;
 import co.elastic.apm.agent.bci.methodmatching.TraceMethodInstrumentation;
 import co.elastic.apm.agent.collections.WeakMapSupplier;
@@ -48,12 +50,16 @@
 import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.NamedElement;
+import net.bytebuddy.description.annotation.AnnotationDescription;
 import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.method.ParameterDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.ClassFileLocator;
 import net.bytebuddy.dynamic.DynamicType;
+import net.bytebuddy.dynamic.loading.ByteArrayClassLoader;
 import net.bytebuddy.dynamic.scaffold.TypeValidation;
 import net.bytebuddy.matcher.ElementMatcher;
+import net.bytebuddy.matcher.ElementMatchers;
 import net.bytebuddy.pool.TypePool;
 import net.bytebuddy.utility.JavaModule;
 import org.slf4j.Logger;
@@ -87,11 +93,11 @@
 import static net.bytebuddy.matcher.ElementMatchers.any;
 import static net.bytebuddy.matcher.ElementMatchers.is;
 import static net.bytebuddy.matcher.ElementMatchers.isAbstract;
+import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
+import static net.bytebuddy.matcher.ElementMatchers.isStatic;
 import static net.bytebuddy.matcher.ElementMatchers.nameContains;
-import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith;
 import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
-import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.none;
 import static net.bytebuddy.matcher.ElementMatchers.not;
 
@@ -289,6 +295,7 @@ public boolean matches(TypeDescription typeDescription, ClassLoader classLoader,
                     }
                 }
             })
+            .transform(new PatchBytecodeVersionTo51Transformer())
             .transform(getTransformer(tracer, instrumentation, logger, methodMatcher))
             .transform(new AgentBuilder.Transformer() {
                 @Override
@@ -302,12 +309,17 @@ public DynamicType.Builder<?> transform(DynamicType.Builder<?> builder, TypeDesc
     private static AgentBuilder.Transformer.ForAdvice getTransformer(final ElasticApmTracer tracer, final ElasticApmInstrumentation instrumentation, final Logger logger, final ElementMatcher<? super MethodDescription> methodMatcher) {
         Advice.WithCustomMapping withCustomMapping = Advice
             .withCustomMapping()
+            .with(new AssignToPostProcessorFactory())
             .bind(new SimpleMethodSignatureOffsetMappingFactory())
             .bind(new AnnotationValueOffsetMappingFactory());
         Advice.OffsetMapping.Factory<?> offsetMapping = instrumentation.getOffsetMapping();
         if (offsetMapping != null) {
             withCustomMapping = withCustomMapping.bind(offsetMapping);
         }
+        if (instrumentation.indyPlugin()) {
+            validateAdvice(instrumentation.getAdviceClass().getName());
+            withCustomMapping = withCustomMapping.bootstrap(IndyBootstrap.getIndyBootstrapMethod());
+        }
         return new AgentBuilder.Transformer.ForAdvice(withCustomMapping)
             .advice(new ElementMatcher<MethodDescription>() {
                 @Override
@@ -335,6 +347,50 @@ public boolean matches(MethodDescription target) {
             .withExceptionHandler(PRINTING);
     }
 
+    /**
+     * Validates invariants explained in {@link ElasticApmInstrumentation#indyPlugin()}
+     *
+     * @param adviceClassName the name of the advice class
+     */
+    private static void validateAdvice(String adviceClassName) {
+        TypePool pool = new TypePool.Default(TypePool.CacheProvider.NoOp.INSTANCE, ClassFileLocator.ForClassLoader.ofSystemLoader(), TypePool.Default.ReaderMode.FAST);
+        TypeDescription typeDescription = pool.describe(adviceClassName).resolve();
+        for (MethodDescription.InDefinedShape enterAdvice : typeDescription.getDeclaredMethods().filter(isStatic().and(isAnnotatedWith(Advice.OnMethodEnter.class)))) {
+            validateAdviceReturnAndParameterTypes(enterAdvice);
+
+            for (AnnotationDescription enter : enterAdvice.getDeclaredAnnotations().filter(ElementMatchers.annotationType(Advice.OnMethodEnter.class))) {
+                if (enter.prepare(Advice.OnMethodEnter.class).load().inline()) {
+                    throw new IllegalStateException(String.format("Indy-dispatched advice %s#%s has to be declared with inline=false", adviceClassName, enterAdvice.getName()));
+                }
+            }
+        }
+        for (MethodDescription.InDefinedShape exitAdvice : typeDescription.getDeclaredMethods().filter(isStatic().and(isAnnotatedWith(Advice.OnMethodExit.class)))) {
+            validateAdviceReturnAndParameterTypes(exitAdvice);
+            if (exitAdvice.getReturnType().getTypeName().startsWith("co.elastic.apm")) {
+                throw new IllegalStateException("Advice return type must be visible from the bootstrap class loader and must not be an agent type.");
+            }
+            for (AnnotationDescription exit : exitAdvice.getDeclaredAnnotations().filter(ElementMatchers.annotationType(Advice.OnMethodExit.class))) {
+                if (exit.prepare(Advice.OnMethodExit.class).load().inline()) {
+                    throw new IllegalStateException(String.format("Indy-dispatched advice %s#%s has to be declared with inline=false", adviceClassName, exitAdvice.getName()));
+                }
+            }
+        }
+        if (adviceClassName.startsWith("co.elastic.apm.agent.") && adviceClassName.split("\\.").length > 6) {
+            throw new IllegalStateException("Indy-dispatched advice class must be at the root of the instrumentation plugin.");
+        }
+    }
+
+    private static void validateAdviceReturnAndParameterTypes(MethodDescription.InDefinedShape advice) {
+        if (advice.getReturnType().getTypeName().startsWith("co.elastic.apm")) {
+            throw new IllegalStateException("Advice return type must not be an agent type: " + advice.toGenericString());
+        }
+        for (ParameterDescription.InDefinedShape parameter : advice.getParameters()) {
+            if (parameter.getName().startsWith("co.elastic.apm")) {
+                throw new IllegalStateException("Advice parameters must not contain an agent type: " + advice.toGenericString());
+            }
+        }
+    }
+
     private static MatcherTimer getOrCreateTimer(Class<? extends ElasticApmInstrumentation> adviceClass) {
         final String name = adviceClass.getName();
         MatcherTimer timer = matcherTimers.get(name);
@@ -386,21 +442,34 @@ public static synchronized void reset() {
         if (instrumentation == null) {
             return;
         }
-
-        if (resettableClassFileTransformer == null) {
-            throw new IllegalStateException("Reset was called before init");
+        Exception exception = null;
+        if (resettableClassFileTransformer != null) {
+            try {
+                resettableClassFileTransformer.reset(instrumentation, RedefinitionStrategy.RETRANSFORMATION);
+            } catch (Exception e) {
+                exception = e;
+            }
+            resettableClassFileTransformer = null;
         }
         dynamicallyInstrumentedClasses.clear();
-        resettableClassFileTransformer.reset(instrumentation, RedefinitionStrategy.RETRANSFORMATION);
-        resettableClassFileTransformer = null;
         for (ResettableClassFileTransformer transformer : dynamicClassFileTransformers) {
-            transformer.reset(instrumentation, RedefinitionStrategy.RETRANSFORMATION);
+            try {
+                transformer.reset(instrumentation, RedefinitionStrategy.RETRANSFORMATION);
+            } catch (Exception e) {
+                if (exception != null) {
+                    exception.addSuppressed(e);
+                } else {
+                    exception = e;
+                }
+            }
         }
         dynamicClassFileTransformers.clear();
         instrumentation = null;
+        HelperClassManager.ForIndyPlugin.clear();
     }
 
-    private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final CoreConfiguration coreConfiguration, Logger logger, AgentBuilder.DescriptionStrategy descriptionStrategy, boolean premain) {
+    private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final CoreConfiguration coreConfiguration, final Logger logger,
+                                                final AgentBuilder.DescriptionStrategy descriptionStrategy, final boolean premain) {
         AgentBuilder.LocationStrategy locationStrategy = AgentBuilder.LocationStrategy.ForClassLoader.WEAK;
         if (agentJarFile != null) {
             try {
@@ -418,6 +487,14 @@ private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final Cor
             // when runtime attaching, only retransform up to 100 classes at once and sleep 100ms in-between as retransformation causes a stop-the-world pause
             .with(premain ? RedefinitionStrategy.BatchAllocator.ForTotal.INSTANCE : RedefinitionStrategy.BatchAllocator.ForFixedSize.ofSize(100))
             .with(premain ? RedefinitionStrategy.Listener.NoOp.INSTANCE : RedefinitionStrategy.Listener.Pausing.of(100, TimeUnit.MILLISECONDS))
+            .with(new RedefinitionStrategy.Listener.Adapter() {
+                @Override
+                public Iterable<? extends List<Class<?>>> onError(int index, List<Class<?>> batch, Throwable throwable, List<Class<?>> types) {
+                    logger.warn("Error while redefining classes {}", throwable.getMessage());
+                    logger.debug(throwable.getMessage(), throwable);
+                    return super.onError(index, batch, throwable, types);
+                }
+            })
             .with(descriptionStrategy)
             .with(locationStrategy)
             .with(new ErrorLoggingListener())
@@ -426,33 +503,9 @@ private static AgentBuilder getAgentBuilder(final ByteBuddy byteBuddy, final Cor
                 ? new SoftlyReferencingTypePoolCache(TypePool.Default.ReaderMode.FAST, 1, isReflectionClassLoader())
                 : AgentBuilder.PoolStrategy.Default.FAST)
             .ignore(any(), isReflectionClassLoader())
+            // avoids instrumenting classes from helper class loaders
+            .or(any(), classLoaderWithName(ByteArrayClassLoader.ChildFirst.class.getName()))
             .or(any(), classLoaderWithName("org.codehaus.groovy.runtime.callsite.CallSiteClassLoader"))
-            // ideally, those bootstrap classpath inclusions should be set at plugin level, see issue #952
-            .or(nameStartsWith("java.")
-                .and(
-                    not(
-                        nameEndsWith("URLConnection")
-                            .or(nameStartsWith("java.util.concurrent."))
-                            .or(named("java.lang.ProcessBuilder"))
-                            .or(named("java.lang.ProcessImpl"))
-                            .or(named("java.lang.Process"))
-                            .or(named("java.lang.UNIXProcess"))
-                    )
-                )
-            )
-            .or(nameStartsWith("com.sun.")
-                .and(
-                    not(
-                        nameStartsWith("com.sun.faces.")
-                            .or(nameEndsWith("URLConnection"))
-                    )
-                )
-            )
-            .or(nameStartsWith("sun")
-                .and(
-                    not(nameEndsWith("URLConnection"))
-                )
-            )
             .or(nameStartsWith("co.elastic.apm.agent.shaded"))
             .or(nameStartsWith("org.aspectj."))
             .or(nameStartsWith("org.groovy."))