Skip to content

Commit

Permalink
Merge pull request #169 from DataDog/ark/c3p0
Browse files Browse the repository at this point in the history 
Instrument Connection Constructor to get connection info
  • Loading branch information
@realark
realark authored Dec 11, 2017
2 parents 28bc1e7 + 4d5c06b commit 5ddcad3
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 103 deletions.
70 changes: 33 additions & 37 deletions ...tests/src/test/groovy/com/datadoghq/agent/integration/jdbc/JDBCInstrumentationTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,23 @@ package com.datadoghq.agent.integration.jdbc

import com.datadoghq.trace.DDTracer
import com.datadoghq.trace.writer.ListWriter
import io.opentracing.util.GlobalTracer
import dd.test.TestUtils
import org.apache.derby.jdbc.EmbeddedDriver
import org.h2.Driver
import org.hsqldb.jdbc.JDBCDriver
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Unroll

import java.lang.reflect.Field
import java.sql.Connection
import java.sql.PreparedStatement
import java.sql.ResultSet
import java.sql.Statement

class JDBCInstrumentationTest extends Specification {

ListWriter writer = new ListWriter()
DDTracer tracer = new DDTracer(writer)
final ListWriter writer = new ListWriter()
final DDTracer tracer = new DDTracer(writer)

@Shared
private Map<String, Connection> connections
Expand All @@ -43,16 +42,8 @@ class JDBCInstrumentationTest extends Specification {
}

def setup() {
try {
GlobalTracer.register(tracer)
} catch (final Exception e) {
// Force it anyway using reflection
final Field field = GlobalTracer.getDeclaredField("tracer")
field.setAccessible(true)
field.set(null, tracer)
}
TestUtils.registerOrReplaceGlobalTracer(tracer)
writer.start()
assert GlobalTracer.isRegistered()
}

@Unroll
Expand Down Expand Up @@ -80,6 +71,7 @@ class JDBCInstrumentationTest extends Specification {

def tags = span.context().tags
tags["db.type"] == driver
tags["db.user"] == username
tags["span.kind"] == "client"
tags["component"] == "java-jdbc-statement"

Expand All @@ -88,16 +80,16 @@ class JDBCInstrumentationTest extends Specification {

tags["thread.name"] != null
tags["thread.id"] != null
tags.size() == 7
tags.size() == username == null ? 7 : 8

cleanup:
statement.close()

where:
driver | connection | query
"h2" | connections.get("h2") | "SELECT 3"
"derby" | connections.get("derby") | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
driver | connection | username | query
"h2" | connections.get("h2") | null | "SELECT 3"
"derby" | connections.get("derby") | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
}

@Unroll
Expand Down Expand Up @@ -126,6 +118,7 @@ class JDBCInstrumentationTest extends Specification {

def tags = span.context().tags
tags["db.type"] == driver
tags["db.user"] == username
tags["span.kind"] == "client"
tags["component"] == "java-jdbc-prepared_statement"

Expand All @@ -134,16 +127,16 @@ class JDBCInstrumentationTest extends Specification {

tags["thread.name"] != null
tags["thread.id"] != null
tags.size() == 7
tags.size() == username == null ? 7 : 8

cleanup:
statement.close()

where:
driver | connection | query
"h2" | connections.get("h2") | "SELECT 3"
"derby" | connections.get("derby") | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
driver | connection | username | query
"h2" | connections.get("h2") | null | "SELECT 3"
"derby" | connections.get("derby") | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
}

@Unroll
Expand Down Expand Up @@ -171,6 +164,7 @@ class JDBCInstrumentationTest extends Specification {

def tags = span.context().tags
tags["db.type"] == driver
tags["db.user"] == username
tags["span.kind"] == "client"
tags["component"] == "java-jdbc-prepared_statement"

Expand All @@ -179,16 +173,16 @@ class JDBCInstrumentationTest extends Specification {

tags["thread.name"] != null
tags["thread.id"] != null
tags.size() == 7
tags.size() == username == null ? 7 : 8

cleanup:
statement.close()

where:
driver | connection | query
"h2" | connections.get("h2") | "SELECT 3"
"derby" | connections.get("derby") | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
driver | connection | username | query
"h2" | connections.get("h2") | null | "SELECT 3"
"derby" | connections.get("derby") | "APP" | "SELECT 3 FROM SYSIBM.SYSDUMMY1"
"hsqldb" | connections.get("hsqldb") | "SA" | "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS"
}

@Unroll
Expand Down Expand Up @@ -217,6 +211,7 @@ class JDBCInstrumentationTest extends Specification {

def tags = span.context().tags
tags["db.type"] == driver
tags["db.user"] == username
tags["span.kind"] == "client"
tags["component"] == "java-jdbc-statement"

Expand All @@ -225,16 +220,16 @@ class JDBCInstrumentationTest extends Specification {

tags["thread.name"] != null
tags["thread.id"] != null
tags.size() == 7
tags.size() == username == null ? 7 : 8

cleanup:
statement.close()

where:
driver | connection | query
"h2" | connections.get("h2") | "CREATE TABLE S_H2 (id INTEGER not NULL, PRIMARY KEY ( id ))"
"derby" | connections.get("derby") | "CREATE TABLE S_DERBY (id INTEGER not NULL, PRIMARY KEY ( id ))"
"hsqldb" | connections.get("hsqldb") | "CREATE TABLE PUBLIC.S_HSQLDB (id INTEGER not NULL, PRIMARY KEY ( id ))"
driver | connection | username | query
"h2" | connections.get("h2") | null | "CREATE TABLE S_H2 (id INTEGER not NULL, PRIMARY KEY ( id ))"
"derby" | connections.get("derby") | "APP" | "CREATE TABLE S_DERBY (id INTEGER not NULL, PRIMARY KEY ( id ))"
"hsqldb" | connections.get("hsqldb") | "SA" | "CREATE TABLE PUBLIC.S_HSQLDB (id INTEGER not NULL, PRIMARY KEY ( id ))"
}

@Unroll
Expand All @@ -261,6 +256,7 @@ class JDBCInstrumentationTest extends Specification {

def tags = span.context().tags
tags["db.type"] == driver
tags["db.user"] == username
tags["span.kind"] == "client"
tags["component"] == "java-jdbc-prepared_statement"

Expand All @@ -269,15 +265,15 @@ class JDBCInstrumentationTest extends Specification {

tags["thread.name"] != null
tags["thread.id"] != null
tags.size() == 7
tags.size() == username == null ? 7 : 8

cleanup:
statement.close()

where:
driver | connection | query
"h2" | connections.get("h2") | "CREATE TABLE PS_H2 (id INTEGER not NULL, PRIMARY KEY ( id ))"
driver | connection | username | query
"h2" | connections.get("h2") | null | "CREATE TABLE PS_H2 (id INTEGER not NULL, PRIMARY KEY ( id ))"
// Derby calls executeLargeUpdate from executeUpdate thus generating a nested span breaking this test.
"hsqldb" | connections.get("hsqldb") | "CREATE TABLE PUBLIC.PS_HSQLDB (id INTEGER not NULL, PRIMARY KEY ( id ))"
"hsqldb" | connections.get("hsqldb") | "SA" | "CREATE TABLE PUBLIC.PS_HSQLDB (id INTEGER not NULL, PRIMARY KEY ( id ))"
}
}
2 changes: 2 additions & 0 deletions dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static net.bytebuddy.matcher.ElementMatchers.any;
import static net.bytebuddy.matcher.ElementMatchers.isBootstrapClassLoader;
import static net.bytebuddy.matcher.ElementMatchers.nameContains;
import static net.bytebuddy.matcher.ElementMatchers.nameMatches;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;

import dd.trace.Instrumenter;
Expand Down Expand Up @@ -77,6 +78,7 @@ public static void addByteBuddy(final Instrumentation inst) {
.or(nameStartsWith("org.slf4j."))
.or(nameContains("javassist"))
.or(nameContains(".asm."))
.or(nameMatches("com\\.mchange\\.v2\\.c3p0\\..*Proxy"))
.ignore(
any(),
isBootstrapClassLoader()
Expand Down
39 changes: 37 additions & 2 deletions ...ent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/ConnectionInstrumentation.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.datadoghq.agent.instrumentation.jdbc;

import static net.bytebuddy.matcher.ElementMatchers.hasSuperType;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.isInterface;
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
import static net.bytebuddy.matcher.ElementMatchers.named;
Expand All @@ -15,11 +16,13 @@
import java.sql.PreparedStatement;
import java.util.Map;
import java.util.WeakHashMap;
import lombok.Data;
import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;

@AutoService(Instrumenter.class)
public final class ConnectionInstrumentation implements Instrumenter {
public static final Map<Connection, DBInfo> connectionInfo = new WeakHashMap<>();
public static final Map<PreparedStatement, String> preparedStatements = new WeakHashMap<>();

@Override
Expand All @@ -32,15 +35,47 @@ public AgentBuilder instrument(final AgentBuilder agentBuilder) {
nameStartsWith("prepare")
.and(takesArgument(0, String.class))
.and(returns(PreparedStatement.class)),
ConnectionAdvice.class.getName()))
ConnectionPrepareAdvice.class.getName()))
.transform(
DDAdvice.create().advice(isConstructor(), ConnectionConstructorAdvice.class.getName()))
.asDecorator();
}

public static class ConnectionAdvice {
public static class ConnectionPrepareAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void addDBInfo(
@Advice.Argument(0) final String sql, @Advice.Return final PreparedStatement statement) {
preparedStatements.put(statement, sql);
}
}

public static class ConnectionConstructorAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void addDBInfo(@Advice.This final Connection connection) {
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];
String user = connection.getMetaData().getUserName();
if (user != null && user.trim().equals("")) {
user = null;
}
connectionInfo.put(connection, new DBInfo(sanitizedURL, type, user));
}
} catch (Throwable t) {
// object may not be fully initialized.
// calling constructor will populate map
}
}
}

