Resource injection for class loader getResourceAsStream (#7476)

In #7447 injected resource is opened with class loader getResourceAsStream. This works only in class loaders where getResourceAsStream delegates to getResource. This is not the case with all class loaders, for example tomcat class loader does not do this. Because of this we also need to instrument class loader getResourceAsStream.
open-telemetry · Jan 2, 2023 · fe540ea · fe540ea
1 parent 8c64a9e
commit fe540ea
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 9 deletions.
diff --git a/...etry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java b/...etry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java
@@ -14,6 +14,8 @@
 import io.opentelemetry.javaagent.bootstrap.HelperResources;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import java.io.IOException;
+import java.io.InputStream;
 import java.net.URL;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -47,6 +49,12 @@ public void transform(TypeTransformer transformer) {
             .and(takesArguments(String.class))
             .and(returns(Enumeration.class)),
         ResourceInjectionInstrumentation.class.getName() + "$GetResourcesAdvice");
+    transformer.applyAdviceToMethod(
+        isMethod()
+            .and(named("getResourceAsStream"))
+            .and(takesArguments(String.class))
+            .and(returns(InputStream.class)),
+        ResourceInjectionInstrumentation.class.getName() + "$GetResourceAsStreamAdvice");
   }
 
   @SuppressWarnings("unused")
@@ -57,6 +65,9 @@ public static void onExit(
         @Advice.This ClassLoader classLoader,
         @Advice.Argument(0) String name,
         @Advice.Return(readOnly = false) URL resource) {
+      if (resource != null) {
+        return;
+      }
 
       URL helper = HelperResources.loadOne(classLoader, name);
       if (helper != null) {
@@ -101,4 +112,27 @@ public static void onExit(
       resources = Collections.enumeration(result);
     }
   }
+
+  @SuppressWarnings("unused")
+  public static class GetResourceAsStreamAdvice {
+
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    public static void onExit(
+        @Advice.This ClassLoader classLoader,
+        @Advice.Argument(0) String name,
+        @Advice.Return(readOnly = false) InputStream inputStream) {
+      if (inputStream != null) {
+        return;
+      }
+
+      URL helper = HelperResources.loadOne(classLoader, name);
+      if (helper != null) {
+        try {
+          inputStream = helper.openStream();
+        } catch (IOException ignored) {
+          // ClassLoader.getResourceAsStream also ignores io exceptions from opening the stream
+        }
+      }
+    }
+  }
 }
diff --git a/...on/internal/internal-class-loader/javaagent/src/test/groovy/TomcatClassloadingTest.groovy b/...on/internal/internal-class-loader/javaagent/src/test/groovy/TomcatClassloadingTest.groovy
@@ -4,28 +4,68 @@
  */
 
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
+import io.opentelemetry.javaagent.bootstrap.HelperResources
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.util.stream.Collectors
 import org.apache.catalina.WebResource
 import org.apache.catalina.WebResourceRoot
 import org.apache.catalina.loader.ParallelWebappClassLoader
+import spock.lang.Shared
 
 class TomcatClassloadingTest extends AgentInstrumentationSpecification {
 
-  WebResourceRoot resources = Mock(WebResourceRoot) {
-    getResource(_) >> Mock(WebResource)
-    listResources(_) >> []
-    // Looks like 9.x.x needs this one:
-    getResources(_) >> []
-  }
-  ParallelWebappClassLoader classloader = new ParallelWebappClassLoader(null)
+  @Shared
+  ParallelWebappClassLoader classloader
 
-  def "tomcat class loading delegates to parent for agent classes"() {
-    setup:
+  def setupSpec() {
+    WebResourceRoot resources = Mock(WebResourceRoot) {
+      getResource(_) >> Mock(WebResource)
+      getClassLoaderResource(_) >> Mock(WebResource)
+      listResources(_) >> []
+      // Looks like 9.x.x needs this one:
+      getResources(_) >> []
+      getClassLoaderResources(_) >> []
+    }
+    def parentClassLoader = new ClassLoader(null) {
+    }
+    classloader = new ParallelWebappClassLoader(parentClassLoader)
     classloader.setResources(resources)
     classloader.init()
     classloader.start()
+  }
 
+  def "tomcat class loading delegates to parent for agent classes"() {
     expect:
     // If instrumentation didn't work this would blow up with NPE due to incomplete resources mocking
     classloader.loadClass("io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge")
   }
+
+  def "test resource injection"() {
+    setup:
+    def tmpFile = Files.createTempFile("hello", "tmp")
+    Files.write(tmpFile, "hello".getBytes(StandardCharsets.UTF_8))
+    def url = tmpFile.toUri().toURL()
+    HelperResources.register(classloader, "hello.txt", Arrays.asList(url))
+
+    expect:
+    classloader.getResource("hello.txt") != null
+
+    and:
+    def resources = classloader.getResources("hello.txt")
+    resources != null
+    resources.hasMoreElements()
+
+    and:
+    def inputStream = classloader.getResourceAsStream("hello.txt")
+    inputStream != null
+    String text = new BufferedReader(
+      new InputStreamReader(inputStream, StandardCharsets.UTF_8))
+      .lines()
+      .collect(Collectors.joining("\n"))
+    text == "hello"
+
+    cleanup:
+    inputStream?.close()
+  }
 }