Skip to content

Commit 9515216

Browse files
deejgregordeejgregor
committed
Disable pool.waiting by default, other tweaks
- Use dd.trace.experimental.jdbc.pool.waiting.enabled=true to enable. - Change instrumentation name from jdbc-datasource to jdbc. - Record exceptions. - Use default instrumentation name (jdbc). - Set resource name to {dbcp2,hikari}.waiting
1 parent ef327ba commit 9515216

File tree

10 files changed

+91
-24
lines changed

10 files changed

+91
-24
lines changed

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2LinkedBlockingDequeInstrumentation.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.google.auto.service.AutoService;
88
import datadog.trace.agent.tooling.Instrumenter;
99
import datadog.trace.agent.tooling.InstrumenterModule;
10+
import datadog.trace.api.InstrumenterConfig;
1011
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
1112
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1213
import net.bytebuddy.asm.Advice;
@@ -16,7 +17,12 @@ public final class Dbcp2LinkedBlockingDequeInstrumentation extends InstrumenterM
1617
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1718

1819
public Dbcp2LinkedBlockingDequeInstrumentation() {
19-
super("jdbc-datasource");
20+
super("jdbc");
21+
}
22+
23+
@Override
24+
protected boolean defaultEnabled() {
25+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
2026
}
2127

2228
@Override
@@ -40,15 +46,21 @@ public static class PollFirstAdvice {
4046
@Advice.OnMethodEnter(suppress = Throwable.class)
4147
public static AgentSpan onEnter() {
4248
if (CallDepthThreadLocalMap.getCallDepth(Dbcp2LinkedBlockingDequeInstrumentation.class) > 0) {
43-
return startSpan("dbcp2", POOL_WAITING);
49+
AgentSpan span = startSpan(POOL_WAITING);
50+
span.setResourceName("dbcp2.waiting");
51+
return span;
4452
} else {
4553
return null;
4654
}
4755
}
4856

4957
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
50-
public static void onExit(@Advice.Enter final AgentSpan span) {
58+
public static void onExit(
59+
@Advice.Enter final AgentSpan span, @Advice.Thrown final Throwable throwable) {
5160
if (span != null) {
61+
if (throwable != null) {
62+
span.addThrowable(throwable);
63+
}
5264
span.finish();
5365
}
5466
}

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2ManagedConnectionInstrumentation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.auto.service.AutoService;
66
import datadog.trace.agent.tooling.Instrumenter;
77
import datadog.trace.agent.tooling.InstrumenterModule;
8+
import datadog.trace.api.InstrumenterConfig;
89
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
910
import net.bytebuddy.asm.Advice;
1011

@@ -13,7 +14,12 @@ public final class Dbcp2ManagedConnectionInstrumentation extends InstrumenterMod
1314
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1415

1516
public Dbcp2ManagedConnectionInstrumentation() {
16-
super("jdbc-datasource");
17+
super("jdbc");
18+
}
19+
20+
@Override
21+
protected boolean defaultEnabled() {
22+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
1723
}
1824

1925
@Override

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PerUserPoolDataSourceInstrumentation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.auto.service.AutoService;
66
import datadog.trace.agent.tooling.Instrumenter;
77
import datadog.trace.agent.tooling.InstrumenterModule;
8+
import datadog.trace.api.InstrumenterConfig;
89
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
910
import net.bytebuddy.asm.Advice;
1011

@@ -13,7 +14,12 @@ public final class Dbcp2PerUserPoolDataSourceInstrumentation extends Instrumente
1314
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1415

1516
public Dbcp2PerUserPoolDataSourceInstrumentation() {
16-
super("jdbc-datasource");
17+
super("jdbc");
18+
}
19+
20+
@Override
21+
protected boolean defaultEnabled() {
22+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
1723
}
1824

1925
@Override

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDataSourceInstrumentation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.auto.service.AutoService;
66
import datadog.trace.agent.tooling.Instrumenter;
77
import datadog.trace.agent.tooling.InstrumenterModule;
8+
import datadog.trace.api.InstrumenterConfig;
89
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
910
import net.bytebuddy.asm.Advice;
1011

@@ -13,7 +14,12 @@ public final class Dbcp2PoolingDataSourceInstrumentation extends InstrumenterMod
1314
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1415

1516
public Dbcp2PoolingDataSourceInstrumentation() {
16-
super("jdbc-datasource");
17+
super("jdbc");
18+
}
19+
20+
@Override
21+
protected boolean defaultEnabled() {
22+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
1723
}
1824

1925
@Override

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2PoolingDriverInstrumentation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.auto.service.AutoService;
66
import datadog.trace.agent.tooling.Instrumenter;
77
import datadog.trace.agent.tooling.InstrumenterModule;
8+
import datadog.trace.api.InstrumenterConfig;
89
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
910
import net.bytebuddy.asm.Advice;
1011

@@ -13,7 +14,12 @@ public final class Dbcp2PoolingDriverInstrumentation extends InstrumenterModule.
1314
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1415

1516
public Dbcp2PoolingDriverInstrumentation() {
16-
super("jdbc-datasource");
17+
super("jdbc");
18+
}
19+
20+
@Override
21+
protected boolean defaultEnabled() {
22+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
1723
}
1824

1925
@Override

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/Dbcp2SharedPoolDataSourceInstrumentation.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.google.auto.service.AutoService;
66
import datadog.trace.agent.tooling.Instrumenter;
77
import datadog.trace.agent.tooling.InstrumenterModule;
8+
import datadog.trace.api.InstrumenterConfig;
89
import datadog.trace.bootstrap.CallDepthThreadLocalMap;
910
import net.bytebuddy.asm.Advice;
1011

@@ -13,7 +14,12 @@ public final class Dbcp2SharedPoolDataSourceInstrumentation extends Instrumenter
1314
implements Instrumenter.ForKnownTypes, Instrumenter.HasMethodAdvice {
1415

1516
public Dbcp2SharedPoolDataSourceInstrumentation() {
16-
super("jdbc-datasource");
17+
super("jdbc");
18+
}
19+
20+
@Override
21+
protected boolean defaultEnabled() {
22+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
1723
}
1824

1925
@Override

‎dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/HikariConcurrentBagInstrumentation.java

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import com.zaxxer.hikari.util.ConcurrentBag;
1212
import datadog.trace.agent.tooling.Instrumenter;
1313
import datadog.trace.agent.tooling.InstrumenterModule;
14+
import datadog.trace.api.InstrumenterConfig;
1415
import datadog.trace.bootstrap.InstrumentationContext;
1516
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1617
import java.lang.reflect.Field;
@@ -39,11 +40,15 @@ public final class HikariConcurrentBagInstrumentation extends InstrumenterModule
3940
implements Instrumenter.ForSingleType,
4041
Instrumenter.HasTypeAdvice,
4142
Instrumenter.HasMethodAdvice {
42-
private static final String INSTRUMENTATION_NAME = "hikari";
4343
private static final String POOL_WAITING = "pool.waiting";
4444

4545
public HikariConcurrentBagInstrumentation() {
46-
super("jdbc-datasource");
46+
super("jdbc");
47+
}
48+
49+
@Override
50+
protected boolean defaultEnabled() {
51+
return InstrumenterConfig.get().isJdbcPoolWaitingEnabled();
4752
}
4853

4954
@Override
@@ -110,16 +115,17 @@ public static void stopSpan(
110115
@Advice.Thrown final Throwable throwable) {
111116
if (HikariWaitingTracker.wasWaiting()) {
112117
final AgentSpan span =
113-
startSpan(
114-
INSTRUMENTATION_NAME,
115-
POOL_WAITING,
116-
TimeUnit.MILLISECONDS.toMicros(startTimeMillis));
118+
startSpan(POOL_WAITING, TimeUnit.MILLISECONDS.toMicros(startTimeMillis));
119+
span.setResourceName("hikari.waiting");
120+
if (throwable != null) {
121+
span.addThrowable(throwable);
122+
}
117123
final String poolName =
118124
InstrumentationContext.get(ConcurrentBag.class, String.class).get(thiz);
119125
if (poolName != null) {
120126
span.setTag(DB_POOL_NAME, poolName);
121127
}
122-
// XXX should we do anything with the throwable?
128+
123129
span.finish();
124130
}
125131
HikariWaitingTracker.clearWaiting();
@@ -175,16 +181,16 @@ public BorrowMethodVisitor(int api, MethodVisitor superMv) {
175181
super(api, superMv);
176182
}
177183

178-
179184
/**
180-
* Adds a call to HikariWaitingTracker.setWaiting whenever Hikari is blocking waiting on a connection from the pool
181-
* to be available whenever either of these method calls happen (which one depends on Hikari version):
182-
* <br/>
183-
* <code>synchronizer.waitUntilSequenceExceeded(startSeq, timeout)</code>
184-
* -- <a href=" https://github.com/brettwooldridge/HikariCP/blob/5adf46c148dfa095886c7c754f365b0644dc04cb/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java#L159">prior to 2.6.0</a>
185-
* <br/>
186-
* <code>handoffQueue.poll(timeout, NANOSECONDS)</code>
187-
* -- <a href="https://github.com/brettwooldridge/HikariCP/blob/22cc9bde6c0fb54c8ac009122a20d2f579e1a54a/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java#L162">2.6.0 and later</a>
185+
* Adds a call to HikariWaitingTracker.setWaiting whenever Hikari is blocking waiting on a
186+
* connection from the pool to be available whenever either of these method calls happen (which
187+
* one depends on Hikari version): <br>
188+
* <code>synchronizer.waitUntilSequenceExceeded(startSeq, timeout)</code> -- <a href="
189+
* https://github.com/brettwooldridge/HikariCP/blob/5adf46c148dfa095886c7c754f365b0644dc04cb/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java#L159">prior
190+
* to 2.6.0</a> <br>
191+
* <code>handoffQueue.poll(timeout, NANOSECONDS)</code> -- <a
192+
* href="https://github.com/brettwooldridge/HikariCP/blob/22cc9bde6c0fb54c8ac009122a20d2f579e1a54a/src/main/java/com/zaxxer/hikari/util/ConcurrentBag.java#L162">2.6.0
193+
* and later</a>
188194
*/
189195
@Override
190196
public void visitMethodInsn(

‎dd-java-agent/instrumentation/jdbc/src/test/groovy/SaturatedPoolBlockingTest.groovy

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,20 @@ import java.sql.SQLTimeoutException
1616
import java.sql.SQLTransientConnectionException
1717
import java.time.Duration
1818

19+
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED
20+
1921
/**
2022
* Ideas taken from Hikari's com.zaxxer.hikari.pool.TestSaturatedPool830.
2123
*/
2224
class SaturatedPoolBlockingTest extends AgentTestRunner {
2325
public static final int CONNECTION_TIMEOUT = 1000
2426

27+
@Override
28+
void configurePreAgent() {
29+
super.configurePreAgent()
30+
injectSysConfig(JDBC_POOL_WAITING_ENABLED, "true")
31+
}
32+
2533
def "saturated pool test"(Closure<DataSource> createDataSource, Long exhaustPoolForMillis, int expectedWaitingSpans, boolean expectedTimeout) {
2634
setup:
2735
TEST_WRITER.setFilter((trace) -> trace.get(0).getOperationName() == "test.when")

‎dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ public final class TraceInstrumentationConfig {
7272

7373
public static final String JDBC_CONNECTION_CLASS_NAME = "trace.jdbc.connection.class.name";
7474

75+
public static final String JDBC_POOL_WAITING_ENABLED = "trace.experimental.jdbc.pool.waiting.enabled";
76+
7577
public static final String AKKA_FORK_JOIN_TASK_NAME = "trace.akka.fork.join.task.name";
7678
public static final String AKKA_FORK_JOIN_EXECUTOR_TASK_NAME =
7779
"trace.akka.fork.join.executor.task.name";

‎internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static datadog.trace.api.config.TraceInstrumentationConfig.INTEGRATIONS_ENABLED;
4646
import static datadog.trace.api.config.TraceInstrumentationConfig.JAX_RS_ADDITIONAL_ANNOTATIONS;
4747
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_CONNECTION_CLASS_NAME;
48+
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED;
4849
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_PREPARED_STATEMENT_CLASS_NAME;
4950
import static datadog.trace.api.config.TraceInstrumentationConfig.MEASURE_METHODS;
5051
import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_CACHE_CONFIG;
@@ -128,6 +129,7 @@ public class InstrumenterConfig {
128129

129130
private final String jdbcPreparedStatementClassName;
130131
private final String jdbcConnectionClassName;
132+
private final boolean jdbcPoolWaitingEnabled;
131133

132134
private final String httpURLConnectionClassName;
133135
private final String axisTransportClassName;
@@ -235,6 +237,7 @@ private InstrumenterConfig() {
235237
jdbcPreparedStatementClassName =
236238
configProvider.getString(JDBC_PREPARED_STATEMENT_CLASS_NAME, "");
237239
jdbcConnectionClassName = configProvider.getString(JDBC_CONNECTION_CLASS_NAME, "");
240+
jdbcPoolWaitingEnabled = configProvider.getBoolean(JDBC_POOL_WAITING_ENABLED, false);
238241

239242
httpURLConnectionClassName = configProvider.getString(HTTP_URL_CONNECTION_CLASS_NAME, "");
240243
axisTransportClassName = configProvider.getString(AXIS_TRANSPORT_CLASS_NAME, "");
@@ -411,6 +414,10 @@ public String getJdbcConnectionClassName() {
411414
return jdbcConnectionClassName;
412415
}
413416

417+
public boolean isJdbcPoolWaitingEnabled() {
418+
return jdbcPoolWaitingEnabled;
419+
}
420+
414421
public String getHttpURLConnectionClassName() {
415422
return httpURLConnectionClassName;
416423
}
@@ -620,6 +627,8 @@ public String toString() {
620627
+ ", jdbcConnectionClassName='"
621628
+ jdbcConnectionClassName
622629
+ '\''
630+
+ ", jdbcPoolWaitingEnabled="
631+
+ jdbcPoolWaitingEnabled
623632
+ ", httpURLConnectionClassName='"
624633
+ httpURLConnectionClassName
625634
+ '\''

0 commit comments

Comments
 (0)