elastic · JonasKunz · Jun 5, 2023 · May 31, 2023 · May 31, 2023 · Jun 5, 2023
@@ -29,6 +29,11 @@ endif::[]
 * Added support for recording AWS lambda transactions even if the JVM crashes or runs into a timeout - {pull}3134[#3134]
 * Add extra built-in metrics: `jvm.fd.*` and `jvm.memory.pool.non_heap.*` - {pull}[#3147]
 
+[float]
+===== Bug fixes
+* Fixed classloading for OpenTelemetry dependencies in external plugins - {pull}3154[#3154]
+
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 

@@ -418,7 +418,9 @@ private static ConstantCallSite internalBootstrap(MethodHandles.Lookup lookup, S
  ClassFileLocator classFileLocator;
  List<String> pluginClasses = new ArrayList<>();
  Map<String, List<String>> requiredModuleOpens = Collections.emptyMap();
+ boolean allowOtelLookupFromParent;
  if (instrumentationClassLoader instanceof ExternalPluginClassLoader) {
+ allowOtelLookupFromParent = true;
  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
@@ -437,6 +439,7 @@ private static ConstantCallSite internalBootstrap(MethodHandles.Lookup lookup, S
  ClassFileLocator.ForJarFile.of(agentJarFile)
  );
  } else {
+ allowOtelLookupFromParent = false;
  String pluginPackage = PluginClassLoaderRootPackageCustomizer.getPluginPackageFromClassName(adviceClassName);
  pluginClasses.addAll(getClassNamesFromBundledPlugin(pluginPackage, instrumentationClassLoader));
  requiredModuleOpens = ElasticApmAgent.getRequiredPluginModuleOpens(pluginPackage);
@@ -453,7 +456,8 @@ private static ConstantCallSite internalBootstrap(MethodHandles.Lookup lookup, S
  isAnnotatedWith(named(GlobalState.class.getName()))
  // if config classes would be loaded from the plugin CL,
  // tracer.getConfig(Config.class) would return null when called from an advice as the classes are not the same
- .or(nameContains("Config").and(hasSuperType(is(ConfigurationOptionProvider.class)))));
+ .or(nameContains("Config").and(hasSuperType(is(ConfigurationOptionProvider.class)))),
+ allowOtelLookupFromParent);
  if (ElasticApmAgent.areModulesSupported() && !requiredModuleOpens.isEmpty()) {
  boolean success = addRequiredModuleOpens(requiredModuleOpens, targetClassLoader, pluginClassLoader);
  if (!success) {

@@ -19,12 +19,12 @@
 package co.elastic.apm.agent.bci;
 
 import co.elastic.apm.agent.bci.classloading.IndyPluginClassLoader;
+import co.elastic.apm.agent.sdk.logging.Logger;
+import co.elastic.apm.agent.sdk.logging.LoggerFactory;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.ClassFileLocator;
 import net.bytebuddy.matcher.ElementMatcher;
 import net.bytebuddy.pool.TypePool;
-import co.elastic.apm.agent.sdk.logging.Logger;
-import co.elastic.apm.agent.sdk.logging.LoggerFactory;
 
 import javax.annotation.Nullable;
 import java.io.IOException;
@@ -51,7 +51,8 @@ public synchronized static ClassLoader getOrCreatePluginClassLoader(@Nullable Cl
  List<String> classesToInject,
  ClassLoader agentClassLoader,
  ClassFileLocator classFileLocator,
- ElementMatcher<? super TypeDescription> exclusionMatcher) throws Exception {
+ ElementMatcher<? super TypeDescription> exclusionMatcher,
+ boolean allowOtelClassesFromAgentCl) throws Exception {
  classesToInject = new ArrayList<>(classesToInject);
 
  Map<Collection<String>, WeakReference<ClassLoader>> injectedClasses = getOrCreateInjectedClasses(targetClassLoader);
@@ -83,7 +84,7 @@ public synchronized static ClassLoader getOrCreatePluginClassLoader(@Nullable Cl
 
  Map<String, byte[]> typeDefinitions = getTypeDefinitions(classesToInjectCopy, classFileLocator);
  // child first semantics are important here as the plugin CL contains classes that are also present in the agent CL
- ClassLoader pluginClassLoader = new IndyPluginClassLoader(targetClassLoader, agentClassLoader, typeDefinitions);
+ ClassLoader pluginClassLoader = new IndyPluginClassLoader(targetClassLoader, agentClassLoader, typeDefinitions, allowOtelClassesFromAgentCl);
  injectedClasses.put(classesToInject, new WeakReference<>(pluginClassLoader));
 
  return pluginClassLoader;

@@ -143,9 +143,15 @@ public Enumeration<URL> getResources(String name) throws IOException {
 
  @Override
  public String toString() {
+ ClassLoader targetClassLoader = null;
+ ElementMatcher<String> targetDiscriminator = null;
+ if (parents.size() > 1) {
+ targetClassLoader = parents.get(1);
+ targetDiscriminator = discriminators.get(1);
+ }
  return "DiscriminatingMultiParentClassLoader{" +
- "agentClassLoader = " + parents.get(0) + " discriminator = "+ discriminators.get(0) +
- ", targetClassLoader =" + parents.get(1) + " discriminator = " + discriminators.get(1) +
+ "agentClassLoader = " + parents.get(0) + " discriminator = " + discriminators.get(0) +
+ ", targetClassLoader =" + targetClassLoader + " discriminator = " + targetDiscriminator +
  '}';
  }
 }
@@ -41,9 +41,9 @@ public class IndyPluginClassLoader extends ByteArrayClassLoader.ChildFirst {
 
  private static final ClassLoader SYSTEM_CLASS_LOADER = ClassLoader.getSystemClassLoader();
 
- public IndyPluginClassLoader(@Nullable ClassLoader targetClassLoader, ClassLoader agentClassLoader, Map<String, byte[]> typeDefinitions) {
+ public IndyPluginClassLoader(@Nullable ClassLoader targetClassLoader, ClassLoader agentClassLoader, Map<String, byte[]> typeDefinitions, boolean allowOtelClassesFromAgentCl) {
  // See getResource on why we're using PersistenceHandler.LATENT over PersistenceHandler.MANIFEST
- super(getParent(targetClassLoader, agentClassLoader),
+ super(getParent(targetClassLoader, agentClassLoader, allowOtelClassesFromAgentCl),
  true,
  typeDefinitions,
  PrivilegedActionUtils.getProtectionDomain(agentClassLoader.getClass()), // inherit protection domain from agent CL
@@ -52,7 +52,7 @@ public IndyPluginClassLoader(@Nullable ClassLoader targetClassLoader, ClassLoade
  }
 
 
- private static ClassLoader getParent(@Nullable ClassLoader targetClassLoader, ClassLoader agentClassLoader) {
+ private static ClassLoader getParent(@Nullable ClassLoader targetClassLoader, ClassLoader agentClassLoader, boolean allowOtelClassesFromAgentCl) {
  if (targetClassLoader == null) {
  // the MultipleParentClassLoader doesn't support null values
  // the agent class loader already has the bootstrap class loader as the parent
@@ -76,12 +76,15 @@ agentClassLoader, startsWith("co.elastic.apm.agent").or(startsWith("net.bytebudd
  // The list of packages not to load should correspond with matching dependency exclusions from the apm-agent-core in apm-agent-plugins/pom.xml
  // As we're using a custom logging facade, plugins don't need to refer to the agent-bundled log4j2 or slf4j.
  // io.opentelemetry is blocked to hide the embedded OpenTelemetry metrics SDK from instrumentation plugins
+ ElementMatcher.Junction<String> agentClassesFilter = not(startsWith("org.apache.logging.log4j"))
+ .and(not(startsWith("org.slf4j")))
+ .and(not(startsWith("co.elastic.logging.log4j2")));
+ if (!allowOtelClassesFromAgentCl) {
+ agentClassesFilter = agentClassesFilter.and(not(startsWith("io.opentelemetry.")));
+ }
  return new DiscriminatingMultiParentClassLoader(
  agentClassLoader,
- not(startsWith("org.apache.logging.log4j"))
- .and(not(startsWith("org.slf4j")))
- .and(not(startsWith("co.elastic.logging.log4j2")))
- .and(not(startsWith("io.opentelemetry."))),
+ agentClassesFilter,
  targetClassLoader, ElementMatchers.<String>any());
  }
  }

@@ -44,10 +44,19 @@
  <build>
  <finalName>${project.artifactId}</finalName>
  <plugins>
+ <plugin>
+ <artifactId>maven-jar-plugin</artifactId>
+ <configuration>
+ <archive>
+ <manifestEntries>
+ <Main-Class>testapp.Main</Main-Class>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </plugin>
  <plugin>
  <artifactId>maven-assembly-plugin</artifactId>
  <configuration>
- <appendAssemblyId>false</appendAssemblyId>
  <archive>
  <manifest>
  <mainClass>testapp.Main</mainClass>

@@ -23,6 +23,8 @@
 
 public class Main {
 
+ private static final boolean OTEL_ON_CLASSPATH = isOtelOnClassPath();
+
  public static void main(String[] args) {
  System.out.println("app start");
  try {
@@ -48,11 +50,33 @@ private static void span() {
  }
 
  private static void checkCurrentSpanVisibleThroughOTel() {
- SpanContext spanContext = Span.current().getSpanContext();
- if (!spanContext.isValid()) {
- System.out.println("no active OTel Span");
- } else {
- System.out.printf("active span ID = %s, trace ID = %s%n", spanContext.getSpanId(), spanContext.getTraceId());
+ if (OTEL_ON_CLASSPATH) {
+ //Why is this wrapping in a Runnable required?
+ //Main is written to be executed with and without Otel on the classpath
+ //If Main would contain direct references to Otel, it would fail to load when running without
+ // Otel on the classpath.
+ //This problem is prevented by doing the actual OTel accesses in an anonymous class, which is
+ // never loaded when Otel is not on the classpath.
+ new Runnable() {
+ @Override
+ public void run() {
+ SpanContext spanContext = Span.current().getSpanContext();
+ if (!spanContext.isValid()) {
+ System.out.println("no active OTel Span");
+ } else {
+ System.out.printf("active span ID = %s, trace ID = %s%n", spanContext.getSpanId(), spanContext.getTraceId());
+ }
+ }
+ }.run();
+ }
+ }
+
+ private static boolean isOtelOnClassPath() {
+ try {
+ Class.forName("io.opentelemetry.api.trace.SpanContext");
+ return true;
+ } catch (ClassNotFoundException e) {
+ return false;
  }
  }
 

@@ -18,7 +18,8 @@
  */
 
 import co.elastic.apm.agent.test.AgentFileAccessor;
-import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.testcontainers.Testcontainers;
 import org.testcontainers.containers.GenericContainer;
 import org.testcontainers.containers.wait.strategy.Wait;
@@ -29,18 +30,21 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class ExternalPluginOTelIT {
 
  private static final String DOCKER_IMAGE = "openjdk:11";
 
- @Test
- void runAppWithTwoExternalPlugins() {
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void runAppWithTwoExternalPlugins(boolean withOtelInApp) {
 
  // remote debug port for container, IDE should be listening to this port
  int debugPort = 5005;
@@ -62,9 +66,10 @@ void runAppWithTwoExternalPlugins() {
  .append("--wait")
  .toString();
 
+ String appJarPath = "target/external-plugin-otel-test-app" + (withOtelInApp ? "-jar-with-dependencies.jar" : ".jar");
  GenericContainer<?> app = new GenericContainer<>(DockerImageName.parse(DOCKER_IMAGE))
  .withCopyFileToContainer(MountableFile.forHostPath(AgentFileAccessor.getPathToJavaagent()), agentJar)
- .withCopyFileToContainer(MountableFile.forHostPath("target/external-plugin-otel-test-app.jar"), appJar)
+ .withCopyFileToContainer(MountableFile.forHostPath(appJarPath), appJar)
  .withCopyFileToContainer(MountableFile.forHostPath("../external-plugin-otel-test-plugin1/target/external-plugin-otel-test-plugin1.jar"), "/tmp/plugins/plugin1.jar")
  .withCopyFileToContainer(MountableFile.forHostPath("../external-plugin-otel-test-plugin2/target/external-plugin-otel-test-plugin2.jar"), "/tmp/plugins/plugin2.jar")
  .withCommand(cmd)
@@ -87,37 +92,44 @@ void runAppWithTwoExternalPlugins() {
  String transactionId = null;
  String spanId = null;
  String traceId = null;
- List<String> otelApiLines = logLines.stream().filter(l -> l.startsWith("active span ID =")).collect(Collectors.toList());
- assertThat(otelApiLines).hasSize(3);
- // first and last should be within transaction thus equal
- assertThat(otelApiLines.get(0)).isEqualTo(otelApiLines.get(2));
-
- Pattern idPattern = Pattern.compile("active span ID = ([a-z0-9]+), trace ID = ([a-z0-9]+)");
- Matcher matcher = idPattern.matcher(otelApiLines.get(0));
- assertThat(matcher.matches()).isTrue();
- assertThat(matcher.groupCount()).isEqualTo(2);
- transactionId = matcher.group(1);
- traceId = matcher.group(2);
- matcher = idPattern.matcher(otelApiLines.get(1));
- assertThat(matcher.matches()).isTrue();
- assertThat(matcher.groupCount()).isEqualTo(2);
- spanId = matcher.group(1);
-
- assertThat(logLines).containsExactly(
+
+ if (withOtelInApp) {
+ List<String> otelApiLines = logLines.stream().filter(l -> l.startsWith("active span ID =")).collect(Collectors.toList());
+ assertThat(otelApiLines).hasSize(3);
+ // first and last should be within transaction thus equal
+ assertThat(otelApiLines.get(0)).isEqualTo(otelApiLines.get(2));
+
+ Pattern idPattern = Pattern.compile("active span ID = ([a-z0-9]+), trace ID = ([a-z0-9]+)");
+ Matcher matcher = idPattern.matcher(otelApiLines.get(0));
+ assertThat(matcher.matches()).isTrue();
+ assertThat(matcher.groupCount()).isEqualTo(2);
+ transactionId = matcher.group(1);
+ traceId = matcher.group(2);
+ matcher = idPattern.matcher(otelApiLines.get(1));
+ assertThat(matcher.matches()).isTrue();
+ assertThat(matcher.groupCount()).isEqualTo(2);
+ spanId = matcher.group(1);
+ }
+
+ List<String> expectedLogs = Stream.of(
  "app start",
  ">> transaction enter", // added by plugin1
  ">> gauge class co.elastic.apm.agent.opentelemetry.metrics.bridge.v1_14.BridgeObservableLongGauge",
+ ">> otel class co.elastic.apm.agent.opentelemetry.global.ElasticOpenTelemetryWithMetrics",
  "start transaction",
- String.format("active span ID = %s, trace ID = %s", transactionId, traceId), // app OTel API
+ withOtelInApp ? String.format("active span ID = %s, trace ID = %s", transactionId, traceId) : null, // app OTel API
  ">> span enter", // added by plugin2
  "start span",
- String.format("active span ID = %s, trace ID = %s", spanId, traceId), // app OTel API
+ withOtelInApp ? String.format("active span ID = %s, trace ID = %s", spanId, traceId) : null, // app OTel API
  "end span",
  "<< span exit", // added by plugin2
- String.format("active span ID = %s, trace ID = %s", transactionId, traceId), // app OTel API
+ withOtelInApp ? String.format("active span ID = %s, trace ID = %s", transactionId, traceId) : null, // app OTel API
  "end transaction",
  "<< transaction exit", // added by plugin1
- "app end");
+ "app end"
+ ).filter(Objects::nonNull).collect(Collectors.toList());
+
+ assertThat(logLines).containsExactlyElementsOf(expectedLogs);
 
  } finally {
 

@@ -20,6 +20,7 @@
 
 import co.elastic.apm.agent.sdk.ElasticApmInstrumentation;
 import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.metrics.ObservableLongGauge;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanKind;
@@ -62,11 +63,13 @@ public static class AdviceClass {
 
  @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
  public static Object onEnter() {
+ OpenTelemetry otel = GlobalOpenTelemetry.get();
 
  System.out.println(">> transaction enter");
  System.out.println(">> gauge class " + gauge.getClass().getName());
+ System.out.println(">> otel class " + otel.getClass().getName());
 
- Tracer tracer = GlobalOpenTelemetry.get().getTracer("plugin1");
+ Tracer tracer = otel.getTracer("plugin1");
  Span span = tracer.spanBuilder("transaction").setSpanKind(SpanKind.SERVER).startSpan();
  return span.makeCurrent();
  }