@Data
public static class DBInfo {
public static DBInfo UNKNOWN = new DBInfo("null", "unknown", null);
private final String url;
private final String type;
private final String user;
}
}
60 changes: 0 additions & 60 deletions ...a-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java
View file

This file was deleted.

5 changes: 3 additions & 2 deletions .../main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ public static ActiveSpan startSpan(@Advice.This final PreparedStatement statemen
return NoopActiveSpanSource.NoopActiveSpan.INSTANCE;
}

DriverInstrumentation.DBInfo dbInfo = DriverInstrumentation.connectionInfo.get(connection);
ConnectionInstrumentation.DBInfo dbInfo =
ConnectionInstrumentation.connectionInfo.get(connection);
if (dbInfo == null) {
dbInfo = DriverInstrumentation.DBInfo.UNKNOWN;
dbInfo = ConnectionInstrumentation.DBInfo.UNKNOWN;
}

final ActiveSpan span =
Expand Down
5 changes: 3 additions & 2 deletions ...gent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ public static ActiveSpan startSpan(
return NoopActiveSpanSource.NoopActiveSpan.INSTANCE;
}

DriverInstrumentation.DBInfo dbInfo = DriverInstrumentation.connectionInfo.get(connection);
ConnectionInstrumentation.DBInfo dbInfo =
ConnectionInstrumentation.connectionInfo.get(connection);
if (dbInfo == null) {
dbInfo = DriverInstrumentation.DBInfo.UNKNOWN;
dbInfo = ConnectionInstrumentation.DBInfo.UNKNOWN;
}

final ActiveSpan span =
Expand Down

0 comments on commit 5ddcad3

Please sign in to comment.