Release pending changes #321

dividedmind · 2026-01-20T16:44:07Z

This releases changes from:

The AppMap agent's logging has been enhanced for better readability and detail. This includes: - Setting a standardized log format for all messages to yyyy-MM-dd HH:mm:ss [thread] AppMap level: message. - Refining the AppMapConfig toString() method to provide a more structured and comprehensive output of the configuration details, including name, config file path, and package information. - Adjusting log levels for system properties output in Agent.java from info to debug, and removing a redundant stack trace in debug mode for cleaner logs.

The logic for extracting parameter names from `LocalVariableAttribute` was quite complex and interleaved with the parameter value construction. This commit extracts that logic into a new private static method `getParameterNames` to improve readability and maintainability of the `Parameters` constructor. As opposed to the previous implementation, the new method traverses all local variable tables (as the spec suggests there can be several) and doesn't spam the logs if debug info is missing. Additionally: - Updated `Parameters` constructor to use the new `getParameterNames` method. - Replaced `this.staticParameters.clone()` with `this.staticParameters.freshCopy()` for clarity, as it's not a deep clone but a copy of value types, kinds and names. - Cleaned up unused imports and removed `clear()` method as it's not used and modifies the object unexpectedly. - Made minor improvements to null checks and error handling within `Parameters` methods for robustness.

Refactor `Hook.apply` to return a `Set<ApplicationAction>` indicating whether the method was marked for ByteBuddy instrumentation or instrumented by Javassist. This allows `ClassFileTransformer` to conditionally apply the `AppMapInstrumented` annotation only when ByteBuddy instrumentation is actually needed. Additionally, add improved logging for Javassist instrumentation failures and guard against excessive logging of these exceptions.

This commit ensures consistent UTF-8 character encoding for AppMap data across file system operations and HTTP responses, preventing corruption on systems where the default charset is not UTF-8 (e.g., Windows-1252). Key changes: - `RecordingSession.java`: Enforce `StandardCharsets.UTF_8` when creating writers for AppMap files, replacing reliance on default `FileWriter`. - `Recording.java`: Explicitly use `StandardCharsets.UTF_8` in `readFully` to correctly decode AppMap content during retrieval. - `ServletRequest.java`: Set `Content-Type: application/json; charset=UTF-8` for remote recording responses and calculate `Content-Length` based on UTF-8 byte size rather than string length. - `agent/test/encoding/`: Add a comprehensive regression test suite (`encoding.bats`, `UnicodeTest.java`, `ReadFullyTest.java`) to verify encoding handling for both reading and writing operations under non-UTF-8 environment settings.

… leaks The fastjson JSONWriter.close() method does not close the underlying Writer passed to its constructor - it only closes the internal SerializeWriter, which has no effect. This caused FileOutputStream and OutputStreamWriter instances to leak in RecordingSession. Changes: - Make AppMapSerializer implement AutoCloseable and track the underlying Writer to close it explicitly - Add try-with-resources for AppMapSerializer in checkpoint() to ensure proper cleanup on exceptions - Add try-finally in stop() to ensure serializer is closed even if write() fails - Consolidate JSON finalization and closing logic into close() method, removing the redundant finish() method Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

The code previously used 9090 which is memorable enough to conflict with other code that could be running on developer machines. New default of 46406 is unlikely to hit such conflicts. Also, some small improvements in the httpcore bats script.

Upgrades the project to use Gradle 9.2.1, allowing the build to run on Java 21 environments while maintaining Java 8 bytecode compatibility. Key changes: - Upgrade Gradle Wrapper to 9.2.1. - Update CI workflows to use Java 21. - Set `options.release = 8` to ensure Java 8 compatibility (replaces obsolete source/target compatibility). - content: upgrade `shadow` plugin to 9.3.1 and `nexus-publish` to 2.0.0. - Fix Gradle 9 deprecation warnings (property assignment syntax). - Fix `ShadowRelocation` task compatibility with new Gradle API. - Bump Gradle version to 8.5 in some test code incompatible with 9. - Make smoke tests (event, access) self-contained and directory-agnostic, removing the need for 'gradlew testClasses' during test installation.

