Skip to content

Commit

Permalink
Align classLoaderMatcher signature with OpenTelemetry (#7125)
Browse files Browse the repository at this point in the history
* Align classLoaderMatcher signature with OpenTelemetry to avoid need for bridge method
* Improve mapping of AgentElementMatchers
* Fix resource leak in RequestImplementationClassLoaderMatcher
  • Loading branch information
mcculls committed Jun 4, 2024
1 parent 244260b commit 239d9a9
Show file tree
Hide file tree
Showing 45 changed files with 100 additions and 60 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package datadog.opentelemetry.tooling;

import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers;
import datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers;
import net.bytebuddy.description.method.MethodDescription;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import net.bytebuddy.matcher.ElementMatchers;

/** Replaces OpenTelemetry's {@code AgentElementMatchers} when mapping extensions. */
public final class OtelElementMatchers {

public static ElementMatcher.Junction<TypeDescription> extendsClass(
ElementMatcher<TypeDescription> matcher) {
return HierarchyMatchers.extendsClass(matcher);
}

public static ElementMatcher.Junction<TypeDescription> implementsInterface(
ElementMatcher<TypeDescription> matcher) {
return HierarchyMatchers.implementsInterface(matcher);
}

public static ElementMatcher.Junction<TypeDescription> hasSuperType(
ElementMatcher<TypeDescription> matcher) {
return HierarchyMatchers.hasSuperType(matcher);
}

public static ElementMatcher.Junction<MethodDescription> methodIsDeclaredByType(
ElementMatcher<? super TypeDescription> matcher) {
return ElementMatchers.isDeclaredBy(matcher);
}

public static ElementMatcher.Junction<MethodDescription> hasSuperMethod(
ElementMatcher<? super MethodDescription> matcher) {
return HierarchyMatchers.hasSuperMethod(matcher);
}

public static ElementMatcher.Junction<ClassLoader> hasClassesNamed(String... classNames) {
return ClassLoaderMatchers.hasClassNamedOneOf(classNames);
}

private OtelElementMatchers() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ public void visit(
@Override
public MethodVisitor visitMethod(
int access, String name, String descriptor, String signature, String[] exceptions) {
if (!UNSUPPORTED_METHODS.contains(name)) {
return super.visitMethod(access, name, descriptor, signature, exceptions);
} else {
if (UNSUPPORTED_METHODS.contains(name)) {
return null; // remove unsupported method
}
return super.visitMethod(access, name, descriptor, signature, exceptions);
}

private String[] removeUnsupportedTypes(String[] interfaces) {
Expand Down Expand Up @@ -84,12 +83,12 @@ static final class Renamer extends Remapper {
RENAMED_TYPES.put(
"io/opentelemetry/javaagent/extension/instrumentation/TypeTransformer",
"datadog/opentelemetry/tooling/OtelTransformer");
RENAMED_TYPES.put(
"io/opentelemetry/javaagent/extension/matcher/AgentElementMatchers",
"datadog/opentelemetry/tooling/OtelElementMatchers");
RENAMED_TYPES.put(
"io/opentelemetry/javaagent/bootstrap/Java8BytecodeBridge",
"datadog/trace/bootstrap/otel/Java8BytecodeBridge");
RENAMED_TYPES.put(
"io/opentelemetry/javaagent/extension/matcher/AgentElementMatchers",
"datadog/trace/agent/tooling/bytebuddy/matcher/HierarchyMatchers");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public String muzzleDirective() {
}

/** Override this to supply additional class-loader requirements. */
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return ANY_CLASS_LOADER;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
public final class ClassLoaderMatchers {
private static final Logger log = LoggerFactory.getLogger(ClassLoaderMatchers.class);

public static final ElementMatcher<ClassLoader> ANY_CLASS_LOADER = any();
public static final ElementMatcher.Junction<ClassLoader> ANY_CLASS_LOADER = any();

private static final ClassLoader BOOTSTRAP_CLASSLOADER = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public String instrumentedType() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("com.linecorp.armeria.server.ServiceRequestContext");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public InvokerInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return ClassLoaderMatchers.hasClassNamed("javax.servlet.ServletRequest")
.or(ClassLoaderMatchers.hasClassNamed("jakarta.servlet.ServletRequest"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public Map<String, String> contextStore() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("com.datastax.driver.core.EndPoint");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public Map<String, String> contextStore() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return not(hasClassNamed("com.datastax.driver.core.EndPoint"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public Elasticsearch7RestClientInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching pre-ES7 releases which have their own instrumentations.
return hasClassNamed("org.elasticsearch.client.RestClient$InternalRequest");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public Elasticsearch73TransportClientInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching pre-ES7 releases which have their own instrumentations.
return hasClassNamed("org.elasticsearch.action.ActionType");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public GradleBuildScopeServicesInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Only instrument Gradle 8.3+
return hasClassNamed("org.gradle.api.file.ConfigurableFilePermissions");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public GradlePluginInjectorInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Only instrument Gradle 8.3+
return hasClassNamed("org.gradle.api.file.ConfigurableFilePermissions");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public GradleBuildListenerInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Only instrument Gradle older than 8.3
return not(hasClassNamed("org.gradle.api.file.ConfigurableFilePermissions"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public String instrumentedType() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("com.google.gson.stream.JsonReader");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public HttpClientInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("java.net.http.HttpClient");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public HttpHeadersInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("java.net.http.HttpRequest");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private Collection<String> getJaxRsAnnotations() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching JAX-RS 2 which has its own instrumentation.
return not(hasClassNamed("javax.ws.rs.container.AsyncResponse"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public Map<String, String> contextStore() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching JAX-RS 1 which has its own instrumentation.
return hasClassNamed("javax.ws.rs.container.AsyncResponse");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public JedisInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching Jedis 3+ which has its own instrumentation.
return not(hasClassNamed("redis.clients.jedis.commands.ProtocolCommand"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,28 +75,26 @@ public void methodAdvice(MethodTransformer transformer) {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return RequestImplementationClassLoaderMatcher.INSTANCE;
}

public static class RequestImplementationClassLoaderMatcher
implements ElementMatcher<ClassLoader> {
public static final ElementMatcher<ClassLoader> INSTANCE =
extends ElementMatcher.Junction.ForNonNullValues<ClassLoader> {
public static final ElementMatcher.Junction<ClassLoader> INSTANCE =
new RequestImplementationClassLoaderMatcher();

@Override
public boolean matches(ClassLoader cl) {
InputStream is = cl.getResourceAsStream("org/eclipse/jetty/server/Request.class");
if (is == null) {
return false;
}
try {
protected boolean doMatch(ClassLoader cl) {
try (InputStream is = cl.getResourceAsStream("org/eclipse/jetty/server/Request.class")) {
if (is == null) {
return false;
}
ClassReader classReader = new ClassReader(is);
final boolean[] foundField = new boolean[1];
final boolean[] foundGetParameters = new boolean[1];
classReader.accept(new ClassLoaderMatcherClassVisitor(foundField, foundGetParameters), 0);
return !foundField[0] && foundGetParameters[0];

} catch (IOException e) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public JUnit5CucumberInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("io.cucumber.junit.platform.engine.CucumberTestEngine");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public JUnit5CucumberItrInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("io.cucumber.junit.platform.engine.CucumberTestEngine");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public JUnit5SpockInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("org.spockframework.runtime.SpockEngine");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public JUnit5SpockItrInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("org.spockframework.runtime.SpockEngine");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public OpensearchTransportClientInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassNamed("org.opensearch.action.ActionType");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public GlobalTracerInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching OT 0.32+ which has its own instrumentation.
return not(hasClassNamed("io.opentracing.tag.Tag"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public BundleReferenceInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching older versions of OSGi that don't have the wiring API.
return hasClassNamed("org.osgi.framework.wiring.BundleWiring");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public HttpClientInstrumentation() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching pre-1.0 releases which are not compatible.
return hasClassNamed("reactor.netty.transport.AddressUtils");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ public String muzzleDirective() {
}

// Avoid matching servlet 3 which has its own instrumentation
static final ElementMatcher<ClassLoader> NOT_SERVLET_3 =
static final ElementMatcher.Junction<ClassLoader> NOT_SERVLET_3 =
not(hasClassNamed("javax.servlet.AsyncEvent"));

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return NOT_SERVLET_3;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ public String muzzleDirective() {
}

// Avoid matching servlet 3 which has its own instrumentation
static final ElementMatcher<ClassLoader> NOT_SERVLET_3 =
static final ElementMatcher.Junction<ClassLoader> NOT_SERVLET_3 =
not(hasClassNamed("javax.servlet.AsyncEvent"));

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return NOT_SERVLET_3;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public String muzzleDirective() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return Servlet2Instrumentation.NOT_SERVLET_3;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public String muzzleDirective() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching request bodies after 3.0.x which have their own instrumentation
return not(hasClassNamed("javax.servlet.ReadListener"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public String muzzleDirective() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching servlet 2 which has its own instrumentation
return hasClassNamed("javax.servlet.AsyncEvent");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public String muzzleDirective() {
}

@Override
public ElementMatcher<ClassLoader> classLoaderMatcher() {
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
// Avoid matching request bodies before 3.1.x which have their own instrumentation
return hasClassNamed("javax.servlet.ReadListener");
}
Expand Down
Loading

0 comments on commit 239d9a9

Please sign in to comment.