Versioned helper resources, part 2 (#3880)

* Versioned helper resources, part 2

* Remove accidentally added javadoc param

* Spotless

* Fix test

Author: trask

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

@@ -58,15 +58,15 @@ AgentBuilder install(
       return parentAgentBuilder;
     }
     List<String> helperClassNames = instrumentationModule.getMuzzleHelperClassNames();
-    HelperResourceBuilderImpl helperResources = new HelperResourceBuilderImpl();
+    HelperResourceBuilderImpl helperResourceBuilder = new HelperResourceBuilderImpl();
     List<String> helperResourceNames = instrumentationModule.helperResourceNames();
     for (String helperResourceName : helperResourceNames) {
-      helperResources.register(helperResourceName);
+      helperResourceBuilder.register(helperResourceName);
     }
-    instrumentationModule.registerHelperResources(helperResources);
+    instrumentationModule.registerHelperResources(helperResourceBuilder);
     List<TypeInstrumentation> typeInstrumentations = instrumentationModule.typeInstrumentations();
     if (typeInstrumentations.isEmpty()) {
-      if (!helperClassNames.isEmpty() || !helperResources.getResourcePathMappings().isEmpty()) {
+      if (!helperClassNames.isEmpty() || !helperResourceBuilder.getResources().isEmpty()) {
         logger.warn(
             "Helper classes and resources won't be injected if no types are instrumented: {}",
             instrumentationModule.instrumentationName());
@@ -82,7 +82,7 @@ AgentBuilder install(
         new HelperInjector(
             instrumentationModule.instrumentationName(),
             helperClassNames,
-            helperResources.getResourcePathMappings(),
+            helperResourceBuilder.getResources(),
             Utils.getExtensionsClassLoader(),
             instrumentation);
     InstrumentationContextProvider contextProvider =

javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/test/HelperInjectionTest.groovy

@@ -28,7 +28,7 @@ class HelperInjectionTest extends Specification {
     ClassLoader helpersSourceLoader = new URLClassLoader(helpersSourceUrls)
 
     String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
-    HelperInjector injector = new HelperInjector("test", [helperClassName], [:], helpersSourceLoader, null)
+    HelperInjector injector = new HelperInjector("test", [helperClassName], [], helpersSourceLoader, null)
     AtomicReference<URLClassLoader> emptyLoader = new AtomicReference<>(new URLClassLoader(new URL[0], (ClassLoader) null))
 
     when:
@@ -60,7 +60,7 @@ class HelperInjectionTest extends Specification {
     ByteBuddyAgent.install()
     AgentInstaller.installBytebuddyAgent(ByteBuddyAgent.getInstrumentation())
     String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
-    HelperInjector injector = new HelperInjector("test", [helperClassName], [:], this.class.classLoader, ByteBuddyAgent.getInstrumentation())
+    HelperInjector injector = new HelperInjector("test", [helperClassName], [], this.class.classLoader, ByteBuddyAgent.getInstrumentation())
     URLClassLoader bootstrapChild = new URLClassLoader(new URL[0], (ClassLoader) null)
 
     when:

muzzle/build.gradle.kts

@@ -9,6 +9,9 @@ dependencies {
   // Only used during compilation by bytebuddy plugin
   compileOnly("com.google.guava:guava")
 
+  compileOnly("com.google.auto.value:auto-value-annotations")
+  annotationProcessor("com.google.auto.value:auto-value")
+
   api("net.bytebuddy:byte-buddy")
 
   implementation(project(":javaagent-bootstrap"))

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java

@@ -7,6 +7,7 @@
 
 import io.opentelemetry.instrumentation.api.caching.Cache;
 import io.opentelemetry.javaagent.bootstrap.HelperResources;
+import io.opentelemetry.javaagent.tooling.muzzle.HelperResource;
 import java.io.File;
 import java.io.IOException;
 import java.lang.instrument.Instrumentation;
@@ -63,7 +64,7 @@ public String toString() {
   private final String requestingName;
 
   private final Set<String> helperClassNames;
-  private final Map<String, String> helperResourcePathMappings;
+  private final List<HelperResource> helperResources;
   @Nullable private final ClassLoader helpersSource;
   @Nullable private final Instrumentation instrumentation;
   private final Map<String, byte[]> dynamicTypeMap = new LinkedHashMap<>();
@@ -87,14 +88,14 @@ public String toString() {
   public HelperInjector(
       String requestingName,
       List<String> helperClassNames,
-      Map<String, String> helperResourcePathMappings,
+      List<HelperResource> helperResources,
       // TODO can this be replaced with the context classloader?
       ClassLoader helpersSource,
       Instrumentation instrumentation) {
     this.requestingName = requestingName;
 
     this.helperClassNames = new LinkedHashSet<>(helperClassNames);
-    this.helperResourcePathMappings = helperResourcePathMappings;
+    this.helperResources = helperResources;
     this.helpersSource = helpersSource;
     this.instrumentation = instrumentation;
   }
@@ -106,7 +107,7 @@ private HelperInjector(
     this.helperClassNames = helperMap.keySet();
     this.dynamicTypeMap.putAll(helperMap);
 
-    this.helperResourcePathMappings = Collections.emptyMap();
+    this.helperResources = Collections.emptyList();
     this.helpersSource = null;
     this.instrumentation = instrumentation;
   }
@@ -149,19 +150,20 @@ public DynamicType.Builder<?> transform(
       classLoader = injectHelperClasses(typeDescription, classLoader, module);
     }
 
-    if (helpersSource != null && !helperResourcePathMappings.isEmpty()) {
-      for (Map.Entry<String, String> entry : helperResourcePathMappings.entrySet()) {
-        String agentResourcePath = entry.getValue();
-        String applicationResourcePath = entry.getKey();
-        URL resource = helpersSource.getResource(agentResourcePath);
+    if (helpersSource != null && !helperResources.isEmpty()) {
+      for (HelperResource helperResource : helperResources) {
+        URL resource = helpersSource.getResource(helperResource.getAgentPath());
         if (resource == null) {
-          logger.debug("Helper resource {} requested but not found.", agentResourcePath);
+          logger.debug(
+              "Helper resource {} requested but not found.", helperResource.getAgentPath());
           continue;
         }
 
         logger.debug(
-            "Injecting resource onto classloader {} -> {}", classLoader, applicationResourcePath);
-        HelperResources.register(classLoader, applicationResourcePath, resource);
+            "Injecting resource onto classloader {} -> {}",
+            classLoader,
+            helperResource.getApplicationPath());
+        HelperResources.register(classLoader, helperResource.getApplicationPath(), resource);
       }
     }
 

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java

@@ -82,17 +82,17 @@ private static List<Mismatch> checkHelperInjection(
     try {
       // verify helper injector works
       List<String> allHelperClasses = instrumentationModule.getMuzzleHelperClassNames();
-      HelperResourceBuilderImpl helperResources = new HelperResourceBuilderImpl();
+      HelperResourceBuilderImpl helperResourceBuilder = new HelperResourceBuilderImpl();
       List<String> helperResourceNames = instrumentationModule.helperResourceNames();
       for (String helperResourceName : helperResourceNames) {
-        helperResources.register(helperResourceName);
+        helperResourceBuilder.register(helperResourceName);
       }
-      instrumentationModule.registerHelperResources(helperResources);
+      instrumentationModule.registerHelperResources(helperResourceBuilder);
       if (!allHelperClasses.isEmpty()) {
         new HelperInjector(
                 instrumentationModule.instrumentationName(),
                 allHelperClasses,
-                helperResources.getResourcePathMappings(),
+                helperResourceBuilder.getResources(),
                 Thread.currentThread().getContextClassLoader(),
                 null)
             .transform(null, null, classLoader, null);

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/HelperResource.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.tooling.muzzle;
+
+import com.google.auto.value.AutoValue;
+
+@AutoValue
+public abstract class HelperResource {
+
+  /**
+   * Create a new helper resource object.
+   *
+   * @param applicationPath The path in the user's class loader at which to inject the resource.
+   * @param agentPath The path in the agent class loader from which to get the content for the
+   *     resource.
+   */
+  public static HelperResource create(String applicationPath, String agentPath) {
+    return new AutoValue_HelperResource(applicationPath, agentPath);
+  }
+
+  /** The path in the user's class loader at which to inject the resource. */
+  public abstract String getApplicationPath();
+
+  /** The path in the agent class loader from which to get the content for the resource. */
+  public abstract String getAgentPath();
+}

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/HelperResourceBuilderImpl.java

@@ -6,30 +6,24 @@
 package io.opentelemetry.javaagent.tooling.muzzle;
 
 import io.opentelemetry.javaagent.extension.instrumentation.HelperResourceBuilder;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.ArrayList;
+import java.util.List;
 
 public class HelperResourceBuilderImpl implements HelperResourceBuilder {
 
-  private final Map<String, String> resourcePathMappings = new HashMap<>();
+  private final List<HelperResource> resources = new ArrayList<>();
 
   @Override
   public void register(String resourcePath) {
-    resourcePathMappings.put(resourcePath, resourcePath);
+    resources.add(HelperResource.create(resourcePath, resourcePath));
   }
 
   @Override
   public void register(String applicationResourcePath, String agentResourcePath) {
-    resourcePathMappings.put(applicationResourcePath, agentResourcePath);
+    resources.add(HelperResource.create(applicationResourcePath, agentResourcePath));
   }
 
-  /**
-   * Returns the registered mappings, where the keys are the paths in the user's class loader at
-   * which to inject the resource ({@code applicationResourcePath}) and the values are the paths in
-   * the agent class loader from which to get the content for the resource ({@code
-   * agentResourcePath}).
-   */
-  public Map<String, String> getResourcePathMappings() {
-    return resourcePathMappings;
+  public List<HelperResource> getResources() {
+    return resources;
   }
 }

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceCollector.java

@@ -74,14 +74,28 @@ public ReferenceCollector(
    *
    * @param resource path to the resource file, same as in {@link ClassLoader#getResource(String)}
    * @see InstrumentationClassPredicate
+   * @deprecated Use {@link #collectReferencesFromResource(HelperResource)} instead.
    */
+  @Deprecated
   public void collectReferencesFromResource(String resource) {
-    if (!isSpiFile(resource)) {
+    collectReferencesFromResource(HelperResource.create(resource, resource));
+  }
+
+  /**
+   * If passed {@code resource} path points to an SPI file (either Java {@link
+   * java.util.ServiceLoader} or AWS SDK {@code ExecutionInterceptor}) reads the file and adds every
+   * implementation as a reference, traversing the graph of classes until a non-instrumentation
+   * (external) class is encountered.
+   *
+   * @see InstrumentationClassPredicate
+   */
+  public void collectReferencesFromResource(HelperResource helperResource) {
+    if (!isSpiFile(helperResource.getApplicationPath())) {
       return;
     }
 
     List<String> spiImplementations = new ArrayList<>();
-    try (InputStream stream = getResourceStream(resource)) {
+    try (InputStream stream = getResourceStream(helperResource.getAgentPath())) {
       BufferedReader reader = new BufferedReader(new InputStreamReader(stream, UTF_8));
       while (reader.ready()) {
         String line = reader.readLine();
@@ -90,7 +104,7 @@ public void collectReferencesFromResource(String resource) {
         }
       }
     } catch (IOException e) {
-      throw new IllegalStateException("Error reading resource " + resource, e);
+      throw new IllegalStateException("Error reading resource " + helperResource.getAgentPath(), e);
     }
 
     visitClassesAndCollectReferences(spiImplementations, /* startsFromAdviceClass= */ false);