From fe540eaad2bb37b6608af4a8fb5c39d2d6cae225 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 2 Jan 2023 12:01:00 +0200 Subject: [PATCH] Resource injection for class loader getResourceAsStream (#7476) In https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/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. --- .../ResourceInjectionInstrumentation.java | 34 +++++++++++ .../test/groovy/TomcatClassloadingTest.groovy | 58 ++++++++++++++++--- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java index cd2404c627..de47454033 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java +++ b/instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java @@ -14,6 +14,8 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; 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 class ResourceInjectionInstrumentation implements TypeInstrumentation { .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 class ResourceInjectionInstrumentation implements TypeInstrumentation { @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 class ResourceInjectionInstrumentation implements TypeInstrumentation { 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/instrumentation/internal/internal-class-loader/javaagent/src/test/groovy/TomcatClassloadingTest.groovy b/instrumentation/internal/internal-class-loader/javaagent/src/test/groovy/TomcatClassloadingTest.groovy index df0a1ce5ac..3d9741a247 100644 --- a/instrumentation/internal/internal-class-loader/javaagent/src/test/groovy/TomcatClassloadingTest.groovy +++ b/instrumentation/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() + } }