make rmi instrumentation indy-compatible + add module opener

instrumentation/rmi/javaagent/build.gradle.kts

@@ -34,14 +34,5 @@ tasks {
   }
   withType<Test>().configureEach {
     jvmArgs("-Djava.rmi.server.hostname=127.0.0.1")
-
-    // Can only export on Java 9+
-    val testJavaVersion =
-      gradle.startParameter.projectProperties.get("testJavaVersion")?.let(JavaVersion::toVersion)
-        ?: JavaVersion.current()
-    if (testJavaVersion.isJava9Compatible) {
-      jvmArgs("--add-exports=java.rmi/sun.rmi.server=ALL-UNNAMED")
-      jvmArgs("--add-exports=java.rmi/sun.rmi.transport=ALL-UNNAMED")
-    }
   }
 }

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/RmiContextPropagationInstrumentationModule.java

@@ -10,25 +10,31 @@
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import io.opentelemetry.javaagent.instrumentation.rmi.context.client.RmiClientContextInstrumentation;
 import io.opentelemetry.javaagent.instrumentation.rmi.context.server.RmiServerContextInstrumentation;
+import java.rmi.Remote;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import net.bytebuddy.utility.JavaModule;
 
 @AutoService(InstrumentationModule.class)
-public class RmiContextPropagationInstrumentationModule extends InstrumentationModule {
+public class RmiContextPropagationInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
   public RmiContextPropagationInstrumentationModule() {
     super("rmi", "rmi-context-propagation");
   }
 
   @Override
-  public boolean isIndyModule() {
-    // java.lang.IllegalAccessError: class
-    // io.opentelemetry.javaagent.instrumentation.rmi.context.client.RmiClientContextInstrumentation$StreamRemoteCallConstructorAdvice (in unnamed module @0x740ee00f) cannot access class sun.rmi.transport.Connection (in module java.rmi) because module java.rmi does not export sun.rmi.transport to unnamed module @0x740ee00f
-    return false;
+  public List<TypeInstrumentation> typeInstrumentations() {
+    return asList(new RmiClientContextInstrumentation(), new RmiServerContextInstrumentation());
   }
 
   @Override
-  public List<TypeInstrumentation> typeInstrumentations() {
-    return asList(new RmiClientContextInstrumentation(), new RmiServerContextInstrumentation());
+  public Map<JavaModule, List<String>> jpmsModulesToOpen() {
+    return Collections.singletonMap(
+        JavaModule.ofType(Remote.class), Arrays.asList("sun.rmi.server", "sun.rmi.transport"));
   }
 }

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/client/RmiClientContextInstrumentation.java

@@ -13,19 +13,13 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.util.VirtualField;
-import io.opentelemetry.javaagent.bootstrap.AgentClassLoader;
-import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
 import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import java.lang.instrument.Instrumentation;
 import java.rmi.server.ObjID;
-import java.util.Collections;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.dynamic.loading.ClassInjector;
 import net.bytebuddy.matcher.ElementMatcher;
-import net.bytebuddy.utility.JavaModule;
 import sun.rmi.transport.Connection;
 
 /**
@@ -63,30 +57,6 @@ public void transform(TypeTransformer transformer) {
             .and(takesArgument(0, named("sun.rmi.transport.Connection")))
             .and(takesArgument(1, named("java.rmi.server.ObjID"))),
         getClass().getName() + "$StreamRemoteCallConstructorAdvice");
-
-    // expose sun.rmi.transport.StreamRemoteCall to helper classes
-    transformer.applyTransformer(
-        (builder, typeDescription, classLoader, javaModule, protectionDomain) -> {
-          if (JavaModule.isSupported()
-              && classLoader == null
-              && "sun.rmi.transport.StreamRemoteCall".equals(typeDescription.getName())
-              && javaModule != null) {
-            Instrumentation instrumentation = InstrumentationHolder.getInstrumentation();
-            ClassInjector.UsingInstrumentation.redefineModule(
-                instrumentation,
-                javaModule,
-                Collections.emptySet(),
-                Collections.emptyMap(),
-                Collections.singletonMap(
-                    "sun.rmi.transport",
-                    // AgentClassLoader is in unnamed module of the bootstrap class loader which is
-                    // where helper classes are also
-                    Collections.singleton(JavaModule.ofType(AgentClassLoader.class))),
-                Collections.emptySet(),
-                Collections.emptyMap());
-          }
-          return builder;
-        });
   }
 
   @SuppressWarnings("unused")

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java

@@ -11,6 +11,8 @@
 import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import net.bytebuddy.utility.JavaModule;
 
 /**
  * This class is internal and is hence not for public use. Its APIs are unstable and can change at
@@ -61,4 +63,19 @@ default String getModuleGroup() {
   default List<String> agentPackagesToHide() {
     return Collections.emptyList();
   }
+
+  /**
+   * Some instrumentation need to access JPMS modules that are not accessible by default, this
+   * method provides a way to access those classes like the "--add-opens" JVM command. <br>
+   * Map key is the name of a "witness class" belonging to the module that will be loaded and used
+   * to get a reference to the module. <br>
+   * Map value is a list of packages to open in the module
+   *
+   * @return map of module to open as key, list of packages as value.
+   */
+  // TODO: when moving this method outside of experimental API, we need to decide using JavaModule
+  // instance or a class FQN in the map entry, as it could lead to some limitations
+  default Map<JavaModule, List<String>> jpmsModulesToOpen() {
+    return Collections.emptyMap();
+  }
 }

javaagent-tooling/javaagent-tooling-java9/src/main/java/io/opentelemetry/javaagent/tooling/ModuleOpener.java

@@ -0,0 +1,84 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling;
+
+import static java.util.logging.Level.FINE;
+import static java.util.logging.Level.WARNING;
+
+import io.opentelemetry.javaagent.bootstrap.AgentClassLoader;
+import java.lang.instrument.Instrumentation;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.logging.Logger;
+import javax.annotation.Nullable;
+import net.bytebuddy.description.type.PackageDescription;
+import net.bytebuddy.dynamic.loading.ClassInjector;
+import net.bytebuddy.utility.JavaModule;
+
+/**
+ * Module opener provides ability to open JPMS modules and allows instrumentation classloader to
+ * access module contents without requiring JVM arguments modification. <br>
+ * Usage of this class must be guarded with an {@code net.bytebuddy.utility.JavaModule#isSupported}
+ * check as it's compiled for Java 9+, otherwise an {@link UnsupportedClassVersionError} will be
+ * thrown for java 8.
+ */
+public class ModuleOpener {
+
+  private static final Logger logger = Logger.getLogger(ModuleOpener.class.getName());
+
+  // AgentClassLoader is in unnamed module of the bootstrap loader
+  private static final JavaModule UNNAMED_BOOT_MODULE = JavaModule.ofType(AgentClassLoader.class);
+
+  private ModuleOpener() {}
+
+  /**
+   * Opens JPMS module to a class loader unnamed module
+   *
+   * @param targetModule target module
+   * @param openTo class loader to open module for, {@literal null} to use the unnamed module of
+   *     bootstrap classloader.
+   * @param packagesToOpen packages to open
+   */
+  public static void open(
+      Instrumentation instrumentation,
+      JavaModule targetModule,
+      @Nullable ClassLoader openTo,
+      Collection<String> packagesToOpen) {
+
+    JavaModule openToModule =
+        openTo != null ? JavaModule.of(openTo.getUnnamedModule()) : UNNAMED_BOOT_MODULE;
+    Set<JavaModule> openToModuleSet = Collections.singleton(openToModule);
+    Map<String, Set<JavaModule>> missingOpens = new HashMap<>();
+    for (String packageName : packagesToOpen) {
+      if (!targetModule.isOpened(new PackageDescription.Simple(packageName), openToModule)) {
+        missingOpens.put(packageName, openToModuleSet);
+        logger.log(
+            FINE,
+            "Exposing package '{0}' in module '{1}' to module '{2}'",
+            new Object[] {packageName, targetModule, openToModule});
+      }
+    }
+    if (missingOpens.isEmpty()) {
+      return;
+    }
+
+    try {
+      ClassInjector.UsingInstrumentation.redefineModule(
+          instrumentation,
+          targetModule,
+          Collections.emptySet(),
+          Collections.emptyMap(),
+          missingOpens,
+          Collections.emptySet(),
+          Collections.emptyMap());
+    } catch (Exception e) {
+      logger.log(WARNING, "Failed to redefine module '" + targetModule.getActualName() + "'", e);
+    }
+  }
+}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java

@@ -17,6 +17,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
 import io.opentelemetry.javaagent.tooling.HelperClassDefinition;
 import io.opentelemetry.javaagent.tooling.HelperInjector;
+import io.opentelemetry.javaagent.tooling.ModuleOpener;
 import io.opentelemetry.javaagent.tooling.TransformSafeLogger;
 import io.opentelemetry.javaagent.tooling.Utils;
 import io.opentelemetry.javaagent.tooling.bytebuddy.LoggingFailSafeMatcher;
@@ -36,11 +37,13 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
 import net.bytebuddy.agent.builder.AgentBuilder;
 import net.bytebuddy.description.annotation.AnnotationSource;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
+import net.bytebuddy.utility.JavaModule;
 
 public final class InstrumentationModuleInstaller {
 
@@ -202,13 +205,32 @@ private AgentBuilder installInjectingModule(
     VirtualFieldImplementationInstaller contextProvider =
         virtualFieldInstallerFactory.create(instrumentationModule);
 
+    AtomicBoolean openerRun = new AtomicBoolean();
     AgentBuilder agentBuilder = parentAgentBuilder;
     for (TypeInstrumentation typeInstrumentation : typeInstrumentations) {
-
       AgentBuilder.Identified.Extendable extendableAgentBuilder =
           setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
               .and(muzzleMatcher)
               .transform(ConstantAdjuster.instance())
+              .transform(
+                  (builder, typeDescription, classLoader, module, protectionDomain) -> {
+                    if (JavaModule.isSupported()
+                        && instrumentationModule instanceof ExperimentalInstrumentationModule
+                        && !openerRun.get()) {
+                      ExperimentalInstrumentationModule experimentalModule =
+                          (ExperimentalInstrumentationModule) instrumentationModule;
+                      experimentalModule
+                          .jpmsModulesToOpen()
+                          .forEach(
+                              (javaModule, packages) -> {
+                                ModuleOpener.open(
+                                    instrumentation, javaModule, classLoader, packages);
+                              });
+                      openerRun.set(true);
+                    }
+
+                    return builder;
+                  })
               .transform(helperInjector);
       extendableAgentBuilder = contextProvider.injectHelperClasses(extendableAgentBuilder);
       extendableAgentBuilder = contextProvider.rewriteVirtualFieldsCalls(extendableAgentBuilder);