Fixed unknown database issue from JDBC Connection constructor exception
Client will now report correct database if JDBC connection is recovered from exception.
MODIFY
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java
dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java
@@ -2,6 +2,7 @@ package datadog.trace.bootstrap;
import java.sql.Connection;
import java.sql.PreparedStatement;
+import java.sql.SQLException;
import java.util.Collections;
import java.util.Map;
import java.util.WeakHashMap;
@@ -39,4 +40,39 @@ public class JDBCMaps {
private static String propToEnvName(final String name) {
return name.toUpperCase().replace(".", "_");
}
+
+ /**
+ * Utility function to get the DBInfo from a JDBC Connection, if the connection was never seen
+ * before, the connectionInfo map will return null and will attempt to extract DBInfo from the
+ * connection. If the DBInfo can't be extracted, then the connection will be stored with the
+ * UNKNOWN DBInfo as the value in the connectionInfo map to avoid retry overhead.
+ *
+ * @param connection The JDBC connection
+ * @return A DBInfo that contains JDBC connection info
+ */
+ public static DBInfo getDBInfo(Connection connection) {
+ DBInfo dbInfo = connectionInfo.get(connection);
+ if (dbInfo == null) {
+ try {
+ final String url = connection.getMetaData().getURL();
+ if (url != null) {
+ // Remove end of url to prevent passwords from leaking:
+ final String sanitizedURL = url.replaceAll("[?;].*", "");
+ final String type = url.split(":", -1)[1];
+ String user = connection.getMetaData().getUserName();
+ if (user != null && user.trim().equals("")) {
+ user = null;
+ }
+ dbInfo = new JDBCMaps.DBInfo(sanitizedURL, type, user);
+ } else {
+ dbInfo = DBInfo.DEFAULT;
+ }
+ } catch (SQLException se) {
+ dbInfo = DBInfo.DEFAULT;
+ }
+ connectionInfo.put(connection, dbInfo);
+ }
+
+ return dbInfo;
+ }
}

MODIFY
dd-java-agent/instrumentation/jdbc/jdbc.gradle
dd-java-agent/instrumentation/jdbc/jdbc.gradle
@@ -7,4 +7,10 @@ dependencies {
compile deps.opentracing
annotationProcessor deps.autoservice
implementation deps.autoservice
+
+ testCompile project(':dd-java-agent:testing')
+ // jdbc unit testing
+ testCompile group: 'com.h2database', name: 'h2', version: '1.4.197'
+ testCompile group: 'org.apache.derby', name: 'derby', version: '10.12.1.1'
+
}

MODIFY
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/ConnectionInstrumentation.java
package datadog.trace.instrumentation.jdbc;
import static net.bytebuddy.matcher.ElementMatchers.failSafe;
-import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.isSubTypeOf;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
@@ -13,11 +12,9 @@ import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.DDAdvice;
import datadog.trace.agent.tooling.DDTransformers;
import datadog.trace.agent.tooling.Instrumenter;
-import datadog.trace.bootstrap.CallDepthThreadLocalMap;
import datadog.trace.bootstrap.JDBCMaps;
import java.sql.Connection;
import java.sql.PreparedStatement;
-import java.sql.SQLException;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;
@@ -40,8 +37,6 @@ public final class ConnectionInstrumentation extends Instrumenter.Configurable {
.and(takesArgument(0, String.class))
.and(returns(PreparedStatement.class)),
ConnectionPrepareAdvice.class.getName()))
- .transform(
- DDAdvice.create().advice(isConstructor(), ConnectionConstructorAdvice.class.getName()))
.asDecorator();
}
@@ -52,34 +47,4 @@ public final class ConnectionInstrumentation extends Instrumenter.Configurable {
JDBCMaps.preparedStatements.put(statement, sql);
}
}
-
- public static class ConnectionConstructorAdvice {
- @Advice.OnMethodEnter(suppress = Throwable.class)
- public static int constructorEnter() {
- // We use this to make sure we only apply the exit instrumentation
- // after the constructors are done calling their super constructors.
- return CallDepthThreadLocalMap.incrementCallDepth(Connection.class);
- }
-
- // Since we're instrumenting the constructor, we can't add onThrowable.
- @Advice.OnMethodExit(suppress = Throwable.class)
- public static void constructorExit(
- @Advice.Enter final int depth, @Advice.This final Connection connection)
- throws SQLException {
- if (depth == 0) {
- CallDepthThreadLocalMap.reset(Connection.class);
- final String url = connection.getMetaData().getURL();
- if (url != null) {
- // Remove end of url to prevent passwords from leaking:
- final String sanitizedURL = url.replaceAll("[?;].*", "");
- final String type = url.split(":", -1)[1];
- String user = connection.getMetaData().getUserName();
- if (user != null && user.trim().equals("")) {
- user = null;
- }
- JDBCMaps.connectionInfo.put(connection, new JDBCMaps.DBInfo(sanitizedURL, type, user));
- }
- }
- }
- }
}

