Skip to content

Commit 62dd98a

Browse files
lauritlaurit
authored
Exclude jdbc wrappers from instrumentation (open-telemetry#14760)
1 parent 644f508 commit 62dd98a

File tree

6 files changed

+208
-28
lines changed

6 files changed

+208
-28
lines changed

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ public static class PrepareAdvice {
6262
@Advice.OnMethodExit(suppress = Throwable.class)
6363
public static void addDbInfo(
6464
@Advice.Argument(0) String sql, @Advice.Return PreparedStatement statement) {
65+
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
66+
return;
67+
}
68+
6569
JdbcData.preparedStatement.set(statement, sql);
6670
}
6771
}
@@ -105,6 +109,10 @@ public void end(@Nullable Throwable throwable) {
105109
@Advice.OnMethodEnter(suppress = Throwable.class)
106110
public static AdviceScope onEnter(
107111
@Advice.This Connection connection, @Advice.Origin("#m") String methodName) {
112+
if (JdbcSingletons.isWrapper(connection, Connection.class)) {
113+
return null;
114+
}
115+
108116
return AdviceScope.start(connection, methodName);
109117
}
110118

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@
1111
import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceAttributesExtractor;
1212
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
1313
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
14+
import io.opentelemetry.instrumentation.api.internal.cache.Cache;
1415
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
1516
import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory;
1617
import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter;
1718
import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
1819
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
1920
import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo;
21+
import java.sql.SQLException;
22+
import java.sql.Wrapper;
2023
import java.util.Collections;
2124
import javax.sql.DataSource;
2225

@@ -68,5 +71,31 @@ public static Instrumenter<DataSource, DbInfo> dataSourceInstrumenter() {
6871
return DATASOURCE_INSTRUMENTER;
6972
}
7073

74+
private static final Cache<Class<?>, Boolean> wrapperClassCache = Cache.weak();
75+
76+
/**
77+
* Returns true if the given object is a wrapper and shouldn't be instrumented. We'll instrument
78+
* the underlying object called by the wrapper instead.
79+
*/
80+
public static <T extends Wrapper> boolean isWrapper(T object, Class<T> clazz) {
81+
return wrapperClassCache.computeIfAbsent(
82+
object.getClass(), key -> isWrapperInternal(object, clazz));
83+
}
84+
85+
private static <T extends Wrapper> boolean isWrapperInternal(T object, Class<T> clazz) {
86+
try {
87+
// we are dealing with a wrapper when the object unwraps to a different instance
88+
if (object.isWrapperFor(clazz)) {
89+
T unwrapped = object.unwrap(clazz);
90+
if (object != unwrapped) {
91+
return true;
92+
}
93+
}
94+
} catch (SQLException | AbstractMethodError e) {
95+
// ignore
96+
}
97+
return false;
98+
}
99+
71100
private JdbcSingletons() {}
72101
}

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ public static JdbcAdviceScope onEnter(@Advice.This PreparedStatement statement)
108108
if (JdbcData.preparedStatement.get(statement) == null) {
109109
return null;
110110
}
111+
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
112+
return null;
113+
}
111114

112115
return JdbcAdviceScope.startPreparedStatement(CallDepth.forClass(Statement.class), statement);
113116
}
@@ -127,6 +130,10 @@ public static class AddBatchAdvice {
127130

128131
@Advice.OnMethodExit(suppress = Throwable.class)
129132
public static void addBatch(@Advice.This PreparedStatement statement) {
133+
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
134+
return;
135+
}
136+
130137
JdbcData.addPreparedStatementBatch(statement);
131138
}
132139
}
@@ -141,6 +148,9 @@ public static void onExit(
141148
if (!CAPTURE_QUERY_PARAMETERS) {
142149
return;
143150
}
151+
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
152+
return;
153+
}
144154

145155
String str = null;
146156

@@ -173,6 +183,9 @@ public static void onExit(
173183
if (!CAPTURE_QUERY_PARAMETERS) {
174184
return;
175185
}
186+
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
187+
return;
188+
}
176189

177190
String str = null;
178191

@@ -205,6 +218,9 @@ public static void onExit(
205218
if (!CAPTURE_QUERY_PARAMETERS) {
206219
return;
207220
}
221+
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
222+
return;
223+
}
208224

209225
String str = null;
210226

‎instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,24 @@ public void transform(TypeTransformer transformer) {
5959
@SuppressWarnings("unused")
6060
public static class StatementAdvice {
6161

62+
@Nullable
6263
@Advice.OnMethodEnter(suppress = Throwable.class)
6364
public static JdbcAdviceScope onEnter(
6465
@Advice.Argument(0) String sql, @Advice.This Statement statement) {
66+
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
67+
return null;
68+
}
69+
6570
return JdbcAdviceScope.startStatement(CallDepth.forClass(Statement.class), sql, statement);
6671
}
6772

6873
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
6974
public static void stopSpan(
70-
@Advice.Thrown @Nullable Throwable throwable, @Advice.Enter JdbcAdviceScope adviceScope) {
71-
adviceScope.end(throwable);
75+
@Advice.Thrown @Nullable Throwable throwable,
76+
@Advice.Enter @Nullable JdbcAdviceScope adviceScope) {
77+
if (adviceScope != null) {
78+
adviceScope.end(throwable);
79+
}
7280
}
7381
}
7482

