From 558a7bfedba573018ab0bb9200e8e776352a1d09 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 23 Sep 2025 14:11:52 +0300 Subject: [PATCH 1/3] Exclude jdbc wrappers from instrumentation --- .../jdbc/ConnectionInstrumentation.java | 8 +++++ .../instrumentation/jdbc/JdbcSingletons.java | 29 +++++++++++++++++++ .../PreparedStatementInstrumentation.java | 17 +++++++++++ .../jdbc/StatementInstrumentation.java | 16 ++++++++-- 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index 3f356fb6db32..652612070db9 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -61,6 +61,10 @@ public static class PrepareAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void addDbInfo( @Advice.Argument(0) String sql, @Advice.Return PreparedStatement statement) { + if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) { + return; + } + JdbcData.preparedStatement.set(statement, sql); } } @@ -74,6 +78,10 @@ public static void onEnter( @Advice.Origin("#m") String methodName, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { + if (JdbcSingletons.isWrapper(connection, Connection.class)) { + return; + } + Context parentContext = currentContext(); DbRequest request = DbRequest.createTransaction(connection, methodName.toUpperCase(Locale.ROOT)); diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java index 5a76dff89e1e..4476a9e2b104 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java @@ -11,12 +11,15 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceAttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.internal.cache.Cache; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory; import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter; import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; +import java.sql.SQLException; +import java.sql.Wrapper; import java.util.Collections; import javax.sql.DataSource; @@ -68,5 +71,31 @@ public static Instrumenter dataSourceInstrumenter() { return DATASOURCE_INSTRUMENTER; } + private static final Cache, Boolean> wrapperClassCache = Cache.weak(); + + /** + * Returns true if the given object is a wrapper and shouldn't be instrumented. We'll instrument + * the underlying object called by the wrapper instead. + */ + public static boolean isWrapper(T object, Class clazz) { + return wrapperClassCache.computeIfAbsent( + object.getClass(), key -> isWrapperInternal(object, clazz)); + } + + private static boolean isWrapperInternal(T object, Class clazz) { + try { + // we are dealing with a wrapper when the object unwraps to a different instance + if (object.isWrapperFor(clazz)) { + T unwrapped = object.unwrap(clazz); + if (object != unwrapped) { + return true; + } + } + } catch (SQLException | AbstractMethodError e) { + // ignore + } + return false; + } + private JdbcSingletons() {} } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java index ea0ec5302c23..acb090544c6d 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -118,6 +118,10 @@ public static void onEnter( return; } + if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) { + return; + } + // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info // this happens before the DB CLIENT span is started (and put in the current context), so this // instrumentation runs again and the shouldStartSpan() check always returns true - and so on @@ -165,6 +169,10 @@ public static class AddBatchAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void addBatch(@Advice.This PreparedStatement statement) { + if (JdbcSingletons.isWrapper(statement, Statement.class)) { + return; + } + JdbcData.addPreparedStatementBatch(statement); } } @@ -179,6 +187,9 @@ public static void onExit( if (!CAPTURE_QUERY_PARAMETERS) { return; } + if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) { + return; + } String str = null; @@ -211,6 +222,9 @@ public static void onExit( if (!CAPTURE_QUERY_PARAMETERS) { return; } + if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) { + return; + } String str = null; @@ -243,6 +257,9 @@ public static void onExit( if (!CAPTURE_QUERY_PARAMETERS) { return; } + if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) { + return; + } String str = null; diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java index 81b252b87fa2..3d10d15c5cb9 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java @@ -72,6 +72,10 @@ public static void onEnter( @Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { + if (JdbcSingletons.isWrapper(statement, Statement.class)) { + return; + } + // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info // this happens before the DB CLIENT span is started (and put in the current context), so this // instrumentation runs again and the shouldStartSpan() check always returns true - and so on @@ -102,7 +106,7 @@ public static void stopSpan( @Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - if (callDepth.decrementAndGet() > 0) { + if (callDepth == null || callDepth.decrementAndGet() > 0) { return; } @@ -121,6 +125,10 @@ public static void addBatch(@Advice.This Statement statement, @Advice.Argument(0 if (statement instanceof PreparedStatement) { return; } + if (JdbcSingletons.isWrapper(statement, Statement.class)) { + return; + } + JdbcData.addStatementBatch(statement, sql); } } @@ -144,6 +152,10 @@ public static void onEnter( @Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { + if (JdbcSingletons.isWrapper(statement, Statement.class)) { + return; + } + // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info // this happens before the DB CLIENT span is started (and put in the current context), so this // instrumentation runs again and the shouldStartSpan() check always returns true - and so on @@ -190,7 +202,7 @@ public static void stopSpan( @Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - if (callDepth.decrementAndGet() > 0) { + if (callDepth == null || callDepth.decrementAndGet() > 0) { return; } From ccb9505ae551cfb4021f03201cca95f6ce146b8d Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 25 Sep 2025 17:51:59 +0300 Subject: [PATCH 2/3] spotless --- .../instrumentation/jdbc/StatementInstrumentation.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java index 8ca35329a43c..9a192535ca87 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java @@ -72,7 +72,8 @@ public static JdbcAdviceScope onEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter @Nullable JdbcAdviceScope adviceScope) { + @Advice.Thrown @Nullable Throwable throwable, + @Advice.Enter @Nullable JdbcAdviceScope adviceScope) { if (adviceScope != null) { adviceScope.end(throwable); } @@ -119,7 +120,8 @@ public static JdbcAdviceScope onEnter(@Advice.This Statement statement) { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( - @Advice.Thrown @Nullable Throwable throwable, @Advice.Enter @Nullable JdbcAdviceScope adviceScope) { + @Advice.Thrown @Nullable Throwable throwable, + @Advice.Enter @Nullable JdbcAdviceScope adviceScope) { if (adviceScope != null) { adviceScope.end(throwable); } From ba15c666f87965280d2421f1c174bf7f470f1db4 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 25 Sep 2025 17:52:13 +0300 Subject: [PATCH 3/3] add test --- .../jdbc/test/JdbcInstrumentationTest.java | 98 ++++++++++++++++++- .../jdbc/test/ProxyStatementFactory.java | 57 ++++++----- 2 files changed, 131 insertions(+), 24 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java index 9c887b560657..b57f1a96a447 100644 --- a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java +++ b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java @@ -1330,7 +1330,7 @@ void testProxyStatement() throws Exception { cleanup.deferCleanup(statement); cleanup.deferCleanup(connection); - Statement proxyStatement = ProxyStatementFactory.proxyStatement(statement); + Statement proxyStatement = ProxyStatementFactory.proxyStatementWithCustomClassLoader(statement); ResultSet resultSet = testing.runWithSpan("parent", () -> proxyStatement.executeQuery("SELECT 3")); @@ -1737,4 +1737,100 @@ static Stream transactionOperationsStream() throws SQLException { Arguments.of( "hsqldb", new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null), "SA", "hsqldb:mem:")); } + + private static PreparedStatement wrapPreparedStatement(PreparedStatement statement) { + return ProxyStatementFactory.proxyPreparedStatement( + (proxy, method, args) -> { + if ("isWrapperFor".equals(method.getName()) + && args.length == 1 + && args[0] == PreparedStatement.class) { + return true; + } + if ("unwrap".equals(method.getName()) + && args.length == 1 + && args[0] == PreparedStatement.class) { + return statement; + } + return testing.runWithSpan("wrapper", () -> method.invoke(statement, args)); + }); + } + + // test that tracing does not start from a wrapper + // regression test for + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/14733 + @Test + void testPreparedStatementWrapper() throws SQLException { + Connection connection = new org.h2.Driver().connect(jdbcUrls.get("h2"), null); + Connection proxyConnection = + ProxyStatementFactory.proxy( + Connection.class, + (proxy, method, args) -> { + // we don't implement unwrapping here as that would also cause the executeQuery + // instrumentation to get skipped for the prepared statement wrapper + if ("prepareStatement".equals(method.getName())) { + return wrapPreparedStatement((PreparedStatement) method.invoke(connection, args)); + } + return method.invoke(connection, args); + }); + PreparedStatement statement = proxyConnection.prepareStatement("SELECT 3"); + cleanup.deferCleanup(statement); + cleanup.deferCleanup(connection); + + ResultSet resultSet = testing.runWithSpan("parent", () -> statement.executeQuery()); + + resultSet.next(); + assertThat(resultSet.getInt(1)).isEqualTo(3); + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), + span -> + span.hasName("wrapper").hasKind(SpanKind.INTERNAL).hasParent(trace.getSpan(0)), + span -> + span.hasName("SELECT " + dbNameLower) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(1)))); + } + + // test that tracing does not start from a wrapper + // regression test for + // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/14733 + @Test + void testStatementWrapper() throws SQLException { + Connection connection = new org.h2.Driver().connect(jdbcUrls.get("h2"), null); + Statement statement = connection.createStatement(); + cleanup.deferCleanup(statement); + cleanup.deferCleanup(connection); + + Statement proxyStatement = + ProxyStatementFactory.proxyStatement( + (proxy, method, args) -> { + if ("isWrapperFor".equals(method.getName()) + && args.length == 1 + && args[0] == Statement.class) { + return true; + } + if ("unwrap".equals(method.getName()) + && args.length == 1 + && args[0] == Statement.class) { + return statement; + } + return testing.runWithSpan("wrapper", () -> method.invoke(statement, args)); + }); + ResultSet resultSet = + testing.runWithSpan("parent", () -> proxyStatement.executeQuery("SELECT 3")); + + resultSet.next(); + assertThat(resultSet.getInt(1)).isEqualTo(3); + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), + span -> + span.hasName("wrapper").hasKind(SpanKind.INTERNAL).hasParent(trace.getSpan(0)), + span -> + span.hasName("SELECT " + dbNameLower) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(1)))); + } } diff --git a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/ProxyStatementFactory.java b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/ProxyStatementFactory.java index f44b26336d9a..3b5cf1dc7b1c 100644 --- a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/ProxyStatementFactory.java +++ b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/ProxyStatementFactory.java @@ -12,50 +12,61 @@ public final class ProxyStatementFactory { - public static Statement proxyStatement(Statement statement) throws Exception { + public static Statement proxyStatementWithCustomClassLoader(Statement statement) + throws Exception { TestClassLoader classLoader = new TestClassLoader(ProxyStatementFactory.class.getClassLoader()); Class testInterface = classLoader.loadClass(TestInterface.class.getName()); if (testInterface.getClassLoader() != classLoader) { throw new IllegalStateException("wrong class loader"); } InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args); - Statement proxyStatement = - (Statement) - Proxy.newProxyInstance( - classLoader, new Class[] {Statement.class, testInterface}, invocationHandler); - // adding package private interface TestInterface to jdk proxy forces defining the proxy class - // in the same package as the package private interface - if (!proxyStatement - .getClass() - .getName() - .startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) { - throw new IllegalStateException("proxy statement is in wrong package"); - } + return proxy( + Statement.class, + classLoader, + new Class[] {Statement.class, testInterface}, + invocationHandler); + } - return proxyStatement; + public static Statement proxyStatement(InvocationHandler invocationHandler) { + return proxy(Statement.class, invocationHandler); } public static PreparedStatement proxyPreparedStatement(PreparedStatement statement) { InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args); - PreparedStatement proxyStatement = - (PreparedStatement) - Proxy.newProxyInstance( - ProxyStatementFactory.class.getClassLoader(), - new Class[] {PreparedStatement.class, TestInterface.class}, - invocationHandler); + return proxyPreparedStatement(invocationHandler); + } + + public static PreparedStatement proxyPreparedStatement(InvocationHandler invocationHandler) { + return proxy(PreparedStatement.class, invocationHandler); + } + + public static T proxy(Class clazz, InvocationHandler invocationHandler) { + return proxy( + clazz, + ProxyStatementFactory.class.getClassLoader(), + new Class[] {clazz, TestInterface.class}, + invocationHandler); + } + + public static T proxy( + Class clazz, + ClassLoader classLoader, + Class[] interfaces, + InvocationHandler invocationHandler) { + T proxy = clazz.cast(Proxy.newProxyInstance(classLoader, interfaces, invocationHandler)); // adding package private interface TestInterface to jdk proxy forces defining the proxy class // in the same package as the package private interface // by default we ignore jdk proxies, having the proxy in a different package ensures it gets // instrumented - if (!proxyStatement + if (!proxy .getClass() .getName() .startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) { - throw new IllegalStateException("proxy statement is in wrong package"); + throw new IllegalStateException("proxy is in wrong package"); } - return proxyStatement; + return proxy; } private ProxyStatementFactory() {}