Skip to content

Commit 427c8e5

Browse files
JonasKunzSylvainJugetrask
authored
Make internal classloader instrumentation indy compatible (#12242)
Co-authored-by: SylvainJuge <[email protected]> Co-authored-by: Trask Stalnaker <[email protected]>
1 parent ca44975 commit 427c8e5

File tree

12 files changed

+281
-74
lines changed

12 files changed

+281
-74
lines changed

‎instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/BootDelegationInstrumentation.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,7 @@ public class BootDelegationInstrumentation implements TypeInstrumentation {
4747
public ElementMatcher<TypeDescription> typeMatcher() {
4848
// just an optimization to exclude common class loaders that are known to delegate to the
4949
// bootstrap loader (or happen to _be_ the bootstrap loader)
50-
return not(namedOneOf(
51-
"java.lang.ClassLoader",
52-
"com.ibm.oti.vm.BootstrapClassLoader",
53-
"io.opentelemetry.javaagent.bootstrap.AgentClassLoader"))
50+
return not(namedOneOf("java.lang.ClassLoader", "com.ibm.oti.vm.BootstrapClassLoader"))
5451
.and(extendsClass(named("java.lang.ClassLoader")));
5552
}
5653

@@ -136,13 +133,14 @@ public static Class<?> onEnter(@Advice.Argument(0) String name) {
136133
return null;
137134
}
138135

139-
@Advice.OnMethodExit(onThrowable = Throwable.class)
140-
public static void onExit(
141-
@Advice.Return(readOnly = false) Class<?> result,
142-
@Advice.Enter Class<?> resultFromBootstrapLoader) {
136+
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
137+
@Advice.AssignReturned.ToReturned
138+
public static Class<?> onExit(
139+
@Advice.Return Class<?> result, @Advice.Enter Class<?> resultFromBootstrapLoader) {
143140
if (resultFromBootstrapLoader != null) {
144-
result = resultFromBootstrapLoader;
141+
return resultFromBootstrapLoader;
145142
}
143+
return result;
146144
}
147145
}
148146
}

‎instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ClassLoaderInstrumentationModule.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,10 @@ public boolean defaultEnabled(ConfigProperties config) {
2525
return true;
2626
}
2727

28-
@Override
29-
public boolean isIndyModule() {
30-
return false;
31-
}
32-
3328
@Override
3429
public boolean isHelperClass(String className) {
30+
// TODO: this can be removed when we drop inlined-advice support
31+
// The advices can directly access this class in the AgentClassLoader with invokedynamic Advice
3532
return className.equals("io.opentelemetry.javaagent.tooling.Constants");
3633
}
3734

‎instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/DefineClassInstrumentation.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,15 @@ public static DefineClassContext onEnter(
5252
classLoader, className, classBytes, offset, length);
5353
}
5454

55+
// TODO: the ToReturned does nothing except for signaling the AdviceTransformer that it must
56+
// not touch this advice
57+
// this is done because we do not want the return values to be wrapped in array types
5558
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
56-
public static void onExit(@Advice.Enter DefineClassContext context) {
59+
@Advice.AssignReturned.ToReturned
60+
public static Class<?> onExit(
61+
@Advice.Enter DefineClassContext context, @Advice.Return Class<?> returned) {
5762
DefineClassHelper.afterDefineClass(context);
63+
return returned;
5864
}
5965
}
6066

@@ -68,9 +74,15 @@ public static DefineClassContext onEnter(
6874
return DefineClassHelper.beforeDefineClass(classLoader, className, classBytes);
6975
}
7076

77+
// TODO: the ToReturned does nothing except for signaling the AdviceTransformer that it must
78+
// not touch this advice
79+
// this is done because we do not want the return values to be wrapped in array types
7180
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
72-
public static void onExit(@Advice.Enter DefineClassContext context) {
81+
@Advice.AssignReturned.ToReturned
82+
public static Class<?> onExit(
83+
@Advice.Enter DefineClassContext context, @Advice.Return Class<?> returned) {
7384
DefineClassHelper.afterDefineClass(context);
85+
return returned;
7486
}
7587
}
7688
}

‎instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/LoadInjectedClassInstrumentation.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,13 @@ public static Class<?> onEnter(
6565
}
6666

6767
@Advice.OnMethodExit(onThrowable = Throwable.class)
68-
public static void onExit(
69-
@Advice.Return(readOnly = false) Class<?> result, @Advice.Enter Class<?> loadedClass) {
68+
@Advice.AssignReturned.ToReturned
69+
public static Class<?> onExit(
70+
@Advice.Return Class<?> result, @Advice.Enter Class<?> loadedClass) {
7071
if (loadedClass != null) {
71-
result = loadedClass;
72+
return loadedClass;
7273
}
74+
return result;
7375
}
7476
}
7577
}

