From daa7de1b0815284a2a7c05befb1c91e2df277672 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sun, 20 Jul 2025 19:39:49 -0700 Subject: [PATCH 1/3] HikariCP: Add a span when waiting on the pool --- .../jdbc/HikariBlockedTracker.java | 18 +++ ...HikariBlockedTrackingSynchronousQueue.java | 18 +++ .../HikariConcurrentBagInstrumentation.java | 121 ++++++++++++++++++ ...edSequenceSynchronizerInstrumentation.java | 39 ++++++ .../groovy/SaturatedPoolBlockingTest.groovy | 71 ++++++++++ .../src/test/groovy/test/TestDataSource.java | 111 ++++++++++++++++ 6 files changed, 378 insertions(+) create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/test/TestDataSource.java 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..3a60a72e6ec --- /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 ThreadLocking 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/HikariBlockedTrackingSynchronousQueue.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java new file mode 100644 index 00000000000..29bff65d0c7 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java @@ -0,0 +1,18 @@ +package datadog.trace.instrumentation.jdbc; + +import java.util.concurrent.SynchronousQueue; +import java.util.concurrent.TimeUnit; + +/** Blocked getConnection() tracking for Hikari starting with commit f0b3c520c. */ +public class HikariBlockedTrackingSynchronousQueue extends SynchronousQueue { + public HikariBlockedTrackingSynchronousQueue() { + // 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..0894792ad9a --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java @@ -0,0 +1,121 @@ +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 java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import com.zaxxer.hikari.pool.HikariPool; +import com.zaxxer.hikari.util.ConcurrentBag; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.lang.reflect.Field; +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. + */ +@AutoService(InstrumenterModule.class) +public final class HikariConcurrentBagInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public HikariConcurrentBagInstrumentation() { + super("jdbc-datasource"); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.util.ConcurrentBag"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".HikariBlockedTrackingSynchronousQueue", packageName + ".HikariBlockedTracker" + }; + } + + @Override + public Map contextStore() { + // For getting the poolName + return singletonMap("com.zaxxer.hikari.util.ConcurrentBag", String.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isConstructor(), HikariConcurrentBagInstrumentation.class.getName() + "$ConstructorAdvice"); + transformer.applyAdvice( + named("borrow"), HikariConcurrentBagInstrumentation.class.getName() + "$BorrowAdvice"); + } + + public static class ConstructorAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + static void after(@Advice.This ConcurrentBag thiz) + throws IllegalAccessException, NoSuchFieldException { + try { + Field handoffQueueField = thiz.getClass().getDeclaredField("handoffQueue"); + handoffQueueField.setAccessible(true); + handoffQueueField.set(thiz, new HikariBlockedTrackingSynchronousQueue<>()); + } catch (NoSuchFieldException e) { + // ignore -- see HikariQueuedSequenceSynchronizerInstrumentation for older Hikari versions + } + + Field hikariPoolField = thiz.getClass().getDeclaredField("listener"); + hikariPoolField.setAccessible(true); + HikariPool hikariPool = (HikariPool) hikariPoolField.get(thiz); + + /* + * In earlier versions of Hikari, poolName is directly inside HikariPool, and + * in later versions it is in the PoolBase superclass. + */ + final Class hikariPoolSuper = hikariPool.getClass().getSuperclass(); + final Class poolNameContainingClass; + if (!hikariPoolSuper.getName().equals("java.lang.Object")) { + poolNameContainingClass = hikariPoolSuper; + } else { + poolNameContainingClass = hikariPool.getClass(); + } + Field poolNameField = poolNameContainingClass.getDeclaredField("poolName"); + poolNameField.setAccessible(true); + String poolName = (String) poolNameField.get(hikariPool); + InstrumentationContext.get(ConcurrentBag.class, String.class).put(thiz, poolName); + } + } + + public static class BorrowAdvice { + private static final String POOL_WAITING = "pool.waiting"; + + @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("hikari", POOL_WAITING, TimeUnit.MILLISECONDS.toMicros(startTimeMillis)); + final String poolName = + InstrumentationContext.get(ConcurrentBag.class, String.class).get(thiz); + if (poolName != null) { + span.setTag(DB_POOL_NAME, poolName); + } + // XXX should we do anything with the throwable? + span.finish(); + } + HikariBlockedTracker.clearBlocked(); + } + } +} 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..9bf90e90cb9 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java @@ -0,0 +1,39 @@ +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; + +/** Blocked getConnection() tracking for Hikari starting before commit f0b3c520c. */ +@AutoService(InstrumenterModule.class) +public final class HikariQueuedSequenceSynchronizerInstrumentation + extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + + public HikariQueuedSequenceSynchronizerInstrumentation() { + super("jdbc-datasource"); + } + + @Override + public String instrumentedType() { + return "com.zaxxer.hikari.util.QueuedSequenceSynchronizer"; + } + + @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/test/groovy/SaturatedPoolBlockingTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy new file mode 100644 index 00000000000..c06f8121bdb --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy @@ -0,0 +1,71 @@ +import datadog.trace.agent.test.AgentTestRunner +import com.zaxxer.hikari.HikariConfig +import com.zaxxer.hikari.HikariDataSource +import test.TestDataSource + +import java.sql.SQLTimeoutException +import java.sql.SQLTransientConnectionException + +/** + * Ideas taken from Hikari's com.zaxxer.hikari.pool.TestSaturatedPool830. + */ +class SaturatedPoolBlockingTest extends AgentTestRunner { + def "saturated pool test"(int connectionTimeout, Long exhaustPoolForMillis, int expectedWaitingSpans, boolean expectedTimeout) { + setup: + TEST_WRITER.setFilter((trace) -> trace.get(0).getOperationName() == "test.when") + + final HikariConfig config = new HikariConfig() + config.setPoolName("testPool") + config.setMaximumPoolSize(1) + config.setConnectionTimeout(connectionTimeout) + config.setDataSourceClassName(TestDataSource.class.getName()) + final HikariDataSource ds = new HikariDataSource(config) + + when: + if (exhaustPoolForMillis != null) { + def saturatedConnection = ds.getConnection() + new Thread(() -> { + Thread.sleep(exhaustPoolForMillis) + saturatedConnection.close() + }, "saturated connection closer").start() + } + + def timedOut = false + def span = TEST_TRACER.startSpan("test", "test.when") + try (def ignore = TEST_TRACER.activateSpan(span)) { + def connection = ds.getConnection() + connection.close() + } catch (SQLTransientConnectionException e) { + if (e.getMessage().contains("request timed out after")) { + // Hikari, newer + timedOut = true + } else { + throw e + } + } catch (SQLTimeoutException ignored) { + // Hikari, older + timedOut = true + } + span.finish() + + then: + def waiting = TEST_WRITER.firstTrace().findAll { + element -> element.getOperationName() == "pool.waiting" + } + + print(TEST_WRITER.firstTrace()) + + verifyAll { + TEST_WRITER.size() == 1 + waiting.size() == expectedWaitingSpans + timedOut == expectedTimeout + } + + where: + connectionTimeout | exhaustPoolForMillis | expectedWaitingSpans | expectedTimeout + 1000 | null | 0 | false + 1000 | null | 0 | false + 1000 | 500 | 1 | false + 1000 | 1500 | 1 | true + } +} 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); + } +} From fec7eac4e5d6dfa71b3293f0201d9957513c67b3 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Thu, 24 Jul 2025 09:46:36 -0700 Subject: [PATCH 2/3] Apache DBCP2: Add a span when waiting on the pool --- .../instrumentation/jdbc/build.gradle | 1 + ...cp2LinkedBlockingDequeInstrumentation.java | 56 ++++++++++ ...Dbcp2ManagedConnectionInstrumentation.java | 45 ++++++++ ...2PerUserPoolDataSourceInstrumentation.java | 46 ++++++++ ...Dbcp2PoolingDataSourceInstrumentation.java | 45 ++++++++ .../Dbcp2PoolingDriverInstrumentation.java | 44 ++++++++ ...p2SharedPoolDataSourceInstrumentation.java | 46 ++++++++ .../groovy/SaturatedPoolBlockingTest.groovy | 103 +++++++++++++++--- .../test/groovy/test/TestConnection.groovy | 2 +- .../src/test/groovy/test/TestDriver.groovy | 2 +- 10 files changed, 375 insertions(+), 15 deletions(-) create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java diff --git a/dd-java-agent/instrumentation/jdbc/build.gradle b/dd-java-agent/instrumentation/jdbc/build.gradle index 1898efd78b4..2b952fb0a98 100644 --- a/dd-java-agent/instrumentation/jdbc/build.gradle +++ b/dd-java-agent/instrumentation/jdbc/build.gradle @@ -27,6 +27,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..5ab0085d898 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java @@ -0,0 +1,56 @@ +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 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.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-datasource"); + } + + @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 void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("pollFirst").and(takesArguments(1)), + Dbcp2LinkedBlockingDequeInstrumentation.class.getName() + "$PollFirstAdvice"); + } + + public static class PollFirstAdvice { + private static final String POOL_WAITING = "pool.waiting"; + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentSpan onEnter() { + if (CallDepthThreadLocalMap.getCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.class) > 0) { + return startSpan("dbcp2", POOL_WAITING); + } else { + return null; + } + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit(@Advice.Enter final AgentSpan span) { + if (span != null) { + 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..5f0c3c626fd --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java @@ -0,0 +1,45 @@ +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.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-datasource"); + } + + @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 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(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.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..6ac6297de1f --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java @@ -0,0 +1,46 @@ +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.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-datasource"); + } + + @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 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(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.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..a428d64b133 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java @@ -0,0 +1,45 @@ +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.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-datasource"); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.PoolingDataSource", // standalone + "org.apache.tomcat.dbcp.dbcp2.PoolingDataSource" // bundled with Tomcat + }; + } + + @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(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.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..8f49488e03e --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java @@ -0,0 +1,44 @@ +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.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-datasource"); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] { + "org.apache.commons.dbcp2.PoolingDriver", // standalone + "org.apache.tomcat.dbcp.dbcp2.PoolingDriver" // bundled with Tomcat + }; + } + + @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(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.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..8366e454a07 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java @@ -0,0 +1,46 @@ +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.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-datasource"); + } + + @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 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(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.class); + } + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy index c06f8121bdb..412479e5147 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy @@ -1,25 +1,31 @@ import datadog.trace.agent.test.AgentTestRunner import com.zaxxer.hikari.HikariConfig import com.zaxxer.hikari.HikariDataSource +import org.apache.commons.dbcp2.BasicDataSource +import org.apache.commons.pool2.BaseObject +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 test.TestDataSource +import test.TestDriver +import javax.sql.DataSource +import java.sql.SQLException import java.sql.SQLTimeoutException import java.sql.SQLTransientConnectionException +import java.time.Duration /** * Ideas taken from Hikari's com.zaxxer.hikari.pool.TestSaturatedPool830. */ class SaturatedPoolBlockingTest extends AgentTestRunner { - def "saturated pool test"(int connectionTimeout, Long exhaustPoolForMillis, int expectedWaitingSpans, boolean expectedTimeout) { + public static final int CONNECTION_TIMEOUT = 1000 + + def "saturated pool test"(Closure createDataSource, Long exhaustPoolForMillis, int expectedWaitingSpans, boolean expectedTimeout) { setup: TEST_WRITER.setFilter((trace) -> trace.get(0).getOperationName() == "test.when") - - final HikariConfig config = new HikariConfig() - config.setPoolName("testPool") - config.setMaximumPoolSize(1) - config.setConnectionTimeout(connectionTimeout) - config.setDataSourceClassName(TestDataSource.class.getName()) - final HikariDataSource ds = new HikariDataSource(config) + final DataSource ds = createDataSource() when: if (exhaustPoolForMillis != null) { @@ -45,6 +51,13 @@ class SaturatedPoolBlockingTest extends AgentTestRunner { } catch (SQLTimeoutException ignored) { // Hikari, older timedOut = true + } catch (SQLException e) { + if (e.getMessage().contains("pool error Timeout waiting for idle object")) { + // dbcp2 + timedOut = true + } else { + throw e + } } span.finish() @@ -62,10 +75,74 @@ class SaturatedPoolBlockingTest extends AgentTestRunner { } where: - connectionTimeout | exhaustPoolForMillis | expectedWaitingSpans | expectedTimeout - 1000 | null | 0 | false - 1000 | null | 0 | false - 1000 | 500 | 1 | false - 1000 | 1500 | 1 | true + createDataSource | exhaustPoolForMillis | expectedWaitingSpans | expectedTimeout + this.&hikariDataSource | null | 0 | false + this.&hikariDataSource | null | 0 | false + this.&hikariDataSource | 500 | 1 | false + this.&hikariDataSource | 1500 | 1 | true + this.&dbcp2DataSource | null | 0 | false + this.&dbcp2DataSource | null | 0 | false + this.&dbcp2DataSource | 500 | 1 | false + this.&dbcp2DataSource | 1500 | 1 | true + } + + def "non-dbcp2 LinkedBlockingDeque"() { + 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 + } + + private static DataSource hikariDataSource() { + final HikariConfig config = new HikariConfig() + config.setPoolName("testPool") + config.setMaximumPoolSize(1) + config.setConnectionTimeout(CONNECTION_TIMEOUT) + config.setDataSourceClassName(TestDataSource.class.getName()) + return new HikariDataSource(config) + } + + private static DataSource dbcp2DataSource() { + final BasicDataSource ds = new BasicDataSource() + ds.setMaxTotal(1) + ds.setMaxWait(Duration.ofMillis(CONNECTION_TIMEOUT)) + ds.setDriverClassName(TestDriver.class.getName()) + ds.start() + return ds } } 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/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 From 41613d30075dce3912a6188b9b1d6de4de69cd0f Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Tue, 12 Aug 2025 17:42:55 -0700 Subject: [PATCH 3/3] Simplify Hikari pool.waiting instrumentation with AsmVisitorWrapper to detect blocking --- .../jdbc/HikariBlockedTracker.java | 18 -- ...HikariBlockedTrackingSynchronousQueue.java | 18 -- .../HikariConcurrentBagInstrumentation.java | 158 +++++++++++++++--- ...edSequenceSynchronizerInstrumentation.java | 39 ----- 4 files changed, 132 insertions(+), 101 deletions(-) delete mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java delete mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java delete mode 100644 dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java 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 deleted file mode 100644 index 3a60a72e6ec..00000000000 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTracker.java +++ /dev/null @@ -1,18 +0,0 @@ -package datadog.trace.instrumentation.jdbc; - -/** Shared blocked getConnection() tracking ThreadLocking 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/HikariBlockedTrackingSynchronousQueue.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java deleted file mode 100644 index 29bff65d0c7..00000000000 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariBlockedTrackingSynchronousQueue.java +++ /dev/null @@ -1,18 +0,0 @@ -package datadog.trace.instrumentation.jdbc; - -import java.util.concurrent.SynchronousQueue; -import java.util.concurrent.TimeUnit; - -/** Blocked getConnection() tracking for Hikari starting with commit f0b3c520c. */ -public class HikariBlockedTrackingSynchronousQueue extends SynchronousQueue { - public HikariBlockedTrackingSynchronousQueue() { - // 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 index 0894792ad9a..22b6a76ed7e 100644 --- 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 @@ -17,6 +17,18 @@ import java.util.Map; import java.util.concurrent.TimeUnit; import net.bytebuddy.asm.Advice; +import net.bytebuddy.asm.AsmVisitorWrapper; +import net.bytebuddy.description.field.FieldDescription; +import net.bytebuddy.description.field.FieldList; +import net.bytebuddy.description.method.MethodList; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.implementation.Implementation; +import net.bytebuddy.jar.asm.ClassVisitor; +import net.bytebuddy.jar.asm.ClassWriter; +import net.bytebuddy.jar.asm.MethodVisitor; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.jar.asm.Type; +import net.bytebuddy.pool.TypePool; /** * Instrument Hikari's ConcurrentBag class to detect when blocking occurs trying to get an entry @@ -24,7 +36,11 @@ */ @AutoService(InstrumenterModule.class) public final class HikariConcurrentBagInstrumentation extends InstrumenterModule.Tracing - implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + implements Instrumenter.ForSingleType, + Instrumenter.HasTypeAdvice, + Instrumenter.HasMethodAdvice { + private static final String INSTRUMENTATION_NAME = "hikari"; + private static final String POOL_WAITING = "pool.waiting"; public HikariConcurrentBagInstrumentation() { super("jdbc-datasource"); @@ -35,19 +51,17 @@ public String instrumentedType() { return "com.zaxxer.hikari.util.ConcurrentBag"; } - @Override - public String[] helperClassNames() { - return new String[] { - packageName + ".HikariBlockedTrackingSynchronousQueue", packageName + ".HikariBlockedTracker" - }; - } - @Override public Map contextStore() { // For getting the poolName return singletonMap("com.zaxxer.hikari.util.ConcurrentBag", String.class.getName()); } + @Override + public void typeAdvice(TypeTransformer transformer) { + transformer.applyAdvice(new ConcurrentBagVisitorWrapper()); + } + @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( @@ -58,19 +72,11 @@ public void methodAdvice(MethodTransformer transformer) { public static class ConstructorAdvice { @Advice.OnMethodExit(suppress = Throwable.class) - static void after(@Advice.This ConcurrentBag thiz) + static void after( + @Advice.This ConcurrentBag thiz, + @Advice.FieldValue("listener") ConcurrentBag.IBagStateListener listener) throws IllegalAccessException, NoSuchFieldException { - try { - Field handoffQueueField = thiz.getClass().getDeclaredField("handoffQueue"); - handoffQueueField.setAccessible(true); - handoffQueueField.set(thiz, new HikariBlockedTrackingSynchronousQueue<>()); - } catch (NoSuchFieldException e) { - // ignore -- see HikariQueuedSequenceSynchronizerInstrumentation for older Hikari versions - } - - Field hikariPoolField = thiz.getClass().getDeclaredField("listener"); - hikariPoolField.setAccessible(true); - HikariPool hikariPool = (HikariPool) hikariPoolField.get(thiz); + HikariPool hikariPool = (HikariPool) listener; /* * In earlier versions of Hikari, poolName is directly inside HikariPool, and @@ -91,11 +97,9 @@ static void after(@Advice.This ConcurrentBag thiz) } public static class BorrowAdvice { - private static final String POOL_WAITING = "pool.waiting"; - @Advice.OnMethodEnter(suppress = Throwable.class) public static Long onEnter() { - HikariBlockedTracker.clearBlocked(); + HikariWaitingTracker.clearWaiting(); return System.currentTimeMillis(); } @@ -104,9 +108,12 @@ public static void stopSpan( @Advice.This ConcurrentBag thiz, @Advice.Enter final Long startTimeMillis, @Advice.Thrown final Throwable throwable) { - if (HikariBlockedTracker.wasBlocked()) { + if (HikariWaitingTracker.wasWaiting()) { final AgentSpan span = - startSpan("hikari", POOL_WAITING, TimeUnit.MILLISECONDS.toMicros(startTimeMillis)); + startSpan( + INSTRUMENTATION_NAME, + POOL_WAITING, + TimeUnit.MILLISECONDS.toMicros(startTimeMillis)); final String poolName = InstrumentationContext.get(ConcurrentBag.class, String.class).get(thiz); if (poolName != null) { @@ -115,7 +122,106 @@ public static void stopSpan( // XXX should we do anything with the throwable? span.finish(); } - HikariBlockedTracker.clearBlocked(); + HikariWaitingTracker.clearWaiting(); + } + } + + private class ConcurrentBagVisitorWrapper implements AsmVisitorWrapper { + @Override + public int mergeWriter(int flags) { + return flags | ClassWriter.COMPUTE_MAXS; + } + + @Override + public int mergeReader(int flags) { + return flags; + } + + @Override + public ClassVisitor wrap( + TypeDescription instrumentedType, + ClassVisitor classVisitor, + Implementation.Context implementationContext, + TypePool typePool, + FieldList fields, + MethodList methods, + int writerFlags, + int readerFlags) { + return new ConcurrentBagClassVisitor(Opcodes.ASM8, classVisitor); + } + } + + public static class ConcurrentBagClassVisitor extends ClassVisitor { + public ConcurrentBagClassVisitor(int api, ClassVisitor cv) { + super(api, cv); + } + + @Override + public MethodVisitor visitMethod( + int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor superMv = super.visitMethod(access, name, descriptor, signature, exceptions); + if ("borrow".equals(name) + && "(JLjava/util/concurrent/TimeUnit;)Lcom/zaxxer/hikari/util/ConcurrentBag$IConcurrentBagEntry;" + .equals(descriptor)) { + return new BorrowMethodVisitor(api, superMv); + } else { + return superMv; + } + } + } + + public static class BorrowMethodVisitor extends MethodVisitor { + public BorrowMethodVisitor(int api, MethodVisitor superMv) { + super(api, superMv); + } + + + /** + * Adds a call to HikariWaitingTracker.setWaiting whenever Hikari is blocking waiting on a connection from the pool + * to be available whenever either of these method calls happen (which one depends on Hikari version): + *
+ * synchronizer.waitUntilSequenceExceeded(startSeq, timeout) + * -- prior to 2.6.0 + *
+ * handoffQueue.poll(timeout, NANOSECONDS) + * -- 2.6.0 and later + */ + @Override + public void visitMethodInsn( + int opcode, String owner, String name, String descriptor, boolean isInterface) { + if ((opcode == Opcodes.INVOKEVIRTUAL + && owner.equals("com/zaxxer/hikari/util/QueuedSequenceSynchronizer") + && name.equals("waitUntilSequenceExceeded") + && descriptor.equals("(JJ)Z")) + || (opcode == Opcodes.INVOKEVIRTUAL + && owner.equals("java/util/concurrent/SynchronousQueue") + && name.equals("poll") + && descriptor.equals("(JLjava/util/concurrent/TimeUnit;)Ljava/lang/Object;"))) { + super.visitMethodInsn( + Opcodes.INVOKESTATIC, + Type.getInternalName(HikariWaitingTracker.class), + "setWaiting", + "()V", + false); + // original stack + } + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + } + } + + public static class HikariWaitingTracker { + private static final ThreadLocal tracker = ThreadLocal.withInitial(() -> false); + + public static void clearWaiting() { + tracker.set(false); + } + + public static void setWaiting() { + tracker.set(true); + } + + public static boolean wasWaiting() { + return tracker.get(); } } } 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 deleted file mode 100644 index 9bf90e90cb9..00000000000 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariQueuedSequenceSynchronizerInstrumentation.java +++ /dev/null @@ -1,39 +0,0 @@ -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; - -/** Blocked getConnection() tracking for Hikari starting before commit f0b3c520c. */ -@AutoService(InstrumenterModule.class) -public final class HikariQueuedSequenceSynchronizerInstrumentation - extends InstrumenterModule.Tracing - implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { - - public HikariQueuedSequenceSynchronizerInstrumentation() { - super("jdbc-datasource"); - } - - @Override - public String instrumentedType() { - return "com.zaxxer.hikari.util.QueuedSequenceSynchronizer"; - } - - @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(); - } - } -}