MODIFY
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PreparedStatementInstrumentation.java
@@ -29,7 +29,6 @@ import net.bytebuddy.asm.Advice;
@AutoService(Instrumenter.class)
public final class PreparedStatementInstrumentation extends Instrumenter.Configurable {
-
public PreparedStatementInstrumentation() {
super("jdbc");
}
@@ -64,10 +63,8 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Configu
return NoopScope.INSTANCE;
}
- JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection);
- if (dbInfo == null) {
- dbInfo = JDBCMaps.DBInfo.DEFAULT;
- }
+ JDBCMaps.DBInfo dbInfo = JDBCMaps.getDBInfo(connection);
+
final Scope scope =
GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(true);

MODIFY
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
@@ -64,16 +64,12 @@ public final class StatementInstrumentation extends Instrumenter.Configurable {
return NoopScope.INSTANCE;
}
- JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection);
- if (dbInfo == null) {
- dbInfo = JDBCMaps.DBInfo.DEFAULT;
- }
+ JDBCMaps.DBInfo dbInfo = JDBCMaps.getDBInfo(connection);
final Scope scope =
GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(true);
final Span span = scope.span();
-
Tags.DB_TYPE.set(span, dbInfo.getType());
Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT);
Tags.COMPONENT.set(span, "java-jdbc-statement");

ADD
dd-java-agent/instrumentation/jdbc/src/test/groovy/DummyThrowingConnection.groovy
+import java.sql.Array
+import java.sql.Blob
+import java.sql.CallableStatement
+import java.sql.Clob
+import java.sql.Connection
+import java.sql.DatabaseMetaData
+import java.sql.NClob
+import java.sql.PreparedStatement
+import java.sql.SQLClientInfoException
+import java.sql.SQLException
+import java.sql.SQLWarning
+import java.sql.SQLXML
+import java.sql.Savepoint
+import java.sql.Statement
+import java.sql.Struct
+import java.util.concurrent.Executor
+
+
+/**
+ * A JDBC connection class that throws an exception in the constructor, used to test
+ */
+class DummyThrowingConnection implements Connection {
+ DummyThrowingConnection() {
+ throw new RuntimeException("Dummy exception")
+ }
+
+ @Override
+ Statement createStatement() throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql) throws SQLException {
+ return null
+ }
+
+ @Override
+ CallableStatement prepareCall(String sql) throws SQLException {
+ return null
+ }
+
+ @Override
+ String nativeSQL(String sql) throws SQLException {
+ return null
+ }
+
+ @Override
+ void setAutoCommit(boolean autoCommit) throws SQLException {
+
+ }
+
+ @Override
+ boolean getAutoCommit() throws SQLException {
+ return false
+ }
+
+ @Override
+ void commit() throws SQLException {
+
+ }
+
+ @Override
+ void rollback() throws SQLException {
+
+ }
+
+ @Override
+ void close() throws SQLException {
+
+ }
+
+ @Override
+ boolean isClosed() throws SQLException {
+ return false
+ }
+
+ @Override
+ DatabaseMetaData getMetaData() throws SQLException {
+ return null
+ }
+
+ @Override
+ void setReadOnly(boolean readOnly) throws SQLException {
+
+ }
+
+ @Override
+ boolean isReadOnly() throws SQLException {
+ return false
+ }
+
+ @Override
+ void setCatalog(String catalog) throws SQLException {
+
+ }
+
+ @Override
+ String getCatalog() throws SQLException {
+ return null
+ }
+
+ @Override
+ void setTransactionIsolation(int level) throws SQLException {
+
+ }
+
+ @Override
+ int getTransactionIsolation() throws SQLException {
+ return 0
+ }
+
+ @Override
+ SQLWarning getWarnings() throws SQLException {
+ return null
+ }
+
+ @Override
+ void clearWarnings() throws SQLException {
+
+ }
+
+ @Override
+ Statement createStatement(int resultSetType, int resultSetConcurrency) throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql, int resultSetType, int resultSetConcurrency) throws SQLException {
+ return null
+ }
+
+ @Override
+ CallableStatement prepareCall(String sql, int resultSetType, int resultSetConcurrency) throws SQLException {
+ return null
+ }
+
+ @Override
+ Map<String, Class<?>> getTypeMap() throws SQLException {
+ return null
+ }
+
+ @Override
+ void setTypeMap(Map<String, Class<?>> map) throws SQLException {
+
+ }
+
+ @Override
+ void setHoldability(int holdability) throws SQLException {
+
+ }
+
+ @Override
+ int getHoldability() throws SQLException {
+ return 0
+ }
+
+ @Override
+ Savepoint setSavepoint() throws SQLException {
+ return null
+ }
+
+ @Override
+ Savepoint setSavepoint(String name) throws SQLException {
+ return null
+ }
+
+ @Override
+ void rollback(Savepoint savepoint) throws SQLException {
+
+ }
+
+ @Override
+ void releaseSavepoint(Savepoint savepoint) throws SQLException {
+
+ }
+
+ @Override
+ Statement createStatement(int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException {
+ return null
+ }
+
+ @Override
+ CallableStatement prepareCall(String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql, int autoGeneratedKeys) throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql, int[] columnIndexes) throws SQLException {
+ return null
+ }
+
+ @Override
+ PreparedStatement prepareStatement(String sql, String[] columnNames) throws SQLException {
+ return null
+ }
+
+ @Override
+ Clob createClob() throws SQLException {
+ return null
+ }
+
+ @Override
+ Blob createBlob() throws SQLException {
+ return null
+ }
+
+ @Override
+ NClob createNClob() throws SQLException {
+ return null
+ }
+
+ @Override
+ SQLXML createSQLXML() throws SQLException {
+ return null
+ }
+
+ @Override
+ boolean isValid(int timeout) throws SQLException {
+ return false
+ }
+
+ @Override
+ void setClientInfo(String name, String value) throws SQLClientInfoException {
+
+ }
+
+ @Override
+ void setClientInfo(Properties properties) throws SQLClientInfoException {
+
+ }
+
+ @Override
+ String getClientInfo(String name) throws SQLException {
+ return null
+ }
+
+ @Override
+ Properties getClientInfo() throws SQLException {
+ return null
+ }
+
+ @Override
+ Array createArrayOf(String typeName, Object[] elements) throws SQLException {
+ return null
+ }
+
+ @Override
+ Struct createStruct(String typeName, Object[] attributes) throws SQLException {
+ return null
+ }
+
+ @Override
+ void setSchema(String schema) throws SQLException {
+
+ }
+
+ @Override
+ String getSchema() throws SQLException {
+ return null
+ }
+
+ @Override
+ void abort(Executor executor) throws SQLException {
+
+ }
+
+ @Override
+ void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException {
+
+ }
+
+ @Override
+ int getNetworkTimeout() throws SQLException {
+ return 0
+ }
+
+ @Override
+ def <T> T unwrap(Class<T> iface) throws SQLException {
+ return null
+ }
+
+ @Override
+ boolean isWrapperFor(Class<?> iface) throws SQLException {
+ return false
+ }
+}

