Enhance JDBC instrumentation #318
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the SqlQuery hooks to provide comprehensive support for JDBC operations, particularly PreparedStatements and batch operations. Key changes: - Refactored prepareStatement/prepareCall hooks to capture at METHOD_RETURN and associate SQL strings with the returned statement objects using WeakHashMaps for proper prepared statement support - Added comprehensive batch operation tracking (addBatch, clearBatch, executeBatch, executeLargeBatch) - Added support for executeLargeUpdate (JDBC 4.2) - Consolidated overloaded methods using @ArgumentArray to reduce code duplication and handle all method signatures uniformly - Added proper exception handling hooks for all execute methods - Implemented caching of database names using WeakHashMap to avoid repeated metadata lookups - Removed nativeSQL hooks (doesn't actually execute SQL, just transforms it) - Fixed potential NoClassDefFoundError by changing exception handling from SQLException to Throwable and removing direct class references The SQLException fix addresses an issue in environments with custom classloaders (e.g., Oracle UCP) where java.sql.SQLException might not be visible to the hook class's classloader. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
This commit adds extensive test infrastructure for validating JDBC hook behavior across multiple database systems. Key additions: - PureJDBCTests: Comprehensive unit tests covering all JDBC operations including Statement, PreparedStatement, CallableStatement, batch operations, and exception handling - OracleRepositoryTests: Spring Data JPA integration tests for Oracle - Test infrastructure supporting both H2 (in-memory) and Oracle databases - Docker Compose configuration for running Oracle database in CI - BATS test harness improvements and helper scripts for snapshot testing - Snapshot-based test validation with expected SQL output for both databases - CI integration: GitHub Actions workflow now includes Oracle database service - Build configuration updated to include Oracle JDBC driver Test utilities: - helper.bash: Common test functions and database connection helpers - regenerate_jdbc_snapshots.sh: Script to regenerate expected SQL snapshots - Snapshot files for both H2 and Oracle to validate SQL generation Also adds *.log pattern to .gitignore to prevent accidental commit of debug logs. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR enhances JDBC instrumentation by refactoring SQL query hooks to properly track PreparedStatement and batch operations, improving exception handling, and introducing comprehensive multi-database testing infrastructure.
Changes:
- Refactored
SqlQuery.javato use argument arrays and return value hooks for tracking PreparedStatement SQL, implementing comprehensive batch operation support, and changing exception handling from SQLException to Throwable - Added comprehensive test suite with
PureJDBCTestsfor direct JDBC validation andOracleRepositoryTestsfor JPA integration, supporting both H2 and Oracle databases - Integrated Oracle database service in CI pipeline with Docker Compose configuration and snapshot-based testing framework
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java | Core refactoring of SQL hooks with database name caching, PreparedStatement tracking, and batch operation support |
| agent/src/test/java/com/appland/appmap/process/hooks/SqlQuerySQLExceptionAvailabilityTest.java | Regression test for NoClassDefFoundError with custom classloaders |
| agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java | Comprehensive JDBC test suite covering Statement, PreparedStatement, CallableStatement, and batch operations |
| agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java | Oracle-specific Spring Data JPA integration test |
| agent/test/jdbc/src/test/resources/application-oracle.properties | Oracle database configuration for Spring tests |
| agent/test/jdbc/helper.bash | Helper functions for snapshot generation and validation |
| agent/test/jdbc/regenerate_jdbc_snapshots.sh | Script to regenerate SQL snapshots for both databases |
| agent/test/jdbc/jdbc.bats | Updated test suite with H2 and Oracle snapshot validation |
| agent/test/jdbc/build.gradle | Added Oracle JDBC driver dependency and test inputs configuration |
| agent/test/jdbc/docker-compose.yml | Docker Compose configuration for local Oracle testing |
| agent/test/jdbc/snapshots/* | SQL snapshot files for both H2 and Oracle databases |
| .github/workflows/build-and-test.yml | Added Oracle service configuration to CI workflow |
| .gitignore | Added *.log pattern to ignore log files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java
Show resolved
Hide resolved
agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java
Show resolved
Hide resolved
agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java
Show resolved
Hide resolved
agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enhances JDBC instrumentation by adding comprehensive support for PreparedStatement and batch operations, and introduces a robust testing framework to validate the instrumentation against multiple database systems.
Key changes:
Enhanced
PreparedStatementand Batch Operation Hooks: The SQL query hooks have been refactored to accurately capture and record SQL strings associated with PreparedStatement and CallableStatement objects. This includes comprehensive tracking of batch operations (addBatch, clearBatch, executeBatch, executeLargeBatch) and support for executeLargeUpdate.Improved Exception Handling: Exception handling within the hooks has been improved to prevent NoClassDefFoundError in environments with custom classloaders by catching Throwable instead of SQLException.
Multi-DB Test Infrastructure: A new test suite has been added to validate JDBC instrumentation against both H2 (in-memory) and Oracle databases. This includes:
CI Integration: The GitHub Actions workflow has been updated to include an Oracle database service, allowing for automated testing of the enhanced JDBC instrumentation.
This PR also includes minor improvements such as caching database names to avoid repeated metadata lookups and adding *.log to .gitignore.