The AppMap runtime JAR must be appended to the bootstrap class loader search path. This ensures that core AppMap runtime classes, such as `HookFunctions`, are available to all application classes, regardless of the specific class loader (e.g., in web servers like Tomcat). This change fixes `NoClassDefFoundError` issues for `HookFunctions`. This commit also adds a new `gretty-tomcat` test to verify the fix in a servlet environment.

- Update `Agent.java` to use `Agent.class.getResource()` instead of `Agent.class.getClassLoader().getResource()` when locating the agent JAR. This prevents a `NullPointerException` when the agent is loaded by the bootstrap class loader (where `getClassLoader()` returns null). - Modify `Properties.java` to automatically default `appmap.debug.disableGit` to `true` if the agent is running on the bootstrap classpath. This avoids crashes in JGit initialization, which relies on `ResourceBundle` loading that is problematic in the bootstrap context. - Add a warning log in `Agent.premain` when running on the bootstrap classpath, advising that this configuration is for troubleshooting only.

Introduces a new configuration property `appmap.hooks.exclude` to allow disabling specific AppMap hook classes by their fully qualified name. This addresses issues where certain hooks, such as `SqlQuery`, might cause `NoClassDefFoundError` due to classloading conflicts or unexpected interactions with the target application's environment. The new property can be set via a system property `-Dappmap.hooks.exclude=<CLASS_NAME>` or an environment variable `APPMAP_HOOKS_EXCLUDE=<CLASS_NAME>`. The agent's `ClassFileTransformer` now checks this exclusion list during hook processing, preventing the instrumentation of specified hook classes.

This commit comprehensively refactors the AppMapPackage class to improve readability, performance, and maintainability. Replace linear exclusion matching with a PrefixTrie data structure, reducing lookup complexity from O(N*M) to O(M), where N is the number of exclusion patterns and M is the length of the class name. This provides dramatic performance improvements for configurations with many exclusion patterns. Exclusion patterns can now be specified relative to the package path (e.g., "internal" instead of "com.example.internal"), improving configuration clarity while maintaining backward compatibility. Add comprehensive documentation explaining the two mutually exclusive configuration modes (exclude mode vs. methods mode). Refactor the complex find() method into three clear helpers with explicit mode detection. Add a warning when both 'exclude' and 'methods' are specified, making the precedence rules explicit to users. Enhance LabelConfig to support matching against both simple and fully qualified class names for better user experience. Remove unused class resolution logic. Add 42 comprehensive tests covering both configuration modes, edge cases, regex patterns, backward compatibility, and complex scenarios. - Fix NullPointerException when 'exclude' field is empty in appmap.yml - Fix package boundary matching (prevent "com.examples" matching "com.example") - Remove unused 'allMethods' field (added in 2022, never implemented) - Remove obsolete pattern threshold warning (no longer needed with PrefixTrie) - Clean up unused imports

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 <noreply@anthropic.com>

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 <noreply@anthropic.com>

Copilot

Pull request overview

This PR prepares for a release by updating build infrastructure and adding new features. The changes primarily focus on modernizing the Gradle build system, consolidating test infrastructure, and adding comprehensive Oracle database and encoding support.

Changes:

Upgraded Gradle from 7.4.2 to 9.2.1 and updated multiple plugins/dependencies
Consolidated Gradle wrapper usage across tests to a single shared wrapper
Added Oracle JDBC testing support with snapshot-based verification
Added new utilities (PrefixTrie) and improved error handling in Hook instrumentation

Reviewed changes

