Skip to content

Commit 1b3d530

Browse files
lauritlaurit
authored
Decide whether to use isolate or inject helper classes based on the advice classes (open-telemetry#17815)
1 parent 8446494 commit 1b3d530

6 files changed

Lines changed: 381 additions & 3 deletions

File tree

‎javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/instrumentation/internal/ExperimentalInstrumentationModule.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,34 @@ default Map<JavaModule, List<String>> jpmsModulesToOpen() {
7676
default List<String> exposedClassNames() {
7777
return emptyList();
7878
}
79+
80+
/**
81+
* Allows instrumentation modules to choose whether the helper classes should be injected into the
82+
* same class loader as the instrumented library, or into an isolated class loader.
83+
*/
84+
default HelperClassStrategy helperClassStrategy() {
85+
return HelperClassStrategy.DEFAULT;
86+
}
87+
88+
/**
89+
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
90+
* any time.
91+
*/
92+
enum HelperClassStrategy {
93+
/**
94+
* Depending on whether the instrumentation uses inline advice or not, helper classes are either
95+
* loaded in the same classloader as the instrumented library, or into an isolated classloader.
96+
*/
97+
DEFAULT,
98+
/**
99+
* Helper classes are loaded in the same classloader as the instrumented library, and are
100+
* visible to the application.
101+
*/
102+
INJECTED,
103+
/**
104+
* Helper classes are loaded into an isolated classloader, and aren't visible to the
105+
* application.
106+
*/
107+
ISOLATED
108+
}
79109
}

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1717
import io.opentelemetry.javaagent.extension.instrumentation.internal.AgentDistributionConfig;
1818
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
19+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule.HelperClassStrategy;
1920
import io.opentelemetry.javaagent.tooling.HelperClassDefinition;
2021
import io.opentelemetry.javaagent.tooling.HelperInjector;
2122
import io.opentelemetry.javaagent.tooling.InjectionMode;
@@ -28,6 +29,7 @@
2829
import io.opentelemetry.javaagent.tooling.instrumentation.indy.ForwardIndyAdviceTransformer;
2930
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyModuleRegistry;
3031
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyTypeTransformerImpl;
32+
import io.opentelemetry.javaagent.tooling.muzzle.AdviceInspector;
3133
import io.opentelemetry.javaagent.tooling.muzzle.HelperResourceBuilderImpl;
3234
import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationModuleMuzzle;
3335
import io.opentelemetry.javaagent.tooling.util.IgnoreFailedTypeMatcher;
@@ -42,6 +44,7 @@
4244
import net.bytebuddy.agent.builder.AgentBuilder;
4345
import net.bytebuddy.description.annotation.AnnotationSource;
4446
import net.bytebuddy.description.type.TypeDescription;
47+
import net.bytebuddy.dynamic.ClassFileLocator;
4548
import net.bytebuddy.matcher.ElementMatcher;
4649
import net.bytebuddy.utility.JavaModule;
4750

@@ -58,6 +61,11 @@ public final class InstrumentationModuleInstaller {
5861
private final Instrumentation instrumentation;
5962
private final VirtualFieldImplementationInstallerFactory virtualFieldInstallerFactory =
6063
VirtualFieldImplementationInstallerFactory.getInstance();
64+
private final AdviceInspector adviceInspector =
65+
new AdviceInspector(
66+
new ClassFileLocator.Compound(
67+
ClassFileLocator.ForClassLoader.of(Utils.getAgentClassLoader()),
68+
ClassFileLocator.ForClassLoader.of(Utils.getExtensionsClassLoader())));
6169

6270
public InstrumentationModuleInstaller(Instrumentation instrumentation) {
6371
this.instrumentation = instrumentation;
@@ -78,13 +86,52 @@ AgentBuilder install(
7886
return parentAgentBuilder;
7987
}
8088

81-
if (AgentDistributionConfig.get().isIndyEnabled()) {
89+
boolean useIndy = useIndy(instrumentationModule);
90+
logger.log(
91+
FINE,
92+
"Instrumentation {0} [class {1}] {2} helper classes",
93+
new Object[] {
94+
instrumentationModule.instrumentationName(),
95+
instrumentationModule.getClass().getName(),
96+
useIndy ? "isolates" : "injects"
97+
});
98+
if (useIndy) {
8299
return installIndyModule(instrumentationModule, parentAgentBuilder);
83100
} else {
84101
return installInjectingModule(instrumentationModule, parentAgentBuilder);
85102
}
86103
}
87104

105+
private boolean useIndy(InstrumentationModule instrumentationModule) {
106+
// currently needs to be enabled with a flag
107+
if (!AgentDistributionConfig.get().isIndyEnabled()) {
108+
return false;
109+
}
110+
// first check whether user has specified how the helper classes should be handled
111+
if (instrumentationModule instanceof ExperimentalInstrumentationModule) {
112+
HelperClassStrategy helperClassStrategy =
113+
((ExperimentalInstrumentationModule) instrumentationModule).helperClassStrategy();
114+
switch (helperClassStrategy) {
115+
case INJECTED:
116+
return false;
117+
case ISOLATED:
118+
return true;
119+
case DEFAULT:
120+
// fallthrough to the next check
121+
}
122+
}
123+
// check whether muzzle has collected information about the advice classes
124+
if (instrumentationModule instanceof InstrumentationModuleMuzzle) {
125+
Boolean useIsolated =
126+
((InstrumentationModuleMuzzle) instrumentationModule).getMuzzleUseIsolatedHelperClasses();
127+
if (useIsolated != null) {
128+
return useIsolated;
129+
}
130+
}
131+
// inspect the advice classes used and decide based on that
132+
return adviceInspector.useIsolatedAdvice(instrumentationModule);
133+
}
134+
88135
private AgentBuilder installIndyModule(
89136
InstrumentationModule instrumentationModule, AgentBuilder parentAgentBuilder) {
90137
List<String> helperClassNames =

‎muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/AdviceInspector.java‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.tooling.muzzle;
7+
8+
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
9+
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
10+
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
11+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
12+
import java.util.Collection;
13+
import java.util.HashSet;
14+
import java.util.Set;
15+
import java.util.function.Function;
16+
import javax.annotation.Nullable;
17+
import net.bytebuddy.agent.builder.AgentBuilder;
18+
import net.bytebuddy.asm.Advice;
19+
import net.bytebuddy.description.annotation.AnnotationDescription;
20+
import net.bytebuddy.description.annotation.AnnotationValue;
21+
import net.bytebuddy.description.method.MethodDescription;
22+
import net.bytebuddy.description.method.MethodList;
23+
import net.bytebuddy.description.type.TypeDescription;
24+
import net.bytebuddy.dynamic.ClassFileLocator;
25+
import net.bytebuddy.matcher.ElementMatcher;
26+
import net.bytebuddy.pool.TypePool;
27+
28+
/**
29+
* Inspect advice classes used by the given instrumentation module and determine whether the
30+
* instrumentation should inject helper classes into the instrumented classloader or load them into
31+
* separate class loader. The decision is made based on the {@code inline} attribute of {@link
32+
* Advice.OnMethodEnter} and {@link Advice.OnMethodExit} annotations.
33+
*/
34+
public class AdviceInspector {
35+
36+
private final ClassFileLocator classFileLocator;
37+
38+
public AdviceInspector(ClassFileLocator classFileLocator) {
39+
this.classFileLocator = classFileLocator;
40+
}
41+
42+
public boolean useIsolatedAdvice(InstrumentationModule instrumentationModule) {
43+
Set<String> adviceClassNames = new HashSet<>();
44+
for (TypeInstrumentation typeInstrumentation : instrumentationModule.typeInstrumentations()) {
45+
typeInstrumentation.transform(
46+
new TypeTransformer() {
47+
@Override
48+
public void applyAdviceToMethod(
49+
ElementMatcher<? super MethodDescription> methodMatcher,
50+
Function<Advice.WithCustomMapping, Advice.WithCustomMapping> mappingCustomizer,
51+
String adviceClassName) {
52+
adviceClassNames.add(adviceClassName);
53+
}
54+
55+
@Override
56+
public void applyTransformer(AgentBuilder.Transformer transformer) {}
57+
});
58+
}
59+
Boolean result = useIsolatedAdvice(instrumentationModule, adviceClassNames);
60+
61+
// we aren't able to tell whether the instrumentation is ready for indy instrumentation or not,
62+
// so we assume that it is not
63+
return result != null ? result : false;
64+
}
65+
66+
@Nullable
67+
public Boolean useIsolatedAdvice(
68+
InstrumentationModule instrumentationModule, Collection<String> adviceClassNames) {
69+
TypePool typePool = AgentBuilder.PoolStrategy.Default.FAST.typePool(classFileLocator, null);
70+
71+
int inlineCount = 0;
72+
int nonInlineCount = 0;
73+
for (String adviceClassName : adviceClassNames) {
74+
TypeDescription type = typePool.describe(adviceClassName).resolve();
75+
MethodList<MethodDescription.InDefinedShape> methodList = type.getDeclaredMethods();
76+
for (MethodDescription.InDefinedShape method : methodList) {
77+
for (AnnotationDescription annotation : method.getDeclaredAnnotations()) {
78+
if (Advice.OnMethodEnter.class.getName().equals(annotation.getAnnotationType().getName())
79+
|| Advice.OnMethodExit.class
80+
.getName()
81+
.equals(annotation.getAnnotationType().getName())) {
82+
AnnotationValue<?, ?> value = annotation.getValue("inline");
83+
// While it is possible to use non-inline advice with the non-indy instrumentation, by
84+
// adding the advice classes as helper classes, we assume that nobody relies on that.
85+
// Having a non-inline advice is treated as a marker that the instrumentation can use
86+
// indy.
87+
// Similarly inline advice could be used with indy instrumentation, but we assume that
88+
// if inline advice is used, then it is not indy ready.
89+
if (value.getState().isDefined() && Boolean.FALSE.equals(value.resolve())) {
90+
nonInlineCount++;
91+
} else {
92+
inlineCount++;
93+
}
94+
}
95+
}
96+
}
97+
}
98+
// mixed inline and non-inline advice
99+
if (inlineCount > 0 && nonInlineCount > 0) {
100+
return null;
101+
}
102+
if (inlineCount > 0) {
103+
return false;
104+
}
105+
// While it is possible to use non-inline advice with the non-indy instrumentation, by
106+
// adding the advice classes as helper classes, we assume that nobody relies on that.
107+
// Having all advice non-inline is treated as a marker that the instrumentation can use indy.
108+
// Similarly inline advice could be used with indy instrumentation. For now, we default to
109+
// non-indy instrumentation in that case, but users can override that by explicitly choosing
110+
// whether to inject or isolate helper classes when mixed advice is used.
111+
if (nonInlineCount > 0) {
112+
return true;
113+
}
114+
115+
// no advice annotations were used so the instrumentation is using an AgentBuilder.Transformer
116+
if (instrumentationModule instanceof ExperimentalInstrumentationModule) {
117+
ExperimentalInstrumentationModule experimentalModule =
118+
(ExperimentalInstrumentationModule) instrumentationModule;
119+
// injected class names makes sense only for indy instrumentation
120+
if (!experimentalModule.injectedClassNames().isEmpty()) {
121+
return true;
122+
}
123+
if (!experimentalModule.exposedClassNames().isEmpty()) {
124+
return true;
125+
}
126+
}
127+
128+
// we aren't able to tell whether the instrumentation is ready for indy instrumentation or not
129+
return null;
130+
}
131+
}

‎muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/InstrumentationModuleMuzzle.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.ArrayList;
1414
import java.util.List;
1515
import java.util.Map;
16+
import javax.annotation.Nullable;
1617

1718
/**
1819
* This interface contains methods that muzzle automatically adds to the {@link
@@ -72,4 +73,14 @@ static List<String> getHelperClassNames(InstrumentationModule module) {
7273
result.addAll(additionalHelperClassNames);
7374
return result;
7475
}
76+
77+
/**
78+
* Returns {@code true} when helper classes should be loaded into an isolated class loader, {@code
79+
* false} when they should be injected to the same class loader as the instrumented library, and
80+
* {@code null} when other means should be used to decide which strategy to use.
81+
*/
82+
@Nullable
83+
default Boolean getMuzzleUseIsolatedHelperClasses() {
84+
return null;
85+
}
7586
}

0 commit comments

Comments
 (0)