@@ -80,6 +88,10 @@ public static void addBatch(@Advice.This Statement statement, @Advice.Argument(0
8088
if (statement instanceof PreparedStatement) {
8189
return;
8290
}
91+
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
92+
return;
93+
}
94+
8395
JdbcData.addStatementBatch(statement, sql);
8496
}
8597
}
@@ -96,15 +108,23 @@ public static void clearBatch(@Advice.This Statement statement) {
96108
@SuppressWarnings("unused")
97109
public static class ExecuteBatchAdvice {
98110

111+
@Nullable
99112
@Advice.OnMethodEnter(suppress = Throwable.class)
100113
public static JdbcAdviceScope onEnter(@Advice.This Statement statement) {
114+
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
115+
return null;
116+
}
117+
101118
return JdbcAdviceScope.startBatch(CallDepth.forClass(Statement.class), statement);
102119
}
103120

104121
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
105122
public static void stopSpan(
106-
@Advice.Thrown @Nullable Throwable throwable, @Advice.Enter JdbcAdviceScope adviceScope) {
107-
adviceScope.end(throwable);
123+
@Advice.Thrown @Nullable Throwable throwable,
124+
@Advice.Enter @Nullable JdbcAdviceScope adviceScope) {
125+
if (adviceScope != null) {
126+
adviceScope.end(throwable);
127+
}
108128
}
109129
}
110130

‎instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1330,7 +1330,7 @@ void testProxyStatement() throws Exception {
13301330
cleanup.deferCleanup(statement);
13311331
cleanup.deferCleanup(connection);
13321332

1333-
Statement proxyStatement = ProxyStatementFactory.proxyStatement(statement);
1333+
Statement proxyStatement = ProxyStatementFactory.proxyStatementWithCustomClassLoader(statement);
13341334
ResultSet resultSet =
13351335
testing.runWithSpan("parent", () -> proxyStatement.executeQuery("SELECT 3"));
13361336

@@ -1737,4 +1737,100 @@ static Stream<Arguments> transactionOperationsStream() throws SQLException {
17371737
Arguments.of(
17381738
"hsqldb", new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null), "SA", "hsqldb:mem:"));
17391739
}
1740+
1741+
private static PreparedStatement wrapPreparedStatement(PreparedStatement statement) {
1742+
return ProxyStatementFactory.proxyPreparedStatement(
1743+
(proxy, method, args) -> {
1744+
if ("isWrapperFor".equals(method.getName())
1745+
&& args.length == 1
1746+
&& args[0] == PreparedStatement.class) {
1747+
return true;
1748+
}
1749+
if ("unwrap".equals(method.getName())
1750+
&& args.length == 1
1751+
&& args[0] == PreparedStatement.class) {
1752+
return statement;
1753+
}
1754+
return testing.runWithSpan("wrapper", () -> method.invoke(statement, args));
1755+
});
1756+
}
1757+
1758+
// test that tracing does not start from a wrapper
1759+
// regression test for
1760+
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/14733
1761+
@Test
1762+
void testPreparedStatementWrapper() throws SQLException {
1763+
Connection connection = new org.h2.Driver().connect(jdbcUrls.get("h2"), null);
1764+
Connection proxyConnection =
1765+
ProxyStatementFactory.proxy(
1766+
Connection.class,
1767+
(proxy, method, args) -> {
1768+
// we don't implement unwrapping here as that would also cause the executeQuery
1769+
// instrumentation to get skipped for the prepared statement wrapper
1770+
if ("prepareStatement".equals(method.getName())) {
1771+
return wrapPreparedStatement((PreparedStatement) method.invoke(connection, args));
1772+
}
1773+
return method.invoke(connection, args);
1774+
});
1775+
PreparedStatement statement = proxyConnection.prepareStatement("SELECT 3");
1776+
cleanup.deferCleanup(statement);
1777+
cleanup.deferCleanup(connection);
1778+
1779+
ResultSet resultSet = testing.runWithSpan("parent", () -> statement.executeQuery());
1780+
1781+
resultSet.next();
1782+
assertThat(resultSet.getInt(1)).isEqualTo(3);
1783+
testing.waitAndAssertTraces(
1784+
trace ->
1785+
trace.hasSpansSatisfyingExactly(
1786+
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
1787+
span ->
1788+
span.hasName("wrapper").hasKind(SpanKind.INTERNAL).hasParent(trace.getSpan(0)),
1789+
span ->
1790+
span.hasName("SELECT " + dbNameLower)
1791+
.hasKind(SpanKind.CLIENT)
1792+
.hasParent(trace.getSpan(1))));
1793+
}
1794+
1795+
// test that tracing does not start from a wrapper
1796+
// regression test for
1797+
// https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/14733
1798+
@Test
1799+
void testStatementWrapper() throws SQLException {
1800+
Connection connection = new org.h2.Driver().connect(jdbcUrls.get("h2"), null);
1801+
Statement statement = connection.createStatement();
1802+
cleanup.deferCleanup(statement);
1803+
cleanup.deferCleanup(connection);
1804+
1805+
Statement proxyStatement =
1806+
ProxyStatementFactory.proxyStatement(
1807+
(proxy, method, args) -> {
1808+
if ("isWrapperFor".equals(method.getName())
1809+
&& args.length == 1
1810+
&& args[0] == Statement.class) {
1811+
return true;
1812+
}
1813+
if ("unwrap".equals(method.getName())
1814+
&& args.length == 1
1815+
&& args[0] == Statement.class) {
1816+
return statement;
1817+
}
1818+
return testing.runWithSpan("wrapper", () -> method.invoke(statement, args));
1819+
});
1820+
ResultSet resultSet =
1821+
testing.runWithSpan("parent", () -> proxyStatement.executeQuery("SELECT 3"));
1822+
1823+
resultSet.next();
1824+
assertThat(resultSet.getInt(1)).isEqualTo(3);
1825+
testing.waitAndAssertTraces(
1826+
trace ->
1827+
trace.hasSpansSatisfyingExactly(
1828+
span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
1829+
span ->
1830+
span.hasName("wrapper").hasKind(SpanKind.INTERNAL).hasParent(trace.getSpan(0)),
1831+
span ->
1832+
span.hasName("SELECT " + dbNameLower)
1833+
.hasKind(SpanKind.CLIENT)
1834+
.hasParent(trace.getSpan(1))));
1835+
}
17401836
}

