make rmi instrumentation indy-compatible + add module opener (#12585)

Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>
Co-authored-by: Lauri Tulmin <tulmin@gmail.com>

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 static java.util.Arrays.asList;
 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;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new RmiClientContextInstrumentation(), new RmiServerContextInstrumentation());
   }
+
+  @Override
+  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 static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 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 class RmiClientContextInstrumentation implements TypeInstrumentation {
             .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")

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/jpms/ExposeRmiModuleInstrumentation.java

@@ -1,82 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.rmi.context.jpms;
-
-import static java.util.logging.Level.FINE;
-import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
-
-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.util.Collections;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.logging.Logger;
-import net.bytebuddy.description.type.TypeDescription;
-import net.bytebuddy.dynamic.loading.ClassInjector;
-import net.bytebuddy.matcher.ElementMatcher;
-import net.bytebuddy.utility.JavaModule;
-
-public class ExposeRmiModuleInstrumentation implements TypeInstrumentation {
-  private static final Logger logger =
-      Logger.getLogger(ExposeRmiModuleInstrumentation.class.getName());
-
-  private final AtomicBoolean instrumented = new AtomicBoolean();
-
-  @Override
-  public ElementMatcher<TypeDescription> typeMatcher() {
-    ElementMatcher.Junction<TypeDescription> notInstrumented =
-        new ElementMatcher.Junction.AbstractBase<TypeDescription>() {
-
-          @Override
-          public boolean matches(TypeDescription target) {
-            return !instrumented.get();
-          }
-        };
-
-    return notInstrumented.and(nameStartsWith("sun.rmi"));
-  }
-
-  @Override
-  public ElementMatcher<ClassLoader> classLoaderOptimization() {
-    return new ElementMatcher.Junction.AbstractBase<ClassLoader>() {
-
-      @Override
-      public boolean matches(ClassLoader target) {
-        // runs only in bootstrap class loader
-        return JavaModule.isSupported() && target == null;
-      }
-    };
-  }
-
-  @Override
-  public void transform(TypeTransformer transformer) {
-    transformer.applyTransformer(
-        (builder, typeDescription, classLoader, javaModule, protectionDomain) -> {
-          if (javaModule != null && javaModule.isNamed()) {
-            // using Java8BytecodeBridge because it's in the unnamed module in the bootstrap
-            // loader, and that's where the rmi instrumentation helper classes will end up
-            JavaModule helperModule = JavaModule.ofType(Java8BytecodeBridge.class);
-            // expose sun.rmi.server package to unnamed module
-            ClassInjector.UsingInstrumentation.redefineModule(
-                InstrumentationHolder.getInstrumentation(),
-                javaModule,
-                Collections.emptySet(),
-                Collections.singletonMap("sun.rmi.server", Collections.singleton(helperModule)),
-                Collections.emptyMap(),
-                Collections.emptySet(),
-                Collections.emptyMap());
-
-            instrumented.set(true);
-            logger.log(
-                FINE,
-                "Exposed package \"sun.rmi.server\" in module {0} to unnamed module",
-                javaModule);
-          }
-          return builder;
-        });
-  }
-}

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/context/jpms/RmiJpmsInstrumentationModule.java

@@ -1,32 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.rmi.context.jpms;
-
-import static java.util.Collections.singletonList;
-
-import com.google.auto.service.AutoService;
-import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
-import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
-import io.opentelemetry.javaagent.instrumentation.rmi.context.server.ContextDispatcher;
-import java.util.List;
-
-/**
- * RMI server instrumentation class {@link ContextDispatcher} implements an internal interface that
- * is not exported by java module system. This is not allowed on jdk17. This instrumentation module
- * exposes JDK internal classes for RMI server instrumentation.
- */
-@AutoService(InstrumentationModule.class)
-public class RmiJpmsInstrumentationModule extends InstrumentationModule {
-
-  public RmiJpmsInstrumentationModule() {
-    super("rmi", "rmi-jpms");
-  }
-
-  @Override
-  public List<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new ExposeRmiModuleInstrumentation());
-  }
-}

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

@@ -11,6 +11,8 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModul
 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,16 @@ public interface ExperimentalInstrumentationModule {
   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.
+   *
+   * @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.Experimenta
 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.lang.instrument.Instrumentation;
 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 @@ public final class InstrumentationModuleInstaller {
     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);

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/IndyModuleRegistry.java

@@ -5,12 +5,16 @@
 
 package io.opentelemetry.javaagent.tooling.instrumentation.indy;
 
+import io.opentelemetry.javaagent.bootstrap.InstrumentationHolder;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
+import io.opentelemetry.javaagent.tooling.ModuleOpener;
 import io.opentelemetry.javaagent.tooling.util.ClassLoaderValue;
+import java.lang.instrument.Instrumentation;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import net.bytebuddy.agent.builder.AgentBuilder;
+import net.bytebuddy.utility.JavaModule;
 
 public class IndyModuleRegistry {
 
@@ -65,6 +69,24 @@ public class IndyModuleRegistry {
               + " yet");
     }
 
+    if (module instanceof ExperimentalInstrumentationModule) {
+      ExperimentalInstrumentationModule experimentalModule =
+          (ExperimentalInstrumentationModule) module;
+
+      Instrumentation instrumentation = InstrumentationHolder.getInstrumentation();
+      if (instrumentation == null) {
+        throw new IllegalStateException("global instrumentation not available");
+      }
+
+      if (JavaModule.isSupported()) {
+        // module opener only usable for java 9+
+        experimentalModule
+            .jpmsModulesToOpen()
+            .forEach(
+                (javaModule, packages) ->
+                    ModuleOpener.open(instrumentation, javaModule, loader, packages));
+      }
+    }
     return loader;
   }