diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/DatabaseClientDecorator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/DatabaseClientDecorator.java index 288e794f25..af03a7dc76 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/DatabaseClientDecorator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/DatabaseClientDecorator.java @@ -3,13 +3,13 @@ package datadog.trace.agent.decorator; import io.opentracing.Span; import io.opentracing.tag.Tags; -public abstract class DatabaseClientDecorator extends ClientDecorator { +public abstract class DatabaseClientDecorator extends ClientDecorator { protected abstract String dbType(); - protected abstract String dbUser(SESSION session); + protected abstract String dbUser(CONNECTION connection); - protected abstract String dbInstance(SESSION session); + protected abstract String dbInstance(CONNECTION connection); @Override public Span afterStart(final Span span) { @@ -18,11 +18,11 @@ public abstract class DatabaseClientDecorator extends ClientDecorator { return super.afterStart(span); } - public Span onSession(final Span span, final SESSION statement) { + public Span onConnection(final Span span, final CONNECTION connection) { assert span != null; - if (statement != null) { - Tags.DB_USER.set(span, dbUser(statement)); - Tags.DB_INSTANCE.set(span, dbInstance(statement)); + if (connection != null) { + Tags.DB_USER.set(span, dbUser(connection)); + Tags.DB_INSTANCE.set(span, dbInstance(connection)); } return span; } diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy index a85ff721cc..07df9e30ee 100644 --- a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy @@ -35,7 +35,7 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { def decorator = newDecorator() when: - decorator.onSession(span, session) + decorator.onConnection(span, session) then: if (session) { @@ -81,7 +81,7 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest { thrown(AssertionError) when: - decorator.onSession(null, null) + decorator.onConnection(null, null) then: thrown(AssertionError) diff --git a/dd-java-agent/instrumentation/datastax-cassandra-2.3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java b/dd-java-agent/instrumentation/datastax-cassandra-2.3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java index 889867281d..d9aa9a3cab 100644 --- a/dd-java-agent/instrumentation/datastax-cassandra-2.3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java +++ b/dd-java-agent/instrumentation/datastax-cassandra-2.3/src/main/java/datadog/trace/instrumentation/datastax/cassandra/TracingSession.java @@ -212,7 +212,7 @@ public class TracingSession implements Session { private Span buildSpan(final String query) { final Span span = tracer.buildSpan("cassandra.execute").start(); DECORATE.afterStart(span); - DECORATE.onSession(span, session); + DECORATE.onConnection(span, session); DECORATE.onStatement(span, query); return span; } diff --git a/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/SlickTest.groovy b/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/SlickTest.groovy index 221ad5576a..0c8f5789b2 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/SlickTest.groovy +++ b/dd-java-agent/instrumentation/java-concurrent/src/slickTest/groovy/SlickTest.groovy @@ -43,7 +43,7 @@ class SlickTest extends AgentTestRunner { "$Tags.DB_TYPE.key" SlickUtils.Driver() "$Tags.DB_USER.key" SlickUtils.Username() - "db.jdbc.url" SlickUtils.Url() + "db.instance" SlickUtils.Url() "span.origin.type" "org.h2.jdbc.JdbcPreparedStatement" defaultTags() 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 f7de0c471c..9c383c2052 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 @@ -11,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.PreparedStatement; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -31,6 +30,13 @@ public final class ConnectionInstrumentation extends Instrumenter.Default { return not(isInterface()).and(safeHasSuperType(named("java.sql.Connection"))); } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JDBCMaps", + }; + } + @Override public Map, String> transformers() { return singletonMap( diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java new file mode 100644 index 0000000000..7de8cca72f --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -0,0 +1,105 @@ +package datadog.trace.instrumentation.jdbc; + +import datadog.trace.agent.decorator.DatabaseClientDecorator; +import datadog.trace.api.DDSpanTypes; +import datadog.trace.api.DDTags; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; +import java.sql.SQLException; + +public class JDBCDecorator extends DatabaseClientDecorator { + public static final JDBCDecorator DECORATE = new JDBCDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"jdbc"}; + } + + @Override + protected String service() { + return "jdbc"; // Overridden by onConnection + } + + @Override + protected String component() { + return "java-jdbc"; // Overridden by onStatement and onPreparedStatement + } + + @Override + protected String spanType() { + return DDSpanTypes.SQL; + } + + @Override + protected String dbType() { + return "jdbc"; + } + + @Override + protected String dbUser(final JDBCMaps.DBInfo info) { + return info.getUser(); + } + + @Override + protected String dbInstance(final JDBCMaps.DBInfo info) { + return info.getUrl(); + } + + public Span onConnection(final Span span, final Connection connection) { + JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection); + /** + * Logic 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 DEFAULT DBInfo + * as the value in the connectionInfo map to avoid retry overhead. + */ + { + if (dbInfo == null) { + try { + final DatabaseMetaData metaData = connection.getMetaData(); + final String url = metaData.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 = metaData.getUserName(); + if (user != null && user.trim().equals("")) { + user = null; + } + dbInfo = new JDBCMaps.DBInfo(sanitizedURL, type, user); + } else { + dbInfo = JDBCMaps.DBInfo.DEFAULT; + } + } catch (final SQLException se) { + dbInfo = JDBCMaps.DBInfo.DEFAULT; + } + JDBCMaps.connectionInfo.put(connection, dbInfo); + } + } + + if (dbInfo != null) { + Tags.DB_TYPE.set(span, dbInfo.getType()); + span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); + } + return super.onConnection(span, dbInfo); + } + + @Override + public Span onStatement(final Span span, final String statement) { + final String resourceName = statement == null ? JDBCMaps.DB_QUERY : statement; + span.setTag(DDTags.RESOURCE_NAME, resourceName); + Tags.COMPONENT.set(span, "java-jdbc-statement"); + return super.onStatement(span, statement); + } + + public Span onPreparedStatement(final Span span, final PreparedStatement statement) { + final String sql = JDBCMaps.preparedStatements.get(statement); + final String resourceName = sql == null ? JDBCMaps.DB_QUERY : sql; + span.setTag(DDTags.RESOURCE_NAME, resourceName); + Tags.COMPONENT.set(span, "java-jdbc-prepared_statement"); + return super.onStatement(span, sql); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCMaps.java similarity index 83% rename from dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java rename to dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCMaps.java index 99b8193278..13bbe8865e 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/JDBCMaps.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCMaps.java @@ -1,7 +1,8 @@ -package datadog.trace.bootstrap; +package datadog.trace.instrumentation.jdbc; import static datadog.trace.bootstrap.WeakMap.Provider.newWeakMap; +import datadog.trace.bootstrap.WeakMap; import java.sql.Connection; import java.sql.PreparedStatement; import lombok.Data; @@ -9,7 +10,7 @@ import lombok.Data; /** * JDBC instrumentation shares a global map of connection info. * - *

In the bootstrap project to ensure visibility by all classes. + *

Should be injected into the bootstrap classpath. */ public class JDBCMaps { public static final WeakMap connectionInfo = newWeakMap(); diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java new file mode 100644 index 0000000000..0fb70291f6 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCUtils.java @@ -0,0 +1,36 @@ +package datadog.trace.instrumentation.jdbc; + +import datadog.trace.bootstrap.ExceptionLogger; +import java.sql.Connection; +import java.sql.Statement; + +public abstract class JDBCUtils { + + /** + * @param statement + * @return the unwrapped connection or null if exception was thrown. + */ + public static Connection connectionFromStatement(final Statement statement) { + Connection connection; + try { + connection = statement.getConnection(); + try { + // unwrap the connection to cache the underlying actual connection and to not cache proxy + // objects + if (connection.isWrapperFor(Connection.class)) { + connection = connection.unwrap(Connection.class); + } + } catch (final Exception | AbstractMethodError e) { + // perhaps wrapping isn't supported? + // ex: org.h2.jdbc.JdbcConnection v1.3.175 + // or: jdts.jdbc which always throws `AbstractMethodError` (at least up to version 1.3) + // Stick with original connection. + } + } catch (final Throwable e) { + // Had some problem getting the connection. + ExceptionLogger.LOGGER.debug("Could not get connection for StatementAdvice", e); + return null; + } + return connection; + } +} 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 3137980c62..56e73233d1 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,7 +1,8 @@ package datadog.trace.instrumentation.jdbc; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.jdbc.JDBCDecorator.DECORATE; +import static datadog.trace.instrumentation.jdbc.JDBCUtils.connectionFromStatement; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -12,21 +13,13 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; -import datadog.trace.bootstrap.ExceptionLogger; -import datadog.trace.bootstrap.JDBCMaps; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.noop.NoopScopeManager.NoopScope; -import io.opentracing.tag.Tags; +import io.opentracing.noop.NoopScopeManager; import io.opentracing.util.GlobalTracer; import java.sql.Connection; -import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; -import java.sql.SQLException; -import java.util.Collections; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -35,6 +28,7 @@ import net.bytebuddy.matcher.ElementMatcher; @AutoService(Instrumenter.class) public final class PreparedStatementInstrumentation extends Instrumenter.Default { + public PreparedStatementInstrumentation() { super("jdbc"); } @@ -44,6 +38,19 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Default return not(isInterface()).and(safeHasSuperType(named("java.sql.PreparedStatement"))); } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".JDBCDecorator", + packageName + ".JDBCMaps", + packageName + ".JDBCMaps$DBInfo", + packageName + ".JDBCUtils", + }; + } + @Override public Map, String> transformers() { return singletonMap( @@ -59,80 +66,18 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Default if (callDepth > 0) { return null; } - final String sql = JDBCMaps.preparedStatements.get(statement); - Connection connection; - try { - connection = statement.getConnection(); - try { - // unwrap the connection to cache the underlying actual connection and to not cache proxy - // objects - if (connection.isWrapperFor(Connection.class)) { - connection = connection.unwrap(Connection.class); - } - } catch (final Exception | AbstractMethodError e) { - // perhaps wrapping isn't supported? - // ex: org.h2.jdbc.JdbcConnection v1.3.175 - // or: jdts.jdbc which always throws `AbstractMethodError` (at least up to version 1.3) - // Stick with original connection. - } - } catch (final Throwable e) { - // Had some problem getting the connection. - ExceptionLogger.LOGGER.debug("Could not get connection for PreparedStatementAdvice", e); - return NoopScope.INSTANCE; + + final Connection connection = connectionFromStatement(statement); + if (connection == null) { + return NoopScopeManager.NoopScope.INSTANCE; } - JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection); - /** - * Logic 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 - * DEFAULT DBInfo as the value in the connectionInfo map to avoid retry overhead. - * - *

This should be a util method to be shared between PreparedStatementInstrumentation and - * StatementInstrumentation class, but java.sql.* are on the platform classloaders in Java 9, - * which prevents us from referencing them in the bootstrap utils. - */ - { - if (dbInfo == null) { - try { - final DatabaseMetaData metaData = connection.getMetaData(); - final String url = metaData.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 = metaData.getUserName(); - if (user != null && user.trim().equals("")) { - user = null; - } - dbInfo = new JDBCMaps.DBInfo(sanitizedURL, type, user); - } else { - dbInfo = JDBCMaps.DBInfo.DEFAULT; - } - } catch (final SQLException se) { - dbInfo = JDBCMaps.DBInfo.DEFAULT; - } - JDBCMaps.connectionInfo.put(connection, dbInfo); - } - } - - final Scope scope = - GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(true); - + final Scope scope = GlobalTracer.get().buildSpan("database.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-prepared_statement"); - - span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); - span.setTag(DDTags.RESOURCE_NAME, sql == null ? JDBCMaps.DB_QUERY : sql); - span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.SQL); + DECORATE.afterStart(span); + DECORATE.onConnection(span, connection); + DECORATE.onPreparedStatement(span, statement); span.setTag("span.origin.type", statement.getClass().getName()); - span.setTag("db.jdbc.url", dbInfo.getUrl()); - - if (dbInfo.getUser() != null) { - Tags.DB_USER.set(span, dbInfo.getUser()); - } return scope; } @@ -140,11 +85,8 @@ public final class PreparedStatementInstrumentation extends Instrumenter.Default public static void stopSpan( @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { if (scope != null) { - if (throwable != null) { - final Span span = scope.span(); - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } + DECORATE.onError(scope.span(), throwable); + DECORATE.beforeFinish(scope.span()); scope.close(); CallDepthThreadLocalMap.reset(PreparedStatement.class); } 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 d51f274793..a39e574f8d 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,7 +1,8 @@ package datadog.trace.instrumentation.jdbc; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static io.opentracing.log.Fields.ERROR_OBJECT; +import static datadog.trace.instrumentation.jdbc.JDBCDecorator.DECORATE; +import static datadog.trace.instrumentation.jdbc.JDBCUtils.connectionFromStatement; import static java.util.Collections.singletonMap; import static net.bytebuddy.matcher.ElementMatchers.isInterface; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -12,21 +13,13 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import com.google.auto.service.AutoService; import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; import datadog.trace.bootstrap.CallDepthThreadLocalMap; -import datadog.trace.bootstrap.ExceptionLogger; -import datadog.trace.bootstrap.JDBCMaps; import io.opentracing.Scope; import io.opentracing.Span; -import io.opentracing.noop.NoopScopeManager.NoopScope; -import io.opentracing.tag.Tags; +import io.opentracing.noop.NoopScopeManager; import io.opentracing.util.GlobalTracer; import java.sql.Connection; -import java.sql.DatabaseMetaData; -import java.sql.SQLException; import java.sql.Statement; -import java.util.Collections; import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.method.MethodDescription; @@ -45,6 +38,19 @@ public final class StatementInstrumentation extends Instrumenter.Default { return not(isInterface()).and(safeHasSuperType(named("java.sql.Statement"))); } + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.DatabaseClientDecorator", + packageName + ".JDBCDecorator", + packageName + ".JDBCMaps", + packageName + ".JDBCMaps$DBInfo", + packageName + ".JDBCUtils", + }; + } + @Override public Map, String> transformers() { return singletonMap( @@ -61,79 +67,18 @@ public final class StatementInstrumentation extends Instrumenter.Default { if (callDepth > 0) { return null; } - Connection connection; - try { - connection = statement.getConnection(); - try { - // unwrap the connection to cache the underlying actual connection and to not cache proxy - // objects - if (connection.isWrapperFor(Connection.class)) { - connection = connection.unwrap(Connection.class); - } - } catch (final Exception | AbstractMethodError e) { - // perhaps wrapping isn't supported? - // ex: org.h2.jdbc.JdbcConnection v1.3.175 - // or: jdts.jdbc which always throws `AbstractMethodError` (at least up to version 1.3) - // Stick with original connection. - } - } catch (final Throwable e) { - // Had some problem getting the connection. - ExceptionLogger.LOGGER.debug("Could not get connection for StatementAdvice", e); - return NoopScope.INSTANCE; + + final Connection connection = connectionFromStatement(statement); + if (connection == null) { + return NoopScopeManager.NoopScope.INSTANCE; } - JDBCMaps.DBInfo dbInfo = JDBCMaps.connectionInfo.get(connection); - /** - * Logic 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 - * DEFAULT DBInfo as the value in the connectionInfo map to avoid retry overhead. - * - *

This should be a util method to be shared between PreparedStatementInstrumentation and - * StatementInstrumentation class, but java.sql.* are on the platform classloaders in Java 9, - * which prevents us from referencing them in the bootstrap utils. - */ - { - if (dbInfo == null) { - try { - final DatabaseMetaData metaData = connection.getMetaData(); - final String url = metaData.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 = metaData.getUserName(); - if (user != null && user.trim().equals("")) { - user = null; - } - dbInfo = new JDBCMaps.DBInfo(sanitizedURL, type, user); - } else { - dbInfo = JDBCMaps.DBInfo.DEFAULT; - } - } catch (final SQLException se) { - dbInfo = JDBCMaps.DBInfo.DEFAULT; - } - JDBCMaps.connectionInfo.put(connection, dbInfo); - } - } - - final Scope scope = - GlobalTracer.get().buildSpan(dbInfo.getType() + ".query").startActive(true); - + final Scope scope = GlobalTracer.get().buildSpan("database.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"); - - span.setTag(DDTags.SERVICE_NAME, dbInfo.getType()); - span.setTag(DDTags.RESOURCE_NAME, sql); - span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.SQL); + DECORATE.afterStart(span); + DECORATE.onConnection(span, connection); + DECORATE.onStatement(span, sql); span.setTag("span.origin.type", statement.getClass().getName()); - span.setTag("db.jdbc.url", dbInfo.getUrl()); - - if (dbInfo.getUser() != null) { - Tags.DB_USER.set(span, dbInfo.getUser()); - } return scope; } @@ -141,11 +86,8 @@ public final class StatementInstrumentation extends Instrumenter.Default { public static void stopSpan( @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) { if (scope != null) { - if (throwable != null) { - final Span span = scope.span(); - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, throwable)); - } + DECORATE.onError(scope.span(), throwable); + DECORATE.beforeFinish(scope.span()); scope.close(); CallDepthThreadLocalMap.reset(Statement.class); } diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy index 8c934909fd..f102a4d6be 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTest.groovy @@ -171,7 +171,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -232,7 +232,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-prepared_statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -288,7 +288,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-prepared_statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -344,7 +344,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-prepared_statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -400,7 +400,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -459,7 +459,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL "component" "java-jdbc-prepared_statement" - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -535,7 +535,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { } "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL - "db.jdbc.url" jdbcUrls.get(driver) + "db.instance" jdbcUrls.get(driver) "span.origin.type" String defaultTags() } @@ -591,27 +591,45 @@ class JDBCInstrumentationTest extends AgentTestRunner { for (int i = 0; i < numQueries; ++i) { res[i] == 3 } - assertTraces(6) { - trace(0, 1) { + assertTraces(5) { + trace(0, 2) { span(0) { + operationName "${dbType}.query" + serviceName dbType + resourceName query + spanType DDSpanTypes.SQL + errored false + tags { + "db.type" dbType + "db.user" "SA" + "component" "java-jdbc-prepared_statement" + "span.kind" Tags.SPAN_KIND_CLIENT + "span.type" DDSpanTypes.SQL + "db.instance" jdbcUrls.get(dbType) + "span.origin.type" String + defaultTags() + } + } + span(1) { operationName "${dbType}.query" serviceName dbType resourceName "CALL USER()" spanType DDSpanTypes.SQL errored false + childOf(span(0)) tags { "db.type" "hsqldb" "db.user" "SA" "component" "java-jdbc-statement" "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL - "db.jdbc.url" jdbcUrls.get(dbType) + "db.instance" jdbcUrls.get(dbType) "span.origin.type" String defaultTags() } } } - for (int i = 1; i < numQueries + 1; ++i) { + for (int i = 1; i < numQueries; ++i) { trace(i, 1) { span(0) { operationName "${dbType}.query" @@ -625,7 +643,7 @@ class JDBCInstrumentationTest extends AgentTestRunner { "component" "java-jdbc-prepared_statement" "span.kind" Tags.SPAN_KIND_CLIENT "span.type" DDSpanTypes.SQL - "db.jdbc.url" jdbcUrls.get(dbType) + "db.instance" jdbcUrls.get(dbType) "span.origin.type" String defaultTags() }