Fix slf4j LinkageErrors for external plugins

.github/dependabot.yml

@@ -32,7 +32,11 @@ updates:
       - dependency-name: "com.datastax.cassandra:cassandra-driver-core"
       - dependency-name: "com.datastax.oss:java-driver-core"
       - dependency-name: "io.micrometer:*"
+      - dependency-name: "jakarta.*:*"
     ignore:
       - dependency-name: "net.bytebuddy:byte-buddy-agent"
         # We deliberately want to keep this older version of Byte Buddy for our runtime attach test
         versions: ["<=1.9.16"]
+      - dependency-name: "org.slf4j:slf4j-api"
+        # A static arbitrary version used within our external plugin test
+        versions: ["<=1.7.25"]

apm-agent-common/src/main/java/co/elastic/apm/agent/common/util/AgentInfo.java

@@ -0,0 +1,67 @@
+/*
+ * 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.
+ */
+package co.elastic.apm.agent.common.util;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+public class AgentInfo {
+
+    private static final Set<String> dependencyPackages = new HashSet<>(Arrays.asList(
+        "org.slf4j",
+        "org.apache.logging",
+        "net.bytebuddy",
+        "org.objectweb.asm",
+        "org.jctools" ,
+        "org.stagemonitor",
+        "org.HdrHistogram",
+        "co.elastic.logging",
+        "com.blogspot.mydailyjava.weaklockfree",
+        "com.lmax.disruptor",
+        "com.dslplatform.json",
+        "com.googlecode.concurrentlinkedhashmap"
+    ));
+
+    private static final Set<String> agentRootPackages = new HashSet<>(Arrays.asList(
+        "co.elastic.apm",
+        "bootstrap.co.elastic.apm",
+        "bootstrap.java.lang"
+    ));
+
+    /**
+     * Returns a list of packages of dependencies used by the agent.
+     * Should be updated manually whenever a new dependency is added to the agent code.
+     * See {@code co.elastic.apm.agent.premain.AgentPackagingIT#validateDependencyPackages()}.
+     * @return a list of root packages of dependencies used by the agent.
+     */
+    public static Set<String> getAgentDependencyPackages() {
+        return dependencyPackages;
+    }
+
+    /**
+     * Returns a list of root packages of agent classes.
+     * Should be updated manually whenever a new root is added to the agent code.
+     * See {@code co.elastic.apm.agent.premain.AgentPackagingIT#validateDependencyPackages()}.
+     * @return a list of agent root packages.
+     */
+    public static Set<String> getAgentRootPackages() {
+        return agentRootPackages;
+    }
+}

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

@@ -195,12 +195,12 @@ public boolean accept(File dir, String name) {
         }
         List<ClassLoader> result = new ArrayList<>(pluginJars.length);
         for (File pluginJar : pluginJars) {
+            logger.info("Loading plugin {}", pluginJar.getName());
             try {
                 result.add(new ExternalPluginClassLoader(pluginJar, ElasticApmAgent.class.getClassLoader()));
-            } catch (IOException e) {
-                logger.error(e.getMessage(), e);
+            } catch (Exception e) {
+                logger.error("Error loading external plugin", e);
             }
-            logger.info("Loading plugin {}", pluginJar.getName());
         }
         return result;
     }

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