‎instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/ProxyStatementFactory.java‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,50 +12,61 @@
1212

1313
public final class ProxyStatementFactory {
1414

15-
public static Statement proxyStatement(Statement statement) throws Exception {
15+
public static Statement proxyStatementWithCustomClassLoader(Statement statement)
16+
throws Exception {
1617
TestClassLoader classLoader = new TestClassLoader(ProxyStatementFactory.class.getClassLoader());
1718
Class<?> testInterface = classLoader.loadClass(TestInterface.class.getName());
1819
if (testInterface.getClassLoader() != classLoader) {
1920
throw new IllegalStateException("wrong class loader");
2021
}
2122
InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args);
22-
Statement proxyStatement =
23-
(Statement)
24-
Proxy.newProxyInstance(
25-
classLoader, new Class<?>[] {Statement.class, testInterface}, invocationHandler);
26-
// adding package private interface TestInterface to jdk proxy forces defining the proxy class
27-
// in the same package as the package private interface
28-
if (!proxyStatement
29-
.getClass()
30-
.getName()
31-
.startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) {
32-
throw new IllegalStateException("proxy statement is in wrong package");
33-
}
23+
return proxy(
24+
Statement.class,
25+
classLoader,
26+
new Class<?>[] {Statement.class, testInterface},
27+
invocationHandler);
28+
}
3429

35-
return proxyStatement;
30+
public static Statement proxyStatement(InvocationHandler invocationHandler) {
31+
return proxy(Statement.class, invocationHandler);
3632
}
3733

3834
public static PreparedStatement proxyPreparedStatement(PreparedStatement statement) {
3935
InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args);
40-
PreparedStatement proxyStatement =
41-
(PreparedStatement)
42-
Proxy.newProxyInstance(
43-
ProxyStatementFactory.class.getClassLoader(),
44-
new Class<?>[] {PreparedStatement.class, TestInterface.class},
45-
invocationHandler);
36+
return proxyPreparedStatement(invocationHandler);
37+
}
38+
39+
public static PreparedStatement proxyPreparedStatement(InvocationHandler invocationHandler) {
40+
return proxy(PreparedStatement.class, invocationHandler);
41+
}
42+
43+
public static <T> T proxy(Class<T> clazz, InvocationHandler invocationHandler) {
44+
return proxy(
45+
clazz,
46+
ProxyStatementFactory.class.getClassLoader(),
47+
new Class<?>[] {clazz, TestInterface.class},
48+
invocationHandler);
49+
}
50+
51+
public static <T> T proxy(
52+
Class<T> clazz,
53+
ClassLoader classLoader,
54+
Class<?>[] interfaces,
55+
InvocationHandler invocationHandler) {
56+
T proxy = clazz.cast(Proxy.newProxyInstance(classLoader, interfaces, invocationHandler));
4657

4758
// adding package private interface TestInterface to jdk proxy forces defining the proxy class
4859
// in the same package as the package private interface
4960
// by default we ignore jdk proxies, having the proxy in a different package ensures it gets
5061
// instrumented
51-
if (!proxyStatement
62+
if (!proxy
5263
.getClass()
5364
.getName()
5465
.startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) {
55-
throw new IllegalStateException("proxy statement is in wrong package");
66+
throw new IllegalStateException("proxy is in wrong package");
5667
}
5768

58-
return proxyStatement;
69+
return proxy;
5970
}
6071

6172
private ProxyStatementFactory() {}

0 commit comments

Comments
 (0)