Skip to content

Unify jdbc tests between javaagent and library instrumentation #15137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
trask merged 7 commits into from
Oct 27, 2025
Merged

Unify jdbc tests between javaagent and library instrumentation #15137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cf1db4c
Unify jdbc tests
trask Oct 22, 2025
c379244
feedback
trask Oct 27, 2025
35abf7d
waitForTraces
trask Oct 27, 2025
0304d33
fix
trask Oct 27, 2025
0dddcf8
Merge remote-tracking branch 'upstream/main' into unify-jdbc-tests-2
trask Oct 27, 2025
a8a700f
fix merge
trask Oct 27, 2025
beb5c45
./gradlew spotlessApply
otelbot[bot] Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions instrumentation/jdbc/javaagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,12 @@ dependencies {
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")

// jdbc unit testing
testLibrary("com.h2database:h2:1.3.169")
// first version jdk 1.6 compatible
testLibrary("org.apache.derby:derby:10.6.1.0")
testLibrary("org.hsqldb:hsqldb:2.0.0")

testLibrary("org.apache.tomcat:tomcat-jdbc:7.0.19")
// tomcat needs this to run
testLibrary("org.apache.tomcat:tomcat-juli:7.0.19")
testLibrary("org.apache.tomcat:tomcat-juli:7.0.19") // tomcat jdbc needs this
testLibrary("com.zaxxer:HikariCP:2.4.0")
testLibrary("com.mchange:c3p0:0.9.5")
testLibrary("com.alibaba:druid:1.2.20")
Expand All @@ -42,7 +39,6 @@ dependencies {
// 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")
Copy link
Member Author

@trask trask Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test seems to pass with version 1.3.169 which is specified above

}

sourceSets {
Expand Down
5 changes: 3 additions & 2 deletions ...ava/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
namedOneOf(
"setBoolean",
"setByte",
Copy link
Member Author

@trask trask Oct 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added to match library instrumentation

Copy link
Contributor

@laurit laurit Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"setShort",
"setInt",
"setLong",
Expand Down Expand Up @@ -155,7 +156,7 @@ public static void onExit(
String str = null;

if (value instanceof Boolean
// Short, Int, Long, Float, Double, BigDecimal
// Byte, Short, Int, Long, Float, Double, BigDecimal
|| value instanceof Number
|| value instanceof String
|| value instanceof Date
Expand Down Expand Up @@ -190,7 +191,7 @@ public static void onExit(
String str = null;

if (value instanceof Boolean
// Short, Int, Long, Float, Double, BigDecimal
// Byte, Short, Int, Long, Float, Double, BigDecimal
|| value instanceof Number
|| value instanceof String
|| value instanceof Date
Expand Down
10 changes: 8 additions & 2 deletions ...tation/jdbc/test/DruicDataSourceTest.java → ...tation/jdbc/test/DruidDataSourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,28 @@
import org.junit.jupiter.api.extension.RegisterExtension;

@SuppressWarnings("deprecation") // using deprecated semconv
class DruicDataSourceTest {
class DruidDataSourceTest {
Copy link
Member Author

@trask trask Oct 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(unrelated fix)


@RegisterExtension
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

private DataSource dataSource;

@BeforeEach
void setUp() {
void setUp() throws SQLException {
DruidDataSource druidDataSource = new DruidDataSource();
druidDataSource.setUrl("jdbc:h2:mem:test");
druidDataSource.setDriverClassName("org.h2.Driver");
druidDataSource.setUsername("sa");
druidDataSource.setPassword("");
druidDataSource.setMaxActive(1);
this.dataSource = druidDataSource;

// Initialize the connection pool to trigger H2's internal schema setup
// This prevents internal H2 spans from appearing in the test assertions
testing.runWithSpan("parent", () -> dataSource.getConnection().close());
testing.waitForTraces(1);
testing.clearData();
}

@AfterEach
Expand Down
Loading
Loading