Skip to content

Commit 03aa810

Browse files
SylvainJugeSylvainJuge
committed
avoid injecting classes for kotlinxcoroutines
1 parent 1d54a14 commit 03aa810

File tree

3 files changed

+62
-56
lines changed

3 files changed

+62
-56
lines changed

‎instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationHelper.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,20 @@
1515
import kotlin.coroutines.Continuation;
1616
import kotlin.coroutines.intrinsics.IntrinsicsKt;
1717

18+
/**
19+
* Instrumentation helper that is called through bytecode instrumentation. When using invokedynamic
20+
* instrumentation this class is called through an injected proxy, and thus it should not pull any
21+
* other class references than the ones that are already present in the target classloader or the
22+
* bootstrap classloader. This is why the {@link MethodRequest} class is here passed as an {@link
23+
* Object} as it allows to avoid having to inject extra classes in the target classloader
24+
*/
25+
@SuppressWarnings("unused") // methods calls injected through bytecode instrumentation
1826
public final class AnnotationInstrumentationHelper {
1927

2028
private static final VirtualField<Continuation<?>, Context> contextField =
2129
VirtualField.find(Continuation.class, Context.class);
2230

23-
public static MethodRequest createMethodRequest(
31+
public static Object createMethodRequest(
2432
Class<?> declaringClass, String methodName, String withSpanValue, String spanKindString) {
2533
SpanKind spanKind = SpanKind.INTERNAL;
2634
if (spanKindString != null) {
@@ -34,11 +42,10 @@ public static MethodRequest createMethodRequest(
3442
return MethodRequest.create(declaringClass, methodName, withSpanValue, spanKind);
3543
}
3644

37-
public static Context enterCoroutine(
38-
int label, Continuation<?> continuation, MethodRequest request) {
45+
public static Context enterCoroutine(int label, Continuation<?> continuation, Object request) {
3946
// label 0 means that coroutine is started, any other label means that coroutine is resumed
4047
if (label == 0) {
41-
Context context = instrumenter().start(Context.current(), request);
48+
Context context = instrumenter().start(Context.current(), (MethodRequest) request);
4249
// null continuation means that this method is not going to be resumed, and we don't need to
4350
// store the context
4451
if (continuation != null) {
@@ -55,18 +62,14 @@ public static Scope openScope(Context context) {
5562
}
5663

5764
public static void exitCoroutine(
58-
Object result,
59-
MethodRequest request,
60-
Continuation<?> continuation,
61-
Context context,
62-
Scope scope) {
65+
Object result, Object request, Continuation<?> continuation, Context context, Scope scope) {
6366
exitCoroutine(null, result, request, continuation, context, scope);
6467
}
6568

6669
public static void exitCoroutine(
6770
Throwable error,
6871
Object result,
69-
MethodRequest request,
72+
Object request,
7073
Continuation<?> continuation,
7174
Context context,
7275
Scope scope) {
@@ -78,7 +81,7 @@ public static void exitCoroutine(
7881
// end the span when this method can not be resumed (coroutine is null) or if it has reached
7982
// final state (returns anything else besides COROUTINE_SUSPENDED)
8083
if (continuation == null || result != IntrinsicsKt.getCOROUTINE_SUSPENDED()) {
81-
instrumenter().end(context, request, null, error);
84+
instrumenter().end(context, (MethodRequest) request, null, error);
8285
}
8386
}
8487

‎instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/AnnotationInstrumentationModule.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1313
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1414
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
15-
import java.util.Arrays;
15+
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
16+
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
1617
import java.util.List;
1718
import net.bytebuddy.matcher.ElementMatcher;
1819

@@ -49,11 +50,15 @@ public List<TypeInstrumentation> typeInstrumentations() {
4950
}
5051

5152
@Override
52-
public List<String> injectedClassNames() {
53-
return Arrays.asList(
54-
"io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationSingletons",
55-
"io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationInstrumentationHelper",
56-
"io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.MethodRequest",
57-
"io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.MethodRequestCodeAttributesGetter");
53+
public void injectClasses(ClassInjector injector) {
54+
// AnnotationInstrumentationHelper is called directly in the instrumented bytecode.
55+
//
56+
// With invokedynamic instrumentation a proxy class can be used as long as it does not pull
57+
// extra types in the method signatures (which would require those types to also be available
58+
// in the instrumented code).
59+
injector
60+
.proxyBuilder(
61+
"io.opentelemetry.javaagent.instrumentation.kotlinxcoroutines.instrumentationannotations.AnnotationInstrumentationHelper")
62+
.inject(InjectionMode.CLASS_ONLY);
5863
}
5964
}

‎instrumentation/kotlinx-coroutines/kotlinx-coroutines-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kotlinxcoroutines/instrumentationannotations/WithSpanInstrumentation.java‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ private static MethodVisitor instrument(
232232
public void visitCode() {
233233
super.visitCode();
234234
// add our local variables after method arguments, this will shift rest of the locals
235-
requestLocal = newLocal(Type.getType(MethodRequest.class));
235+
requestLocal = newLocal(Type.getType(Object.class));
236236
ourContinuationLocal = newLocal(Type.getType(Continuation.class));
237237
contextLocal = newLocal(Type.getType(Context.class));
238238
scopeLocal = newLocal(Type.getType(Scope.class));
@@ -360,32 +360,26 @@ public void visitEnd() {
360360
} else {
361361
temp.visitInsn(Opcodes.ACONST_NULL);
362362
}
363-
temp.visitMethodInsn(
364-
Opcodes.INVOKESTATIC,
365-
Type.getInternalName(AnnotationInstrumentationHelper.class),
363+
visitInvokeHelperMethod(
364+
temp,
366365
"createMethodRequest",
367-
"(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)"
368-
+ Type.getDescriptor(MethodRequest.class),
369-
false);
366+
"(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)Ljava/lang/Object;");
370367
temp.visitInsn(Opcodes.DUP);
371368
temp.visitVarInsn(Opcodes.ASTORE, requestLocal);
372-
temp.visitMethodInsn(
373-
Opcodes.INVOKESTATIC,
374-
Type.getInternalName(AnnotationInstrumentationHelper.class),
369+
visitInvokeHelperMethod(
370+
temp,
375371
"enterCoroutine",
376-
"(ILkotlin/coroutines/Continuation;"
377-
+ Type.getDescriptor(MethodRequest.class)
378-
+ ")"
379-
+ Type.getDescriptor(Context.class),
380-
false);
372+
"(ILkotlin/coroutines/Continuation;Ljava/lang/Object;)"
373+
+ Type.getDescriptor(Context.class));
381374
temp.visitInsn(Opcodes.DUP);
382375
temp.visitVarInsn(Opcodes.ASTORE, contextLocal);
383-
temp.visitMethodInsn(
384-
Opcodes.INVOKESTATIC,
385-
Type.getInternalName(AnnotationInstrumentationHelper.class),
376+
visitInvokeHelperMethod(
377+
temp,
386378
"openScope",
387-
"(" + Type.getDescriptor(Context.class) + ")" + Type.getDescriptor(Scope.class),
388-
false);
379+
"("
380+
+ Type.getDescriptor(Context.class)
381+
+ ")"
382+
+ Type.getDescriptor(Scope.class));
389383
temp.visitVarInsn(Opcodes.ASTORE, scopeLocal);
390384
// @SpanAttribute handling
391385
for (Parameter parameter : annotatedParameters) {
@@ -396,14 +390,12 @@ public void visitEnd() {
396390
boolean primitive =
397391
parameter.type.getSort() != Type.ARRAY
398392
&& parameter.type.getSort() != Type.OBJECT;
399-
temp.visitMethodInsn(
400-
Opcodes.INVOKESTATIC,
401-
Type.getInternalName(AnnotationInstrumentationHelper.class),
393+
visitInvokeHelperMethod(
394+
temp,
402395
"setSpanAttribute",
403396
"(ILjava/lang/String;"
404397
+ (primitive ? parameter.type.getDescriptor() : "Ljava/lang/Object;")
405-
+ ")V",
406-
false);
398+
+ ")V");
407399
}
408400
// pop label
409401
temp.visitInsn(Opcodes.POP);
@@ -446,7 +438,7 @@ public void visitEnd() {
446438
// in this handler we are using only the locals we added, we don't care about method
447439
// arguments and this, so we don't list them in the stack frame
448440
Arrays.fill(locals, Opcodes.TOP);
449-
locals[requestLocal] = Type.getInternalName(MethodRequest.class);
441+
locals[requestLocal] = Type.getInternalName(Object.class);
450442
locals[ourContinuationLocal] = Type.getInternalName(Continuation.class);
451443
locals[contextLocal] = Type.getInternalName(Context.class);
452444
locals[scopeLocal] = Type.getInternalName(Scope.class);
@@ -463,17 +455,15 @@ public void visitEnd() {
463455
temp.visitVarInsn(Opcodes.ALOAD, ourContinuationLocal);
464456
temp.visitVarInsn(Opcodes.ALOAD, contextLocal);
465457
temp.visitVarInsn(Opcodes.ALOAD, scopeLocal);
466-
temp.visitMethodInsn(
467-
Opcodes.INVOKESTATIC,
468-
Type.getInternalName(AnnotationInstrumentationHelper.class),
458+
visitInvokeHelperMethod(
459+
temp,
469460
"exitCoroutine",
470461
"(Ljava/lang/Throwable;Ljava/lang/Object;"
471-
+ Type.getDescriptor(MethodRequest.class)
462+
+ "Ljava/lang/Object;"
472463
+ Type.getDescriptor(Continuation.class)
473464
+ Type.getDescriptor(Context.class)
474465
+ Type.getDescriptor(Scope.class)
475-
+ ")V",
476-
false);
466+
+ ")V");
477467

478468
// rethrow the exception
479469
temp.visitInsn(Opcodes.ATHROW);
@@ -498,17 +488,15 @@ public void visitEnd() {
498488
temp.visitVarInsn(Opcodes.ALOAD, ourContinuationLocal);
499489
temp.visitVarInsn(Opcodes.ALOAD, contextLocal);
500490
temp.visitVarInsn(Opcodes.ALOAD, scopeLocal);
501-
temp.visitMethodInsn(
502-
Opcodes.INVOKESTATIC,
503-
Type.getInternalName(AnnotationInstrumentationHelper.class),
491+
visitInvokeHelperMethod(
492+
temp,
504493
"exitCoroutine",
505494
"(Ljava/lang/Object;"
506-
+ Type.getDescriptor(MethodRequest.class)
495+
+ "Ljava/lang/Object;"
507496
+ Type.getDescriptor(Continuation.class)
508497
+ Type.getDescriptor(Context.class)
509498
+ Type.getDescriptor(Scope.class)
510-
+ ")V",
511-
false);
499+
+ ")V");
512500
methodNode.instructions.insertBefore(instruction, temp.instructions);
513501
}
514502
}
@@ -519,5 +507,15 @@ public void visitEnd() {
519507

520508
return generatorAdapter;
521509
}
510+
511+
private static void visitInvokeHelperMethod(
512+
MethodNode methodNode, String methodName, String descriptor) {
513+
methodNode.visitMethodInsn(
514+
Opcodes.INVOKESTATIC,
515+
Type.getInternalName(AnnotationInstrumentationHelper.class),
516+
methodName,
517+
descriptor,
518+
false);
519+
}
522520
}
523521
}

0 commit comments

Comments
 (0)