Copilot reviewed 107 out of 125 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
gradle/wrapper/gradle-wrapper.properties	Upgraded Gradle to 9.2.1
build.gradle	Updated nexus-publish plugin (1.1.0→2.0.0) and checkstyle (9.3→13.0.0)
buildSrc/build.gradle	Migrated from jcenter to mavenCentral, upgraded Shadow plugin
runtime/build.gradle	Removed jcenter, replaced sourceCompatibility with options.release
annotation/build.gradle	Updated to use options.release and modern Gradle syntax
agent/test/helper.bash	Added shared gradlew function and improved process management
agent/test/jdbc/*	Added comprehensive Oracle JDBC testing with snapshots
agent/test/encoding/*	Added encoding tests for Windows-1252 and UTF-8 handling
agent/test/gretty-tomcat/*	Added new Gretty-Tomcat integration test
agent/src/main/java/com/appland/appmap/util/PrefixTrie.java	New utility for efficient prefix matching
agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java	Improved error handling and added debug logging

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T16:50:28Z

agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java

+
+    try {
+      // note this will fail to prepare on h2 but only fail on execution on oracle
+      connection.prepareStatement("INVALID SQL").execute();


This PreparedStatement is not always closed on method exit.

Copilot · 2026-01-20T16:50:28Z

agent/src/main/java/com/appland/appmap/record/RecordingSession.java

      this.tmpPath = Files.createTempFile(null, ".appmap.json");
      this.tmpPath.toFile().deleteOnExit();
-      this.serializer = AppMapSerializer.open(new FileWriter(this.tmpPath.toFile()));
+      FileOutputStream fileOutputStream = new FileOutputStream(this.tmpPath.toFile());


This FileOutputStream is not always closed on method exit.

Copilot · 2026-01-20T16:50:28Z

agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java

+
+  @AfterEach
+  public void tearDown() throws SQLException {
+    try (Statement statement = connection.createStatement()) {


Variable connection may be null at this access as suggested by this null guard.

Copilot · 2026-01-20T16:50:29Z

agent/src/main/java/com/appland/appmap/record/RecordingSession.java

+      try (RandomAccessFile raf = new RandomAccessFile(targetPath.toFile(), "rw")) {
+        Writer fw = new OutputStreamWriter(new OutputStream() {
+          @Override
+          public void write(int b) throws IOException {
+            raf.write(b);
+          }
+        }, StandardCharsets.UTF_8);
+        raf.seek(targetPath.toFile().length());
+
+        if (eventReceived) {


This class extends 'java.io.OutputStream' and implements write, but does not override 'write(byte[],int,int)'.

Suggested change

try (RandomAccessFile raf = new RandomAccessFile(targetPath.toFile(), "rw")) {

Writer fw = new OutputStreamWriter(new OutputStream() {

@Override

public void write(int b) throws IOException {

raf.write(b);

}

}, StandardCharsets.UTF_8);

raf.seek(targetPath.toFile().length());

if (eventReceived) {

// By using RandomAccessFile we can erase that character.

// If we don't let the JSON writer write the "begin object" token, it refuses

// to do anything else properly either.

try (RandomAccessFile raf = new RandomAccessFile(targetPath.toFile(), "rw")) {

Writer fw = new OutputStreamWriter(new OutputStream() {

@Override

public void write(int b) throws IOException {

raf.write(b);

}

@Override

public void write(byte[] b, int off, int len) throws IOException {

raf.write(b, off, len);

}

}, StandardCharsets.UTF_8);

Copilot · 2026-01-20T16:50:29Z

agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java

-      logger.debug(e);
+    String beforeSrcBlock = beforeSrcBlock(uniqueLocks.toString(),
+        invocations[MethodEvent.METHOD_INVOCATION.getIndex()]);
+    logger.trace("{}: beforeSrcBlock:\n{}", targetBehavior::getName, beforeSrcBlock::toString);


Redundant call to 'toString' on a String object.

Copilot · 2026-01-20T16:50:31Z

agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java

+  @ArgumentArray
  @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN)
-  public static void execute(Event event, Statement s, Object returnValue, String sql, String[] columnNames) {
+  public static void execute(Event event, Statement s, Object returnValue, Object[] args) {


Method SqlQuery.execute(..) could be confused with overloaded method execute, since dispatch depends on static types.

Copilot · 2026-01-20T16:50:31Z

agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java


  @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN)
-  public static void addBatch(Event event, Statement s, Object returnValue, String sql) {
+  public static void executeLargeBatch(Event event, Statement s, Object returnValue) {


Method SqlQuery.executeLargeBatch(..) could be confused with overloaded method executeLargeBatch, since dispatch depends on static types.

Suggested change

public static void executeLargeBatch(Event event, Statement s, Object returnValue) {

public static void executeLargeBatch(Event event, Statement s, long[] returnValue) {

Copilot · 2026-01-20T16:50:32Z

agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java

-  @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN)
-  public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, int[] columnIndexes) {
+  @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN)
+  public static void executeBatch(Event event, Statement s, Object returnValue) {


Method SqlQuery.executeBatch(..) could be confused with overloaded method executeBatch, since dispatch depends on static types.

Suggested change

public static void executeBatch(Event event, Statement s, Object returnValue) {

public static void executeBatch(Event event, Statement s, int[] updateCounts) {

Copilot · 2026-01-20T16:50:32Z

agent/src/main/java/com/appland/appmap/record/RecordingSession.java

+        try (AppMapSerializer serializer = AppMapSerializer.reopen(fw, raf)) {
+          serializer.write(this.getClassMap(), this.metadata, this.eventUpdates);


Potentially confusing name: method checkpoint also refers to field serializer (as this.serializer).

Suggested change

try (AppMapSerializer serializer = AppMapSerializer.reopen(fw, raf)) {

serializer.write(this.getClassMap(), this.metadata, this.eventUpdates);

try (AppMapSerializer checkpointSerializer = AppMapSerializer.reopen(fw, raf)) {

checkpointSerializer.write(this.getClassMap(), this.metadata, this.eventUpdates);

Copilot · 2026-01-20T16:50:32Z

agent/src/main/java/com/appland/appmap/record/RecordingSession.java

+        if (this.serializer != null) {
+          this.serializer.close();
+        }


This check is useless. this.serializer cannot be null at this check, since it is guarded by ... == ....

Suggested change

if (this.serializer != null) {

this.serializer.close();

}

this.serializer.close();

dividedmind and others added 19 commits January 15, 2026 04:19

ci: Add github token to avoid rate limiting in intellij tests

02801aa

fix: Don't throw when loading logging config fails

6704c09

chore: Update Jackson dependency

5ce1574

chore: Update org.reflections dependency

ac25e78

chore: Update checkstyle dependency

be30cc0

chore: Update commons-lang3

4844090

dividedmind requested a review from Copilot January 20, 2026 16:44

dividedmind self-assigned this Jan 20, 2026

dividedmind requested review from hleb-rubanau and kgilpin January 20, 2026 16:44

dividedmind marked this pull request as ready for review January 20, 2026 16:44

Copilot started reviewing on behalf of dividedmind January 20, 2026 16:44 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Release pending changes #321

Release pending changes #321

Uh oh!

dividedmind commented Jan 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	public static void executeLargeBatch(Event event, Statement s, Object returnValue) {
	public static void executeLargeBatch(Event event, Statement s, long[] returnValue) {

	public static void executeBatch(Event event, Statement s, Object returnValue) {
	public static void executeBatch(Event event, Statement s, int[] updateCounts) {

		try (AppMapSerializer serializer = AppMapSerializer.reopen(fw, raf)) {
		serializer.write(this.getClassMap(), this.metadata, this.eventUpdates);

Release pending changes #321

Are you sure you want to change the base?

Release pending changes #321

Uh oh!

Conversation

dividedmind commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dividedmind commented Jan 20, 2026 •

edited

Loading