Route classloads to classloader by package (DataDog/dd-trace-java#1598)
MODIFY
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/AgentClassLoader.java
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/AgentClassLoader.java
@@ -41,6 +41,7 @@ public class AgentClassLoader extends URLClassLoader {
// As a workaround, we keep a reference to the bootstrap jar
// to use only for resource lookups.
private final BootstrapClassLoaderProxy bootstrapProxy;
+
/**
* Construct a new AgentClassLoader
*
@@ -59,20 +60,13 @@ public class AgentClassLoader extends URLClassLoader {
? new BootstrapClassLoaderProxy(new URL[0])
: new BootstrapClassLoaderProxy(new URL[] {bootstrapJarLocation});
+ InternalJarURLHandler internalJarURLHandler =
+ new InternalJarURLHandler(internalJarFileName, bootstrapJarLocation);
try {
// The fields of the URL are mostly dummy. InternalJarURLHandler is the only important
// field. If extending this class from Classloader instead of URLClassloader required less
// boilerplate it could be used and the need for dummy fields would be reduced
-
- final URL internalJarURL =
- new URL(
- "x-internal-jar",
- null,
- 0,
- "/",
- new InternalJarURLHandler(internalJarFileName, bootstrapJarLocation));
-
- addURL(internalJarURL);
+ addURL(new URL("x-internal-jar", null, 0, "/", internalJarURLHandler));
} catch (final MalformedURLException e) {
// This can't happen with current URL constructor
log.error("URL malformed. Unsupported JDK?", e);

MODIFY
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/InternalJarURLHandler.java
agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/InternalJarURLHandler.java
package io.opentelemetry.auto.bootstrap;
+import io.opentelemetry.auto.bootstrap.instrumentation.api.Pair;
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.lang.ref.WeakReference;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLConnection;
import java.net.URLStreamHandler;
-import java.nio.file.NoSuchFileException;
import java.security.Permission;
import java.util.Enumeration;
import java.util.HashMap;
@@ -35,24 +36,33 @@ import lombok.extern.slf4j.Slf4j;
@Slf4j
public class InternalJarURLHandler extends URLStreamHandler {
+
+ private static final WeakReference<Pair<String, JarEntry>> NULL = new WeakReference<>(null);
+
+ private final String name;
+ private final FileNotInInternalJar notFound;
private final Map<String, JarEntry> filenameToEntry = new HashMap<>();
- private JarFile bootstrapJarFile;
+ private final JarFile bootstrapJarFile;
+
+ private WeakReference<Pair<String, JarEntry>> cache = NULL;
InternalJarURLHandler(final String internalJarFileName, final URL bootstrapJarLocation) {
+ name = internalJarFileName;
+ notFound = new FileNotInInternalJar(internalJarFileName);
final String filePrefix = internalJarFileName + "/";
-
+ JarFile jarFile = null;
try {
if (bootstrapJarLocation != null) {
- bootstrapJarFile = new JarFile(new File(bootstrapJarLocation.toURI()), false);
- final Enumeration<JarEntry> entries = bootstrapJarFile.entries();
+ jarFile = new JarFile(new File(bootstrapJarLocation.toURI()), false);
+ final Enumeration<JarEntry> entries = jarFile.entries();
while (entries.hasMoreElements()) {
final JarEntry entry = entries.nextElement();
-
if (!entry.isDirectory() && entry.getName().startsWith(filePrefix)) {
String name = entry.getName();
// remove data suffix
int end = name.endsWith(".classdata") ? name.length() - 4 : name.length();
- filenameToEntry.put(name.substring(internalJarFileName.length(), end), entry);
+ String fileName = name.substring(internalJarFileName.length(), end);
+ filenameToEntry.put(fileName, entry);
}
}
}
@@ -63,6 +73,7 @@ public class InternalJarURLHandler extends URLStreamHandler {
if (filenameToEntry.isEmpty()) {
log.warn("No internal jar entries found");
}
+ bootstrapJarFile = jarFile;
}
@Override
@@ -75,12 +86,27 @@ public class InternalJarURLHandler extends URLStreamHandler {
// nullInputStream() is not available until Java 11
return new InternalJarURLConnection(url, new ByteArrayInputStream(new byte[0]));
}
+ // believe it or not, we're going to get called twice for this,
+ // and the key will be a new object each time.
+ Pair<String, JarEntry> pair = cache.get();
+ if (null == pair || !filename.equals(pair.getLeft())) {
final JarEntry entry = filenameToEntry.get(filename);
if (null != entry) {
- return new InternalJarURLConnection(url, bootstrapJarFile.getInputStream(entry));
+ pair = Pair.of(filename, entry);
+ // the cache field is not volatile as a performance optimization
+ // in the rare event this write is not visible to another thread,
+ // it just means the same work is recomputed but does not affect consistency
+ cache = new WeakReference<>(pair);
+ } else {
+ log.debug("{} not found in {}", filename, name);
+ throw notFound;
+ }
} else {
- throw new NoSuchFileException(url.getFile(), null, url.getFile() + " not in internal jar");
+ // hack: just happen to know this only ever happens twice,
+ // so dismiss cache after a hit
+ cache = NULL;
}
+ return new InternalJarURLConnection(url, bootstrapJarFile.getInputStream(pair.getRight()));
}
private static class InternalJarURLConnection extends URLConnection {
@@ -107,4 +133,16 @@ public class InternalJarURLHandler extends URLStreamHandler {
return null;
}
}
+
+ private static class FileNotInInternalJar extends IOException {
+
+ public FileNotInInternalJar(String jarName) {
+ super("class not found in " + jarName);
+ }
+
+ @Override
+ public Throwable fillInStackTrace() {
+ return this;
+ }
+ }
}

ADD
agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/InternalJarURLHandlerTest.groovy
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package datadog.trace.bootstrap
+
+import io.opentelemetry.auto.bootstrap.InternalJarURLHandler
+import io.opentelemetry.auto.util.test.AgentSpecification
+import spock.lang.Shared
+
+class InternalJarURLHandlerTest extends AgentSpecification {
+
+ @Shared
+ URL testJarLocation = new File("src/test/resources/classloader-test-jar/testjar-jdk8").toURI().toURL()
+
+ def "test get URL"() {
+ setup:
+ InternalJarURLHandler handler = new InternalJarURLHandler(dir, testJarLocation)
+ when:
+ URLConnection connection = handler.openConnection(new URL('file://' + file))
+ assert connection != null
+ byte[] data = new byte[128]
+ int read = connection.getInputStream().read(data)
+ then:
+ read > 0
+
+ where:
+ dir | file
+ "isolated" | '/a/A.class'
+ "isolated" | '/a/b/B.class'
+ "isolated" | '/a/b/c/C.class'
+ }
+
+
+ // guards against InternalJarURLHandler caching and re-using the same stream twice
+ def "test read class twice"() {
+ setup:
+ InternalJarURLHandler handler = new InternalJarURLHandler(dir, testJarLocation)
+ when:
+ URLConnection connection = handler.openConnection(new URL('file://' + file))
+ assert connection != null
+ InputStream is = connection.getInputStream()
+ is.close()
+ connection = handler.openConnection(new URL('file://' + file))
+ assert connection != null
+ is = connection.getInputStream()
+ byte[] data = new byte[128]
+ int read = is.read(data)
+
+ then:
+ read > 0
+
+ where:
+ dir | file
+ "isolated" | '/a/A.class'
+ "isolated" | '/a/b/B.class'
+ "isolated" | '/a/b/c/C.class'
+ }
+
+ def "handle not found"() {
+ setup:
+ InternalJarURLHandler handler = new InternalJarURLHandler(dir, testJarLocation)
+ when:
+ handler.openConnection(new File(file).toURI().toURL())
+ then:
+ // not going to specify (and thereby constrain) the sub type because it doesn't matter
+ thrown IOException
+
+ // permuted
+ where:
+ dir | file
+ "isolated" | '/x/X.class'
+ }
+}

ADD
agent-bootstrap/src/test/resources/classloader-test-jar/testjar-jdk8
agent-bootstrap/src/test/resources/classloader-test-jar/testjar-jdk8
Binary files /dev/null and b/agent-bootstrap/src/test/resources/classloader-test-jar/testjar-jdk8 differ

ADD
agent-bootstrap/src/test/resources/classloader-test-jar/testjar-readme.md
+Jar file has a structure similar to the agent jar:
+
+```bash
+jar -tf testjar
+
+META-INF/
+META-INF/MANIFEST.MF
+isolated/
+isolated/a/
+isolated/a/A.classdata
+isolated/a/b/
+isolated/a/b/B.classdata
+isolated/a/b/c/
+isolated/a/b/c/C.classdata
+```

MODIFY
opentelemetry-javaagent/src/test/java/io/opentelemetry/auto/test/IntegrationTestUtils.java
opentelemetry-javaagent/src/test/java/io/opentelemetry/auto/test/IntegrationTestUtils.java
@@ -25,6 +25,7 @@ import java.io.InputStreamReader;
import java.lang.management.ManagementFactory;
import java.lang.management.RuntimeMXBean;
import java.lang.reflect.Field;
+import java.lang.reflect.Method;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
@@ -70,13 +71,8 @@ public class IntegrationTestUtils {
/** Returns the URL to the jar the agent appended to the bootstrap classpath * */
public static ClassLoader getBootstrapProxy() throws Exception {
final ClassLoader agentClassLoader = getAgentClassLoader();
- final Field field = agentClassLoader.getClass().getDeclaredField("bootstrapProxy");
- try {
- field.setAccessible(true);
- return (ClassLoader) field.get(agentClassLoader);
- } finally {
- field.setAccessible(false);
- }
+ final Method getBootstrapProxy = agentClassLoader.getClass().getMethod("getBootstrapProxy");
+ return (ClassLoader) getBootstrapProxy.invoke(agentClassLoader);
}
/** See {@link IntegrationTestUtils#createJarWithClasses(String, Class[])} */

Route classloads to classloader by package (DataDog/dd-trace-java#1598)