Better support for wrapping premain (#3302)

* Better support for wrapping premain

* fix

* Fix test

* Sync comment from test
This commit is contained in:
Trask Stalnaker 2021-06-14 23:27:10 -07:00 committed by GitHub
parent 0a21abe15a
commit 48a90b090c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 20 additions and 11 deletions

View File

@ -71,6 +71,9 @@ public final class OpenTelemetryAgent {
File javaagentFile = new File(codeSource.getLocation().toURI()); File javaagentFile = new File(codeSource.getLocation().toURI());
if (javaagentFile.isFile()) { if (javaagentFile.isFile()) {
// passing verify false for vendors who sign the agent jar, because jar file signature
// verification is very slow before the JIT compiler starts up, which on Java 8 is not until
// after premain executes
JarFile agentJar = new JarFile(javaagentFile, false); JarFile agentJar = new JarFile(javaagentFile, false);
verifyJarManifestMainClassIsThis(javaagentFile, agentJar); verifyJarManifestMainClassIsThis(javaagentFile, agentJar);
inst.appendToBootstrapClassLoaderSearch(agentJar); inst.appendToBootstrapClassLoaderSearch(agentJar);
@ -164,18 +167,25 @@ public final class OpenTelemetryAgent {
} }
} }
// this protects against the case where someone adds the contents of opentelemetry-javaagent.jar
// by mistake to their application's "uber.jar"
//
// the reason this can cause issues is because we locate the agent jar based on the CodeSource of
// the OpenTelemetryAgent class, and then we add that jar file to the bootstrap class path
//
// but if we find the OpenTelemetryAgent class in an uber jar file, and we add that (whole) uber
// jar file to the bootstrap class loader, that can cause some applications to break, as there's a
// lot of application and library code that doesn't handle getClassLoader() returning null
// (e.g. https://github.com/qos-ch/logback/pull/291)
private static void verifyJarManifestMainClassIsThis(File jarFile, JarFile agentJar) private static void verifyJarManifestMainClassIsThis(File jarFile, JarFile agentJar)
throws IOException { throws IOException {
Manifest manifest = agentJar.getManifest(); Manifest manifest = agentJar.getManifest();
String mainClass = manifest.getMainAttributes().getValue("Main-Class"); if (manifest.getMainAttributes().getValue("Premain-Class") == null) {
if (!thisClass.getCanonicalName().equals(mainClass)) {
throw new IllegalStateException( throw new IllegalStateException(
"opentelemetry-javaagent is not installed, because class '" "The agent was not installed, because the agent was found in '"
+ thisClass.getCanonicalName()
+ "' is located in '"
+ jarFile + jarFile
+ "'. Make sure you don't have this .class file anywhere, " + "', which doesn't contain a Premain-Class manifest attribute. Make sure that you"
+ "besides opentelemetry-javaagent.jar"); + " haven't included the agent jar file inside of an application uber jar.");
} }
} }

View File

@ -24,10 +24,10 @@ class AgentLoadedIntoBootstrapTest extends Specification {
// to their application's "uber.jar" // to their application's "uber.jar"
// //
// the reason this can cause issues is because we locate the agent jar based on the CodeSource of // the reason this can cause issues is because we locate the agent jar based on the CodeSource of
// the AgentBootstrap class, and then we add that jar file to the bootstrap class path // the OpenTelemetryAgent class, and then we add that jar file to the bootstrap class path
// //
// but if we find the AgentBootstrap class in an uber jar file, and we add that (whole) uber jar // but if we find the OpenTelemetryAgent class in an uber jar file, and we add that (whole) uber
// file to the bootstrap class loader, that can cause some applications to break, as there's a // jar file to the bootstrap class loader, that can cause some applications to break, as there's a
// lot of application and library code that doesn't handle getClassLoader() returning null // lot of application and library code that doesn't handle getClassLoader() returning null
// (e.g. https://github.com/qos-ch/logback/pull/291) // (e.g. https://github.com/qos-ch/logback/pull/291)
def "application uber jar should not be added to the bootstrap class loader"() { def "application uber jar should not be added to the bootstrap class loader"() {

View File

@ -90,7 +90,6 @@ public class IntegrationTestUtils {
Attributes mainAttributes = manifest.getMainAttributes(); Attributes mainAttributes = manifest.getMainAttributes();
mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0"); mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0");
mainAttributes.put(Attributes.Name.MAIN_CLASS, mainClassname); mainAttributes.put(Attributes.Name.MAIN_CLASS, mainClassname);
mainAttributes.put(new Attributes.Name("Premain-Class"), mainClassname);
} }
JarOutputStream target = new JarOutputStream(new FileOutputStream(tmpJar), manifest); JarOutputStream target = new JarOutputStream(new FileOutputStream(tmpJar), manifest);
for (Class<?> clazz : classes) { for (Class<?> clazz : classes) {