Transform inline advice to delegating advice and test indy modules (#9508)

Author: laurit

.github/workflows/build-pull-request.yml

@@ -21,6 +21,11 @@ jobs:
     with:
       cache-read-only: true
 
+  test-indy:
+    uses: ./.github/workflows/reusable-test-indy.yml
+    with:
+      cache-read-only: true
+
   muzzle:
     uses: ./.github/workflows/reusable-muzzle.yml
     with:

.github/workflows/reusable-test-indy.yml

@@ -0,0 +1,92 @@
+name: Reusable - Test latest deps
+
+on:
+  workflow_call:
+    inputs:
+      cache-read-only:
+        type: boolean
+        required: false
+      no-build-cache:
+        type: boolean
+        required: false
+    secrets:
+      GRADLE_ENTERPRISE_ACCESS_KEY:
+        required: false
+      GE_CACHE_USERNAME:
+        required: false
+      GE_CACHE_PASSWORD:
+        required: false
+
+jobs:
+  test-indy:
+    name: testIndy${{ matrix.test-partition }}
+    if: ${{ contains(github.event.pull_request.labels.*.name, 'test indy') }}
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        test-partition:
+          - 0
+          - 1
+          - 2
+          - 3
+      fail-fast: false
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Free disk space
+        run: .github/scripts/gha-free-disk-space.sh
+
+      - name: Set up JDK for running Gradle
+        uses: actions/setup-java@v3
+        with:
+          distribution: temurin
+          java-version: 17.0.6
+
+      - name: Increase gradle daemon heap size
+        run: |
+          sed -i "s/org.gradle.jvmargs=/org.gradle.jvmargs=-Xmx3g /" gradle.properties
+
+      # vaadin 14 tests fail with node 18
+      - name: Set up Node
+        uses: actions/setup-node@v3
+        with:
+          node-version: 16
+
+      # vaadin tests use pnpm
+      - name: Cache pnpm modules
+        uses: actions/cache@v3
+        with:
+          path: ~/.pnpm-store
+          key: ${{ runner.os }}-test-latest-cache-pnpm-modules
+
+      - name: List tests
+        env:
+          GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
+          GE_CACHE_USERNAME: ${{ secrets.GE_CACHE_USERNAME }}
+          GE_CACHE_PASSWORD: ${{ secrets.GE_CACHE_PASSWORD }}
+        uses: gradle/gradle-build-action@v2
+        with:
+          arguments: >
+            check -x spotlessCheck
+            listTestsInPartition
+            -PtestPartition=${{ matrix.test-partition }}
+          cache-read-only: true
+
+      - name: Set test tasks
+        run: |
+          echo "test-tasks=$(cat test-tasks.txt | xargs echo | sed 's/\n/ /g')" >> $GITHUB_ENV
+
+      - name: Test
+        uses: gradle/gradle-build-action@v2
+        env:
+          GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }}
+          GE_CACHE_USERNAME: ${{ secrets.GE_CACHE_USERNAME }}
+          GE_CACHE_PASSWORD: ${{ secrets.GE_CACHE_PASSWORD }}
+        with:
+          arguments: >
+            ${{ env.test-tasks }}
+            -PtestIndy=true
+            ${{ inputs.no-build-cache && ' --no-build-cache' || '' }}
+          cache-read-only: ${{ inputs.cache-read-only }}
+          # gradle enterprise is used for the build cache
+          gradle-home-cache-excludes: caches/build-cache-1

conventions/src/main/kotlin/io.opentelemetry.instrumentation.javaagent-testing.gradle.kts

