indy instrumentation - leftovers migration

instrumentation/internal/internal-lambda/javaagent/build.gradle.kts

@@ -4,7 +4,6 @@ plugins {
 
 dependencies {
   compileOnly(project(":javaagent-bootstrap"))
-  implementation(project(":instrumentation:internal:internal-lambda-java9:javaagent"))
 
   testImplementation(project(":javaagent-bootstrap"))
 }

instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaInstrumentationModule.java

@@ -10,13 +10,14 @@
 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.sdk.autoconfigure.spi.ConfigProperties;
-import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
-import net.bytebuddy.utility.JavaModule;
 
 @AutoService(InstrumentationModule.class)
-public class LambdaInstrumentationModule extends InstrumentationModule {
+public class LambdaInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
   public LambdaInstrumentationModule() {
     super("internal-lambda");
   }
@@ -28,21 +29,16 @@ public boolean defaultEnabled(ConfigProperties config) {
   }
 
   @Override
-  public boolean isIndyModule() {
-    return false;
+  public List<String> injectedClassNames() {
+    return Collections.singletonList(
+        "io.opentelemetry.javaagent.instrumentation.internal.lambda.LambdaTransformer");
   }
 
   @Override
   public List<String> getAdditionalHelperClassNames() {
     // this instrumentation uses ASM not ByteBuddy so muzzle doesn't automatically add helper
     // classes
-    List<String> classNames = new ArrayList<>();
-    classNames.add("io.opentelemetry.javaagent.instrumentation.internal.lambda.LambdaTransformer");
-    if (JavaModule.isSupported()) {
-      classNames.add(
-          "io.opentelemetry.javaagent.instrumentation.internal.lambda.Java9LambdaTransformer");
-    }
-    return classNames;
+    return injectedClassNames();
   }
 
   @Override

instrumentation/internal/internal-lambda/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/lambda/LambdaTransformer.java

@@ -11,21 +11,11 @@
 
 /** Helper class for transforming lambda class bytes. */
 public final class LambdaTransformer {
-  private static final boolean IS_JAVA_9 = isJava9();
 
   private LambdaTransformer() {}
 
-  private static boolean isJava9() {
-    try {
-      Class.forName("java.lang.Module", false, null);
-      return true;
-    } catch (ClassNotFoundException exception) {
-      return false;
-    }
-  }
-
   /**
-   * Called from {@code java.lang.invoke.InnerClassLambdaMetafactory} to transform lambda class
+   * Called from {@code java.lang.invoke.InnerClassLambdaMetaFactory} to transform lambda class
    * bytes.
    */
   @SuppressWarnings("unused")
@@ -34,29 +24,25 @@ public static byte[] transform(byte[] classBytes, String slashClassName, Class<?
     if (InjectedClassHelper.isHelperClass(targetClass)) {
       return classBytes;
     }
+    ClassFileTransformer transformer = ClassFileTransformerHolder.getLambdaClassFileTransformer();
+    if (transformer == null) {
+      return classBytes;
+    }
 
-    ClassFileTransformer transformer = ClassFileTransformerHolder.getClassFileTransformer();
-    if (transformer != null) {
-      try {
-        byte[] result;
-        if (IS_JAVA_9) {
-          result =
-              Java9LambdaTransformer.transform(
-                  transformer, classBytes, slashClassName, targetClass);
-        } else {
-          result =
-              transformer.transform(
-                  targetClass.getClassLoader(), slashClassName, null, null, classBytes);
-        }
-        if (result != null) {
-          return result;
-        }
-      } catch (Throwable throwable) {
-        // sun.instrument.TransformerManager catches Throwable from ClassFileTransformer and ignores
-        // it, we do the same.
+    try {
+      // The 'targetClass' needs to be non-null as it is used in Java9LambdaClassFileTransformer
+      // to get a reference to the jpms module. However, a null value must be passed to the
+      // underlying transformer as this code is called when the lambda is defined.
+      byte[] result =
+          transformer.transform(
+              targetClass.getClassLoader(), slashClassName, targetClass, null, classBytes);
+      if (result != null) {
+        classBytes = result;
       }
+    } catch (Throwable throwable) {
+      // sun.instrument.TransformerManager catches Throwable from ClassFileTransformer and ignores
+      // it, we do the same.
     }
-
     return classBytes;
   }
 }

instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java

@@ -11,12 +11,15 @@
 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 java.util.Arrays;
 import java.util.List;
 import net.bytebuddy.matcher.ElementMatcher;
 
 /** Instrumentation for methods annotated with {@code WithSpan} annotation. */
 @AutoService(InstrumentationModule.class)
-public class AnnotationInstrumentationModule extends InstrumentationModule {
+public class AnnotationInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public AnnotationInstrumentationModule() {
     super(
@@ -26,12 +29,6 @@ public AnnotationInstrumentationModule() {
         "opentelemetry-instrumentation-annotations");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // needs helper classes in the same class loader
-    return false;
-  }
-
   @Override
   public int order() {
     // Run first to ensure other automatic instrumentation is added after and therefore is executed
@@ -50,4 +47,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new WithSpanInstrumentation());
   }
+
+  @Override
+  public List<String> injectedClassNames() {
+    return Arrays.asList(
+        "io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationSingletons",
+        "io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationInstrumentationHelper",
+        "io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.MethodRequest",
+        "io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.MethodRequestCodeAttributesGetter");
+  }
 }

javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/ClassFileTransformerHolder.java

@@ -16,11 +16,21 @@ public final class ClassFileTransformerHolder {
 
   private static volatile ClassFileTransformer classFileTransformer;
 
-  public static ClassFileTransformer getClassFileTransformer() {
+  /**
+   * get class transformer for defining lambdas
+   *
+   * @return class transformer for defining lambdas
+   */
+  public static ClassFileTransformer getLambdaClassFileTransformer() {
     return classFileTransformer;
   }
 
-  public static void setClassFileTransformer(ClassFileTransformer transformer) {
+  /**
+   * set class transformer for defining lambdas
+   *
+   * @param transformer transformer
+   */
+  public static void setLambdaClassFileTransformer(ClassFileTransformer transformer) {
     classFileTransformer = transformer;
   }
 

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

@@ -0,0 +1,42 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling;
+
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
+import java.security.ProtectionDomain;
+
+/** {@link ClassFileTransformer} for lambda instrumentation with java9 jpms module compatibility */
+public class Java9LambdaClassFileTransformer implements ClassFileTransformer {
+
+  private final ClassFileTransformer delegate;
+
+  public Java9LambdaClassFileTransformer(ClassFileTransformer delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  public byte[] transform(
+      ClassLoader loader,
+      String className,
+      Class<?> targetClass,
+      ProtectionDomain protectionDomain,
+      byte[] classfileBuffer)
+      throws IllegalClassFormatException {
+
+    if (targetClass == null) {
+      // we expect the target class argument to be non-null, however it's only used to resolve
+      // the JPMS module
+      throw new IllegalStateException("can't resolve module without class");
+    }
+    Module module = targetClass.getModule();
+
+    // lambda instrumentation happens only when the lambda is defined, thus the classBeingRedefined
+    // must be null otherwise we get a partial instrumentation, for example virtual fields are not
+    // properly applied
+    return delegate.transform(module, loader, className, null, null, classfileBuffer);
+  }
+}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java

@@ -48,6 +48,7 @@
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.SdkAutoconfigureAccess;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -63,7 +64,6 @@
 import net.bytebuddy.ByteBuddy;
 import net.bytebuddy.agent.builder.AgentBuilder;
 import net.bytebuddy.agent.builder.AgentBuilderUtil;
-import net.bytebuddy.agent.builder.ResettableClassFileTransformer;
 import net.bytebuddy.description.type.TypeDefinition;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.DynamicType;
@@ -199,9 +199,17 @@ private static void installBytebuddyAgent(
     logger.log(FINE, "Installed {0} extension(s)", numberOfLoadedExtensions);
 
     agentBuilder = AgentBuilderUtil.optimize(agentBuilder);
-    ResettableClassFileTransformer resettableClassFileTransformer = agentBuilder.installOn(inst);
+    ClassFileTransformer transformer = agentBuilder.installOn(inst);
+    if (JavaModule.isSupported()) {
+      // wrapping in a JPMS compliant implementation
+      transformer = new Java9LambdaClassFileTransformer(transformer);
+    } else {
+      // wrapping in a java 8 compliant transformer
+      transformer = new LambdaClassFileTransformer(transformer);
+    }
+    ClassFileTransformerHolder.setLambdaClassFileTransformer(transformer);
+
     instrumentationInstalled = true;
-    ClassFileTransformerHolder.setClassFileTransformer(resettableClassFileTransformer);
 
     addHttpServerResponseCustomizers(extensionClassLoader);
 

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/LambdaClassFileTransformer.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling;
+
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.IllegalClassFormatException;
+import java.security.ProtectionDomain;
+
+/** {@link ClassFileTransformer} for lambda instrumentation without jpms modules */
+public class LambdaClassFileTransformer implements ClassFileTransformer {
+
+  private final ClassFileTransformer delegate;
+
+  public LambdaClassFileTransformer(ClassFileTransformer delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  public byte[] transform(
+      ClassLoader loader,
+      String className,
+      Class<?> classBeingRedefined,
+      ProtectionDomain protectionDomain,
+      byte[] classfileBuffer)
+      throws IllegalClassFormatException {
+
+    // lambda instrumentation happens only when the lambda is defined, thus the classBeingRedefined
+    // must be null otherwise we get a partial instrumentation, for example virtual fields are not
+    // properly applied.
+    return delegate.transform(loader, className, null, null, classfileBuffer);
+  }
+}

settings.gradle.kts

@@ -257,7 +257,6 @@ include(":instrumentation:internal:internal-class-loader:javaagent")
 include(":instrumentation:internal:internal-class-loader:javaagent-integration-tests")
 include(":instrumentation:internal:internal-eclipse-osgi-3.6:javaagent")
 include(":instrumentation:internal:internal-lambda:javaagent")
-include(":instrumentation:internal:internal-lambda-java9:javaagent")
 include(":instrumentation:internal:internal-reflection:javaagent")
 include(":instrumentation:internal:internal-reflection:javaagent-integration-tests")
 include(":instrumentation:internal:internal-url-class-loader:javaagent")

testing-common/integration-tests/build.gradle.kts

@@ -52,15 +52,13 @@ tasks {
       includeTestsMatching("InstrumentOldBytecode")
     }
     include("**/InstrumentOldBytecode.*")
-    jvmArgs("-Dotel.instrumentation.inline-ibm-resource-level.enabled=false")
   }
 
   val testInlineModuleOldBytecodeInstrumentation by registering(Test::class) {
     filter {
       includeTestsMatching("InstrumentOldBytecode")
     }
     include("**/InstrumentOldBytecode.*")
-    jvmArgs("-Dotel.instrumentation.indy-ibm-resource-level.enabled=false")
   }
 
   test {

testing-common/integration-tests/src/main/java/InlineIbmResourceLevelInstrumentationModule.java → testing-common/integration-tests/src/main/java/IbmResourceLevelInstrumentationModule.java

@@ -11,13 +11,13 @@
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class InlineIbmResourceLevelInstrumentationModule extends InstrumentationModule {
-  public InlineIbmResourceLevelInstrumentationModule() {
-    super("inline-ibm-resource-level");
+public class IbmResourceLevelInstrumentationModule extends InstrumentationModule {
+  public IbmResourceLevelInstrumentationModule() {
+    super("ibm-resource-level");
   }
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new InlineResourceLevelInstrumentation());
+    return singletonList(new ResourceLevelInstrumentation());
   }
 }