Skip to content

Commit 545f3b4

Browse files
authored
Remove the need to bump class version when indy advice is used (#15258)
1 parent efef43a commit 545f3b4

File tree

11 files changed

+228
-512
lines changed

11 files changed

+228
-512
lines changed
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.bootstrap.advice;
7+
8+
import java.lang.invoke.MethodHandles;
9+
import java.util.function.Supplier;
10+
11+
/**
12+
* Helper class that provides a MethodHandles.Lookup that allows defining classes in this package.
13+
*/
14+
public final class AdviceForwardLookupSupplier implements Supplier<MethodHandles.Lookup> {
15+
16+
@Override
17+
public MethodHandles.Lookup get() {
18+
return MethodHandles.lookup();
19+
}
20+
}

javaagent-tooling/build.gradle.kts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,6 @@ testing {
103103
}
104104
}
105105

106-
val testPatchBytecodeVersion by registering(JvmTestSuite::class) {
107-
dependencies {
108-
implementation(project(":javaagent-bootstrap"))
109-
implementation(project(":javaagent-tooling"))
110-
implementation("net.bytebuddy:byte-buddy-dep")
111-
112-
// Used by byte-buddy but not brought in as a transitive dependency.
113-
compileOnly("com.google.code.findbugs:annotations")
114-
}
115-
}
116-
117106
val testConfigFile by registering(JvmTestSuite::class) {
118107
dependencies {
119108
implementation(project(":javaagent-tooling"))

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@
2525
import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstaller;
2626
import io.opentelemetry.javaagent.tooling.field.VirtualFieldImplementationInstallerFactory;
2727
import io.opentelemetry.javaagent.tooling.instrumentation.indy.ClassInjectorImpl;
28+
import io.opentelemetry.javaagent.tooling.instrumentation.indy.ForwardIndyAdviceTransformer;
2829
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyModuleRegistry;
2930
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl;
30-
import io.opentelemetry.javaagent.tooling.instrumentation.indy.PatchByteCodeVersionTransformer;
3131
import io.opentelemetry.javaagent.tooling.muzzle.HelperResourceBuilderImpl;
3232
import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationModuleMuzzle;
3333
import io.opentelemetry.javaagent.tooling.util.IgnoreFailedTypeMatcher;
@@ -134,7 +134,7 @@ private AgentBuilder installIndyModule(
134134
return helpers;
135135
};
136136

137-
AgentBuilder.Transformer helperInjector =
137+
HelperInjector helperInjector =
138138
new HelperInjector(
139139
instrumentationModule.instrumentationName(),
140140
helperGenerator,
@@ -150,13 +150,9 @@ private AgentBuilder installIndyModule(
150150
AgentBuilder.Identified.Extendable extendableAgentBuilder =
151151
setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
152152
.and(muzzleMatcher)
153-
.transform(new PatchByteCodeVersionTransformer());
153+
.transform(ConstantAdjuster.instance())
154+
.transform(new ForwardIndyAdviceTransformer(helperInjector));
154155

155-
// TODO (Jonas): we are not calling
156-
// contextProvider.rewriteVirtualFieldsCalls(extendableAgentBuilder) anymore
157-
// As a result the advices should store `VirtualFields` as static variables instead of having
158-
// the lookup inline
159-
// We need to update our documentation on that
160156
extendableAgentBuilder =
161157
IndyModuleRegistry.initializeModuleLoaderOnMatch(
162158
instrumentationModule, extendableAgentBuilder);
@@ -166,7 +162,6 @@ private AgentBuilder installIndyModule(
166162
new IndyTypeTransformerImpl(extendableAgentBuilder, instrumentationModule);
167163
typeInstrumentation.transform(typeTransformer);
168164
extendableAgentBuilder = typeTransformer.getAgentBuilder();
169-
// TODO (Jonas): make instrumentation of bytecode older than 1.4 opt-in via a config option
170165
extendableAgentBuilder = contextProvider.injectFields(extendableAgentBuilder);
171166

172167
agentBuilder = extendableAgentBuilder;
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.tooling.instrumentation.indy;
7+
8+
import io.opentelemetry.javaagent.bootstrap.IndyBootstrapDispatcher;
9+
import io.opentelemetry.javaagent.bootstrap.advice.AdviceForwardLookupSupplier;
10+
import io.opentelemetry.javaagent.extension.instrumentation.internal.AsmApi;
11+
import io.opentelemetry.javaagent.tooling.HelperInjector;
12+
import java.security.ProtectionDomain;
13+
import java.util.HashMap;
14+
import java.util.Map;
15+
import java.util.concurrent.atomic.AtomicInteger;
16+
import java.util.function.Supplier;
17+
import net.bytebuddy.ClassFileVersion;
18+
import net.bytebuddy.agent.builder.AgentBuilder;
19+
import net.bytebuddy.asm.AsmVisitorWrapper;
20+
import net.bytebuddy.description.field.FieldDescription;
21+
import net.bytebuddy.description.field.FieldList;
22+
import net.bytebuddy.description.method.MethodList;
23+
import net.bytebuddy.description.type.TypeDescription;
24+
import net.bytebuddy.dynamic.DynamicType;
25+
import net.bytebuddy.implementation.Implementation;
26+
import net.bytebuddy.pool.TypePool;
27+
import net.bytebuddy.utility.JavaModule;
28+
import org.objectweb.asm.ClassVisitor;
29+
import org.objectweb.asm.ClassWriter;
30+
import org.objectweb.asm.Handle;
31+
import org.objectweb.asm.MethodVisitor;
32+
import org.objectweb.asm.Opcodes;
33+
import org.objectweb.asm.Type;
34+
import org.objectweb.asm.commons.GeneratorAdapter;
35+
36+
/**
37+
* Replaces {@code INVOKEDYNAMIC} instructions used for invoking advice with {@code INVOKESTATIC}
38+
* instructions in a helper class that contains the original {@code INVOKEDYNAMIC} instruction in
39+
* classes that do not support {@code INVOKEDYNAMIC} (i.e. pre Java 7 class files).
40+
*/
41+
public class ForwardIndyAdviceTransformer implements AgentBuilder.Transformer {
42+
private static final AtomicInteger counter = new AtomicInteger();
43+
private static final String bootForwardClassPackage =
44+
AdviceForwardLookupSupplier.class.getPackage().getName();
45+
46+
private final HelperInjector helperInjector;
47+
48+
public ForwardIndyAdviceTransformer(HelperInjector helperInjector) {
49+
this.helperInjector = helperInjector;
50+
}
51+
52+
private static boolean isAtLeastJava7(TypeDescription typeDescription) {
53+
ClassFileVersion classFileVersion = typeDescription.getClassFileVersion();
54+
return classFileVersion != null && classFileVersion.getJavaVersion() >= 7;
55+
}
56+
57+
@Override
58+
public DynamicType.Builder<?> transform(
59+
DynamicType.Builder<?> builder,
60+
TypeDescription typeDescription,
61+
ClassLoader classLoader,
62+
JavaModule javaModule,
63+
ProtectionDomain protectionDomain) {
64+
65+
// java 7+ class files already support invokedynamic
66+
if (isAtLeastJava7(typeDescription)) {
67+
return builder;
68+
}
69+
70+
return builder.visit(
71+
new AsmVisitorWrapper.AbstractBase() {
72+
@Override
73+
public ClassVisitor wrap(
74+
TypeDescription typeDescription,
75+
ClassVisitor classVisitor,
76+
Implementation.Context context,
77+
TypePool typePool,
78+
FieldList<FieldDescription.InDefinedShape> fieldList,
79+
MethodList<?> methodList,
80+
int writerFlags,
81+
int readerFlags) {
82+
83+
return new ClassVisitor(AsmApi.VERSION, classVisitor) {
84+
final Map<String, Supplier<byte[]>> injectedClasses = new HashMap<>();
85+
86+
@Override
87+
public void visitEnd() {
88+
super.visitEnd();
89+
90+
// inject helper classes that forward to the advice using invokedynamic
91+
if (!injectedClasses.isEmpty()) {
92+
helperInjector.injectHelperClasses(classLoader, injectedClasses);
93+
}
94+
}
95+
96+
@Override
97+
public MethodVisitor visitMethod(
98+
int access,
99+
String name,
100+
String descriptor,
101+
String signature,
102+
String[] exceptions) {
103+
MethodVisitor mv =
104+
super.visitMethod(access, name, descriptor, signature, exceptions);
105+
return new MethodVisitor(api, mv) {
106+
@Override
107+
public void visitInvokeDynamicInsn(
108+
String name,
109+
String descriptor,
110+
Handle bootstrapMethodHandle,
111+
Object... bootstrapMethodArguments) {
112+
if (Type.getInternalName(IndyBootstrapDispatcher.class)
113+
.equals(bootstrapMethodHandle.getOwner())) {
114+
115+
String adviceClassName = (String) bootstrapMethodArguments[3];
116+
String forwardClassDotName =
117+
classLoader == null
118+
? bootForwardClassPackage + ".Forward$$" + counter.incrementAndGet()
119+
: adviceClassName + "$$Forward$$" + counter.incrementAndGet();
120+
String forwardClassSlasName = forwardClassDotName.replace('.', '/');
121+
122+
Supplier<byte[]> forwardClassBytes =
123+
generateForwardClass(
124+
forwardClassSlasName,
125+
name,
126+
descriptor,
127+
bootstrapMethodHandle,
128+
bootstrapMethodArguments);
129+
injectedClasses.put(forwardClassDotName, forwardClassBytes);
130+
131+
// replace invokedynamic with invokestatic to the generated forwarder class
132+
// the forwarder class will contain the original invokedynamic instruction
133+
super.visitMethodInsn(
134+
Opcodes.INVOKESTATIC, forwardClassSlasName, name, descriptor, false);
135+
return;
136+
}
137+
138+
super.visitInvokeDynamicInsn(
139+
name, descriptor, bootstrapMethodHandle, bootstrapMethodArguments);
140+
}
141+
};
142+
}
143+
};
144+
}
145+
});
146+
}
147+
148+
private static Supplier<byte[]> generateForwardClass(
149+
String forwardClassSlasName,
150+
String methodName,
151+
String methodDescriptor,
152+
Handle bootstrapMethodHandle,
153+
Object[] bootstrapMethodArguments) {
154+
return () -> {
155+
ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
156+
cw.visit(
157+
Opcodes.V1_8,
158+
Opcodes.ACC_PUBLIC,
159+
forwardClassSlasName,
160+
null,
161+
Type.getInternalName(Object.class),
162+
null);
163+
MethodVisitor mv =
164+
cw.visitMethod(
165+
Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, methodName, methodDescriptor, null, null);
166+
GeneratorAdapter ga =
167+
new GeneratorAdapter(
168+
mv, Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC, methodName, methodDescriptor);
169+
ga.loadArgs();
170+
mv.visitInvokeDynamicInsn(
171+
methodName, methodDescriptor, bootstrapMethodHandle, bootstrapMethodArguments);
172+
ga.returnValue();
173+
mv.visitMaxs(0, 0);
174+
mv.visitEnd();
175+
cw.visitEnd();
176+
177+
return cw.toByteArray();
178+
};
179+
}
180+
}

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/PatchByteCodeVersionTransformer.java

Lines changed: 0 additions & 103 deletions
This file was deleted.

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/util/ClassLoaderMap.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@
2020
class ClassLoaderMap {
2121
private static final Cache<ClassLoader, WeakReference<Map<Object, Object>>> data = Cache.weak();
2222
private static final Map<Object, Object> bootLoaderData = new ConcurrentHashMap<>();
23+
private static final HelperInjector helperInjector =
24+
HelperInjector.forDynamicTypes(
25+
ClassLoaderMap.class.getSimpleName(), Collections.emptyList(), null);
2326
static Injector defaultInjector =
2427
(classLoader, className, bytes) -> {
25-
HelperInjector.injectHelperClasses(
28+
helperInjector.injectHelperClasses(
2629
classLoader, Collections.singletonMap(className, () -> bytes));
2730
return Class.forName(className, false, classLoader);
2831
};

0 commit comments

Comments
 (0)