ADD
dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy
+import datadog.trace.agent.test.AgentTestRunner
+import org.apache.derby.jdbc.EmbeddedDriver
+import org.h2.Driver
+import spock.lang.Shared
+import spock.lang.Unroll
+
+import java.sql.Connection
+import java.sql.ResultSet
+import java.sql.Statement
+
+class JDBCInstrumentationTest extends AgentTestRunner {
+ @Shared
+ def dbName = "jdbcUnitTest"
+
+ @Unroll
+ def "Connection constructor throwing then generating correct spans after recovery using #db connection (prepare statement = #prepareStatement)"() {
+ setup:
+ Connection connection = null
+
+ when:
+ try {
+ connection = new DummyThrowingConnection()
+ } catch (Exception e) {
+ connection = driver.connect(url, null)
+ }
+
+ Statement statement = null
+ ResultSet rs = null
+ if (prepareStatement) {
+ statement = connection.prepareStatement(query)
+ rs = statement.executeQuery()
+ } else {
+ statement = connection.createStatement()
+ rs = statement.executeQuery(query)
+ }
+
+ then:
+ rs.next()
+ rs.getInt(1) == 3
+ TEST_WRITER.size() == 1
+
+ def trace = TEST_WRITER.firstTrace()
+ trace.size() == 1
+
+ and:
+ def span = trace[0]
+ span.context().operationName == "${db}.query"
+ span.serviceName == db
+ span.resourceName == query
+ span.type == "sql"
+ !span.context().getErrorFlag()
+ span.context().parentId == 0
+
+ def tags = span.context().tags
+ tags["db.type"] == db
+ tags["db.user"] == user
+ tags["span.kind"] == "client"
+ if (prepareStatement) {
+ tags["component"] == "java-jdbc-prepared_statement"
+ } else {
+ tags["component"] == "java-jdbc-statement"
+ }
+
+ tags["db.jdbc.url"].contains(db)
+ tags["span.origin.type"] != null
+
+ tags["thread.name"] != null
+ tags["thread.id"] != null
+ tags.size() == user == null ? 7 : 8
+
+ cleanup:
+ if (statement != null) {
+ statement.close()
+ }
+ if (connection != null) {
+ connection.close()
+ }
+
+ where:
+ prepareStatement | db | driver | url | user | query
+ true | "h2" | new Driver() | "jdbc:h2:mem:" + dbName | null | "SELECT 3;"
+ true | "derby" | new EmbeddedDriver() | "jdbc:derby:memory:" + dbName + ";create=true" | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
+ false | "h2" | new Driver() | "jdbc:h2:mem:" + dbName | null | "SELECT 3;"
+ false | "derby" | new EmbeddedDriver() | "jdbc:derby:memory:" + dbName + ";create=true" | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
+ }
+
+}

Fixed unknown database issue from JDBC Connection constructor exception

Client will now report correct database if JDBC connection is recovered from exception.