Skip to content

Commit a2f01e5

Browse files
lauritlaurit
authored
Make more tests run with indy (#9729)
1 parent a172384 commit a2f01e5

File tree

13 files changed

+102
-48
lines changed

13 files changed

+102
-48
lines changed

‎instrumentation/couchbase/couchbase-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/couchbase/v2_0/CouchbaseInstrumentationModule.java

+9-6
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@
66
package io.opentelemetry.javaagent.instrumentation.couchbase.v2_0;
77

88
import static java.util.Arrays.asList;
9+
import static java.util.Collections.singletonList;
910

1011
import com.google.auto.service.AutoService;
1112
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1213
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
14+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1315
import java.util.List;
1416

1517
@AutoService(InstrumentationModule.class)
16-
public class CouchbaseInstrumentationModule extends InstrumentationModule {
18+
public class CouchbaseInstrumentationModule extends InstrumentationModule
19+
implements ExperimentalInstrumentationModule {
20+
1721
public CouchbaseInstrumentationModule() {
1822
super("couchbase", "couchbase-2.0");
1923
}
@@ -24,13 +28,12 @@ public boolean isHelperClass(String className) {
2428
}
2529

2630
@Override
27-
public boolean isIndyModule() {
28-
// rx.__OpenTelemetryTracingUtil is used for accessing a package private field
29-
return false;
31+
public List<TypeInstrumentation> typeInstrumentations() {
32+
return asList(new CouchbaseBucketInstrumentation(), new CouchbaseClusterInstrumentation());
3033
}
3134

3235
@Override
33-
public List<TypeInstrumentation> typeInstrumentations() {
34-
return asList(new CouchbaseBucketInstrumentation(), new CouchbaseClusterInstrumentation());
36+
public List<String> injectedClassNames() {
37+
return singletonList("rx.__OpenTelemetryTracingUtil");
3538
}
3639
}

‎instrumentation/hystrix-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/hystrix/HystrixInstrumentationModule.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
import com.google.auto.service.AutoService;
1111
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1212
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
13+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1314
import java.util.List;
1415

1516
@AutoService(InstrumentationModule.class)
16-
public class HystrixInstrumentationModule extends InstrumentationModule {
17+
public class HystrixInstrumentationModule extends InstrumentationModule
18+
implements ExperimentalInstrumentationModule {
1719

1820
public HystrixInstrumentationModule() {
1921
super("hystrix", "hystrix-1.4");
@@ -25,13 +27,12 @@ public boolean isHelperClass(String className) {
2527
}
2628

2729
@Override
28-
public boolean isIndyModule() {
29-
// rx.__OpenTelemetryTracingUtil is used for accessing a package private field
30-
return false;
30+
public List<TypeInstrumentation> typeInstrumentations() {
31+
return singletonList(new HystrixCommandInstrumentation());
3132
}
3233

3334
@Override
34-
public List<TypeInstrumentation> typeInstrumentations() {
35-
return singletonList(new HystrixCommandInstrumentation());
35+
public List<String> injectedClassNames() {
36+
return singletonList("rx.__OpenTelemetryTracingUtil");
3637
}
3738
}

‎instrumentation/ktor/ktor-2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ktor/v2_0/KtorInstrumentationModule.java

-8
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,6 @@ public boolean isHelperClass(String className) {
2424
return className.startsWith("io.opentelemetry.extension.kotlin.");
2525
}
2626

27-
@Override
28-
public boolean isIndyModule() {
29-
// java.lang.LinkageError: loader constraint violation in interface itable initialization for
30-
// class
31-
// io.opentelemetry.javaagent.shaded.instrumentation.ktor.v2_0.client.KtorClientTracing$Companion: when selecting method 'java.lang.Object io.ktor.client.plugins.HttpClientPlugin.prepare(kotlin.jvm.functions.Function1)' the class loader 'app' for super interface io.ktor.client.plugins.HttpClientPlugin, and the class loader io.opentelemetry.javaagent.tooling.instrumentation.indy.InstrumentationModuleClassLoader @2565a7d0 of the selected method's class, io.opentelemetry.javaagent.shaded.instrumentation.ktor.v2_0.client.KtorClientTracing$Companion have different Class objects for the type kotlin.jvm.functions.Function1 used in the signature (io.ktor.client.plugins.HttpClientPlugin is in unnamed module of loader 'app'; io.opentelemetry.javaagent.shaded.instrumentation.ktor.v2_0.client.KtorClientTracing$Companion is in unnamed module of loader io.opentelemetry.javaagent.tooling.instrumentation.indy.InstrumentationModuleClassLoader @2565a7d0, parent loader io.opentelemetry.javaagent.bootstrap.AgentClassLoader @ea30797)
32-
return false;
33-
}
34-
3527
@Override
3628
public List<TypeInstrumentation> typeInstrumentations() {
3729
return asList(new ServerInstrumentation(), new HttpClientInstrumentation());

‎instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java

-7
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,6 @@ public OkHttp3InstrumentationModule() {
1919
super("okhttp", "okhttp-3.0");
2020
}
2121

22-
@Override
23-
public boolean isIndyModule() {
24-
// java.lang.LinkageError: bad method type alias: (Builder,Map)void not visible from class
25-
// io.opentelemetry.javaagent.instrumentation.okhttp.v3_0.OkHttp3Instrumentation$ConstructorAdvice
26-
return false;
27-
}
28-
2922
@Override
3023
public List<TypeInstrumentation> typeInstrumentations() {
3124
return asList(new OkHttp3Instrumentation(), new OkHttp3DispatcherInstrumentation());

‎instrumentation/vertx/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxSqlClientInstrumentationModule.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
package io.opentelemetry.javaagent.instrumentation.vertx.v4_0.sql;
77

88
import static java.util.Arrays.asList;
9+
import static java.util.Collections.singletonList;
910

1011
import com.google.auto.service.AutoService;
1112
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1213
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
14+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1315
import java.util.List;
1416

1517
@AutoService(InstrumentationModule.class)
16-
public class VertxSqlClientInstrumentationModule extends InstrumentationModule {
18+
public class VertxSqlClientInstrumentationModule extends InstrumentationModule
19+
implements ExperimentalInstrumentationModule {
1720

1821
public VertxSqlClientInstrumentationModule() {
1922
super("vertx-sql-client", "vertx-sql-client-4.0", "vertx");
@@ -25,9 +28,8 @@ public boolean isHelperClass(String className) {
2528
}
2629

2730
@Override
28-
public boolean isIndyModule() {
29-
// QueryExecutorUtil accesses package private class io.vertx.sqlclient.impl.QueryExecutor
30-
return false;
31+
public List<String> injectedClassNames() {
32+
return singletonList("io.vertx.sqlclient.impl.QueryExecutorUtil");
3133
}
3234

3335
@Override

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

+11
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55

66
package io.opentelemetry.javaagent.extension.instrumentation.internal;
77

8+
import static java.util.Collections.emptyList;
9+
810
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
911
import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
12+
import java.util.List;
1013

1114
/**
1215
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
@@ -25,4 +28,12 @@ public interface ExperimentalInstrumentationModule {
2528
* @param injector the builder for injecting classes
2629
*/
2730
default void injectClasses(ClassInjector injector) {}
31+
32+
/**
33+
* Returns a list of helper classes that will be defined in the class loader of the instrumented
34+
* library.
35+
*/
36+
default List<String> injectedClassNames() {
37+
return emptyList();
38+
}
2839
}

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

+40-10
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import io.opentelemetry.javaagent.tooling.util.NamedMatcher;
3232
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
3333
import java.lang.instrument.Instrumentation;
34+
import java.util.Collections;
3435
import java.util.List;
3536
import net.bytebuddy.agent.builder.AgentBuilder;
3637
import net.bytebuddy.description.annotation.AnnotationSource;
@@ -69,46 +70,75 @@ AgentBuilder install(
6970
}
7071

7172
if (instrumentationModule.isIndyModule()) {
72-
return installIndyModule(instrumentationModule, parentAgentBuilder);
73+
return installIndyModule(instrumentationModule, parentAgentBuilder, config);
7374
} else {
7475
return installInjectingModule(instrumentationModule, parentAgentBuilder, config);
7576
}
7677
}
7778

7879
private AgentBuilder installIndyModule(
79-
InstrumentationModule instrumentationModule, AgentBuilder parentAgentBuilder) {
80-
81-
IndyModuleRegistry.registerIndyModule(instrumentationModule);
82-
80+
InstrumentationModule instrumentationModule,
81+
AgentBuilder parentAgentBuilder,
82+
ConfigProperties config) {
83+
List<String> helperClassNames =
84+
InstrumentationModuleMuzzle.getHelperClassNames(instrumentationModule);
8385
HelperResourceBuilderImpl helperResourceBuilder = new HelperResourceBuilderImpl();
8486
instrumentationModule.registerHelperResources(helperResourceBuilder);
87+
List<TypeInstrumentation> typeInstrumentations = instrumentationModule.typeInstrumentations();
88+
if (typeInstrumentations.isEmpty()) {
89+
if (!helperClassNames.isEmpty() || !helperResourceBuilder.getResources().isEmpty()) {
90+
logger.log(
91+
WARNING,
92+
"Helper classes and resources won't be injected if no types are instrumented: {0}",
93+
instrumentationModule.instrumentationName());
94+
}
95+
96+
return parentAgentBuilder;
97+
}
98+
99+
List<String> injectedHelperClassNames = Collections.emptyList();
100+
if (instrumentationModule instanceof ExperimentalInstrumentationModule) {
101+
ExperimentalInstrumentationModule experimentalInstrumentationModule =
102+
(ExperimentalInstrumentationModule) instrumentationModule;
103+
injectedHelperClassNames = experimentalInstrumentationModule.injectedClassNames();
104+
}
105+
106+
IndyModuleRegistry.registerIndyModule(instrumentationModule);
85107

86108
ClassInjectorImpl injectedClassesCollector = new ClassInjectorImpl(instrumentationModule);
87109
if (instrumentationModule instanceof ExperimentalInstrumentationModule) {
88110
((ExperimentalInstrumentationModule) instrumentationModule)
89111
.injectClasses(injectedClassesCollector);
90112
}
91113

114+
MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config);
115+
92116
AgentBuilder.Transformer helperInjector =
93117
new HelperInjector(
94118
instrumentationModule.instrumentationName(),
95-
injectedClassesCollector.getClassesToInject(),
119+
injectedHelperClassNames,
96120
helperResourceBuilder.getResources(),
97121
instrumentationModule.getClass().getClassLoader(),
98122
instrumentation);
123+
AgentBuilder.Transformer indyHelperInjector =
124+
new HelperInjector(
125+
instrumentationModule.instrumentationName(),
126+
injectedClassesCollector.getClassesToInject(),
127+
Collections.emptyList(),
128+
instrumentationModule.getClass().getClassLoader(),
129+
instrumentation);
99130

100-
// TODO (Jonas): Adapt MuzzleMatcher to use the same type lookup strategy as the
101-
// InstrumentationModuleClassLoader (see IndyModuleTypePool)
102-
// MuzzleMatcher muzzleMatcher = new MuzzleMatcher(logger, instrumentationModule, config);
103131
VirtualFieldImplementationInstaller contextProvider =
104132
virtualFieldInstallerFactory.create(instrumentationModule);
105133

106134
AgentBuilder agentBuilder = parentAgentBuilder;
107135
for (TypeInstrumentation typeInstrumentation : instrumentationModule.typeInstrumentations()) {
108136
AgentBuilder.Identified.Extendable extendableAgentBuilder =
109137
setTypeMatcher(agentBuilder, instrumentationModule, typeInstrumentation)
138+
.and(muzzleMatcher)
110139
.transform(new PatchByteCodeVersionTransformer())
111-
.transform(helperInjector);
140+
.transform(helperInjector)
141+
.transform(indyHelperInjector);
112142

113143
// TODO (Jonas): we are not calling
114144
// contextProvider.rewriteVirtualFieldsCalls(extendableAgentBuilder) anymore

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

+5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.opentelemetry.javaagent.tooling.TransformSafeLogger;
1515
import io.opentelemetry.javaagent.tooling.Utils;
1616
import io.opentelemetry.javaagent.tooling.config.AgentConfig;
17+
import io.opentelemetry.javaagent.tooling.instrumentation.indy.IndyModuleRegistry;
1718
import io.opentelemetry.javaagent.tooling.muzzle.Mismatch;
1819
import io.opentelemetry.javaagent.tooling.muzzle.ReferenceMatcher;
1920
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
@@ -60,6 +61,10 @@ public boolean matches(
6061
ProtectionDomain protectionDomain) {
6162
if (classLoader == BOOTSTRAP_LOADER) {
6263
classLoader = Utils.getBootstrapProxy();
64+
} else if (instrumentationModule.isIndyModule()) {
65+
classLoader =
66+
IndyModuleRegistry.getInstrumentationClassloader(
67+
instrumentationModule.getClass().getName(), classLoader);
6368
}
6469
return matchCache.computeIfAbsent(classLoader, this::doesMatch);
6570
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
1010
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
1111
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
12+
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
1213
import io.opentelemetry.javaagent.tooling.muzzle.InstrumentationModuleMuzzle;
1314
import java.lang.ref.WeakReference;
1415
import java.util.HashSet;
@@ -84,6 +85,9 @@ static InstrumentationModuleClassLoader createInstrumentationModuleClassloader(
8485
// TODO (Jonas): Make muzzle include advice classes as helper classes
8586
// so that we don't have to include them here
8687
toInject.addAll(getModuleAdviceNames(module));
88+
if (module instanceof ExperimentalInstrumentationModule) {
89+
toInject.removeAll(((ExperimentalInstrumentationModule) module).injectedClassNames());
90+
}
8791

8892
ClassLoader agentOrExtensionCl = module.getClass().getClassLoader();
8993
Map<String, ClassCopySource> injectedClasses =

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

+15-1
Original file line numberDiff line numberDiff line change
@@ -54,16 +54,26 @@ public class InstrumentationModuleClassLoader extends ClassLoader {
5454
private volatile MethodHandles.Lookup cachedLookup;
5555

5656
private final ClassLoader instrumentedCl;
57+
private final boolean delegateAllToAgent;
5758

5859
public InstrumentationModuleClassLoader(
5960
ClassLoader instrumentedCl,
6061
ClassLoader agentOrExtensionCl,
6162
Map<String, ClassCopySource> injectedClasses) {
63+
this(instrumentedCl, agentOrExtensionCl, injectedClasses, false);
64+
}
65+
66+
InstrumentationModuleClassLoader(
67+
ClassLoader instrumentedCl,
68+
ClassLoader agentOrExtensionCl,
69+
Map<String, ClassCopySource> injectedClasses,
70+
boolean delegateAllToAgent) {
6271
// agent/extension-classloader is "main"-parent, but class lookup is overridden
6372
super(agentOrExtensionCl);
6473
additionalInjectedClasses = injectedClasses;
6574
this.agentOrExtensionCl = agentOrExtensionCl;
6675
this.instrumentedCl = instrumentedCl;
76+
this.delegateAllToAgent = delegateAllToAgent;
6777
}
6878

6979
/**
@@ -110,7 +120,7 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
110120
}
111121
}
112122
}
113-
if (result == null) {
123+
if (result == null && shouldLoadFromAgent(name)) {
114124
result = tryLoad(agentOrExtensionCl, name);
115125
}
116126
if (result == null) {
@@ -128,6 +138,10 @@ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundE
128138
}
129139
}
130140

141+
private boolean shouldLoadFromAgent(String dotClassName) {
142+
return delegateAllToAgent || dotClassName.startsWith("io.opentelemetry.javaagent");
143+
}
144+
131145
private static Class<?> tryLoad(ClassLoader cl, String name) {
132146
try {
133147
return cl.loadClass(name);

‎javaagent-tooling/src/test/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/InstrumentationModuleClassLoaderTest.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ void checkLookup() throws Throwable {
4343
ClassLoader dummyParent = new URLClassLoader(new URL[] {}, null);
4444

4545
InstrumentationModuleClassLoader m1 =
46-
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject);
46+
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject, true);
4747
InstrumentationModuleClassLoader m2 =
48-
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject);
48+
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject, true);
4949

5050
// MethodHandles.publicLookup() always succeeds on the first invocation
5151
lookupAndInvokeFoo(m1);
@@ -79,7 +79,7 @@ void checkInjectedClassesHavePackage() throws Throwable {
7979

8080
ClassLoader dummyParent = new URLClassLoader(new URL[] {}, null);
8181
InstrumentationModuleClassLoader m1 =
82-
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject);
82+
new InstrumentationModuleClassLoader(dummyParent, dummyParent, toInject, true);
8383

8484
Class<?> injected = Class.forName(A.class.getName(), true, m1);
8585
// inject two classes from the same package to trigger errors if we try to redefine the package
@@ -120,7 +120,7 @@ void checkClassLookupPrecedence(@TempDir Path tempDir) throws Exception {
120120
toInject.put(C.class.getName(), ClassCopySource.create(C.class.getName(), moduleSourceCl));
121121

122122
InstrumentationModuleClassLoader moduleCl =
123-
new InstrumentationModuleClassLoader(appCl, agentCl, toInject);
123+
new InstrumentationModuleClassLoader(appCl, agentCl, toInject, true);
124124

125125
// Verify precedence for classloading
126126
Class<?> clA = moduleCl.loadClass(A.class.getName());

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

-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ public static void setHelperInjectorListener(HelperInjectorListener listener) {
160160
private Map<String, Supplier<byte[]>> getHelperMap(ClassLoader targetClassloader) {
161161
Map<String, Supplier<byte[]>> result = new LinkedHashMap<>();
162162
if (dynamicTypeMap.isEmpty()) {
163-
164163
for (String helperClassName : helperClassNames) {
165164
result.put(
166165
helperClassName,

‎testing-common/integration-tests/src/main/java/indy/IndyInstrumentationTestModule.java ‎testing-common/integration-tests/src/main/java/io/opentelemetry/javaagent/IndyInstrumentationTestModule.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
package indy;
6+
package io.opentelemetry.javaagent;
77

88
import static net.bytebuddy.implementation.bytecode.assign.Assigner.Typing.DYNAMIC;
99
import static net.bytebuddy.matcher.ElementMatchers.named;

0 commit comments

Comments
 (0)