Skip to content

Commit fd9229e

Browse files
trasktrask
committed
Fix JDBC suppression: match Pool interface in PoolInstrumentation typeMatcher
PoolInstrumentation's typeMatcher used implementsInterface(named(Pool)) which only matches classes/interfaces that implement Pool, but NOT the Pool interface itself. Since Pool.pool() is a static interface method, the advice never fired and connectOptions was never propagated through the ThreadLocal chain. Fix: Add .or(named(Pool)) to the typeMatcher so the Pool interface itself is matched, enabling advice on its static pool() factory methods. This makes the connectOptions propagation chain work: Pool.pool() -> ThreadLocal -> SqlClientBase constructor -> VirtualField -> SqlClientBase.query() -> ThreadLocal -> QueryExecutor -> VirtualField. The null-connectOptions check in QueryAdvice now correctly identifies JDBC pools since JDBCPool.pool() uses its own factory (not Pool.pool()), so connectOptions stays null for JDBC-backed connections.
1 parent 27b9daf commit fd9229e

6 files changed

Lines changed: 232 additions & 2 deletions

File tree

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/PoolInstrumentation.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ public ElementMatcher<ClassLoader> classLoaderOptimization() {
4343

4444
@Override
4545
public ElementMatcher<TypeDescription> typeMatcher() {
46-
return implementsInterface(named("io.vertx.sqlclient.Pool"));
46+
// Match both the Pool interface (for static pool() factory methods) and classes/interfaces
47+
// that implement/extend Pool (for instance methods like getConnection())
48+
return implementsInterface(named("io.vertx.sqlclient.Pool"))
49+
.or(named("io.vertx.sqlclient.Pool"));
4750
}
4851

4952
@Override

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/QueryExecutorInstrumentation.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ public static AdviceScope start(Object queryExecutor, Object[] arguments) {
104104
}
105105

106106
SqlConnectOptions connectOptions = QueryExecutorUtil.getConnectOptions(queryExecutor);
107+
// connectOptions is null when the pool was created via JDBCPool which bypasses the
108+
// Pool.pool() factory, in that case we skip vertx-sql-client span creation and let JDBC
109+
// instrumentation handle it
110+
if (connectOptions == null) {
111+
return new AdviceScope(callDepth);
112+
}
107113
// Try db system stored from pool class first (handles generic SqlConnectOptions),
108114
// fall back to class name detection on the connect options itself
109115
String dbSystem = VertxSqlClientSingletons.getConnectOptionsDbSystem(connectOptions);

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/sql/VertxJdbcClientTest.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.vertx.v4_0.sql;
7+
8+
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
9+
import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable;
10+
import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStableDbSystemName;
11+
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
12+
import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_SUMMARY;
13+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_CONNECTION_STRING;
14+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME;
15+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
16+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE;
17+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
18+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
19+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_USER;
20+
import static java.util.concurrent.TimeUnit.SECONDS;
21+
22+
import io.opentelemetry.api.trace.SpanKind;
23+
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
24+
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
25+
import io.vertx.core.Vertx;
26+
import io.vertx.jdbcclient.JDBCConnectOptions;
27+
import io.vertx.jdbcclient.JDBCPool;
28+
import io.vertx.sqlclient.PoolOptions;
29+
import org.junit.jupiter.api.AfterAll;
30+
import org.junit.jupiter.api.BeforeAll;
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.api.extension.RegisterExtension;
33+
34+
/**
35+
* Tests that vertx-sql-client instrumentation is suppressed for JDBC-backed connections and JDBC
36+
* instrumentation handles them instead.
37+
*/
38+
@SuppressWarnings("deprecation") // using deprecated semconv
39+
class VertxJdbcClientTest {
40+
41+
private static final String DB = "testdb";
42+
43+
@RegisterExtension
44+
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
45+
46+
private static Vertx vertx;
47+
private static io.vertx.sqlclient.Pool pool;
48+
49+
@BeforeAll
50+
static void setUp() throws Exception {
51+
vertx = Vertx.vertx();
52+
pool =
53+
JDBCPool.pool(
54+
vertx,
55+
new JDBCConnectOptions().setJdbcUrl("jdbc:hsqldb:mem:" + DB),
56+
new PoolOptions().setMaxSize(4));
57+
pool.query("create table test(id int primary key, name varchar(255))")
58+
.execute()
59+
.compose(r -> pool.query("insert into test values (1, 'Hello'), (2, 'World')").execute())
60+
.toCompletionStage()
61+
.toCompletableFuture()
62+
.get(30, SECONDS);
63+
}
64+
65+
@AfterAll
66+
static void cleanUp() {
67+
pool.close();
68+
vertx.close();
69+
}
70+
71+
@Test
72+
void testSimpleSelect() throws Exception {
73+
testing
74+
.runWithSpan("parent", () -> pool.query("select * from test").execute())
75+
.toCompletionStage()
76+
.toCompletableFuture()
77+
.get(30, SECONDS);
78+
79+
testing.waitAndAssertTraces(
80+
trace ->
81+
trace.hasSpansSatisfyingExactly(
82+
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL),
83+
span ->
84+
span.hasName(
85+
emitStableDatabaseSemconv() ? "SELECT test" : "SELECT " + DB + ".test")
86+
.hasKind(SpanKind.CLIENT)
87+
.hasParent(trace.getSpan(0))
88+
.hasAttributesSatisfyingExactly(
89+
equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName("hsqldb")),
90+
equalTo(maybeStable(DB_NAME), DB),
91+
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : "SA"),
92+
equalTo(
93+
DB_CONNECTION_STRING,
94+
emitStableDatabaseSemconv() ? null : "hsqldb:mem:"),
95+
equalTo(maybeStable(DB_STATEMENT), "select * from test"),
96+
equalTo(
97+
DB_QUERY_SUMMARY,
98+
emitStableDatabaseSemconv() ? "SELECT test" : null),
99+
equalTo(
100+
maybeStable(DB_OPERATION),
101+
emitStableDatabaseSemconv() ? null : "SELECT"),
102+
equalTo(
103+
maybeStable(DB_SQL_TABLE),
104+
emitStableDatabaseSemconv() ? null : "test"))));
105+
}
106+
}

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v5_0/sql/PoolInstrumentation.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,10 @@ public ElementMatcher<ClassLoader> classLoaderOptimization() {
4141

4242
@Override
4343
public ElementMatcher<TypeDescription> typeMatcher() {
44-
return implementsInterface(named("io.vertx.sqlclient.Pool"));
44+
// Match both the Pool interface (for static pool() factory methods) and classes/interfaces
45+
// that implement/extend Pool (for instance methods like getConnection())
46+
return implementsInterface(named("io.vertx.sqlclient.Pool"))
47+
.or(named("io.vertx.sqlclient.Pool"));
4548
}
4649

4750
@Override

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v5_0/sql/QueryExecutorInstrumentation.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,12 @@ public static AdviceScope start(Object queryExecutor, Object[] arguments) {
103103
}
104104

105105
SqlConnectOptions connectOptions = QueryExecutorUtil.getConnectOptions(queryExecutor);
106+
// connectOptions is null when the pool was created via JDBCPool which bypasses the
107+
// Pool.pool() factory, in that case we skip vertx-sql-client span creation and let JDBC
108+
// instrumentation handle it
109+
if (connectOptions == null) {
110+
return new AdviceScope(callDepth);
111+
}
106112
String dbSystem = VertxSqlClientSingletons.getConnectOptionsDbSystem(connectOptions);
107113
VertxSqlClientRequest otelRequest =
108114
new VertxSqlClientRequest(sql, connectOptions, preparedStatement, dbSystem);

‎instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/v5_0/sql/VertxJdbcClientTest.java‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.javaagent.instrumentation.vertx.v5_0.sql;
7+
8+
import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv;
9+
import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable;
10+
import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStableDbSystemName;
11+
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo;
12+
import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_SUMMARY;
13+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_CONNECTION_STRING;
14+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME;
15+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
16+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE;
17+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
18+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
19+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_USER;
20+
import static java.util.concurrent.TimeUnit.SECONDS;
21+
22+
import io.opentelemetry.api.trace.SpanKind;
23+
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
24+
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
25+
import io.vertx.core.Vertx;
26+
import io.vertx.jdbcclient.JDBCConnectOptions;
27+
import io.vertx.jdbcclient.JDBCPool;
28+
import io.vertx.sqlclient.PoolOptions;
29+
import org.junit.jupiter.api.AfterAll;
30+
import org.junit.jupiter.api.BeforeAll;
31+
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.api.extension.RegisterExtension;
33+
34+
/**
35+
* Tests that vertx-sql-client instrumentation is suppressed for JDBC-backed connections and JDBC
36+
* instrumentation handles them instead.
37+
*/
38+
@SuppressWarnings("deprecation") // using deprecated semconv
39+
class VertxJdbcClientTest {
40+
41+
private static final String DB = "testdb";
42+
43+
@RegisterExtension
44+
private static final InstrumentationExtension testing = AgentInstrumentationExtension.create();
45+
46+
private static Vertx vertx;
47+
private static io.vertx.sqlclient.Pool pool;
48+
49+
@BeforeAll
50+
static void setUp() throws Exception {
51+
vertx = Vertx.vertx();
52+
pool =
53+
JDBCPool.pool(
54+
vertx,
55+
new JDBCConnectOptions().setJdbcUrl("jdbc:hsqldb:mem:" + DB),
56+
new PoolOptions().setMaxSize(4));
57+
pool.query("create table test(id int primary key, name varchar(255))")
58+
.execute()
59+
.compose(r -> pool.query("insert into test values (1, 'Hello'), (2, 'World')").execute())
60+
.toCompletionStage()
61+
.toCompletableFuture()
62+
.get(30, SECONDS);
63+
}
64+
65+
@AfterAll
66+
static void cleanUp() {
67+
pool.close();
68+
vertx.close();
69+
}
70+
71+
@Test
72+
void testSimpleSelect() throws Exception {
73+
testing
74+
.runWithSpan("parent", () -> pool.query("select * from test").execute())
75+
.toCompletionStage()
76+
.toCompletableFuture()
77+
.get(30, SECONDS);
78+
79+
testing.waitAndAssertTraces(
80+
trace ->
81+
trace.hasSpansSatisfyingExactly(
82+
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL),
83+
span ->
84+
span.hasName(
85+
emitStableDatabaseSemconv() ? "SELECT test" : "SELECT " + DB + ".test")
86+
.hasKind(SpanKind.CLIENT)
87+
.hasParent(trace.getSpan(0))
88+
.hasAttributesSatisfyingExactly(
89+
equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName("hsqldb")),
90+
equalTo(maybeStable(DB_NAME), DB),
91+
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : "SA"),
92+
equalTo(
93+
DB_CONNECTION_STRING,
94+
emitStableDatabaseSemconv() ? null : "hsqldb:mem:"),
95+
equalTo(maybeStable(DB_STATEMENT), "select * from test"),
96+
equalTo(
97+
DB_QUERY_SUMMARY,
98+
emitStableDatabaseSemconv() ? "SELECT test" : null),
99+
equalTo(
100+
maybeStable(DB_OPERATION),
101+
emitStableDatabaseSemconv() ? null : "SELECT"),
102+
equalTo(
103+
maybeStable(DB_SQL_TABLE),
104+
emitStableDatabaseSemconv() ? null : "test"))));
105+
}
106+
}

0 commit comments

Comments
 (0)