‎instrumentation/internal/internal-class-loader/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/internal/classloader/ResourceInjectionInstrumentation.java‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -61,37 +61,37 @@ public void transform(TypeTransformer transformer) {
6161
public static class GetResourceAdvice {
6262

6363
@Advice.OnMethodExit(suppress = Throwable.class)
64-
public static void onExit(
64+
@Advice.AssignReturned.ToReturned
65+
public static URL onExit(
6566
@Advice.This ClassLoader classLoader,
6667
@Advice.Argument(0) String name,
67-
@Advice.Return(readOnly = false) URL resource) {
68-
if (resource != null) {
69-
return;
70-
}
71-
72-
URL helper = HelperResources.loadOne(classLoader, name);
73-
if (helper != null) {
74-
resource = helper;
68+
@Advice.Return URL resource) {
69+
if (resource == null) {
70+
URL helper = HelperResources.loadOne(classLoader, name);
71+
if (helper != null) {
72+
return helper;
73+
}
7574
}
75+
return resource;
7676
}
7777
}
7878

7979
@SuppressWarnings("unused")
8080
public static class GetResourcesAdvice {
8181

8282
@Advice.OnMethodExit(suppress = Throwable.class)
83-
public static void onExit(
83+
@Advice.AssignReturned.ToReturned
84+
public static Enumeration<URL> onExit(
8485
@Advice.This ClassLoader classLoader,
8586
@Advice.Argument(0) String name,
86-
@Advice.Return(readOnly = false) Enumeration<URL> resources) {
87+
@Advice.Return Enumeration<URL> resources) {
8788
List<URL> helpers = HelperResources.loadAll(classLoader, name);
8889
if (helpers.isEmpty()) {
89-
return;
90+
return resources;
9091
}
9192

9293
if (!resources.hasMoreElements()) {
93-
resources = Collections.enumeration(helpers);
94-
return;
94+
return Collections.enumeration(helpers);
9595
}
9696

9797
List<URL> result = Collections.list(resources);
@@ -108,31 +108,30 @@ public static void onExit(
108108
result.add(helperUrl);
109109
}
110110
}
111-
112-
resources = Collections.enumeration(result);
111+
return Collections.enumeration(result);
113112
}
114113
}
115114

116115
@SuppressWarnings("unused")
117116
public static class GetResourceAsStreamAdvice {
118117

119118
@Advice.OnMethodExit(suppress = Throwable.class)
120-
public static void onExit(
119+
@Advice.AssignReturned.ToReturned
120+
public static InputStream onExit(
121121
@Advice.This ClassLoader classLoader,
122122
@Advice.Argument(0) String name,
123-
@Advice.Return(readOnly = false) InputStream inputStream) {
124-
if (inputStream != null) {
125-
return;
126-
}
127-
128-
URL helper = HelperResources.loadOne(classLoader, name);
129-
if (helper != null) {
130-
try {
131-
inputStream = helper.openStream();
132-
} catch (IOException ignored) {
133-
// ClassLoader.getResourceAsStream also ignores io exceptions from opening the stream
123+
@Advice.Return InputStream inputStream) {
124+
if (inputStream == null) {
125+
URL helper = HelperResources.loadOne(classLoader, name);
126+
if (helper != null) {
127+
try {
128+
return helper.openStream();
129+
} catch (IOException ignored) {
130+
// ClassLoader.getResourceAsStream also ignores io exceptions from opening the stream
131+
}
134132
}
135133
}
134+
return inputStream;
136135
}
137136
}
138137
}

‎javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/IndyBootstrapDispatcher.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ public static CallSite bootstrap(
5454
return callSite;
5555
}
5656

57-
// package visibility for testing
58-
static MethodHandle generateNoopMethodHandle(MethodType methodType) {
57+
public static MethodHandle generateNoopMethodHandle(MethodType methodType) {
5958
Class<?> returnType = methodType.returnType();
6059
MethodHandle noopNoArg;
6160
if (returnType == void.class) {

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
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 java.lang.invoke.MutableCallSite;
9+
import java.util.HashMap;
10+
import java.util.Map;
11+
import java.util.function.Function;
12+
import java.util.function.Supplier;
13+
import javax.annotation.Nullable;
14+
15+
class AdviceBootstrapState implements AutoCloseable {
16+
17+
private static final ThreadLocal<Map<Key, AdviceBootstrapState>> stateForCurrentThread =
18+
ThreadLocal.withInitial(HashMap::new);
19+
20+
private final Key key;
21+
private int recursionDepth;
22+
@Nullable private MutableCallSite nestedCallSite;
23+
24+
/**
25+
* We have to eagerly initialize to not cause a lambda construction during {@link #enter(Class,
26+
* String, String, String, String)}.
27+
*/
28+
private static final Function<Key, AdviceBootstrapState> CONSTRUCTOR = AdviceBootstrapState::new;
29+
30+
private AdviceBootstrapState(Key key) {
31+
this.key = key;
32+
// enter will increment it by one, so 0 is the value for non-recursive calls
33+
recursionDepth = -1;
34+
}
35+
36+
static void initialize() {
37+
// Eager initialize everything because we could run into recursions doing this during advice
38+
// bootstrapping
39+
stateForCurrentThread.get();
40+
stateForCurrentThread.remove();
41+
try {
42+
Class.forName(Key.class.getName());
43+
} catch (ClassNotFoundException e) {
44+
throw new IllegalStateException(e);
45+
}
46+
}
47+
48+
static AdviceBootstrapState enter(
49+
Class<?> instrumentedClass,
50+
String moduleClassName,
51+
String adviceClassName,
52+
String adviceMethodName,
53+
String adviceMethodDescriptor) {
54+
Key key =
55+
new Key(
56+
instrumentedClass,
57+
moduleClassName,
58+
adviceClassName,
59+
adviceMethodName,
60+
adviceMethodDescriptor);
61+
AdviceBootstrapState state = stateForCurrentThread.get().computeIfAbsent(key, CONSTRUCTOR);
62+
state.recursionDepth++;
63+
return state;
64+
}
65+
66+
public boolean isNestedInvocation() {
67+
return recursionDepth > 0;
68+
}
69+
70+
public MutableCallSite getOrInitMutableCallSite(Supplier<MutableCallSite> initializer) {
71+
if (nestedCallSite == null) {
72+
nestedCallSite = initializer.get();
73+
}
74+
return nestedCallSite;
75+
}
76+
77+
public void initMutableCallSite(MutableCallSite mutableCallSite) {
78+
if (nestedCallSite != null) {
79+
throw new IllegalStateException("callsite has already been initialized");
80+
}
81+
nestedCallSite = mutableCallSite;
82+
}
83+
84+
@Nullable
85+
public MutableCallSite getMutableCallSite() {
86+
return nestedCallSite;
87+
}
88+
89+
@Override
90+
public void close() {
91+
if (recursionDepth == 0) {
92+
Map<Key, AdviceBootstrapState> stateMap = stateForCurrentThread.get();
93+
stateMap.remove(key);
94+
if (stateMap.isEmpty()) {
95+
// Do not leave an empty map dangling as thread local
96+
stateForCurrentThread.remove();
97+
}
98+
} else {
99+
recursionDepth--;
100+
}
101+
}
102+
103+
/** Key uniquely identifying a single invokedynamic instruction inserted for an advice */
104+
private static class Key {
105+
106+
private final Class<?> instrumentedClass;
107+
private final String moduleClassName;
108+
private final String adviceClassName;
109+
private final String adviceMethodName;
110+
private final String adviceMethodDescriptor;
111+
112+
private Key(
113+
Class<?> instrumentedClass,
114+
String moduleClassName,
115+
String adviceClassName,
116+
String adviceMethodName,
117+
String adviceMethodDescriptor) {
118+
this.instrumentedClass = instrumentedClass;
119+
this.moduleClassName = moduleClassName;
120+
this.adviceClassName = adviceClassName;
121+
this.adviceMethodName = adviceMethodName;
122+
this.adviceMethodDescriptor = adviceMethodDescriptor;
123+
}
124+
125+
@Override
126+
public boolean equals(Object o) {
127+
if (this == o) {
128+
return true;
129+
}
130+
if (o == null || !(o instanceof Key)) {
131+
return false;
132+
}
133+
134+
Key that = (Key) o;
135+
return instrumentedClass.equals(that.instrumentedClass)
136+
&& moduleClassName.equals(that.moduleClassName)
137+
&& adviceClassName.equals(that.adviceClassName)
138+
&& adviceMethodName.equals(that.adviceMethodName)
139+
&& adviceMethodDescriptor.equals(that.adviceMethodDescriptor);
140+
}
141+
142+
@Override
143+
public int hashCode() {
144+
int result = instrumentedClass.hashCode();
145+
result = 31 * result + moduleClassName.hashCode();
146+
result = 31 * result + adviceClassName.hashCode();
147+
result = 31 * result + adviceMethodName.hashCode();
148+
result = 31 * result + adviceMethodDescriptor.hashCode();
149+
return result;
150+
}
151+
}
152+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ static byte[] transform(byte[] bytes) {
6767
}));
6868

6969
TransformationContext context = new TransformationContext();
70+
if (justDelegateAdvice) {
71+
context.disableReturnTypeChange();
72+
}
7073
ClassVisitor cv =
7174
new ClassVisitor(AsmApi.VERSION, cw) {
7275

0 commit comments

Comments
 (0)