Skip repeated AbstractMethodError from JDBC getClientInfo

dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java

@@ -29,6 +29,7 @@
 import datadog.trace.bootstrap.instrumentation.jdbc.DBQueryInfo;
 import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionContext;
 import datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser;
+import datadog.trace.util.ClassLatch;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.sql.ClientInfoStatus;
@@ -48,6 +49,15 @@ public class JDBCDecorator extends DatabaseClientDecorator<DBInfo> {
 
   private static final Logger log = LoggerFactory.getLogger(JDBCDecorator.class);
 
+  /** Old drivers and pool proxies may not implement getClientInfo at all. */
+  private static final ClassLatch<Connection, Properties, SQLException> CLIENT_INFO_LATCH =
+      new ClassLatch<Connection, Properties, SQLException>() {
+        @Override
+        protected Properties apply(Connection connection) throws SQLException {
+          return handleAbstractMethod(connection, Connection::getClientInfo);
+        }
+      };
+
   public static final JDBCDecorator DECORATE = new JDBCDecorator();
   public static final CharSequence JAVA_JDBC = UTF8BytesString.create("java-jdbc");
   public static final CharSequence DATABASE_QUERY = UTF8BytesString.create("database.query");
@@ -246,9 +256,10 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) {
       if (metaData != null && (url = metaData.getURL()) != null) {
         Properties clientInfo = null;
         try {
-          clientInfo = connection.getClientInfo();
+          clientInfo = CLIENT_INFO_LATCH.tryApplyOrNull(connection);
         } catch (final Throwable ex) {
-          // getClientInfo is likely not allowed, we can still extract info from the url alone
+          // getClientInfo can fail in many ways (old drivers, pool proxies, test doubles), and we
+          // can still extract info from the url alone
           log.debug(LogCollector.EXCLUDE_TELEMETRY, "Could not get client info from DB", ex);
         }
         dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo);

dd-java-agent/instrumentation/jdbc/src/test/java/datadog/trace/instrumentation/jdbc/ParseDBInfoClientInfoTest.java

@@ -0,0 +1,133 @@
+package datadog.trace.instrumentation.jdbc;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo;
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Proxy;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.SQLException;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.junit.jupiter.api.Test;
+
+/** How {@code parseDBInfoFromConnection} copes with connections whose getClientInfo fails. */
+class ParseDBInfoClientInfoTest {
+  private static final String URL = "jdbc:postgresql://db.example.com:5432/orders";
+
+  interface ClientInfoAnswer {
+    Properties get() throws Throwable;
+  }
+
+  private static Connection connection(AtomicInteger clientInfoCalls, ClientInfoAnswer answer) {
+    DatabaseMetaData metaData =
+        (DatabaseMetaData)
+            Proxy.newProxyInstance(
+                DatabaseMetaData.class.getClassLoader(),
+                new Class<?>[] {DatabaseMetaData.class},
+                (proxy, method, args) -> "getURL".equals(method.getName()) ? URL : null);
+    InvocationHandler handler =
+        (proxy, method, args) -> {
+          switch (method.getName()) {
+            case "getMetaData":
+              return metaData;
+            case "getClientInfo":
+              clientInfoCalls.incrementAndGet();
+              try {
+                return answer.get();
+              } catch (InvocationTargetException e) {
+                throw e.getCause();
+              }
+            default:
+              return null;
+          }
+        };
+    return (Connection)
+        Proxy.newProxyInstance(
+            Connection.class.getClassLoader(), new Class<?>[] {Connection.class}, handler);
+  }
+
+  @Test
+  void urlIsStillParsedWhenGetClientInfoThrowsSqlException() {
+    AtomicInteger calls = new AtomicInteger();
+    Connection connection =
+        connection(
+            calls,
+            () -> {
+              throw new SQLException("not allowed");
+            });
+
+    DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection);
+
+    assertEquals("postgresql", info.getType());
+    assertEquals("orders", info.getDb());
+  }
+
+  @Test
+  void urlIsStillParsedWhenGetClientInfoIsUnsupported() {
+    AtomicInteger calls = new AtomicInteger();
+    Connection connection =
+        connection(
+            calls,
+            () -> {
+              throw new UnsupportedOperationException();
+            });
+
+    DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection);
+
+    assertEquals("postgresql", info.getType());
+    assertEquals("orders", info.getDb());
+  }
+
+  @Test
+  void urlIsStillParsedWhenGetClientInfoIsMissing() {
+    AtomicInteger calls = new AtomicInteger();
+    Connection connection =
+        connection(
+            calls,
+            () -> {
+              throw new AbstractMethodError("driver predates JDBC 4.0");
+            });
+
+    DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection);
+
+    assertEquals("postgresql", info.getType());
+    assertEquals("orders", info.getDb());
+  }
+
+  @Test
+  void anyOtherFailureStillYieldsUrlBasedDbInfo() {
+    AtomicInteger calls = new AtomicInteger();
+    for (Throwable failure :
+        new Throwable[] {
+          new IllegalStateException("unexpected"), new Throwable("not even an Exception")
+        }) {
+      Connection connection =
+          connection(
+              calls,
+              () -> {
+                throw failure;
+              });
+
+      // getClientInfo can fail in any way; the URL alone is still enough for the DB info
+      DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection);
+
+      assertEquals("postgresql", info.getType());
+      assertEquals("orders", info.getDb());
+    }
+  }
+
+  @Test
+  void returnsTheClientInfoWhenAvailable() {
+    AtomicInteger calls = new AtomicInteger();
+    Properties clientInfo = new Properties();
+    Connection connection = connection(calls, () -> clientInfo);
+
+    DBInfo info = JDBCDecorator.parseDBInfoFromConnection(connection);
+
+    assertEquals("postgresql", info.getType());
+    assertEquals(1, calls.get());
+  }
+}