@@ -207,6 +207,7 @@ public class IndyBootstrap {
      * Caches the names of classes that are defined within a package and it's subpackages
      */
     private static final ConcurrentMap<String, List<String>> classesByPackage = new ConcurrentHashMap<>();
+
     @Nullable
     static Method indyBootstrapMethod;
 
@@ -362,10 +363,19 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup,
             MethodHandle instrumentedMethod = args.length >= 5 ? (MethodHandle) args[4] : null;
 
             ClassLoader instrumentationClassLoader = ElasticApmAgent.getInstrumentationClassLoader(adviceClassName);
+            ClassLoader targetClassLoader = lookup.lookupClass().getClassLoader();
             ClassFileLocator classFileLocator;
             List<String> pluginClasses = new ArrayList<>();
             if (instrumentationClassLoader instanceof ExternalPluginClassLoader) {
-                pluginClasses.addAll(((ExternalPluginClassLoader) instrumentationClassLoader).getClassNames());
+                List<String> externalPluginClasses = ((ExternalPluginClassLoader) instrumentationClassLoader).getClassNames();
+                for (String externalPluginClass : externalPluginClasses) {
+                    if (// API classes have no dependencies and don't need to be loaded by an IndyPluginCL
+                        !(externalPluginClass.startsWith("co.elastic.apm.api")) &&
+                        !(externalPluginClass.startsWith("co.elastic.apm.opentracing"))
+                    ) {
+                        pluginClasses.add(externalPluginClass);
+                    }
+                }
                 File agentJarFile = ElasticApmAgent.getAgentJarFile();
                 if (agentJarFile == null) {
                     throw new IllegalStateException("External plugin cannot be applied - can't find agent jar");
@@ -380,7 +390,7 @@ public static ConstantCallSite bootstrap(MethodHandles.Lookup lookup,
             }
             pluginClasses.add(LOOKUP_EXPOSER_CLASS_NAME);
             ClassLoader pluginClassLoader = IndyPluginClassLoaderFactory.getOrCreatePluginClassLoader(
-                lookup.lookupClass().getClassLoader(),
+                targetClassLoader,
                 pluginClasses,
                 // we provide the instrumentation class loader as the agent class loader, but it could actually be an
                 // ExternalPluginClassLoader, of which parent is the agent class loader, so this works as well.

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

@@ -22,6 +22,7 @@
 import co.elastic.apm.agent.matcher.WildcardMatcher;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
+import co.elastic.apm.agent.util.ClassLoaderUtils;
 import co.elastic.apm.agent.util.Version;
 import net.bytebuddy.description.NamedElement;
 import net.bytebuddy.description.annotation.AnnotationSource;
@@ -54,7 +55,7 @@ public class CustomElementMatchers {
     private static final ElementMatcher.Junction.AbstractBase<ClassLoader> AGENT_CLASS_LOADER_MATCHER = new ElementMatcher.Junction.AbstractBase<ClassLoader>() {
         @Override
         public boolean matches(@Nullable ClassLoader classLoader) {
-            return classLoader != null && classLoader.getClass().getName().startsWith("co.elastic.apm");
+            return ClassLoaderUtils.isAgentClassLoader(classLoader);
         }
     };
 

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

@@ -18,8 +18,10 @@
  */
 package co.elastic.apm.agent.bci.classloading;
 
+import co.elastic.apm.agent.common.util.AgentInfo;
 import co.elastic.apm.agent.configuration.CoreConfiguration;
 import co.elastic.apm.agent.sdk.ElasticApmInstrumentation;
+import co.elastic.apm.agent.sdk.logging.LoggerFactory;
 
 import java.io.File;
 import java.io.IOException;
@@ -44,7 +46,8 @@ public ExternalPluginClassLoader(File pluginJar, ClassLoader agentClassLoader) t
         super(new URL[]{pluginJar.toURI().toURL()}, agentClassLoader);
         classNames = Collections.unmodifiableList(scanForClasses(pluginJar));
         if (classNames.contains(ElasticApmInstrumentation.class.getName())) {
-            throw new IllegalStateException("The plugin %s contains the plugin SDK. Please make sure the scope for the dependency apm-agent-plugin-sdk is set to provided.");
+            throw new IllegalStateException(String.format("The plugin %s contains the plugin SDK. Please make sure the " +
+                "scope for the dependency apm-agent-plugin-sdk is set to provided.", pluginJar.getName()));
         }
     }
 
@@ -55,7 +58,27 @@ private List<String> scanForClasses(File pluginJar) throws IOException {
             while (entries.hasMoreElements()) {
                 JarEntry jarEntry = entries.nextElement();
                 if (jarEntry.getName().endsWith(".class")) {
-                    tempClassNames.add(jarEntry.getName().replace('/', '.').substring(0, jarEntry.getName().length() - 6));
+                    String fqcn = jarEntry.getName().replace('/', '.').substring(0, jarEntry.getName().length() - 6);
+                    if (fqcn.startsWith("org.slf4j") || fqcn.startsWith("org.apache.logging")) {
+                        throw new IllegalStateException(String.format("Package \"%s\" is used within plugin %s. This is not allowed " +
+                                "because it is already used by the agent. For logging purposes, use the agent SDK logging facade - %s",
+                            fqcn.substring(0, fqcn.lastIndexOf('.')),
+                            pluginJar.getName(),
+                            LoggerFactory.class.getName()
+                        ));
+                    }
+                    for (String agentDependencyPackage : AgentInfo.getAgentDependencyPackages()) {
+                        if (fqcn.startsWith(agentDependencyPackage)) {
+                            throw new IllegalStateException(String.format("Package \"%s\" is used within plugin %s. This is not allowed " +
+                                "because the same dependency is used by the agent. Please either replace the corresponding dependency or " +
+                                "make sure its scope is set to provided. See the full list of such packages in %s",
+                                fqcn.substring(0, fqcn.lastIndexOf('.')),
+                                pluginJar.getName(),
+                                AgentInfo.class.getName()
+                            ));
+                        }
+                    }
+                    tempClassNames.add(fqcn);
                 }
             }
         }

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/ElasticApmTracer.java

@@ -24,7 +24,6 @@
 import co.elastic.apm.agent.context.ClosableLifecycleListenerAdapter;
 import co.elastic.apm.agent.context.LifecycleListener;
 import co.elastic.apm.agent.impl.error.ErrorCapture;
-import co.elastic.apm.agent.impl.metadata.MetaData;
 import co.elastic.apm.agent.impl.metadata.MetaDataFuture;
 import co.elastic.apm.agent.impl.sampling.ProbabilitySampler;
 import co.elastic.apm.agent.impl.sampling.Sampler;
@@ -57,11 +56,9 @@
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.ThreadPoolExecutor;
 

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/AbstractSpan.java

@@ -390,15 +390,18 @@ public boolean isExit() {
         return isExit;
     }
 
-
-    public void captureException(long epochMicros, Throwable t) {
-        tracer.captureAndReportException(epochMicros, t, this);
+    @Nullable
+    public String captureExceptionAndGetErrorId(long epochMicros, @Nullable Throwable t) {
+        if (t != null) {
+            hasCapturedExceptions = true;
+            return tracer.captureAndReportException(epochMicros, t, this);
+        }
+        return null;
     }
 
     public T captureException(@Nullable Throwable t) {
         if (t != null) {
-            hasCapturedExceptions = true;
-            captureException(getTraceContext().getClock().getEpochMicros(), t);
+            captureExceptionAndGetErrorId(getTraceContext().getClock().getEpochMicros(), t);
         }
         return (T) this;
     }
@@ -409,7 +412,7 @@ public void endExceptionally(@Nullable Throwable t) {
 
     @Nullable
     public String captureExceptionAndGetErrorId(@Nullable Throwable t) {
-        return tracer.captureAndReportException(getTraceContext().getClock().getEpochMicros(), t, this);
+        return captureExceptionAndGetErrorId(getTraceContext().getClock().getEpochMicros(), t);
     }
 
     public void addLabel(String key, String value) {

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/TraceContext.java

@@ -26,6 +26,7 @@
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
 import co.elastic.apm.agent.util.ByteUtils;
+import co.elastic.apm.agent.util.ClassLoaderUtils;
 import co.elastic.apm.agent.util.HexUtils;
 import co.elastic.apm.agent.sdk.logging.Logger;
 import co.elastic.apm.agent.sdk.logging.LoggerFactory;
@@ -712,14 +713,15 @@ public int hashCode() {
     }
 
     void setApplicationClassLoader(@Nullable ClassLoader classLoader) {
-        if (classLoader != null) {
-            WeakReference<ClassLoader> local = classLoaderWeakReferenceCache.get(classLoader);
-            if (local == null) {
-                local = new WeakReference<>(classLoader);
-                classLoaderWeakReferenceCache.putIfAbsent(classLoader, local);
-            }
-            applicationClassLoader = local;
+        if (ClassLoaderUtils.isBootstrapClassLoader(classLoader) || ClassLoaderUtils.isAgentClassLoader(classLoader)) {
+            return;
+        }
+        WeakReference<ClassLoader> local = classLoaderWeakReferenceCache.get(classLoader);
+        if (local == null) {
+            local = new WeakReference<>(classLoader);
+            classLoaderWeakReferenceCache.putIfAbsent(classLoader, local);
         }
+        applicationClassLoader = local;
     }
 
     @Nullable

apm-agent-core/src/main/java/co/elastic/apm/agent/util/ClassLoaderUtils.java

@@ -42,4 +42,12 @@ public static boolean isPersistentClassLoader(@Nullable ClassLoader classLoader)
             return false;
         }
     }
+
+    public static boolean isAgentClassLoader(@Nullable ClassLoader classLoader) {
+        return classLoader != null && classLoader.getClass().getName().startsWith("co.elastic.apm");
+    }
+
+    public static boolean isBootstrapClassLoader(@Nullable ClassLoader classLoader) {
+        return classLoader == null;
+    }
 }

apm-agent-core/src/test/java/co/elastic/apm/agent/impl/ElasticApmTracerTest.java

@@ -27,6 +27,7 @@
 import co.elastic.apm.agent.impl.sampling.ConstantSampler;
 import co.elastic.apm.agent.impl.stacktrace.StacktraceConfiguration;
 import co.elastic.apm.agent.impl.transaction.AbstractSpan;
+import co.elastic.apm.agent.impl.transaction.Outcome;
 import co.elastic.apm.agent.impl.transaction.Span;
 import co.elastic.apm.agent.impl.transaction.TraceContext;
 import co.elastic.apm.agent.impl.transaction.Transaction;
@@ -484,6 +485,7 @@ void testCaptureExceptionAndGetErrorId() {
         Transaction transaction = startTestRootTransaction();
         String errorId = transaction.captureExceptionAndGetErrorId(new Exception("test"));
         transaction.end();
+        assertThat(transaction.getOutcome()).isEqualTo(Outcome.FAILURE);
 
         assertThat(reporter.getErrors()).hasSize(1);
         assertThat(errorId).isNotNull();

apm-agent-plugins/apm-api-plugin/src/test/java/co/elastic/apm/agent/pluginapi/TransactionInstrumentationTest.java

@@ -255,6 +255,8 @@ public void testGetErrorIdWithTransactionCaptureException() {
         }
         endTransaction();
         assertThat(errorId).isNotNull();
+        assertThat(reporter.getTransactions()).hasSize(1);
+        assertThat(reporter.getFirstTransaction().getOutcome()).isEqualTo(convertOutcome(Outcome.FAILURE));
     }
 
     @Test
@@ -271,6 +273,10 @@ public void testGetErrorIdWithSpanCaptureException() {
         }
         endTransaction();
         assertThat(errorId).isNotNull();
+        assertThat(reporter.getTransactions()).hasSize(1);
+        assertThat(reporter.getFirstTransaction().getOutcome()).isEqualTo(convertOutcome(Outcome.SUCCESS));
+        assertThat(reporter.getSpans()).hasSize(1);
+        assertThat(reporter.getFirstSpan().getOutcome()).isEqualTo(convertOutcome(Outcome.FAILURE));
     }
 
     @Test
@@ -308,4 +314,8 @@ private void endTransaction() {
         transaction.end();
         assertThat(reporter.getTransactions()).hasSize(1);
     }
+
+    private co.elastic.apm.agent.impl.transaction.Outcome convertOutcome(Outcome apiOutcome) {
+        return co.elastic.apm.agent.impl.transaction.Outcome.valueOf(apiOutcome.toString());
+    }
 }

apm-agent-plugins/apm-opentracing-plugin/src/main/java/co/elastic/apm/agent/opentracingimpl/ApmSpanInstrumentation.java

@@ -125,7 +125,7 @@ public static void log(@Advice.FieldValue(value = "dispatcher", typing = Assigne
                         final Object errorObject = fields.get("error.object");
                         if (errorObject instanceof Throwable) {
                             if (epochTimestampMicros > 0) {
-                                span.captureException(epochTimestampMicros, (Throwable) errorObject);
+                                span.captureExceptionAndGetErrorId(epochTimestampMicros, (Throwable) errorObject);
                             } else {
                                 span.captureException((Throwable) errorObject);
                             }