Skip to content

Commit b7038a8

Browse files
committed
Remove the need to bump class version when indy advice is used
1 parent 2c1f91b commit b7038a8

File tree

5 files changed

+240
-21
lines changed

5 files changed

+240
-21
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/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
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;
3031
import io.opentelemetry.javaagent.tooling.instrumentation.indy.PatchByteCodeVersionTransformer;
@@ -134,7 +135,7 @@ private AgentBuilder installIndyModule(
134135
return helpers;
135136
};
136137

137-
AgentBuilder.Transformer helperInjector =
138+
HelperInjector helperInjector =
138139
new HelperInjector(
139140
instrumentationModule.instrumentationName(),
140141
helperGenerator,
@@ -145,18 +146,24 @@ private AgentBuilder installIndyModule(
145146
VirtualFieldImplementationInstaller contextProvider =
146147
virtualFieldInstallerFactory.create(instrumentationModule);
147148

149+
boolean allowClassVersionChange =
150+
config.getBoolean("otel.javaagent.experimental.allow-class-version-change", false);
148151
AgentBuilder agentBuilder = parentAgentBuilder;
149152
for (TypeInstrumentation typeInstrumentation : instrumentationModule.typeInstrumentations()) {
150-
AgentBuilder.Identified.Extendable extendableAgentBuilder =
153+
AgentBuilder.Identified.Extendable extendableAgentBuilder;
154+
AgentBuilder.Identified.Narrowable narrowableAgentBuilder =
151155
setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
152-
.and(muzzleMatcher)
153-
.transform(new PatchByteCodeVersionTransformer());
156+
.and(muzzleMatcher);
157+
if (allowClassVersionChange) {
158+
extendableAgentBuilder =
159+
narrowableAgentBuilder.transform(new PatchByteCodeVersionTransformer());
160+
} else {
161+
extendableAgentBuilder =
162+
narrowableAgentBuilder
163+
.transform(ConstantAdjuster.instance())
164+
.transform(new ForwardIndyAdviceTransformer(helperInjector));
165+
}
154166

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
160167
extendableAgentBuilder =
161168
IndyModuleRegistry.initializeModuleLoaderOnMatch(
162169
instrumentationModule, extendableAgentBuilder);
@@ -166,7 +173,6 @@ private AgentBuilder installIndyModule(
166173
new IndyTypeTransformerImpl(extendableAgentBuilder, instrumentationModule);
167174
typeInstrumentation.transform(typeTransformer);
168175
extendableAgentBuilder = typeTransformer.getAgentBuilder();
169-
// TODO (Jonas): make instrumentation of bytecode older than 1.4 opt-in via a config option
170176
extendableAgentBuilder = contextProvider.injectFields(extendableAgentBuilder);
171177

172178
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+
// temporarily disable version check for testing
66+
if (false) {
67+
// java 7+ class files already support invokedynamic
68+
if (isAtLeastJava7(typeDescription)) {
69+
return builder;
70+
}
71+
}
72+
73+
return builder.visit(
74+
new AsmVisitorWrapper.AbstractBase() {
75+
@Override
76+
public ClassVisitor wrap(
77+
TypeDescription typeDescription,
78+
ClassVisitor classVisitor,
79+
Implementation.Context context,
80+
TypePool typePool,
81+
FieldList<FieldDescription.InDefinedShape> fieldList,
82+
MethodList<?> methodList,
83+
int writerFlags,
84+
int readerFlags) {
85+
86+
return new ClassVisitor(AsmApi.VERSION, classVisitor) {
87+
final Map<String, Supplier<byte[]>> injectedClasses = new HashMap<>();
88+
89+
@Override
90+
public void visitEnd() {
91+
super.visitEnd();
92+
93+
if (!injectedClasses.isEmpty()) {
94+
helperInjector.injectHelperClasses(classLoader, injectedClasses);
95+
}
96+
}
97+
98+
@Override
99+
public MethodVisitor visitMethod(
100+
int access,
101+
String name,
102+
String descriptor,
103+
String signature,
104+
String[] exceptions) {
105+
MethodVisitor mv =
106+
super.visitMethod(access, name, descriptor, signature, exceptions);
107+
return new MethodVisitor(api, mv) {
108+
@Override
109+
public void visitInvokeDynamicInsn(
110+
String name,
111+
String descriptor,
112+
Handle bootstrapMethodHandle,
113+
Object... bootstrapMethodArguments) {
114+
if (Type.getInternalName(IndyBootstrapDispatcher.class)
115+
.equals(bootstrapMethodHandle.getOwner())) {
116+
117+
String adviceClassName = (String) bootstrapMethodArguments[3];
118+
String forwardClassDotName =
119+
classLoader == null
120+
? bootForwardClassPackage + ".Forward$$" + counter.incrementAndGet()
121+
: adviceClassName + "$$Forward$$" + counter.incrementAndGet();
122+
String forwardClassSlasName = forwardClassDotName.replace('.', '/');
123+
124+
Supplier<byte[]> forwardClassBytes =
125+
generateForwardClass(
126+
forwardClassSlasName,
127+
name,
128+
descriptor,
129+
bootstrapMethodHandle,
130+
bootstrapMethodArguments);
131+
injectedClasses.put(forwardClassDotName, forwardClassBytes);
132+
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/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
};

muzzle/src/main/java/io/opentelemetry/javaagent/tooling/HelperInjector.java

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import io.opentelemetry.javaagent.bootstrap.HelperResources;
1414
import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper;
1515
import io.opentelemetry.javaagent.bootstrap.InjectedClassHelper.HelperClassInfo;
16+
import io.opentelemetry.javaagent.bootstrap.advice.AdviceForwardLookupSupplier;
1617
import io.opentelemetry.javaagent.bootstrap.field.VirtualFieldLookupSupplier;
1718
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
1819
import io.opentelemetry.javaagent.instrumentation.executors.ExecutorLookupSupplier;
@@ -289,26 +290,30 @@ private void injectHelperClasses(
289290
if (isBootClassLoader(classLoader)) {
290291
injectBootstrapClassLoader(classnameToBytes);
291292
}
292-
} catch (Exception e) {
293+
} catch (RuntimeException e) {
293294
if (logger.isLoggable(SEVERE)) {
294295
logger.log(
295296
SEVERE,
296297
"Error preparing helpers while processing {0} for {1}. Failed to inject helper classes into instance {2}",
297298
new Object[] {typeDescription, requestingName, classLoader},
298299
e);
299300
}
300-
throw new IllegalStateException(e);
301+
throw e;
301302
}
302303
}
303304

304-
public static void injectHelperClasses(
305+
public void injectHelperClasses(
305306
ClassLoader classLoader, Map<String, Supplier<byte[]>> classNameToBytes) {
306-
if (isBootClassLoader(classLoader)) {
307-
throw new UnsupportedOperationException("boot loader not supported");
308-
}
309307
if (classNameToBytes.isEmpty()) {
310308
return;
311309
}
310+
if (classLoader == null) {
311+
if (instrumentation == null) {
312+
throw new UnsupportedOperationException("boot loader not supported");
313+
}
314+
injectBootstrapClassLoader(classNameToBytes);
315+
return;
316+
}
312317

313318
Map<String, HelperClass> map =
314319
helperClasses.computeIfAbsent(classLoader, (unused) -> new ConcurrentHashMap<>());
@@ -339,6 +344,7 @@ private static Map<String, byte[]> resolve(Map<String, Supplier<byte[]>> classes
339344
// because all generated virtual field classes are in the same package we can use lookup to
340345
// define them
341346
addPackageLookup(new VirtualFieldLookupSupplier());
347+
addPackageLookup(new AdviceForwardLookupSupplier());
342348
}
343349

344350
private static void addPackageLookup(Supplier<MethodHandles.Lookup> supplier) {
@@ -350,7 +356,7 @@ private static ClassInjector getClassInjector(String packageName) {
350356
return lookup != null ? ClassInjector.UsingLookup.of(lookup) : null;
351357
}
352358

353-
private void injectBootstrapClassLoader(Map<String, Supplier<byte[]>> inject) throws IOException {
359+
private void injectBootstrapClassLoader(Map<String, Supplier<byte[]>> inject) {
354360
Map<String, byte[]> classnameToBytes = resolve(inject);
355361
if (helperInjectorListener != null) {
356362
helperInjectorListener.onInjection(classnameToBytes);
@@ -404,7 +410,7 @@ private void injectBootstrapClassLoader(Map<String, Supplier<byte[]>> inject) th
404410
// a reference count -- but for now, starting simple.
405411

406412
// Failures to create a tempDir are propagated as IOException and handled by transform
407-
if (!classnameToBytes.isEmpty()) {
413+
if (!classnameToBytes.isEmpty() && instrumentation != null) {
408414
File tempDir = createTempDir();
409415
try {
410416
ClassInjector.UsingInstrumentation.of(
@@ -417,8 +423,12 @@ private void injectBootstrapClassLoader(Map<String, Supplier<byte[]>> inject) th
417423
}
418424
}
419425

420-
private static File createTempDir() throws IOException {
421-
return Files.createTempDirectory("opentelemetry-temp-jars").toFile();
426+
private static File createTempDir() {
427+
try {
428+
return Files.createTempDirectory("opentelemetry-temp-jars").toFile();
429+
} catch (IOException exception) {
430+
throw new IllegalStateException("Failed to create temporary directory.", exception);
431+
}
422432
}
423433

424434
private static void deleteTempDir(File file) {

0 commit comments

Comments
 (0)