From b40bcf997337115ac3b6b414c778e1462776fd67 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 20 Oct 2017 15:24:26 -0700 Subject: [PATCH 1/5] Add automatic instrumentation for JDBC This instrumentation creates spans for Statements and PreparedStatements. It also captures the corresponding SQL and additional connection info. ResultSet could be considered for future instrumentation to capture even more of the DB interaction time. This integration uses Bytebuddy instead of Byteman as the many methods to instrument would have been messy in Byteman. --- README.md | 21 +---- dd-java-agent/dd-java-agent.gradle | 5 +- .../com/datadoghq/agent/TracingAgent.java | 71 ++++++++++++++++ .../agent/instrumentation/Instrumenter.java | 7 ++ .../jdbc/ConnectionInstrumentation.java | 44 ++++++++++ .../jdbc/DriverInstrumentation.java | 55 +++++++++++++ .../PreparedStatementInstrumentation.java | 82 +++++++++++++++++++ .../jdbc/StatementInstrumentation.java | 81 ++++++++++++++++++ dd-trace-examples/spring-boot-jdbc/README.md | 6 -- .../spring-boot-jdbc/spring-boot-jdbc.gradle | 1 - .../src/main/resources/application.properties | 4 +- 11 files changed, 346 insertions(+), 31 deletions(-) create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/Instrumenter.java create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/ConnectionInstrumentation.java create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java diff --git a/README.md b/README.md index 7f7241a698..142d918bea 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Finally, add the following JVM argument when starting your application—in your -javaagent:/path/to/the/dd-java-agent.jar ``` -The Java Agent—once passed to your application—automatically traces requests to the frameworks, application servers, and databases shown below. It does this by using various libraries from [opentracing-contrib](https://github.com/opentracing-contrib). In most cases you don't need to install or configure anything; traces will automatically show up in your Datadog dashboards. The exception is [any database library that uses JDBC](#jdbc). +The Java Agent—once passed to your application—automatically traces requests to the frameworks, application servers, and databases shown below. It does this by using various libraries from [opentracing-contrib](https://github.com/opentracing-contrib). In most cases you don't need to install or configure anything; traces will automatically show up in your Datadog dashboards. #### Application Servers @@ -87,8 +87,7 @@ Also, frameworks like Spring Boot and Dropwizard inherently work because they us | Database | Versions | Comments | | ------------- |:-------------:| ----- | -| Spring JDBC| 4.x | **NOT traced automatically**—see [JDBC instructions](#jdbc) | -| Hibernate | 5.x | **NOT traced automatically**—see [JDBC instructions](#jdbc) | +| JDBC | 4.x | Intercepts calls to JDBC compatible clients | | [MongoDB](https://github.com/opentracing-contrib/java-mongo-driver) | 3.x | Intercepts all the calls from the MongoDB client | | [Cassandra](https://github.com/opentracing-contrib/java-cassandra-driver) | 3.2.x | Intercepts all the calls from the Cassandra client | @@ -103,22 +102,6 @@ disabledInstrumentations: ["opentracing-apache-httpclient", "opentracing-mongo-d See [this YAML file](dd-java-agent/src/main/resources/dd-trace-supported-framework.yaml) for the proper names of all supported libraries (i.e. the names as you must list them in `disabledInstrumentations`). -#### JDBC - -The Java Agent doesn't automatically trace requests to databases whose drivers are JDBC-based. For such databases, you must: - -1. Add the opentracing-jdbc dependency to your project, e.g. for Maven, add this to pom.xml: - -``` - - io.opentracing.contrib - opentracing-jdbc - 0.0.3 - -``` - -2. Modify your code's database connection strings, e.g. for a connection string `jdbc:h2:mem:test`, make it `jdbc:tracing:h2:mem:test`. - ### The `@Trace` Annotation The Java Agent lets you add a `@Trace` annotation to any method to measure its execution time. Setup the [Java Agent](#java-agent-setup) first if you haven't done so. diff --git a/dd-java-agent/dd-java-agent.gradle b/dd-java-agent/dd-java-agent.gradle index e11979ce3e..76f35a41b4 100644 --- a/dd-java-agent/dd-java-agent.gradle +++ b/dd-java-agent/dd-java-agent.gradle @@ -20,6 +20,7 @@ dependencies { compile project(':dd-trace') compile project(':dd-trace-annotations') + compile group: 'net.bytebuddy', name: 'byte-buddy', version: '1.7.6' compile group: 'org.jboss.byteman', name: 'byteman', version: '3.0.10' compile group: 'org.reflections', name: 'reflections', version: '0.9.11' @@ -36,9 +37,6 @@ dependencies { testCompile(project(path: ':dd-java-agent:integrations:helpers')) { transitive = false } - - // Not bundled in with the agent. Usage requires being on the app's classpath (eg. Spring Boot's executable jar) - compileOnly group: 'io.opentracing.contrib', name: 'opentracing-jdbc', version: '0.0.3' } project(':dd-java-agent:integrations:helpers').afterEvaluate { helperProject -> @@ -90,6 +88,7 @@ shadowJar { relocate 'com.fasterxml', 'dd.deps.com.fasterxml' relocate 'javassist', 'dd.deps.javassist' + relocate 'net.bytebuddy', 'dd.deps.net.bytebuddy' relocate 'org.reflections', 'dd.deps.org.reflections' relocate('org.jboss.byteman', 'dd.deps.org.jboss.byteman') { // Renaming these causes a verify error in the tests. diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java b/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java index 4af6ad45d6..39444a58b4 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java @@ -16,8 +16,16 @@ */ package com.datadoghq.agent; +import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; + +import com.datadoghq.agent.instrumentation.Instrumenter; import java.lang.instrument.Instrumentation; +import java.util.ServiceLoader; import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.utility.JavaModule; /** * This class provides a wrapper around the ByteMan agent, to establish required system properties @@ -27,12 +35,14 @@ import lombok.extern.slf4j.Slf4j; public class TracingAgent { public static void premain(String agentArgs, final Instrumentation inst) throws Exception { + addByteBuddy(inst); agentArgs = addManager(agentArgs); log.debug("Using premain for loading {}", TracingAgent.class.getSimpleName()); org.jboss.byteman.agent.Main.premain(agentArgs, inst); } public static void agentmain(String agentArgs, final Instrumentation inst) throws Exception { + addByteBuddy(inst); agentArgs = addManager(agentArgs); log.debug("Using agentmain for loading {}", TracingAgent.class.getSimpleName()); org.jboss.byteman.agent.Main.agentmain(agentArgs, inst); @@ -48,4 +58,65 @@ public class TracingAgent { log.debug("Agent args=: {}", agentArgs); return agentArgs; } + + public static void addByteBuddy(final Instrumentation inst) { + + AgentBuilder agentBuilder = + new AgentBuilder.Default() + .disableClassFormatChanges() + .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) + .with(new Listener()) + .ignore(nameStartsWith("com.datadoghq.agent.integration")); + + for (final Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class)) { + agentBuilder = instrumenter.instrument(agentBuilder); + } + + agentBuilder.installOn(inst); + } + + @Slf4j + static class Listener implements AgentBuilder.Listener { + + @Override + public void onError( + final String typeName, + final ClassLoader classLoader, + final JavaModule module, + final boolean loaded, + final Throwable throwable) { + log.warn("Failed to handle " + typeName + " for transformation", throwable); + } + + @Override + public void onTransformation( + final TypeDescription typeDescription, + final ClassLoader classLoader, + final JavaModule module, + final boolean loaded, + final DynamicType dynamicType) { + log.debug("Transformed {0}", typeDescription); + } + + @Override + public void onIgnored( + final TypeDescription typeDescription, + final ClassLoader classLoader, + final JavaModule module, + final boolean loaded) {} + + @Override + public void onComplete( + final String typeName, + final ClassLoader classLoader, + final JavaModule module, + final boolean loaded) {} + + @Override + public void onDiscovery( + final String typeName, + final ClassLoader classLoader, + final JavaModule module, + final boolean loaded) {} + } } diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/Instrumenter.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/Instrumenter.java new file mode 100644 index 0000000000..5348b90523 --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/Instrumenter.java @@ -0,0 +1,7 @@ +package com.datadoghq.agent.instrumentation; + +import net.bytebuddy.agent.builder.AgentBuilder; + +public interface Instrumenter { + AgentBuilder instrument(AgentBuilder agentBuilder); +} diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/ConnectionInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/ConnectionInstrumentation.java new file mode 100644 index 0000000000..6284ff5163 --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -0,0 +1,44 @@ +package com.datadoghq.agent.instrumentation.jdbc; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +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; + +import com.datadoghq.agent.instrumentation.Instrumenter; +import com.google.auto.service.AutoService; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.util.Map; +import java.util.WeakHashMap; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public final class ConnectionInstrumentation implements Instrumenter { + public static final Map preparedStatements = new WeakHashMap<>(); + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(Connection.class.getName())))) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + nameStartsWith("prepare") + .and(takesArgument(0, String.class)) + .and(returns(PreparedStatement.class)), + ConnectionAdvice.class.getName())); + } + + public static class ConnectionAdvice { + @Advice.OnMethodExit + public static void addDBInfo( + @Advice.Argument(0) final String sql, @Advice.Return final PreparedStatement statement) { + preparedStatements.put(statement, sql); + } + } +} diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java new file mode 100644 index 0000000000..f6a1b2b442 --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java @@ -0,0 +1,55 @@ +package com.datadoghq.agent.instrumentation.jdbc; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.datadoghq.agent.instrumentation.Instrumenter; +import com.google.auto.service.AutoService; +import java.sql.Connection; +import java.sql.Driver; +import java.util.Map; +import java.util.Properties; +import java.util.WeakHashMap; +import lombok.Data; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public final class DriverInstrumentation implements Instrumenter { + public static final Map connectionInfo = new WeakHashMap<>(); + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(Driver.class.getName())))) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("connect").and(takesArguments(String.class, Properties.class)), + DriverAdvice.class.getName())); + } + + public static class DriverAdvice { + @Advice.OnMethodExit + public static void addDBInfo( + @Advice.Argument(0) final String url, + @Advice.Argument(1) final Properties info, + @Advice.Return final Connection connection) { + // Remove end of url to prevent passwords from leaking: + final String sanitizedURL = url.replaceAll("[?;].*", ""); + final String type = url.split(":")[1]; + final String dbUser = info.getProperty("user"); + connectionInfo.put(connection, new DBInfo(sanitizedURL, type, dbUser)); + } + } + + @Data + public static class DBInfo { + private final String url; + private final String type; + private final String user; + } +} diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java new file mode 100644 index 0000000000..ea955641f2 --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -0,0 +1,82 @@ +package com.datadoghq.agent.instrumentation.jdbc; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +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; + +import com.datadoghq.agent.instrumentation.Instrumenter; +import com.google.auto.service.AutoService; +import io.opentracing.ActiveSpan; +import io.opentracing.NoopActiveSpanSource; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.util.Collections; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public final class PreparedStatementInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(PreparedStatement.class.getName())))) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + nameStartsWith("execute").and(takesArguments(0)), + PreparedStatementAdvice.class.getName())) + .asDecorator(); + } + + public static class PreparedStatementAdvice { + + @Advice.OnMethodEnter + public static ActiveSpan startSpan(@Advice.This final PreparedStatement statement) { + // TODO: Should this happen always instead of just inside an existing tracer? + if (GlobalTracer.get().activeSpan() == null) { + return NoopActiveSpanSource.NoopActiveSpan.INSTANCE; + } + + final String sql = ConnectionInstrumentation.preparedStatements.get(statement); + + final ActiveSpan span = GlobalTracer.get().buildSpan("sql.prepared_statement").startActive(); + Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); + Tags.COMPONENT.set(span, "java-jdbc"); + Tags.DB_STATEMENT.set(span, sql); + span.setTag("span.origin.type", statement.getClass().getName()); + + try { + final Connection connection = statement.getConnection(); + final DriverInstrumentation.DBInfo dbInfo = + DriverInstrumentation.connectionInfo.get(connection); + + span.setTag("db.jdbc.url", dbInfo.getUrl()); + span.setTag("db.schema", connection.getSchema()); + + Tags.DB_TYPE.set(span, dbInfo.getType()); + if (dbInfo.getUser() != null) { + Tags.DB_USER.set(span, dbInfo.getUser()); + } + } finally { + return span; + } + } + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void stopSpan( + @Advice.Enter final ActiveSpan activeSpan, + @Advice.Thrown(readOnly = false) final Throwable throwable) { + if (throwable != null) { + Tags.ERROR.set(activeSpan, true); + activeSpan.log(Collections.singletonMap("error.object", throwable)); + } + activeSpan.deactivate(); + } + } +} diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java new file mode 100644 index 0000000000..1f93889a7e --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java @@ -0,0 +1,81 @@ +package com.datadoghq.agent.instrumentation.jdbc; + +import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +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; + +import com.datadoghq.agent.instrumentation.Instrumenter; +import com.google.auto.service.AutoService; +import io.opentracing.ActiveSpan; +import io.opentracing.NoopActiveSpanSource; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import java.sql.Connection; +import java.sql.Statement; +import java.util.Collections; +import net.bytebuddy.agent.builder.AgentBuilder; +import net.bytebuddy.asm.Advice; + +@AutoService(Instrumenter.class) +public final class StatementInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(not(isInterface()).and(hasSuperType(named(Statement.class.getName())))) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + nameStartsWith("execute").and(takesArgument(0, String.class)), + StatementAdvice.class.getName())) + .asDecorator(); + } + + public static class StatementAdvice { + + @Advice.OnMethodEnter + public static ActiveSpan startSpan( + @Advice.Argument(0) final String sql, @Advice.This final Statement statement) { + // TODO: Should this happen always instead of just inside an existing tracer? + if (GlobalTracer.get().activeSpan() == null) { + return NoopActiveSpanSource.NoopActiveSpan.INSTANCE; + } + + final ActiveSpan span = GlobalTracer.get().buildSpan("sql.statement").startActive(); + Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); + Tags.COMPONENT.set(span, "java-jdbc"); + Tags.DB_STATEMENT.set(span, sql); + span.setTag("span.origin.type", statement.getClass().getName()); + + try { + final Connection connection = statement.getConnection(); + final DriverInstrumentation.DBInfo dbInfo = + DriverInstrumentation.connectionInfo.get(connection); + + span.setTag("db.jdbc.url", dbInfo.getUrl()); + span.setTag("db.schema", connection.getSchema()); + + Tags.DB_TYPE.set(span, dbInfo.getType()); + if (dbInfo.getUser() != null) { + Tags.DB_USER.set(span, dbInfo.getUser()); + } + } finally { + return span; + } + } + + @Advice.OnMethodExit(onThrowable = Throwable.class) + public static void stopSpan( + @Advice.Enter final ActiveSpan activeSpan, + @Advice.Thrown(readOnly = false) final Throwable throwable) { + if (throwable != null) { + Tags.ERROR.set(activeSpan, true); + activeSpan.log(Collections.singletonMap("error.object", throwable)); + } + activeSpan.deactivate(); + } + } +} diff --git a/dd-trace-examples/spring-boot-jdbc/README.md b/dd-trace-examples/spring-boot-jdbc/README.md index 66c0854d5b..3c0e7cb77b 100644 --- a/dd-trace-examples/spring-boot-jdbc/README.md +++ b/dd-trace-examples/spring-boot-jdbc/README.md @@ -57,9 +57,3 @@ instruments: - The JDBC driver The Java Agent embeds the [OpenTracing Java Agent](https://github.com/opentracing-contrib/java-agent). - -#### Note for JDBC tracing configuration - -[JDBC is not automatically instrumented by the Java Agent](../../README.md#jdbc), so we changed the `application.properties` -[file](src/main/resources/application.properties) to use the OpenTracing Driver and included it as a dependency in `spring-boot-jdbc.gradle`. -Without these steps in your applications, the TracingDriver will not work. diff --git a/dd-trace-examples/spring-boot-jdbc/spring-boot-jdbc.gradle b/dd-trace-examples/spring-boot-jdbc/spring-boot-jdbc.gradle index 869be5d6fb..cdb5523ae0 100644 --- a/dd-trace-examples/spring-boot-jdbc/spring-boot-jdbc.gradle +++ b/dd-trace-examples/spring-boot-jdbc/spring-boot-jdbc.gradle @@ -11,7 +11,6 @@ description = 'spring-boot-jdbc' dependencies { compile group: 'org.apache.httpcomponents', name: 'httpclient', version: '4.5.3' - compile group: 'io.opentracing.contrib', name: 'opentracing-jdbc', version: '0.0.3' compile group: 'com.h2database', name: 'h2', version: '1.4.196' compile group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.4.RELEASE' compile group: 'org.springframework.boot', name: 'spring-boot-starter-data-jpa', version: '1.5.4.RELEASE' diff --git a/dd-trace-examples/spring-boot-jdbc/src/main/resources/application.properties b/dd-trace-examples/spring-boot-jdbc/src/main/resources/application.properties index 399d5e9819..bee9bc11eb 100644 --- a/dd-trace-examples/spring-boot-jdbc/src/main/resources/application.properties +++ b/dd-trace-examples/spring-boot-jdbc/src/main/resources/application.properties @@ -1,6 +1,6 @@ # you must set the following so that OpenTracing traced driver is used -spring.datasource.driver-class-name=io.opentracing.contrib.jdbc.TracingDriver -spring.datasource.url=jdbc:tracing:h2:mem:spring-test;DB_CLOSE_ON_EXIT=FALSE +spring.datasource.driver-class-name=org.h2.Driver +spring.datasource.url=jdbc:h2:mem:spring-test;DB_CLOSE_ON_EXIT=FALSE # set the logging level logging.level.root=INFO From ef5481cc277c68fe9cec99f1c0a616f1fa4fafb6 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 23 Oct 2017 17:12:37 -0700 Subject: [PATCH 2/5] Change operation name and set service name. --- .../PreparedStatementInstrumentation.java | 46 ++++++++++--------- .../jdbc/StatementInstrumentation.java | 45 ++++++++++-------- 2 files changed, 50 insertions(+), 41 deletions(-) diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java index ea955641f2..62adef10d8 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -8,6 +8,7 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.datadoghq.agent.instrumentation.Instrumenter; +import com.datadoghq.trace.DDTags; import com.google.auto.service.AutoService; import io.opentracing.ActiveSpan; import io.opentracing.NoopActiveSpanSource; @@ -38,40 +39,43 @@ public final class PreparedStatementInstrumentation implements Instrumenter { @Advice.OnMethodEnter public static ActiveSpan startSpan(@Advice.This final PreparedStatement statement) { - // TODO: Should this happen always instead of just inside an existing tracer? - if (GlobalTracer.get().activeSpan() == null) { + final String sql = ConnectionInstrumentation.preparedStatements.get(statement); + final Connection connection; + try { + connection = statement.getConnection(); + } catch (final Throwable e) { + // Had some problem getting the connection. return NoopActiveSpanSource.NoopActiveSpan.INSTANCE; } - final String sql = ConnectionInstrumentation.preparedStatements.get(statement); + final DriverInstrumentation.DBInfo dbInfo = + DriverInstrumentation.connectionInfo.get(connection); - final ActiveSpan span = GlobalTracer.get().buildSpan("sql.prepared_statement").startActive(); - Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(span, "java-jdbc"); + final ActiveSpan span = + GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(); + Tags.DB_TYPE.set(span, dbInfo.getType()); Tags.DB_STATEMENT.set(span, sql); + Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); + Tags.COMPONENT.set(span, "java-jdbc-prepared_statement"); + + span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); span.setTag("span.origin.type", statement.getClass().getName()); - + span.setTag("db.jdbc.url", dbInfo.getUrl()); try { - final Connection connection = statement.getConnection(); - final DriverInstrumentation.DBInfo dbInfo = - DriverInstrumentation.connectionInfo.get(connection); - - span.setTag("db.jdbc.url", dbInfo.getUrl()); span.setTag("db.schema", connection.getSchema()); - - Tags.DB_TYPE.set(span, dbInfo.getType()); - if (dbInfo.getUser() != null) { - Tags.DB_USER.set(span, dbInfo.getUser()); - } - } finally { - return span; + } catch (final Throwable e) { + // Ignore... } + + if (dbInfo.getUser() != null) { + Tags.DB_USER.set(span, dbInfo.getUser()); + } + return span; } @Advice.OnMethodExit(onThrowable = Throwable.class) public static void stopSpan( - @Advice.Enter final ActiveSpan activeSpan, - @Advice.Thrown(readOnly = false) final Throwable throwable) { + @Advice.Enter final ActiveSpan activeSpan, @Advice.Thrown final Throwable throwable) { if (throwable != null) { Tags.ERROR.set(activeSpan, true); activeSpan.log(Collections.singletonMap("error.object", throwable)); diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java index 1f93889a7e..b3b782442c 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java @@ -8,6 +8,7 @@ import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.datadoghq.agent.instrumentation.Instrumenter; +import com.datadoghq.trace.DDTags; import com.google.auto.service.AutoService; import io.opentracing.ActiveSpan; import io.opentracing.NoopActiveSpanSource; @@ -39,38 +40,42 @@ public final class StatementInstrumentation implements Instrumenter { @Advice.OnMethodEnter public static ActiveSpan startSpan( @Advice.Argument(0) final String sql, @Advice.This final Statement statement) { - // TODO: Should this happen always instead of just inside an existing tracer? - if (GlobalTracer.get().activeSpan() == null) { + final Connection connection; + try { + connection = statement.getConnection(); + } catch (final Throwable e) { + // Had some problem getting the connection. return NoopActiveSpanSource.NoopActiveSpan.INSTANCE; } - final ActiveSpan span = GlobalTracer.get().buildSpan("sql.statement").startActive(); - Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); - Tags.COMPONENT.set(span, "java-jdbc"); + final DriverInstrumentation.DBInfo dbInfo = + DriverInstrumentation.connectionInfo.get(connection); + + final ActiveSpan span = + GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(); + Tags.DB_TYPE.set(span, dbInfo.getType()); Tags.DB_STATEMENT.set(span, sql); + Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); + Tags.COMPONENT.set(span, "java-jdbc-statement"); + + span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); span.setTag("span.origin.type", statement.getClass().getName()); - + span.setTag("db.jdbc.url", dbInfo.getUrl()); try { - final Connection connection = statement.getConnection(); - final DriverInstrumentation.DBInfo dbInfo = - DriverInstrumentation.connectionInfo.get(connection); - - span.setTag("db.jdbc.url", dbInfo.getUrl()); span.setTag("db.schema", connection.getSchema()); - - Tags.DB_TYPE.set(span, dbInfo.getType()); - if (dbInfo.getUser() != null) { - Tags.DB_USER.set(span, dbInfo.getUser()); - } - } finally { - return span; + } catch (final Throwable e) { + // Ignore... } + + if (dbInfo.getUser() != null) { + Tags.DB_USER.set(span, dbInfo.getUser()); + } + return span; } @Advice.OnMethodExit(onThrowable = Throwable.class) public static void stopSpan( - @Advice.Enter final ActiveSpan activeSpan, - @Advice.Thrown(readOnly = false) final Throwable throwable) { + @Advice.Enter final ActiveSpan activeSpan, @Advice.Thrown final Throwable throwable) { if (throwable != null) { Tags.ERROR.set(activeSpan, true); activeSpan.log(Collections.singletonMap("error.object", throwable)); From 0d64192d91b7e939be37a77bd73ad42e558cb006 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 24 Oct 2017 07:59:29 -0700 Subject: [PATCH 3/5] Set resource name and span type correctly. --- .../instrumentation/jdbc/PreparedStatementInstrumentation.java | 3 ++- .../agent/instrumentation/jdbc/StatementInstrumentation.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java index 62adef10d8..5310228627 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -54,11 +54,12 @@ public final class PreparedStatementInstrumentation implements Instrumenter { final ActiveSpan span = GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(); Tags.DB_TYPE.set(span, dbInfo.getType()); - Tags.DB_STATEMENT.set(span, sql); Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); Tags.COMPONENT.set(span, "java-jdbc-prepared_statement"); span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); + span.setTag(DDTags.RESOURCE_NAME, sql); + span.setTag(DDTags.SPAN_TYPE, "sql"); span.setTag("span.origin.type", statement.getClass().getName()); span.setTag("db.jdbc.url", dbInfo.getUrl()); try { diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java index b3b782442c..ced42a6c87 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java @@ -54,11 +54,12 @@ public final class StatementInstrumentation implements Instrumenter { final ActiveSpan span = GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(); Tags.DB_TYPE.set(span, dbInfo.getType()); - Tags.DB_STATEMENT.set(span, sql); Tags.SPAN_KIND.set(span, Tags.SPAN_KIND_CLIENT); Tags.COMPONENT.set(span, "java-jdbc-statement"); span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); + span.setTag(DDTags.RESOURCE_NAME, sql); + span.setTag(DDTags.SPAN_TYPE, "sql"); span.setTag("span.origin.type", statement.getClass().getName()); span.setTag("db.jdbc.url", dbInfo.getUrl()); try { From 14b834a358ddc8a092ade336fcfe92a9c489a0f8 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 25 Oct 2017 10:36:37 -0700 Subject: [PATCH 4/5] Ignore generic classes and proxy classes --- .../com/datadoghq/agent/TracingAgent.java | 27 ++++++++++++++++--- .../agent/utils/ClassLoaderNameMatcher.java | 27 +++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 dd-java-agent/src/main/java/com/datadoghq/agent/utils/ClassLoaderNameMatcher.java diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java b/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java index 39444a58b4..cc1e95c062 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/TracingAgent.java @@ -16,6 +16,11 @@ */ package com.datadoghq.agent; +import static com.datadoghq.agent.utils.ClassLoaderNameMatcher.classLoaderWithName; +import static com.datadoghq.agent.utils.ClassLoaderNameMatcher.isReflectionClassLoader; +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.nameStartsWith; import com.datadoghq.agent.instrumentation.Instrumenter; @@ -60,13 +65,29 @@ public class TracingAgent { } public static void addByteBuddy(final Instrumentation inst) { - AgentBuilder agentBuilder = new AgentBuilder.Default() .disableClassFormatChanges() .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION) .with(new Listener()) - .ignore(nameStartsWith("com.datadoghq.agent.integration")); + .ignore(nameStartsWith("com.datadoghq.agent.integration")) + .or(nameStartsWith("java.")) + .or(nameStartsWith("com.sun.")) + .or(nameStartsWith("sun.")) + .or(nameStartsWith("jdk.")) + .or(nameStartsWith("org.aspectj.")) + .or(nameStartsWith("org.groovy.")) + .or(nameStartsWith("com.p6spy.")) + .or(nameStartsWith("org.slf4j.")) + .or(nameContains("javassist")) + .or(nameContains(".asm.")) + .ignore( + any(), + isBootstrapClassLoader() + .or(isReflectionClassLoader()) + .or( + classLoaderWithName( + "org.codehaus.groovy.runtime.callsite.CallSiteClassLoader"))); for (final Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class)) { agentBuilder = instrumenter.instrument(agentBuilder); @@ -95,7 +116,7 @@ public class TracingAgent { final JavaModule module, final boolean loaded, final DynamicType dynamicType) { - log.debug("Transformed {0}", typeDescription); + log.debug("Transformed {}", typeDescription); } @Override diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/utils/ClassLoaderNameMatcher.java b/dd-java-agent/src/main/java/com/datadoghq/agent/utils/ClassLoaderNameMatcher.java new file mode 100644 index 0000000000..1c5e4090c4 --- /dev/null +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/utils/ClassLoaderNameMatcher.java @@ -0,0 +1,27 @@ +package com.datadoghq.agent.utils; + +import net.bytebuddy.matcher.ElementMatcher; + +// Borrowed from https://github.com/stagemonitor/stagemonitor/blob/master/stagemonitor-core/src/main/java/org/stagemonitor/core/instrument/ClassLoaderNameMatcher.java +public class ClassLoaderNameMatcher extends ElementMatcher.Junction.AbstractBase { + + private final String name; + + private ClassLoaderNameMatcher(final String name) { + this.name = name; + } + + public static ElementMatcher.Junction.AbstractBase classLoaderWithName( + final String name) { + return new ClassLoaderNameMatcher(name); + } + + public static ElementMatcher.Junction.AbstractBase isReflectionClassLoader() { + return new ClassLoaderNameMatcher("sun.reflect.DelegatingClassLoader"); + } + + @Override + public boolean matches(final ClassLoader target) { + return target != null && name.equals(target.getClass().getName()); + } +} From 25029b41092cc4f3f5be40d76f320afdfd2d8308 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 25 Oct 2017 10:37:37 -0700 Subject: [PATCH 5/5] Add integration tests for in-memory jdbc drivers. --- .../dd-java-agent-ittests.gradle | 5 + .../jdbc/JDBCInstrumentationTest.groovy | 283 ++++++++++++++++++ .../jdbc/DriverInstrumentation.java | 2 +- .../PreparedStatementInstrumentation.java | 8 +- .../jdbc/StatementInstrumentation.java | 8 +- 5 files changed, 293 insertions(+), 13 deletions(-) create mode 100644 dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/jdbc/JDBCInstrumentationTest.groovy diff --git a/dd-java-agent-ittests/dd-java-agent-ittests.gradle b/dd-java-agent-ittests/dd-java-agent-ittests.gradle index 6c85002148..9196577168 100644 --- a/dd-java-agent-ittests/dd-java-agent-ittests.gradle +++ b/dd-java-agent-ittests/dd-java-agent-ittests.gradle @@ -28,6 +28,11 @@ dependencies { testCompile group: 'javax.jms', name: 'javax.jms-api', version: '2.0.1' testCompile group: 'org.apache.activemq.tooling', name: 'activemq-junit', version: '5.14.5' testCompile group: 'org.apache.activemq', name: 'activemq-broker', version: '5.14.5' + + // JDBC tests: + testCompile group: 'com.h2database', name: 'h2', version: '1.4.196' + testCompile group: 'org.hsqldb', name: 'hsqldb', version: '2.4.0' + testCompile group: 'org.apache.derby', name: 'derby', version: '10.14.1.0' } configurations.all { diff --git a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/jdbc/JDBCInstrumentationTest.groovy b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/jdbc/JDBCInstrumentationTest.groovy new file mode 100644 index 0000000000..b998f8b2da --- /dev/null +++ b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/jdbc/JDBCInstrumentationTest.groovy @@ -0,0 +1,283 @@ +package com.datadoghq.agent.integration.jdbc + +import com.datadoghq.trace.DDTracer +import com.datadoghq.trace.writer.ListWriter +import io.opentracing.util.GlobalTracer +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) + + @Shared + private Map connections + + def setupSpec() { + Connection h2Connection = new Driver().connect("jdbc:h2:mem:integ-test", null) + Connection hsqlConnection = new JDBCDriver().connect("jdbc:hsqldb:mem:integTest", null) + Connection derbyConnection = new EmbeddedDriver().connect("jdbc:derby:memory:integTest;create=true", null) + + connections = [ + h2 : h2Connection, + derby : derbyConnection, + hsqldb: hsqlConnection, + ] + } + + def cleanupSpec() { + connections.values().each { + it.close() + } + } + + 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) + } + writer.start() + assert GlobalTracer.isRegistered() + } + + @Unroll + def "basic statement on #driver generates spans"() { + setup: + Statement statement = connection.createStatement() + ResultSet resultSet = statement.executeQuery(query) + + expect: + resultSet.next() + resultSet.getInt(1) == 3 + writer.size() == 1 + + def trace = writer.firstTrace() + trace.size() == 1 + def span = trace[0] + + span.context().operationName == "${driver}.query" + span.serviceName == driver + span.resourceName == query + span.type == "sql" + !span.context().getErrorFlag() + span.context().parentId == 0 + + + def tags = span.context().tags + tags["db.type"] == driver + tags["span.kind"] == "client" + tags["component"] == "java-jdbc-statement" + + tags["db.jdbc.url"].contains(driver) + tags["span.origin.type"] != null + + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 7 + + 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" + } + + @Unroll + def "prepared statement execute on #driver generates a span"() { + setup: + PreparedStatement statement = connection.prepareStatement(query) + assert statement.execute() + ResultSet resultSet = statement.resultSet + + expect: + resultSet.next() + resultSet.getInt(1) == 3 + writer.size() == 1 + + def trace = writer.firstTrace() + trace.size() == 1 + def span = trace[0] + + span.context().operationName == "${driver}.query" + span.serviceName == driver + span.resourceName == query + span.type == "sql" + !span.context().getErrorFlag() + span.context().parentId == 0 + + + def tags = span.context().tags + tags["db.type"] == driver + tags["span.kind"] == "client" + tags["component"] == "java-jdbc-prepared_statement" + + tags["db.jdbc.url"].contains(driver) + tags["span.origin.type"] != null + + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 7 + + 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" + } + + @Unroll + def "prepared statement query on #driver generates a span"() { + setup: + PreparedStatement statement = connection.prepareStatement(query) + ResultSet resultSet = statement.executeQuery() + + expect: + resultSet.next() + resultSet.getInt(1) == 3 + writer.size() == 1 + + def trace = writer.firstTrace() + trace.size() == 1 + def span = trace[0] + + span.context().operationName == "${driver}.query" + span.serviceName == driver + span.resourceName == query + span.type == "sql" + !span.context().getErrorFlag() + span.context().parentId == 0 + + + def tags = span.context().tags + tags["db.type"] == driver + tags["span.kind"] == "client" + tags["component"] == "java-jdbc-prepared_statement" + + tags["db.jdbc.url"].contains(driver) + tags["span.origin.type"] != null + + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 7 + + 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" + } + + @Unroll + def "statement update on #driver generates a span"() { + setup: + Statement statement = connection.createStatement() + def sql = connection.nativeSQL(query) + + expect: + !statement.execute(sql) + statement.updateCount == 0 + + writer.size() == 1 + + def trace = writer.firstTrace() + trace.size() == 1 + def span = trace[0] + + span.context().operationName == "${driver}.query" + span.serviceName == driver + span.resourceName == query + span.type == "sql" + !span.context().getErrorFlag() + span.context().parentId == 0 + + + def tags = span.context().tags + tags["db.type"] == driver + tags["span.kind"] == "client" + tags["component"] == "java-jdbc-statement" + + tags["db.jdbc.url"].contains(driver) + tags["span.origin.type"] != null + + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 7 + + 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 ))" + } + + @Unroll + def "prepared statement update on #driver generates a span"() { + setup: + def sql = connection.nativeSQL(query) + PreparedStatement statement = connection.prepareStatement(sql) + + expect: + statement.executeUpdate() == 0 + writer.size() == 1 + + def trace = writer.firstTrace() + trace.size() == 1 + def span = trace[0] + + span.context().operationName == "${driver}.query" + span.serviceName == driver + span.resourceName == query + span.type == "sql" + !span.context().getErrorFlag() + span.context().parentId == 0 + + + def tags = span.context().tags + tags["db.type"] == driver + tags["span.kind"] == "client" + tags["component"] == "java-jdbc-prepared_statement" + + tags["db.jdbc.url"].contains(driver) + tags["span.origin.type"] != null + + tags["thread.name"] != null + tags["thread.id"] != null + tags.size() == 7 + + cleanup: + statement.close() + + where: + driver | connection | query + "h2" | connections.get("h2") | "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 ))" + } +} diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java index f6a1b2b442..21d470e7f5 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/DriverInstrumentation.java @@ -41,7 +41,7 @@ public final class DriverInstrumentation implements Instrumenter { // Remove end of url to prevent passwords from leaking: final String sanitizedURL = url.replaceAll("[?;].*", ""); final String type = url.split(":")[1]; - final String dbUser = info.getProperty("user"); + final String dbUser = info == null ? null : info.getProperty("user"); connectionInfo.put(connection, new DBInfo(sanitizedURL, type, dbUser)); } } diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java index 5310228627..82e74452e1 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -2,6 +2,7 @@ package com.datadoghq.agent.instrumentation.jdbc; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -30,7 +31,7 @@ public final class PreparedStatementInstrumentation implements Instrumenter { .transform( new AgentBuilder.Transformer.ForAdvice() .advice( - nameStartsWith("execute").and(takesArguments(0)), + nameStartsWith("execute").and(takesArguments(0)).and(isPublic()), PreparedStatementAdvice.class.getName())) .asDecorator(); } @@ -62,11 +63,6 @@ public final class PreparedStatementInstrumentation implements Instrumenter { span.setTag(DDTags.SPAN_TYPE, "sql"); span.setTag("span.origin.type", statement.getClass().getName()); span.setTag("db.jdbc.url", dbInfo.getUrl()); - try { - span.setTag("db.schema", connection.getSchema()); - } catch (final Throwable e) { - // Ignore... - } if (dbInfo.getUser() != null) { Tags.DB_USER.set(span, dbInfo.getUser()); diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java index ced42a6c87..21fdcd4c1f 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/instrumentation/jdbc/StatementInstrumentation.java @@ -2,6 +2,7 @@ package com.datadoghq.agent.instrumentation.jdbc; import static net.bytebuddy.matcher.ElementMatchers.hasSuperType; import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; @@ -30,7 +31,7 @@ public final class StatementInstrumentation implements Instrumenter { .transform( new AgentBuilder.Transformer.ForAdvice() .advice( - nameStartsWith("execute").and(takesArgument(0, String.class)), + nameStartsWith("execute").and(takesArgument(0, String.class)).and(isPublic()), StatementAdvice.class.getName())) .asDecorator(); } @@ -62,11 +63,6 @@ public final class StatementInstrumentation implements Instrumenter { span.setTag(DDTags.SPAN_TYPE, "sql"); span.setTag("span.origin.type", statement.getClass().getName()); span.setTag("db.jdbc.url", dbInfo.getUrl()); - try { - span.setTag("db.schema", connection.getSchema()); - } catch (final Throwable e) { - // Ignore... - } if (dbInfo.getUser() != null) { Tags.DB_USER.set(span, dbInfo.getUser());