diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 235310d31b3..7e28f4ceefc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,6 +52,7 @@ The default JVM to build and run tests from the command line should be Java 8. * `Project Structure` -> `Project` -> `SDK` -> `Download JDK...` -> `Version: 1.8` -> `Download` * Configure Java and Groovy import formatting: * `Settings...` ->`Editor` > `Code Style` > `Java` > `Imports` + * `Use single class import`: checked * `Class count to use import with '*'`: `9999` (some number sufficiently large that is unlikely to matter) * `Names count to use static import with '*'`: `9999` * Use the following import layout to ensure consistency with google-java-format: diff --git a/dd-java-agent/instrumentation/jdbc/build.gradle b/dd-java-agent/instrumentation/jdbc/build.gradle index c39dee6fc03..5289f57dad8 100644 --- a/dd-java-agent/instrumentation/jdbc/build.gradle +++ b/dd-java-agent/instrumentation/jdbc/build.gradle @@ -31,6 +31,7 @@ dependencies { testImplementation group: 'com.h2database', name: 'h2', version: '[1.3.168,1.3.169]'// first jdk 1.6 compatible testImplementation group: 'org.apache.derby', name: 'derby', version: '10.6.1.0' testImplementation group: 'org.hsqldb', name: 'hsqldb', version: '2.0.0' + testImplementation group: 'org.apache.commons', name: 'commons-dbcp2', version: '2.10.0' testImplementation group: 'org.apache.tomcat', name: 'tomcat-jdbc', version: '7.0.19' // tomcat needs this to run diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java new file mode 100644 index 00000000000..0718f0f4f39 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java @@ -0,0 +1,72 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.jdbc.PoolWaitingDecorator.DECORATE; +import static datadog.trace.instrumentation.jdbc.PoolWaitingDecorator.POOL_WAITING; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2LinkedBlockingDequeInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2LinkedBlockingDequeInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.pool2.impl.LinkedBlockingDeque", // standalone + "org.apache.tomcat.dbcp.pool2.impl.LinkedBlockingDeque" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("pollFirst").and(takesArguments(1)), + Dbcp2LinkedBlockingDequeInstrumentation.class.getName() + "$PollFirstAdvice"); + } + + public static class PollFirstAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentSpan onEnter() { + if (CallDepthThreadLocalMap.getCallDepth(PoolWaitingDecorator.class) > 0) { + AgentSpan span = startSpan(POOL_WAITING); + DECORATE.afterStart(span); + span.setResourceName("dbcp2.waiting"); + return span; + } else { + return null; + } + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit( + @Advice.Enter final AgentSpan span, @Advice.Thrown final Throwable throwable) { + if (span != null) { + DECORATE.onError(span, throwable); + span.finish(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java new file mode 100644 index 00000000000..25fa6e2d595 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java @@ -0,0 +1,56 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2ManagedConnectionInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2ManagedConnectionInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.managed.ManagedConnection", // standalone + "org.apache.tomcat.dbcp.dbcp2.managed.ManagedConnection" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("updateTransactionStatus"), + Dbcp2ManagedConnectionInstrumentation.class.getName() + "$UpdateTransactionStatusAdvice"); + } + + public static class UpdateTransactionStatusAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(PoolWaitingDecorator.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(PoolWaitingDecorator.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java new file mode 100644 index 00000000000..44ac190c7d2 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java @@ -0,0 +1,57 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2PerUserPoolDataSourceInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2PerUserPoolDataSourceInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.datasources.PerUserPoolDataSource", // standalone + "org.apache.tomcat.dbcp.dbcp2.datasources.PerUserPoolDataSource" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("getPooledConnectionAndInfo"), + Dbcp2PerUserPoolDataSourceInstrumentation.class.getName() + + "$GetPooledConnectionAndInfoAdvice"); + } + + public static class GetPooledConnectionAndInfoAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(PoolWaitingDecorator.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(PoolWaitingDecorator.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java new file mode 100644 index 00000000000..c19c35fb36f --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java @@ -0,0 +1,56 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2PoolingDataSourceInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2PoolingDataSourceInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.PoolingDataSource", // standalone + "org.apache.tomcat.dbcp.dbcp2.PoolingDataSource" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("getConnection"), + Dbcp2PoolingDataSourceInstrumentation.class.getName() + "$GetConnectionAdvice"); + } + + public static class GetConnectionAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(PoolWaitingDecorator.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(PoolWaitingDecorator.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java new file mode 100644 index 00000000000..08648a5947c --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java @@ -0,0 +1,55 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2PoolingDriverInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2PoolingDriverInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.PoolingDriver", // standalone + "org.apache.tomcat.dbcp.dbcp2.PoolingDriver" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("connect"), Dbcp2PoolingDriverInstrumentation.class.getName() + "$ConnectAdvice"); + } + + public static class ConnectAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(PoolWaitingDecorator.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(PoolWaitingDecorator.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java new file mode 100644 index 00000000000..0ba8e5f5d80 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java @@ -0,0 +1,57 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public final class Dbcp2SharedPoolDataSourceInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice { + + public Dbcp2SharedPoolDataSourceInstrumentation() { + super("jdbc", "dbcp2"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.datasources.SharePoolDataSource", // standalone + "org.apache.tomcat.dbcp.dbcp2.datasources.SharedPoolPoolDataSource" // bundled with Tomcat + }; + } + + @Override + public String[] helperClassNames() { + return new String[] {packageName + ".PoolWaitingDecorator"}; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("getPooledConnectionAndInfo"), + Dbcp2SharedPoolDataSourceInstrumentation.class.getName() + + "$GetPooledConnectionAndInfoAdvice"); + } + + public static class GetPooledConnectionAndInfoAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(PoolWaitingDecorator.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(PoolWaitingDecorator.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java new file mode 100644 index 00000000000..6409104173c --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java @@ -0,0 +1,18 @@ +package datadog.trace.instrumentation.jdbc; + +/** Shared blocked getConnection() tracking {@link ThreadLocal} for Hikari. */ +public class HikariBlockedTracker { + private static final ThreadLocal tracker = ThreadLocal.withInitial(() -> false); + + public static void clearBlocked() { + tracker.set(false); + } + + public static void setBlocked() { + tracker.set(true); + } + + public static boolean wasBlocked() { + return tracker.get(); + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagHandoffQueueInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagHandoffQueueInstrumentation.java new file mode 100644 index 00000000000..dec821d96e9 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagHandoffQueueInstrumentation.java @@ -0,0 +1,83 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.declaresField; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.TimeUnit; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * Detect blocking for newer Hikari versions starting with commit f0b3c520c (>=2.6.0) by looking for + * calls to handoffQueue.poll(timeout, NANOSECONDS). + */ +@AutoService(InstrumenterModule.class) +public final class HikariConcurrentBagHandoffQueueInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, + Instrumenter.HasMethodAdvice, + Instrumenter.WithTypeStructure { + + public HikariConcurrentBagHandoffQueueInstrumentation() { + super("jdbc", "hikari"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.util.ConcurrentBag"; + } + + @Override + public ElementMatcher structureMatcher() { + return declaresField(named("handoffQueue")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".HikariBlockedTracker", + packageName + + ".HikariConcurrentBagHandoffQueueInstrumentation$BlockedTrackingSynchronousQueue", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isConstructor(), + HikariConcurrentBagHandoffQueueInstrumentation.class.getName() + "$ConstructorAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + static void after( + @Advice.FieldValue(value = "handoffQueue", readOnly = false) + SynchronousQueue handoffQueue) { + handoffQueue = new BlockedTrackingSynchronousQueue<>(); + } + } + + public static class BlockedTrackingSynchronousQueue extends SynchronousQueue { + public BlockedTrackingSynchronousQueue() { + // This assumes the initialization of the SynchronousQueue in ConcurrentBag doesn't change + super(true); + } + + @Override + public T poll(long timeout, TimeUnit unit) throws InterruptedException { + HikariBlockedTracker.setBlocked(); + return super.poll(timeout, unit); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java new file mode 100644 index 00000000000..acac1a85dc1 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java @@ -0,0 +1,106 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.bootstrap.instrumentation.api.Tags.DB_POOL_NAME; +import static datadog.trace.instrumentation.jdbc.PoolWaitingDecorator.DECORATE; +import static datadog.trace.instrumentation.jdbc.PoolWaitingDecorator.POOL_WAITING; +import static java.util.Collections.singletonMap; + +import com.google.auto.service.AutoService; +import com.zaxxer.hikari.util.ConcurrentBag; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import net.bytebuddy.asm.Advice; + +/** + * Instrument Hikari's ConcurrentBag class to detect when blocking occurs trying to get an entry + * from the connection pool. There are two related instrumentations to detect blocking for different + * versions of Hikari. The {@link HikariBlockedTracker} contextStore is used to pass blocking state + * from the other instrumentations to this class. + * + *
    + *
  • Before commit f0b3c520c (2.4.0 <= version < 2.6.0): calls to + * synchronizer.waitUntilSequenceExceeded(startSeq, timeout) with {@link + * HikariQueuedSequenceSynchronizerInstrumentation} + *
  • Commit f0b3c520c and later (version >= 2.6.0): calls to + * handoffQueue.poll(timeout, NANOSECONDS) with {@link + * HikariQueuedSequenceSynchronizerInstrumentation} + *
+ */ +@AutoService(InstrumenterModule.class) +public final class HikariConcurrentBagInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public HikariConcurrentBagInstrumentation() { + super("jdbc", "hikari"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.util.ConcurrentBag"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".HikariBlockedTracker", packageName + ".PoolWaitingDecorator" + }; + } + + @Override + public Map contextStore() { + // The contextStore Map is populated by HikariPoolInstrumentation + return singletonMap("com.zaxxer.hikari.util.ConcurrentBag", String.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("borrow"), HikariConcurrentBagInstrumentation.class.getName() + "$BorrowAdvice"); + } + + /** + * Instead of always starting and ending a span, a pool.waiting span is only created if blocking + * is detected when attempting to get a connection from the pool. + */ + public static class BorrowAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Long onEnter() { + HikariBlockedTracker.clearBlocked(); + return System.currentTimeMillis(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.This ConcurrentBag thiz, + @Advice.Enter final Long startTimeMillis, + @Advice.Thrown final Throwable throwable) { + if (HikariBlockedTracker.wasBlocked()) { + final AgentSpan span = + startSpan(POOL_WAITING, TimeUnit.MILLISECONDS.toMicros(startTimeMillis)); + DECORATE.afterStart(span); + DECORATE.onError(span, throwable); + span.setResourceName("hikari.waiting"); + final String poolName = + InstrumentationContext.get(ConcurrentBag.class, String.class).get(thiz); + if (poolName != null) { + span.setTag(DB_POOL_NAME, poolName); + } + + span.finish(); + } + HikariBlockedTracker.clearBlocked(); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariPoolInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariPoolInstrumentation.java new file mode 100644 index 00000000000..9de43ec8429 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariPoolInstrumentation.java @@ -0,0 +1,56 @@ +package datadog.trace.instrumentation.jdbc; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import com.zaxxer.hikari.util.ConcurrentBag; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.InstrumenterConfig; +import datadog.trace.bootstrap.InstrumentationContext; +import java.util.Map; +import net.bytebuddy.asm.Advice; + +/** + * Store the poolName associated with a {@link ConcurrentBag} for later use in {@link + * HikariConcurrentBagInstrumentation}. + */ +@AutoService(InstrumenterModule.class) +public final class HikariPoolInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public HikariPoolInstrumentation() { + super("jdbc", "hikari"); + } + + @Override + protected boolean defaultEnabled() { + return InstrumenterConfig.get().isJdbcPoolWaitingEnabled(); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.pool.HikariPool"; + } + + @Override + public Map contextStore() { + return singletonMap("com.zaxxer.hikari.util.ConcurrentBag", String.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isConstructor(), HikariPoolInstrumentation.class.getName() + "$ConstructorAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + static void after( + @Advice.FieldValue("connectionBag") ConcurrentBag concurrentBag, + @Advice.FieldValue("poolName") String poolName) { + InstrumentationContext.get(ConcurrentBag.class, String.class).put(concurrentBag, poolName); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java new file mode 100644 index 00000000000..7fd3f1c3ff0 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java @@ -0,0 +1,49 @@ +package datadog.trace.instrumentation.jdbc; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import net.bytebuddy.asm.Advice; + +/** + * Detect blocking for older Hikari versions before commit f0b3c520c (<2.6.0) by looking for calls + * to synchronizer.waitUntilSequenceExceeded(startSeq, timeout). + */ +@AutoService(InstrumenterModule.class) +public final class HikariQueuedSequenceSynchronizerInstrumentation + extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public HikariQueuedSequenceSynchronizerInstrumentation() { + super("jdbc", "hikari"); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.util.QueuedSequenceSynchronizer"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".HikariBlockedTracker", + }; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("waitUntilSequenceExceeded"), + HikariQueuedSequenceSynchronizerInstrumentation.class.getName() + + "$WaitUntilSequenceExceededAdvice"); + } + + public static class WaitUntilSequenceExceededAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + HikariBlockedTracker.setBlocked(); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PoolWaitingDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PoolWaitingDecorator.java new file mode 100644 index 00000000000..8a3569209a0 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/PoolWaitingDecorator.java @@ -0,0 +1,27 @@ +package datadog.trace.instrumentation.jdbc; + +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.bootstrap.instrumentation.decorator.BaseDecorator; + +public class PoolWaitingDecorator extends BaseDecorator { + public static final CharSequence POOL_WAITING = UTF8BytesString.create("pool.waiting"); + public static final CharSequence JAVA_JDBC_POOL_WAITING = + UTF8BytesString.create("java-jdbc-pool-waiting"); + + public static final PoolWaitingDecorator DECORATE = new PoolWaitingDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"jdbc"}; + } + + @Override + protected CharSequence component() { + return JAVA_JDBC_POOL_WAITING; + } + + @Override + protected CharSequence spanType() { + return null; + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy index f8d78abf68c..882ff27ca42 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy @@ -6,6 +6,11 @@ import datadog.trace.api.Config import datadog.trace.api.DDSpanTypes import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags import datadog.trace.bootstrap.instrumentation.api.Tags +import org.apache.commons.dbcp2.BasicDataSource +import org.apache.commons.pool2.PooledObject +import org.apache.commons.pool2.PooledObjectFactory +import org.apache.commons.pool2.impl.DefaultPooledObject +import org.apache.commons.pool2.impl.GenericObjectPool import org.apache.derby.jdbc.EmbeddedDataSource import org.h2.jdbcx.JdbcDataSource import spock.lang.Shared @@ -18,11 +23,16 @@ import java.sql.Connection import java.sql.Driver import java.sql.PreparedStatement import java.sql.ResultSet +import java.sql.SQLException +import java.sql.SQLTimeoutException +import java.sql.SQLTransientConnectionException import java.sql.Statement +import java.time.Duration import static datadog.trace.agent.test.utils.TraceUtils.basicSpan import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE +import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { @@ -97,6 +107,22 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { return ds } + def createDbcp2DS(String dbType, String jdbcUrl) { + BasicDataSource ds = new BasicDataSource() + def jdbcUrlToSet = dbType == "derby" ? jdbcUrl + ";create=true" : jdbcUrl + ds.setUrl(jdbcUrlToSet) + ds.setDriverClassName(jdbcDriverClassNames.get(dbType)) + String username = jdbcUserNames.get(dbType) + if (username != null) { + ds.setUsername(username) + } + ds.setPassword(jdbcPasswords.get(dbType)) + ds.setMaxTotal(1) // to test proper caching, having > 1 max active connection will be hard to + // determine whether the connection is properly cached + ds.setMaxWait(Duration.ofMillis(1000)) + return ds + } + def createHikariDS(String dbType, String jdbcUrl) { HikariConfig config = new HikariConfig() def jdbcUrlToSet = dbType == "derby" ? jdbcUrl + ";create=true" : jdbcUrl @@ -110,6 +136,7 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { config.addDataSourceProperty("prepStmtCacheSize", "250") config.addDataSourceProperty("prepStmtCacheSqlLimit", "2048") config.setMaximumPoolSize(1) + config.setConnectionTimeout(1000) return new HikariDataSource(config) } @@ -133,6 +160,9 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { if (connectionPoolName == "tomcat") { ds = createTomcatDS(dbType, jdbcUrl) } + if (connectionPoolName == "dbcp2") { + ds = createDbcp2DS(dbType, jdbcUrl) + } if (connectionPoolName == "hikari") { ds = createHikariDS(dbType, jdbcUrl) } @@ -148,6 +178,7 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { injectSysConfig("dd.trace.jdbc.prepared.statement.class.name", "test.TestPreparedStatement") injectSysConfig("dd.integration.jdbc-datasource.enabled", "true") + injectSysConfig(JDBC_POOL_WAITING_ENABLED, "true") } def setupSpec() { @@ -814,6 +845,151 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase { "c3p0" | _ } + def "#connectionPoolName should have pool.waiting span when pool exhausted for #exhaustPoolForMillis with exception thrown #expectException"() { + setup: + String dbType = "hsqldb" + DataSource ds = createDS(connectionPoolName, dbType, jdbcUrls.get(dbType)) + + if (exhaustPoolForMillis != null) { + def saturatedConnection = ds.getConnection() + new Thread(() -> { + Thread.sleep(exhaustPoolForMillis) + saturatedConnection.close() + }, "saturated connection closer").start() + } + + when: + Throwable timedOutException = null + runUnderTrace("parent") { + try { + ds.getConnection().close() + } catch (SQLTransientConnectionException e) { + if (e.getMessage().contains("request timed out after")) { + // Hikari, newer + timedOutException = e + } else { + throw e + } + } catch (SQLTimeoutException e) { + // Hikari, older + timedOutException = e + } catch (SQLException e) { + if (e.getMessage().contains("pool error Timeout waiting for idle object")) { + // dbcp2 + timedOutException = e + } else { + throw e + } + } + } + + then: + assertTraces(1) { + trace(connectionPoolName == "dbcp2" ? 4 : 3) { + basicSpan(it, "parent") + + span { + operationName "database.connection" + resourceName "${ds.class.simpleName}.getConnection" + childOf span(0) + errored timedOutException != null + tags { + "$Tags.COMPONENT" "java-jdbc-connection" + defaultTagsNoPeerService() + if (timedOutException) { + errorTags(timedOutException) + } + } + } + + // dbcp2 will have two database.connection spans + if (connectionPoolName == "dbcp2") { + span { + operationName "database.connection" + resourceName "PoolingDataSource.getConnection" + childOf span(1) + errored timedOutException != null + tags { + "$Tags.COMPONENT" "java-jdbc-connection" + defaultTagsNoPeerService() + if (timedOutException) { + errorTags(timedOutException) + } + } + } + } + + span { + operationName "pool.waiting" + resourceName "${connectionPoolName}.waiting" + childOf span(connectionPoolName == "dbcp2" ? 2 : 1) + tags { + "$Tags.COMPONENT" "java-jdbc-pool-waiting" + if (connectionPoolName == "hikari") { + "$Tags.DB_POOL_NAME" String + } + defaultTagsNoPeerService() + } + } + } + } + assert expectException == (timedOutException != null) + + cleanup: + if (ds instanceof Closeable) { + ds.close() + } + + where: + connectionPoolName | exhaustPoolForMillis | expectException + "hikari" | 500 | false + "dbcp2" | 500 | false + "hikari" | 1500 | true + "dbcp2" | 1500 | true + } + + def "Ensure LinkedBlockingDeque.pollFirst called outside of DBCP2 does not create spans"() { + setup: + def pool = new GenericObjectPool<>(new PooledObjectFactory() { + + @Override + void activateObject(PooledObject p) throws Exception { + } + + @Override + void destroyObject(PooledObject p) throws Exception { + } + + @Override + PooledObject makeObject() throws Exception { + return new DefaultPooledObject(new Object()) + } + + @Override + void passivateObject(PooledObject p) throws Exception { + } + + @Override + boolean validateObject(PooledObject p) { + return false + } + }) + pool.setMaxTotal(1) + + when: + def exhaustPoolForMillis = 500 + def saturatedConnection = pool.borrowObject() + new Thread(() -> { + Thread.sleep(exhaustPoolForMillis) + pool.returnObject(saturatedConnection) + }, "saturated connection closer").start() + + pool.borrowObject(1000) + + then: + TEST_WRITER.size() == 0 + } + Driver driverFor(String db) { return newDriver(jdbcDriverClassNames.get(db)) } diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy index 9e75ef23d26..c710a6c89bb 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestConnection.groovy @@ -227,7 +227,7 @@ class TestConnection implements Connection { @Override boolean isValid(int timeout) throws SQLException { - return false + return true } @Override diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDataSource.java b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDataSource.java new file mode 100644 index 00000000000..b5508b5dc7a --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDataSource.java @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2013 Brett Wooldridge + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package test; + +import java.io.PrintWriter; +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.util.logging.Logger; +import javax.sql.DataSource; + +/** + * Test DataSource. Derived from Hikari's StubDataSource. + * + * @author Brett Wooldridge + */ +public class TestDataSource implements DataSource { + private String user; + private String password; + private PrintWriter logWriter; + private SQLException throwException; + private int loginTimeout; + + public String getUser() { + return user; + } + + public void setUser(String user) { + this.user = user; + } + + public String getPassword() { + return password; + } + + public void setURL(String url) { + // we don't care + } + + /** {@inheritDoc} */ + @Override + public PrintWriter getLogWriter() throws SQLException { + return logWriter; + } + + /** {@inheritDoc} */ + @Override + public void setLogWriter(PrintWriter out) throws SQLException { + this.logWriter = out; + } + + /** {@inheritDoc} */ + @Override + public void setLoginTimeout(int seconds) throws SQLException { + this.loginTimeout = seconds; + } + + /** {@inheritDoc} */ + @Override + public int getLoginTimeout() throws SQLException { + return loginTimeout; + } + + /** {@inheritDoc} */ + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return null; + } + + /** {@inheritDoc} */ + @SuppressWarnings("unchecked") + @Override + public T unwrap(Class iface) throws SQLException { + if (iface.isInstance(this)) { + return (T) this; + } + + throw new SQLException("Wrapped DataSource is not an instance of " + iface); + } + + /** {@inheritDoc} */ + @Override + public boolean isWrapperFor(Class iface) throws SQLException { + return false; + } + + /** {@inheritDoc} */ + @Override + public Connection getConnection() throws SQLException { + return new TestConnection(false); + } + + /** {@inheritDoc} */ + @Override + public Connection getConnection(String username, String password) throws SQLException { + return new TestConnection(false); + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy index 7983583ad23..8fe6090d213 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDriver.groovy @@ -15,7 +15,7 @@ class TestDriver implements Driver { @Override boolean acceptsURL(String url) throws SQLException { - return false + return true } @Override diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index 195f7a43383..e96900c073d 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -72,6 +72,9 @@ public final class TraceInstrumentationConfig { public static final String JDBC_CONNECTION_CLASS_NAME = "trace.jdbc.connection.class.name"; + public static final String JDBC_POOL_WAITING_ENABLED = + "trace.experimental.jdbc.pool.waiting.enabled"; + public static final String AKKA_FORK_JOIN_TASK_NAME = "trace.akka.fork.join.task.name"; public static final String AKKA_FORK_JOIN_EXECUTOR_TASK_NAME = "trace.akka.fork.join.executor.task.name"; diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 38ec6957f69..f7e3fabff2a 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -45,6 +45,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.INTEGRATIONS_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_ADDITIONAL_ANNOTATIONS; import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_CONNECTION_CLASS_NAME; +import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_PREPARED_STATEMENT_CLASS_NAME; import static datadog.trace.api.config.TraceInstrumentationConfig.MEASURE_METHODS; import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_CACHE_CONFIG; @@ -130,6 +131,7 @@ public class InstrumenterConfig { private final String jdbcPreparedStatementClassName; private final String jdbcConnectionClassName; + private final boolean jdbcPoolWaitingEnabled; private final String httpURLConnectionClassName; private final String axisTransportClassName; @@ -242,6 +244,7 @@ private InstrumenterConfig() { jdbcPreparedStatementClassName = configProvider.getString(JDBC_PREPARED_STATEMENT_CLASS_NAME, ""); jdbcConnectionClassName = configProvider.getString(JDBC_CONNECTION_CLASS_NAME, ""); + jdbcPoolWaitingEnabled = configProvider.getBoolean(JDBC_POOL_WAITING_ENABLED, false); httpURLConnectionClassName = configProvider.getString(HTTP_URL_CONNECTION_CLASS_NAME, ""); axisTransportClassName = configProvider.getString(AXIS_TRANSPORT_CLASS_NAME, ""); @@ -418,6 +421,10 @@ public String getJdbcConnectionClassName() { return jdbcConnectionClassName; } + public boolean isJdbcPoolWaitingEnabled() { + return jdbcPoolWaitingEnabled; + } + public String getHttpURLConnectionClassName() { return httpURLConnectionClassName; } @@ -627,6 +634,8 @@ public String toString() { + ", jdbcConnectionClassName='" + jdbcConnectionClassName + '\'' + + ", jdbcPoolWaitingEnabled=" + + jdbcPoolWaitingEnabled + ", httpURLConnectionClassName='" + httpURLConnectionClassName + '\''