Unify jdbc tests between javaagent and library instrumentation #15137

trask · 2025-10-24T18:57:10Z

No description provided.

trask · 2025-10-25T16:00:07Z

...a/io/opentelemetry/instrumentation/jdbc/testing/AbstractPreparedStatementParametersTest.java

only change compared to the old javaagent PreparedStatementParametersTest is calling wrap()

EDIT and added test for setByte

trask · 2025-10-25T17:27:46Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+    if (init != null) {
+      init.accept(ds);
+    }
+    Class<?> originalDatasourceClass = ds.getClass();


capturing the original class here before to use in verification later

trask · 2025-10-25T18:04:25Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

see review comments for changes to this file other than calling wrap()

trask · 2025-10-25T18:07:20Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+    return Stream.of(
+        Arguments.of(
+            new JdbcDataSource(),
+            (Consumer<DataSource>) ds -> ((JdbcDataSource) ds).setURL(jdbcUrls.get("h2")),


this changed from setUrl in the javaagent test to setURL in this module because in the javaagent module, h2 version is overridden for slick test

trask · 2025-10-25T18:20:45Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+    try {
+      ds.setDriverClass(jdbcDriverClassNames.get(dbType));
+    } catch (PropertyVetoException e) {
+      throw new IllegalStateException(e);


this changed from RuntimeException to IllegalStateException b/c it's now in src/main and so errorprone rules apply

trask · 2025-10-25T20:14:57Z

.../src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/DruidDataSourceTest.java


 @SuppressWarnings("deprecation") // using deprecated semconv
-class DruicDataSourceTest {
+class DruidDataSourceTest {


(unrelated fix)

trask · 2025-10-25T20:18:03Z

...ntation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestConnection.java

-  private String url;
-  Consumer<String> sqlConsumer = unused -> {};

-  public TestConnection() {}
+  private final String url;
+  private final Consumer<String> sqlConsumer;


unrelated simplification

trask · 2025-10-25T20:38:11Z

...rc/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java

removed tests from here that are now covered by the shared tests

trask · 2025-10-25T20:54:22Z

...ng/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractSqlCommenterTest.java

only change compared to the old javaagent PreparedStatementParametersTest is calling wrap()

trask · 2025-10-26T20:23:53Z

...n/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java

    transformer.applyAdviceToMethod(
        namedOneOf(
                "setBoolean",
+                "setByte",


added to match library instrumentation

good catch. Down below there are 2 lines with comment // Short, Int, Long, Float, Double, BigDecimal could also add Byte to the list there

trask · 2025-10-27T02:07:50Z

instrumentation/jdbc/javaagent/build.gradle.kts

  // these dependencies are for SlickTest
  testImplementation("org.scala-lang:scala-library:2.11.12")
  testImplementation("com.typesafe.slick:slick_2.11:3.2.0")
-  testImplementation("com.h2database:h2:1.4.197")


test seems to pass with version 1.3.169 which is specified above

trask · 2025-10-27T02:08:48Z

...main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java

+    putParameter(parameterIndex, x);
  }

  @SuppressWarnings("UngroupedOverloads")
  @Override
  public void setObject(int parameterIndex, Object x) throws SQLException {
    delegate.setObject(parameterIndex, x);
+    putParameter(parameterIndex, x);


for parity with javaagent instrumentation

agent only captures the object if it is of a recognized type, see

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

Lines 157 to 167 in 5c145a8

if (value instanceof Boolean

// Short, Int, Long, Float, Double, BigDecimal

|| value instanceof Number

|| value instanceof String

|| value instanceof Date

|| value instanceof Time

|| value instanceof Timestamp

|| value instanceof URL

|| value instanceof RowId) {

str = value.toString();

}

trask · 2025-10-27T02:11:04Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+        Arguments.of(
+            "derby",
+            new EmbeddedDriver().connect(jdbcUrls.get("derby"), null),
+            "APP",
+            "CREATE TABLE PS_LARGE_DERBY (id INTEGER not NULL, PRIMARY KEY ( id ))",
+            "CREATE TABLE jdbcunittest.PS_LARGE_DERBY",
+            "derby:memory:",
+            "PS_LARGE_DERBY"),
+        Arguments.of(
+            "hsqldb",
+            new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null),
+            "SA",
+            "CREATE TABLE PUBLIC.PS_LARGE_HSQLDB (id INTEGER not NULL, PRIMARY KEY ( id ))",
+            "CREATE TABLE PUBLIC.PS_LARGE_HSQLDB",
+            "hsqldb:mem:",
+            "PUBLIC.PS_LARGE_HSQLDB"));


added derby and hsqldb for more "large" method testing

trask · 2025-10-27T02:11:51Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+    if (Boolean.getBoolean("testLatestDeps")) {
+      testPreparedStatementUpdateImpl(
+          system,
+          connection,
+          username,
+          query,
+          spanName,
+          url,
+          table,
+          statement -> assertThat(statement.executeLargeUpdate()).isEqualTo(0));
+    } else {
+      // Older drivers don't support JDBC 4.2, expect UnsupportedOperationException
+      // This is the correct behavior - instrumentation should not change driver behavior
+      String sql = connection.nativeSQL(query);
+      PreparedStatement statement = connection.prepareStatement(sql);
+      cleanup.deferCleanup(statement);
+      assertThatThrownBy(statement::executeLargeUpdate)
+          .isInstanceOf(UnsupportedOperationException.class);
+    }


verifies both new and old driver behavior now

trask · 2025-10-27T02:12:08Z

...main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java

+    if (Boolean.getBoolean("testLatestDeps")) {
+      assertThat(statement.executeLargeBatch()).isEqualTo(new long[] {1, 1});
+    } else {
+      // Older drivers don't support JDBC 4.2, expect UnsupportedOperationException
+      // This is the correct behavior - instrumentation should not change driver behavior
+      assertThatThrownBy(statement::executeLargeBatch)
+          .isInstanceOf(UnsupportedOperationException.class);
+    }


verifies both new and old driver behavior now

trask · 2025-10-27T02:51:46Z

.../src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/DruidDataSourceTest.java

+
+    // Initialize the connection pool to trigger H2's internal schema setup
+    // This prevents internal H2 spans from appearing in the test assertions
+    dataSource.getConnection().close();
+    testing.clearData();


this change was triggered by the h2 dependency version change in the javaagent module

Often we add testing.waitForTraces(1); before clearData to ensure that the trace is received before we clear it.

trask · 2025-10-27T04:58:26Z

...sting/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/ProxyStatementFactory.java

+  // need to unwrap in order to preserve SQLException behavior
+  private static Object invokeWithUnwrappedTarget(Object target, Method method, Object[] args)


This change also triggered by h2 dependency version change in the javaagent tests

otelbot-java-instrumentation · 2025-10-27T19:32:43Z

🔧 The result from spotlessApply was committed to the PR branch.

github-actions bot added the test native This label can be applied to PRs to trigger them to run native tests label Oct 24, 2025

trask force-pushed the unify-jdbc-tests-2 branch 10 times, most recently from 2d63bcd to 6e4b8c0 Compare October 26, 2025 00:35

trask commented Oct 26, 2025

View reviewed changes

trask force-pushed the unify-jdbc-tests-2 branch from 0ea49ce to 4928d36 Compare October 27, 2025 01:54

trask commented Oct 27, 2025

View reviewed changes

Unify jdbc tests

cf1db4c

trask force-pushed the unify-jdbc-tests-2 branch from 8c72788 to cf1db4c Compare October 27, 2025 04:13

trask commented Oct 27, 2025

View reviewed changes

trask marked this pull request as ready for review October 27, 2025 04:58

trask requested a review from a team as a code owner October 27, 2025 04:58

laurit approved these changes Oct 27, 2025

View reviewed changes

trask added 2 commits October 27, 2025 08:51

feedback

c379244

waitForTraces

35abf7d

trask enabled auto-merge (squash) October 27, 2025 16:37

fix

0304d33

trask disabled auto-merge October 27, 2025 17:37

trask and others added 3 commits October 27, 2025 10:43

Merge remote-tracking branch 'upstream/main' into unify-jdbc-tests-2

0dddcf8

fix merge

a8a700f

./gradlew spotlessApply

beb5c45

trask merged commit bdc098f into open-telemetry:main Oct 27, 2025
83 checks passed

trask deleted the unify-jdbc-tests-2 branch October 27, 2025 20:24

	if (value instanceof Boolean
	// Short, Int, Long, Float, Double, BigDecimal
	\|\| value instanceof Number
	\|\| value instanceof String
	\|\| value instanceof Date
	\|\| value instanceof Time
	\|\| value instanceof Timestamp
	\|\| value instanceof URL
	\|\| value instanceof RowId) {
	str = value.toString();
	}

		// need to unwrap in order to preserve SQLException behavior
		private static Object invokeWithUnwrappedTarget(Object target, Method method, Object[] args)

Uh oh!

Unify jdbc tests between javaagent and library instrumentation #15137

Unify jdbc tests between javaagent and library instrumentation #15137

Conversation

trask commented Oct 24, 2025

Uh oh!

trask Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trask Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

otelbot-java-instrumentation bot commented Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trask Oct 25, 2025 •

edited

Loading

trask Oct 25, 2025 •

edited

Loading