From c66bd24d3ad0d9e9b5d1462840bffa43d3382d15 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 30 Jul 2018 16:37:18 -0400 Subject: [PATCH] Use `safeHasSuperType` instead of `isSubType` `isSubType` may fail on certain class lookup problems, even on classes unrelated to given instrumentation, preventing instrumentation from being applied. --- .../classloaders/ClassLoaderInstrumentation.java | 5 ++--- .../HttpUrlConnectionInstrumentation.java | 12 ++++++------ .../jdbc/ConnectionInstrumentation.java | 6 +++--- .../jdbc/PreparedStatementInstrumentation.java | 5 +++-- .../jdbc/StatementInstrumentation.java | 5 +++-- 5 files changed, 17 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java index 396c5f70b4..a7e99e63b5 100644 --- a/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java +++ b/dd-java-agent/instrumentation/classloaders/src/main/java/datadog/trace/instrumentation/classloaders/ClassLoaderInstrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.classloaders; import static net.bytebuddy.matcher.ElementMatchers.isConstructor; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import com.google.auto.service.AutoService; import datadog.opentracing.DDTracer; @@ -28,12 +27,12 @@ public final class ClassLoaderInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return isSubTypeOf(ClassLoader.class); + return safeHasSuperType(named("java.lang.ClassLoader")); } @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put(isConstructor(), ClassloaderAdvice.class.getName()); return transformers; } diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java index 1592c77c83..c5e82c4d88 100644 --- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java @@ -1,9 +1,9 @@ package datadog.trace.instrumentation.http_url_connection; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -43,7 +43,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return isSubTypeOf(HttpURLConnection.class) + return safeHasSuperType(named("java.net.HttpURLConnection")) // This class is a simple delegator. Skip because it does not update its `connected` field. .and(not(named("sun.net.www.protocol.https.HttpsURLConnectionImpl"))); } @@ -58,7 +58,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { @Override public Map transformers() { - Map transformers = new HashMap<>(); + final Map transformers = new HashMap<>(); transformers.put( isMethod() .and(isPublic()) @@ -177,7 +177,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { private static final Map STATE_MAP = Collections.synchronizedMap(new WeakHashMap()); - public static HttpURLState get(HttpURLConnection connection) { + public static HttpURLState get(final HttpURLConnection connection) { HttpURLState state = STATE_MAP.get(connection); if (state == null) { // not thread-safe, but neither is HttpURLConnection @@ -195,8 +195,8 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default { return hasDoneIO; } - public void setHasDoneIO(boolean value) { - this.hasDoneIO = value; + public void setHasDoneIO(final boolean value) { + hasDoneIO = value; } } } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java index 25877f74f1..d7f8d5de4d 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java @@ -1,8 +1,9 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -10,7 +11,6 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.bootstrap.JDBCMaps; -import java.sql.Connection; import java.sql.PreparedStatement; import java.util.HashMap; import java.util.Map; @@ -26,7 +26,7 @@ public final class ConnectionInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(Connection.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.Connection"))); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java index 8756e89a44..d15ac53689 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -1,10 +1,11 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; @@ -36,7 +37,7 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Default @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(PreparedStatement.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.PreparedStatement"))); } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index dffd5b1354..65795a781e 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -1,10 +1,11 @@ package datadog.trace.instrumentation.jdbc; +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; +import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; @@ -37,7 +38,7 @@ public final class StatementInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { - return not(isInterface()).and(isSubTypeOf(Statement.class)); + return not(isInterface()).and(safeHasSuperType(named("java.sql.Statement"))); } @Override