@@ -86,6 +86,7 @@ class JavaagentTestArgumentsProvider(
     // in smoke tests instead.
     "-Dotel.javaagent.add-thread-details=false",
     "-Dotel.metrics.exporter=otlp",
+    "-Dotel.javaagent.experimental.indy=${findProperty("testIndy") == "true"}",
     // suppress repeated logging of "No metric data to export - skipping export."
     // since PeriodicMetricReader is configured with a short interval
     "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.opentelemetry.sdk.metrics.export.PeriodicMetricReader=INFO",

instrumentation/akka/akka-actor-fork-join-2.5/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/akkaactor/AkkaForkJoinPoolInstrumentation.java

@@ -48,8 +48,7 @@ public void transform(TypeTransformer transformer) {
   public static class SetAkkaForkJoinStateAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static PropagatedContext enterJobSubmit(
-        @Advice.Argument(value = 0, readOnly = false) ForkJoinTask<?> task) {
+    public static PropagatedContext enterJobSubmit(@Advice.Argument(0) ForkJoinTask<?> task) {
       Context context = Java8BytecodeBridge.currentContext();
       if (ExecutorAdviceHelper.shouldPropagateContext(context, task)) {
         VirtualField<ForkJoinTask<?>, PropagatedContext> virtualField =

instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AbstractAwsSdkInstrumentationModule.java

@@ -28,6 +28,11 @@ public boolean isHelperClass(String className) {
     return className.startsWith("io.opentelemetry.contrib.awsxray.");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // We don't actually transform it but want to make sure we only apply the instrumentation when

instrumentation/aws-sdk/aws-sdk-1.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v1_11/AwsSdkInstrumentationModule.java

@@ -23,6 +23,11 @@ public boolean isHelperClass(String className) {
     return className.startsWith("io.opentelemetry.contrib.awsxray.");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/aws-sdk/aws-sdk-2.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/awssdk/v2_2/AbstractAwsSdkInstrumentationModule.java

@@ -27,6 +27,11 @@ public boolean isHelperClass(String className) {
     return className.startsWith("io.opentelemetry.contrib.awsxray.");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // We don't actually transform it but want to make sure we only apply the instrumentation when

instrumentation/azure-core/azure-core-1.14/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_14/AzureSdkInstrumentationModule.java

@@ -35,6 +35,11 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder)
         "azure-core-1.14/META-INF/services/com.azure.core.util.tracing.Tracer");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     return hasClassesNamed("com.azure.core.util.tracing.Tracer")

instrumentation/azure-core/azure-core-1.19/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_19/AzureSdkInstrumentationModule.java

@@ -35,6 +35,11 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder)
         "azure-core-1.19/META-INF/services/com.azure.core.util.tracing.Tracer");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // this class was introduced in azure-core 1.19

instrumentation/azure-core/azure-core-1.36/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/azurecore/v1_36/AzureSdkInstrumentationModule.java

@@ -37,6 +37,11 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder)
         "azure-core-1.36/META-INF/services/com.azure.core.util.tracing.Tracer");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // this class was introduced in azure-core 1.36

instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java

@@ -23,6 +23,12 @@ public boolean isHelperClass(String className) {
     return className.equals("rx.__OpenTelemetryTracingUtil");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // rx.__OpenTelemetryTracingUtil is used for accessing a package private field
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new CouchbaseBucketInstrumentation(), new CouchbaseClusterInstrumentation());

instrumentation/elasticsearch/elasticsearch-api-client-7.16/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/apiclient/ElasticsearchApiClientInstrumentationModule.java

@@ -30,6 +30,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     return not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation"));
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // java.lang.ClassCastException: class
+    // io.opentelemetry.javaagent.shaded.instrumentation.elasticsearch.rest.internal.ElasticsearchEndpointDefinition cannot be cast to class io.opentelemetry.javaagent.shaded.instrumentation.elasticsearch.rest.internal.ElasticsearchEndpointDefinition (io.opentelemetry.javaagent.shaded.instrumentation.elasticsearch.rest.internal.ElasticsearchEndpointDefinition is in unnamed module of loader io.opentelemetry.javaagent.tooling.instrumentation.indy.InstrumentationModuleClassLoader @6baee63b; io.opentelemetry.javaagent.shaded.instrumentation.elasticsearch.rest.internal.ElasticsearchEndpointDefinition is in unnamed module of loader 'app')
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v7_0/ElasticsearchRest7InstrumentationModule.java

@@ -32,6 +32,12 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
         .and(not(hasClassesNamed("co.elastic.clients.transport.instrumentation.Instrumentation")));
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // shares a virtual field with elasticsearch-api-client
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new RestClientInstrumentation());

instrumentation/elasticsearch/elasticsearch-rest-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/elasticsearch/rest/v7_0/RestClientInstrumentation.java

@@ -80,7 +80,7 @@ public static void onEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Return(readOnly = false) Response response,
+        @Advice.Return Response response,
         @Advice.Thrown Throwable throwable,
         @Advice.Local("otelRequest") ElasticsearchRestRequest otelRequest,
         @Advice.Local("otelContext") Context context,

instrumentation/executors/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/executors/ExecutorsInstrumentationModule.java

@@ -19,6 +19,11 @@ public ExecutorsInstrumentationModule() {
     super("executors");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/executors/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/executors/JavaExecutorInstrumentation.java

@@ -114,8 +114,7 @@ public static void exitJobSubmit(
   public static class SetJavaForkJoinStateAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static PropagatedContext enterJobSubmit(
-        @Advice.Argument(value = 0, readOnly = false) ForkJoinTask<?> task) {
+    public static PropagatedContext enterJobSubmit(@Advice.Argument(0) ForkJoinTask<?> task) {
       Context context = Java8BytecodeBridge.currentContext();
       if (ExecutorAdviceHelper.shouldPropagateContext(context, task)) {
         VirtualField<ForkJoinTask<?>, PropagatedContext> virtualField =
@@ -165,8 +164,7 @@ public static void exitJobSubmit(
   public static class SetCallableStateAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static PropagatedContext enterJobSubmit(
-        @Advice.Argument(value = 0, readOnly = false) Callable<?> task) {
+    public static PropagatedContext enterJobSubmit(@Advice.Argument(0) Callable<?> task) {
       Context context = Java8BytecodeBridge.currentContext();
       if (ExecutorAdviceHelper.shouldPropagateContext(context, task)) {
         VirtualField<Callable<?>, PropagatedContext> virtualField =
@@ -195,7 +193,7 @@ public static class SetCallableStateForCallableCollectionAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static Collection<?> submitEnter(
-        @Advice.Argument(value = 0, readOnly = false) Collection<? extends Callable<?>> tasks) {
+        @Advice.Argument(0) Collection<? extends Callable<?>> tasks) {
       if (tasks == null) {
         return Collections.emptyList();
       }

instrumentation/guava-10.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/guava/v10_0/GuavaListenableFutureInstrumentation.java

@@ -49,8 +49,7 @@ public static void onConstruction() {
   public static class AddListenerAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static PropagatedContext addListenerEnter(
-        @Advice.Argument(value = 0, readOnly = false) Runnable task) {
+    public static PropagatedContext addListenerEnter(@Advice.Argument(0) Runnable task) {
       Context context = Java8BytecodeBridge.currentContext();
       if (ExecutorAdviceHelper.shouldPropagateContext(context, task)) {
         VirtualField<Runnable, PropagatedContext> virtualField =

instrumentation/hibernate/hibernate-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_0/HibernateInstrumentationModule.java

@@ -30,6 +30,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
         "org.hibernate.Criteria");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // shares classes with hibernate-procedure-call-4.3, these classes should be in the same class
+    // loader
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/hibernate/hibernate-6.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v6_0/HibernateInstrumentationModule.java

@@ -28,6 +28,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
         "org.hibernate.query.spi.SqmQuery");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // shares classes with hibernate-procedure-call-4.3, these classes should be in the same class
+    // loader
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

instrumentation/hibernate/hibernate-procedure-call-4.3/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hibernate/v4_3/HibernateInstrumentationModule.java

@@ -25,6 +25,13 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     return hasClassesNamed("org.hibernate.procedure.ProcedureCall");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // uses SessionInfo class from hibernate common which is now in separate class loader for all
+    // instrumentations
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new ProcedureCallInstrumentation(), new SessionInstrumentation());

instrumentation/hystrix-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java

@@ -24,6 +24,12 @@ public boolean isHelperClass(String className) {
     return className.equals("rx.__OpenTelemetryTracingUtil");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    // rx.__OpenTelemetryTracingUtil is used for accessing a package private field
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new HystrixCommandInstrumentation());

instrumentation/internal/internal-class-loader/javaagent-integration-tests/src/main/java/instrumentation/TestInstrumentationModule.java

@@ -36,6 +36,11 @@ public void registerHelperResources(HelperResourceBuilder helperResourceBuilder)
         "test-resources/test-resource.txt", "test-resources/test-resource-2.txt");
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   public static class TestTypeInstrumentation implements TypeInstrumentation {
     @Override
     public ElementMatcher<ClassLoader> classLoaderOptimization() {

instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java

@@ -25,6 +25,11 @@ public boolean defaultEnabled(ConfigProperties config) {
     return true;
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public boolean isHelperClass(String className) {
     return className.equals("io.opentelemetry.javaagent.tooling.Constants");

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

@@ -27,6 +27,11 @@ public boolean defaultEnabled(ConfigProperties config) {
     return true;
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public List<String> getAdditionalHelperClassNames() {
     // this instrumentation uses ASM not ByteBuddy so muzzle doesn't automatically add helper

instrumentation/internal/internal-reflection/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/reflection/ReflectionInstrumentationModule.java

@@ -25,6 +25,11 @@ public boolean defaultEnabled(ConfigProperties config) {
     return true;
   }
 
+  @Override
+  public boolean isIndyModule() {
+    return false;
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new ClassInstrumentation(), new ReflectionInstrumentation());