From 50f7f21759d2044d2e91644e068db95591835e0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Sun, 4 Jan 2026 05:52:34 +0100 Subject: [PATCH 01/21] feat(logging): Improve AppMap agent logging and config output 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. --- .../main/java/com/appland/appmap/Agent.java | 3 +-- .../appland/appmap/config/AppMapConfig.java | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/Agent.java b/agent/src/main/java/com/appland/appmap/Agent.java index 3de302b7..2aca22a0 100644 --- a/agent/src/main/java/com/appland/appmap/Agent.java +++ b/agent/src/main/java/com/appland/appmap/Agent.java @@ -72,8 +72,7 @@ public static void premain(String agentArgs, Instrumentation inst) { logger.info("Agent version {}, current time mills: {}", implementationVersion, start); logger.info("config: {}", AppMapConfig.get()); - logger.info("System properties: {}", System.getProperties()); - logger.debug(new Exception(), "whereAmI"); + logger.debug("System properties: {}", System.getProperties()); addAgentJars(agentArgs, inst); diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java b/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java index 846d962a..cc49f313 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java +++ b/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java @@ -317,6 +317,8 @@ public static TaggedLogger configureLogging() { // tinylog freezes its configuration after the first call to any of its // methods other than those in Configuration. So, get everything ready // before returning the logger for this class; + Configuration.set("writer.format", "{date:yyyy-MM-dd HH:mm:ss} [{thread}] AppMap {level}: {message}"); + if (Properties.Debug) { Configuration.set("level", "debug"); } @@ -365,6 +367,25 @@ private static Path findDefaultOutputDirectory(FileSystem fs) { @Override public String toString() { - return JSON.toJSONString(this, true); + StringBuilder sb = new StringBuilder(); + sb.append("name: ").append(name).append("\n"); + if (configFile != null) { + sb.append("configFile: ").append(configFile).append("\n"); + } + sb.append("packages: "); + if (packages == null || packages.length == 0) { + sb.append("[]"); + } else { + for (AppMapPackage pkg : packages) { + sb.append("\n - path: ").append(pkg.path); + if (pkg.shallow) { + sb.append("\n shallow: true"); + } + if (pkg.exclude != null && pkg.exclude.length > 0) { + sb.append("\n exclude: ").append(Arrays.toString(pkg.exclude)); + } + } + } + return sb.toString(); } } From 377b850e19d5d0525e93dfa9f9f281de74056ed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Sun, 4 Jan 2026 06:02:42 +0100 Subject: [PATCH 02/21] refactor(parameters): Extract parameter name resolution to a new method 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. --- .../appland/appmap/output/v1/Parameters.java | 137 +++++++++--------- .../appmap/transform/annotations/Hook.java | 2 +- 2 files changed, 67 insertions(+), 72 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/output/v1/Parameters.java b/agent/src/main/java/com/appland/appmap/output/v1/Parameters.java index dd3779fa..be04629e 100644 --- a/agent/src/main/java/com/appland/appmap/output/v1/Parameters.java +++ b/agent/src/main/java/com/appland/appmap/output/v1/Parameters.java @@ -7,11 +7,10 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import javassist.bytecode.AttributeInfo; import org.tinylog.TaggedLogger; import com.appland.appmap.config.AppMapConfig; -import com.appland.appmap.config.Properties; -import com.appland.appmap.util.Logger; import javassist.CtBehavior; import javassist.CtClass; @@ -30,7 +29,7 @@ public class Parameters implements Iterable { private static final TaggedLogger logger = AppMapConfig.getLogger(null); - private final ArrayList values = new ArrayList(); + private final ArrayList values = new ArrayList<>(); public Parameters() { } @@ -48,7 +47,7 @@ public Parameters(CtBehavior behavior) { "." + behavior.getName() + methodInfo.getDescriptor(); - CtClass[] paramTypes = null; + CtClass[] paramTypes; try { paramTypes = behavior.getParameterTypes(); } catch (NotFoundException e) { @@ -71,51 +70,11 @@ public Parameters(CtBehavior behavior) { return; } - CodeAttribute codeAttribute = methodInfo.getCodeAttribute(); - LocalVariableAttribute locals = null; - if (codeAttribute != null) { - locals = (LocalVariableAttribute) codeAttribute.getAttribute(javassist.bytecode.LocalVariableAttribute.tag); - } else { - logger.debug("No code attribute for {}", fqn); - } - + String[] paramNames = getParameterNames(methodInfo, paramTypes); int numParams = paramTypes.length; - String[] paramNames = new String[numParams]; - if (locals != null && numParams > 0) { - int numLocals = locals.tableLength(); - - // This is handy when debugging this code, but produces too much - // noise for general use. - if (Properties.DebugLocals) { - logger.debug("local variables for {}", fqn); - for (int idx = 0; idx < numLocals; idx++) { - logger.debug(" {} {} {}", idx, locals.variableName(idx), locals.index(idx)); - } - } - - // Iterate through the local variables to find the ones that match the argument slots. - // Arguments are pushed into consecutive slots, starting at 0 (for this or the first argument), - // and then incrementing by 1 for each argument, unless the argument is an unboxed long or double, - // in which case it takes up two slots. - int slot = Modifier.isStatic(behavior.getModifiers()) ? 0 : 1; // ignore `this` - for (int i = 0; i < numParams; i++) { - try { - // note that the slot index is not the same as the - // parameter index or the local variable index - paramNames[i] = locals.variableNameByIndex(slot); - } catch (Exception e) { - // the debug info might be corrupted or partial, let's not crash in this case - logger.debug(e, "Failed to get local variable name for slot {} in {}", slot, fqn); - } finally { - // note these only correspond to unboxed types — boxed double and long will still have width 1 - int width = paramTypes[i] == CtClass.doubleType || paramTypes[i] == CtClass.longType ? 2 : 1; - slot += width; - } - } - } Value[] paramValues = new Value[numParams]; - for (int i = 0; i < paramTypes.length; ++i) { + for (int i = 0; i < numParams; ++i) { // Use a real parameter name if we have it, a fake one if we // don't. String paramName = paramNames[i]; @@ -130,11 +89,61 @@ public Parameters(CtBehavior behavior) { paramValues[i] = param; } - for (int i = 0; i < paramValues.length; ++i) { - this.add(paramValues[i]); + for (Value paramValue : paramValues) { + this.add(paramValue); } } + /** + * Iterate through the LocalVariableTables to get parameter names. + * Local variable tables are debugging metadata containing information about local variables. + * Variables are organized into slots; first slots are used for parameters, then for local variables. + * + * @param methodInfo for the method + * @param paramTypes types of the parameters (used to calculate slot positions) + * @return Array of parameter names (ignoring this), with null for any names that could not be determined. + * Length of the array matches length of paramTypes. + * @see The Java Virtual Machine Specification: The LocalVariableTable Attribute + */ + private static String[] getParameterNames(MethodInfo methodInfo, CtClass[] paramTypes) { + String[] paramNames = new String[paramTypes.length]; + + CodeAttribute codeAttribute = methodInfo.getCodeAttribute(); + if (codeAttribute != null) { + boolean isStatic = Modifier.isStatic(methodInfo.getAccessFlags()); + + // count number of slots taken by all the parameters + int slotCount = isStatic ? 0 : 1; // account for `this` + for (CtClass paramType : paramTypes) { + slotCount += (paramType == CtClass.doubleType || paramType == CtClass.longType) ? 2 : 1; + } + + String[] namesBySlot = new String[slotCount]; + + for (AttributeInfo attr : codeAttribute.getAttributes()) { + if (attr instanceof LocalVariableAttribute) { + LocalVariableAttribute localVarAttr = (LocalVariableAttribute) attr; + + for (int i = 0; i < localVarAttr.tableLength(); i++) { + int index = localVarAttr.index(i); + if (index < slotCount) { + namesBySlot[index] = localVarAttr.variableName(i); + } + } + } + } + + int slot = isStatic ? 0 : 1; // ignore `this` + for (int i = 0; i < paramTypes.length; i++) { + paramNames[i] = namesBySlot[slot]; + int width = paramTypes[i] == CtClass.doubleType || paramTypes[i] == CtClass.longType ? 2 : 1; + slot += width; + } + } + + return paramNames; + } + /** * Get an iterator for each {@link Value}. * @return A {@link Value} iterator @@ -172,26 +181,16 @@ public int size() { return this.values.size(); } - /** - * Clears the internal value array. - */ - public void clear() { - this.values.clear(); - } - - /** - * Gets a {@Value} object stored by this Parameters object by name/identifier. + * Gets a {@link Value} object stored by this Parameters object by name/identifier. * @param name The name or identifier of the @{link Value} to be returned * @return The {@link Value} object found - * @throws NoSuchElementException If no @{link Value} object is found + * @throws NoSuchElementException If no {@link Value} object is found */ public Value get(String name) throws NoSuchElementException { - if (this.values != null) { - for (Value param : this.values) { - if (param.name.equals(name)) { - return param; - } + for (Value param : this.values) { + if (param.name.equals(name)) { + return param; } } @@ -199,16 +198,12 @@ public Value get(String name) throws NoSuchElementException { } /** - * Gets a {@Value} object stored by this Parameters object by index. + * Gets a {@link Value} object stored by this Parameters object by index. * @param index The index of the @{link Value} to be returned * @return The {@link Value} object at the given index - * @throws NoSuchElementException if no @{link Value} object is found at the given index + * @throws NoSuchElementException if no {@link Value} object is found at the given index */ public Value get(Integer index) throws NoSuchElementException { - if (this.values == null) { - throw new NoSuchElementException(); - } - try { return this.values.get(index); } catch (NullPointerException | IndexOutOfBoundsException e) { @@ -233,10 +228,10 @@ public Boolean validate(Integer index, String type) { } /** - * Performs a deep copy of the Parameters object and all of its values. + * Creates a copy of the parameters object with the value types, kinds and names preserved. * @return A new Parameters object */ - public Parameters clone() { + public Parameters freshCopy() { Parameters clonedParams = new Parameters(); for (Value param : this.values) { clonedParams.add(new Value(param)); diff --git a/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java b/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java index 3ff0e28e..c8f237bd 100644 --- a/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java +++ b/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java @@ -73,7 +73,7 @@ public void buildParameters() { } public Parameters getRuntimeParameters(HookBinding binding) { - Parameters runtimeParameters = this.staticParameters.clone(); + Parameters runtimeParameters = this.staticParameters.freshCopy(); Stream.concat(Stream.of(this.sourceSystem), this.optionalSystems.stream()) .sorted(Comparator.comparingInt(ISystem::getParameterPriority)) .forEach(system -> { From 0cb2362baaa3b94a3f381dde764dd3ef4c74016e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Sun, 4 Jan 2026 06:05:50 +0100 Subject: [PATCH 03/21] refactor(agent): Refactor Hook.apply and ClassFileTransformer Refactor `Hook.apply` to return a `Set` 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. --- .../transform/ClassFileTransformer.java | 61 +++----- .../appmap/transform/annotations/Hook.java | 136 +++++++++++------- 2 files changed, 109 insertions(+), 88 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java b/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java index 34764ffc..2a84a138 100644 --- a/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java +++ b/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java @@ -189,9 +189,7 @@ private void processClass(CtClass ctClass) { } } - private boolean applyHooks(CtBehavior behavior) { - boolean traceClass = tracePrefix == null || behavior.getDeclaringClass().getName().startsWith(tracePrefix); - + private Set applyHooks(CtBehavior behavior, boolean traceClass) { try { List hookSites = getHookSites(behavior); @@ -199,37 +197,16 @@ private boolean applyHooks(CtBehavior behavior) { if (traceClass) { logger.trace("no hook sites"); } - return false; + return java.util.Collections.emptySet(); } - Hook.apply(behavior, hookSites); - - if (logger.isDebugEnabled()) { - for (HookSite hookSite : hookSites) { - final Hook hook = hookSite.getHook(); - String className = behavior.getDeclaringClass().getName(); - if (tracePrefix != null && !className.startsWith(tracePrefix)) { - continue; - } - - if (traceClass) { - logger.trace("hooked {}.{}{} on ({},{}) with {}", - className, - behavior.getName(), - behavior.getMethodInfo().getDescriptor(), - hook.getMethodEvent().getEventString(), - hook.getPosition(), - hook); - } - } - } - return true; + return Hook.apply(behavior, hookSites, traceClass); } catch (NoSourceAvailableException e) { Logger.println(e); } - return false; + return java.util.Collections.emptySet(); } public List getHookSites(CtBehavior behavior) { @@ -292,7 +269,7 @@ public byte[] transform(ClassLoader loader, try { ClassPool classPool = AppMapClassPool.get(); if (traceClass) { - logger.debug("className: {}", className); + logger.trace("className: {}", className); } ctClass = classPool.makeClass(new ByteArrayInputStream(bytes)); @@ -317,7 +294,8 @@ public byte[] transform(ClassLoader loader, return null; } - boolean hookApplied = false; + boolean bytecodeModified = false; + boolean needsByteBuddy = false; for (CtBehavior behavior : ctClass.getDeclaredBehaviors()) { if (traceClass) { logger.trace("behavior: {}", behavior.getLongName()); @@ -331,24 +309,27 @@ public byte[] transform(ClassLoader loader, } methodsExamined++; - if (this.applyHooks(behavior)) { - hookApplied = true; + Set actions = this.applyHooks(behavior, traceClass); + if (!actions.isEmpty()) { + bytecodeModified = true; methodsHooked++; + if (actions.contains(Hook.ApplicationAction.MARKED)) { + needsByteBuddy = true; + } } } - if (hookApplied) { - // One or more of the methods in the the class were marked as needing to + if (bytecodeModified) { + // One or more of the methods in the class were marked as needing to // be instrumented. Mark the class so the bytebuddy transformer will // know it needs to be instrumented. - ClassFile classFile = ctClass.getClassFile(); - ConstPool constPool = classFile.getConstPool(); - Annotation annot = new Annotation(AppMapInstrumented.class.getName(), constPool); - AnnotationUtil.setAnnotation(new AnnotatedClass(ctClass), annot); - - if (traceClass) { - logger.trace("hooks applied to {}", className); + if (needsByteBuddy) { + ClassFile classFile = ctClass.getClassFile(); + ConstPool constPool = classFile.getConstPool(); + Annotation annot = new Annotation(AppMapInstrumented.class.getName(), constPool); + AnnotationUtil.setAnnotation(new AnnotatedClass(ctClass), annot); } + if (logger.isDebugEnabled()) { packagesHooked.compute(ctClass.getPackageName(), (k, v) -> v == null ? 1 : v + 1); } diff --git a/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java b/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java index c8f237bd..5a695ef4 100644 --- a/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java +++ b/agent/src/main/java/com/appland/appmap/transform/annotations/Hook.java @@ -2,6 +2,7 @@ import java.util.Arrays; import java.util.Comparator; +import java.util.EnumSet; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -107,25 +108,72 @@ public HookSite prepare(CtBehavior targetBehavior, Map hookConte return new HookSite(this, behaviorOrdinal, binding); } - public static void apply(CtBehavior targetBehavior, List hookSites) { + public enum ApplicationAction { + MARKED, + INSTRUMENTED + } + + // We only log the first exception to avoid flooding the logs at the debug level. + // Note this variable is not thread safe, but this is okay; the worst that can happen is + // that we log more than one exception in a multi-threaded scenario. + private static boolean exceptionLogged = false; + + public static Set apply(CtBehavior targetBehavior, List hookSites, boolean traceClass) { + Set actions = EnumSet.noneOf(ApplicationAction.class); MethodInfo methodInfo = targetBehavior.getMethodInfo(); AnnotationsAttribute attr = (AnnotationsAttribute)methodInfo.getAttribute(AnnotationsAttribute.visibleTag); - // If the behavior is marked as an app method, update the annotation with - // the behavior ordinals so the bytebuddy transformer can instrument it. - if (attr.getAnnotation(AppMapAppMethod.class.getName()) != null) { - setBehaviorOrdinals(targetBehavior, hookSites); - } + if (attr != null) { + // If the behavior is marked as an app method, update the annotation with + // the behavior ordinals so the bytebuddy transformer can instrument it. + if (attr.getAnnotation(AppMapAppMethod.class.getName()) != null) { + setBehaviorOrdinals(targetBehavior, hookSites); + actions.add(ApplicationAction.MARKED); + if (traceClass) { + logger.debug("tracing {}.{}{}", + targetBehavior.getDeclaringClass().getName(), + targetBehavior.getName(), + targetBehavior.getMethodInfo().getDescriptor()); + } + } - // If it's (also) marked as an agent method, it needs to be instrumented - // by javassist. - if (attr.getAnnotation(AppMapAgentMethod.class.getName()) != null) { - instrument(targetBehavior, hookSites); + // If it's (also) marked as an agent method, it needs to be instrumented + // by javassist. + if (attr.getAnnotation(AppMapAgentMethod.class.getName()) != null) { + try { + instrument(targetBehavior, hookSites); + actions.add(ApplicationAction.INSTRUMENTED); + if (traceClass) { + String hooks = hookSites.stream() + .map(h -> h.getHook().toString()) + .collect(Collectors.joining(", ")); + logger.debug("{}.{}{} instrumented with hooks: {}", + targetBehavior.getDeclaringClass().getName(), + targetBehavior.getName(), + targetBehavior.getMethodInfo().getDescriptor(), + hooks); + } + } catch (CannotCompileException | NotFoundException e) { + String msg = String.format("failed to instrument %s.%s: %s", + targetBehavior.getDeclaringClass().getName(), targetBehavior.getName(), e.getMessage()); + if (!exceptionLogged) { + logger.debug(e, msg); + exceptionLogged = true; + } else { + // Log at trace level after the first one to avoid flooding the debug logs + logger.trace(e, msg); + logger.debug(msg); + } + } + } } + + return actions; } - public static void instrument(CtBehavior targetBehavior, List hookSites) { + public static void instrument(CtBehavior targetBehavior, List hookSites) + throws CannotCompileException, NotFoundException { final CtClass returnType = getReturnType(targetBehavior); final Boolean returnsVoid = (returnType == CtClass.voidType); @@ -150,44 +198,36 @@ public static void instrument(CtBehavior targetBehavior, List hookSite } - try { - String beforeSrcBlock = beforeSrcBlock(uniqueLocks.toString(), - invocations[MethodEvent.METHOD_INVOCATION.getIndex()]); - logger.trace("{}: beforeSrcBlock:\n{}", targetBehavior::getName, beforeSrcBlock::toString); - targetBehavior.insertBefore( - beforeSrcBlock); - - String afterSrcBlock = afterSrcBlock(invocations[MethodEvent.METHOD_RETURN.getIndex()]); - logger.trace("{}: afterSrcBlock:\n{}", targetBehavior::getName, afterSrcBlock::toString); - - targetBehavior.insertAfter( - afterSrcBlock); - - ClassPool cp = AppMapClassPool.get(); - String exitEarlyCatchSrc = "{com.appland.appmap.process.ThreadLock.current().exit();return;}"; - if (returnsVoid) { - targetBehavior.addCatch(exitEarlyCatchSrc, - cp.get("com.appland.appmap.process.ExitEarly")); - } else if (!returnType.isPrimitive()) { - exitEarlyCatchSrc = "{com.appland.appmap.process.ThreadLock.current().exit();return(" - + returnType.getName() + ")$e.getReturnValue();}"; - targetBehavior - .addCatch(exitEarlyCatchSrc, cp.get("com.appland.appmap.process.ExitEarly")); - } - logger.trace("{}: catch1Src:\n{}", targetBehavior::getName, exitEarlyCatchSrc::toString); - - String catchSrcBlock = catchSrcBlock(invocations[MethodEvent.METHOD_EXCEPTION.getIndex()]); - targetBehavior.addCatch( - catchSrcBlock, - cp.get("java.lang.Throwable")); - logger.trace("{}: catchSrcBlock:\n{}", targetBehavior::getName, catchSrcBlock::toString); - - } catch (CannotCompileException e) { - logger.debug(e, "failed to compile {}.{}", targetBehavior.getDeclaringClass().getName(), - targetBehavior.getName()); - } catch (NotFoundException e) { - logger.debug(e); + String beforeSrcBlock = beforeSrcBlock(uniqueLocks.toString(), + invocations[MethodEvent.METHOD_INVOCATION.getIndex()]); + logger.trace("{}: beforeSrcBlock:\n{}", targetBehavior::getName, beforeSrcBlock::toString); + targetBehavior.insertBefore( + beforeSrcBlock); + + String afterSrcBlock = afterSrcBlock(invocations[MethodEvent.METHOD_RETURN.getIndex()]); + logger.trace("{}: afterSrcBlock:\n{}", targetBehavior::getName, afterSrcBlock::toString); + + targetBehavior.insertAfter( + afterSrcBlock); + + ClassPool cp = AppMapClassPool.get(); + String exitEarlyCatchSrc = "{com.appland.appmap.process.ThreadLock.current().exit();return;}"; + if (returnsVoid) { + targetBehavior.addCatch(exitEarlyCatchSrc, + cp.get("com.appland.appmap.process.ExitEarly")); + } else if (!returnType.isPrimitive()) { + exitEarlyCatchSrc = "{com.appland.appmap.process.ThreadLock.current().exit();return(" + + returnType.getName() + ")$e.getReturnValue();}"; + targetBehavior + .addCatch(exitEarlyCatchSrc, cp.get("com.appland.appmap.process.ExitEarly")); } + logger.trace("{}: catch1Src:\n{}", targetBehavior::getName, exitEarlyCatchSrc::toString); + + String catchSrcBlock = catchSrcBlock(invocations[MethodEvent.METHOD_EXCEPTION.getIndex()]); + targetBehavior.addCatch( + catchSrcBlock, + cp.get("java.lang.Throwable")); + logger.trace("{}: catchSrcBlock:\n{}", targetBehavior::getName, catchSrcBlock::toString); } private static void setBehaviorOrdinals(CtBehavior behavior, From 95cd9570acc013b3304cbe20fc03eaa57dd0ab39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Thu, 18 Dec 2025 16:46:43 +0100 Subject: [PATCH 04/21] feat(gretty-tomcat): Add gretty-tomcat test project and fix HookFunctions NoClassDefFoundError - Added a new `gretty-tomcat` subdirectory under `agent/test` with a minimal web application using Gretty and Tomcat. - Updated `HelloServlet.java` to use `jakarta.servlet` imports, aligning with Tomcat 10 and Jakarta EE. - Modified `Agent.java` to append the runtime JAR to the bootstrap class loader search path. This resolves a `java.lang.NoClassDefFoundError: com/appland/appmap/runtime/HookFunctions` by ensuring AppMap's runtime classes are available to all application class loaders, particularly in web server environments. - Updated `wait_for_ws` in `agent/test/helper.bash` to accept an optional URL argument for health checks. --- .../main/java/com/appland/appmap/Agent.java | 13 +++---- agent/test/gretty-tomcat/appmap.yml | 3 ++ agent/test/gretty-tomcat/build.gradle | 27 +++++++++++++++ agent/test/gretty-tomcat/gradlew | 1 + agent/test/gretty-tomcat/gretty-tomcat.bats | 34 +++++++++++++++++++ agent/test/gretty-tomcat/settings.gradle | 1 + .../main/java/org/example/HelloServlet.java | 15 ++++++++ agent/test/helper.bash | 5 +-- 8 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 agent/test/gretty-tomcat/appmap.yml create mode 100644 agent/test/gretty-tomcat/build.gradle create mode 120000 agent/test/gretty-tomcat/gradlew create mode 100755 agent/test/gretty-tomcat/gretty-tomcat.bats create mode 100644 agent/test/gretty-tomcat/settings.gradle create mode 100644 agent/test/gretty-tomcat/src/main/java/org/example/HelloServlet.java diff --git a/agent/src/main/java/com/appland/appmap/Agent.java b/agent/src/main/java/com/appland/appmap/Agent.java index 2aca22a0..1c283437 100644 --- a/agent/src/main/java/com/appland/appmap/Agent.java +++ b/agent/src/main/java/com/appland/appmap/Agent.java @@ -213,13 +213,14 @@ private static void setupRuntime(Path agentJarPath, JarFile agentJar, Instrument System.exit(1); } - // Adding the runtime jar to the boot class loader means the classes it - // contains will be available everywhere. This avoids issues caused by any - // filtering the app's class loader might be doing (e.g. the Scala runtime - // when running a Play app). + // It's critical to append the runtime JAR to the bootstrap class loader + // search path, not the system class loader search path. This ensures that + // AppMap's core runtime classes, such as HookFunctions, are available to + // all application classes, including those loaded by different class loaders + // (e.g., in web servers like Tomcat or other complex environments), which + // fixes `NoClassDefFoundError` for `HookFunctions`. JarFile runtimeJar = new JarFile(runtimeJarPath.toFile()); - inst.appendToSystemClassLoaderSearch(runtimeJar); - // inst.appendToBootstrapClassLoaderSearch(runtimeJar); + inst.appendToBootstrapClassLoaderSearch(runtimeJar); // HookFunctions can only be referenced after the runtime jar has been // appended to the boot class loader. diff --git a/agent/test/gretty-tomcat/appmap.yml b/agent/test/gretty-tomcat/appmap.yml new file mode 100644 index 00000000..32e1ffc0 --- /dev/null +++ b/agent/test/gretty-tomcat/appmap.yml @@ -0,0 +1,3 @@ +name: gretty-tomcat +packages: +- path: org.example diff --git a/agent/test/gretty-tomcat/build.gradle b/agent/test/gretty-tomcat/build.gradle new file mode 100644 index 00000000..4d8da04f --- /dev/null +++ b/agent/test/gretty-tomcat/build.gradle @@ -0,0 +1,27 @@ +plugins { + id 'war' + id 'org.gretty' version '4.1.6' +} + +repositories { + mavenCentral() +} + +def appmapJar = System.env.AGENT_JAR + +gretty { + servletContainer = 'tomcat10' + contextPath = '/' + jvmArgs = [ + "-Dappmap.config.file=appmap.yml", + "-Dappmap.debug.file=../../build/logs/gretty-tomcat-appmap.log" + ] + if (appmapJar) { + jvmArgs << "-javaagent:${appmapJar}" + } +} + +dependencies { + providedCompile 'jakarta.servlet:jakarta.servlet-api:5.0.0' +} + diff --git a/agent/test/gretty-tomcat/gradlew b/agent/test/gretty-tomcat/gradlew new file mode 120000 index 00000000..ab9334b0 --- /dev/null +++ b/agent/test/gretty-tomcat/gradlew @@ -0,0 +1 @@ +../../../gradlew \ No newline at end of file diff --git a/agent/test/gretty-tomcat/gretty-tomcat.bats b/agent/test/gretty-tomcat/gretty-tomcat.bats new file mode 100755 index 00000000..d0f34e1d --- /dev/null +++ b/agent/test/gretty-tomcat/gretty-tomcat.bats @@ -0,0 +1,34 @@ +#!/usr/bin/env bats + +load '../helper' + +setup_file() { + mkdir -p build/log + + export LOG="$(getcwd)/build/log/gretty-tomcat.log" + export SERVER_PORT=8080 + export WS_URL="http://localhost:${SERVER_PORT}" + + cd ${BATS_TEST_DIRNAME} + _configure_logging + + ./gradlew appStart -Pgretty.httpPort=${SERVER_PORT} &> $LOG & + export JVM_MAIN_CLASS=org.gradle.wrapper.GradleWrapperMain + + wait_for_ws "${WS_URL}/hello" +} + +teardown_file() { + ./gradlew appStop || true + # stop_ws might fail if /exit is not there, but it also waits for process to die. + # We can try to just kill the gradle process if it's still running. + pkill -P $$ -f "GradleWrapperMain" || true +} + +@test "hello world" { + run _curl -sXGET "${WS_URL}/hello" + assert_success + assert_output "Hello, World!" +} + + diff --git a/agent/test/gretty-tomcat/settings.gradle b/agent/test/gretty-tomcat/settings.gradle new file mode 100644 index 00000000..76ed3897 --- /dev/null +++ b/agent/test/gretty-tomcat/settings.gradle @@ -0,0 +1 @@ +rootProject.name = 'gretty-tomcat' diff --git a/agent/test/gretty-tomcat/src/main/java/org/example/HelloServlet.java b/agent/test/gretty-tomcat/src/main/java/org/example/HelloServlet.java new file mode 100644 index 00000000..0006c5c6 --- /dev/null +++ b/agent/test/gretty-tomcat/src/main/java/org/example/HelloServlet.java @@ -0,0 +1,15 @@ +package org.example; + +import java.io.IOException; +import jakarta.servlet.ServletException; +import jakarta.servlet.annotation.WebServlet; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + +@WebServlet(name = "HelloServlet", urlPatterns = {"/hello"}) +public class HelloServlet extends HttpServlet { + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + response.getWriter().print("Hello, World!"); + } +} diff --git a/agent/test/helper.bash b/agent/test/helper.bash index e11df0b5..cac7c40e 100644 --- a/agent/test/helper.bash +++ b/agent/test/helper.bash @@ -154,7 +154,8 @@ check_ws_running() { } wait_for_ws() { - while ! curl -Isf "${WS_URL}" >/dev/null; do + local url="${1:-$WS_URL}" + while ! curl -Isf "${url}" >/dev/null; do if ! jcmd $JVM_MAIN_CLASS VM.uptime >&/dev/null; then echo "$JVM_MAIN_CLASS failed" exit 1 @@ -296,4 +297,4 @@ getcwd() { # This seems slightly ridiculous. But, it produces a path that's understood by both Bash and Java, # and it works in all dev and CI environments. git rev-parse --show-toplevel --show-prefix | tr -s '\n' '/' -} \ No newline at end of file +} From 2762a15749297c5d5418ac66cf7eb91f3d4f8fb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Thu, 18 Dec 2025 17:33:53 +0100 Subject: [PATCH 05/21] fix(agent): Support running agent on bootstrap classpath - 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. --- .../main/java/com/appland/appmap/Agent.java | 18 ++++++++++++++---- .../com/appland/appmap/config/Properties.java | 7 ++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/Agent.java b/agent/src/main/java/com/appland/appmap/Agent.java index 1c283437..3de6be8c 100644 --- a/agent/src/main/java/com/appland/appmap/Agent.java +++ b/agent/src/main/java/com/appland/appmap/Agent.java @@ -74,6 +74,10 @@ public static void premain(String agentArgs, Instrumentation inst) { logger.info("config: {}", AppMapConfig.get()); logger.debug("System properties: {}", System.getProperties()); + if (Agent.class.getClassLoader() == null) { + logger.warn("AppMap agent is running on the bootstrap classpath. This is not a recommended configuration and should only be used for troubleshooting. Git integration will be disabled."); + } + addAgentJars(agentArgs, inst); @@ -162,12 +166,18 @@ private static void addAgentJars(String agentArgs, Instrumentation inst) { Path agentJarPath = null; try { Class agentClass = Agent.class; - URL resourceURL = agentClass.getClassLoader() - .getResource(agentClass.getName().replace('.', '/') + ".class"); + // When the agent is loaded by the bootstrap class loader (e.g., via -Xbootclasspath/a:), + // agentClass.getClassLoader() returns null, leading to a NullPointerException. To handle + // this, we use Class.getResource() which correctly resolves resources even when the + // class is loaded by the bootstrap class loader. The leading '/' in the resource name + // is crucial for absolute path resolution when using Class.getResource(). + URL resourceURL = agentClass.getResource("/" + agentClass.getName().replace('.', '/') + ".class"); + // During testing of the agent itself, classes get loaded from a directory, and will have the // protocol "file". The rest of the time (i.e. when it's actually deployed), they'll always - // come from a jar file. - if (resourceURL.getProtocol().equals("jar")) { + // come from a jar file. We must also check that resourceURL is not null before using it, + // as getResource() can return null if the resource is not found. + if (resourceURL != null && resourceURL.getProtocol().equals("jar")) { String resourcePath = resourceURL.getPath(); URL jarURL = new URL(resourcePath.substring(0, resourcePath.indexOf('!'))); logger.debug("jarURL: {}", jarURL); diff --git a/agent/src/main/java/com/appland/appmap/config/Properties.java b/agent/src/main/java/com/appland/appmap/config/Properties.java index 5c4c168e..0d4163bd 100644 --- a/agent/src/main/java/com/appland/appmap/config/Properties.java +++ b/agent/src/main/java/com/appland/appmap/config/Properties.java @@ -21,7 +21,12 @@ public class Properties { public static final String DebugClassPrefix = resolveProperty("appmap.debug.classPrefix", (String) null); public static final Boolean SaveInstrumented = resolveProperty("appmap.debug.saveInstrumented", false); - public static final Boolean DisableGit = resolveProperty("appmap.debug.disableGit", false); + public static final Boolean DisableGit = + // Git integration (JGit) uses resource bundles, which are not reliably available + // when the agent is loaded by the bootstrap class loader (i.e., when + // getClassLoader() returns null). In such cases, automatically disable Git + // to prevent NullPointerExceptions during initialization. + resolveProperty("appmap.debug.disableGit", Properties.class.getClassLoader() == null); public static final Boolean RecordingAuto = resolveProperty("appmap.recording.auto", false); public static final String RecordingName = resolveProperty("appmap.recording.name", (String) null); From be04979edfe50ac7100a40376e6f0ac9385cca28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Thu, 18 Dec 2025 18:00:00 +0100 Subject: [PATCH 06/21] feat(tests): Add Java version check for bats tests - Introduced a `_require_java_version` helper function in `agent/test/helper.bash` to allow bats tests to specify a minimum required Java version. Tests will be skipped if the current Java version is below the requirement. - Applied the Java 11 requirement to the `gretty-tomcat` bats tests to ensure they run in a compatible environment. --- agent/test/gretty-tomcat/gretty-tomcat.bats | 2 ++ agent/test/helper.bash | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/agent/test/gretty-tomcat/gretty-tomcat.bats b/agent/test/gretty-tomcat/gretty-tomcat.bats index d0f34e1d..6d821876 100755 --- a/agent/test/gretty-tomcat/gretty-tomcat.bats +++ b/agent/test/gretty-tomcat/gretty-tomcat.bats @@ -3,6 +3,8 @@ load '../helper' setup_file() { + _require_java_version 11 + mkdir -p build/log export LOG="$(getcwd)/build/log/gretty-tomcat.log" diff --git a/agent/test/helper.bash b/agent/test/helper.bash index cac7c40e..1b8c16bc 100644 --- a/agent/test/helper.bash +++ b/agent/test/helper.bash @@ -14,6 +14,13 @@ export JAVA_VERSION export JAVA_OUTPUT_OPTIONS="-Xshare:off" +_require_java_version() { + local required_version=$1 + if [[ "$(echo -e "$required_version\n$JAVA_VERSION" | sort -V | head -n1)" != "$required_version" ]]; then + skip "Java version $required_version or higher is required (current: $JAVA_VERSION)" + fi +} + _curl() { curl -sfH 'Accept: application/json,*/*' "${@}" } From 800882797f6548a8f5bb94e9ca12dc771dad4065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 10:04:42 +0100 Subject: [PATCH 07/21] fix(tests): Add timeout to wait_for_ws in helper.bash Adds a 60-second timeout to the `wait_for_ws` function in `agent/test/helper.bash`. Previously, this function could hang indefinitely in CI environments if the web server failed to start but the monitored JVM process (e.g., Gradle wrapper) remained active. This commonly occurred when a forked application server (like Tomcat via Gretty) crashed during startup. With this change: - The test will now fail explicitly after 60 seconds if the web service at `WS_URL` is unreachable. - Upon timeout or JVM failure, the content of the `$LOG` file will be printed, providing crucial debugging information for CI failures. --- agent/test/helper.bash | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/agent/test/helper.bash b/agent/test/helper.bash index 1b8c16bc..f52df480 100644 --- a/agent/test/helper.bash +++ b/agent/test/helper.bash @@ -162,11 +162,23 @@ check_ws_running() { wait_for_ws() { local url="${1:-$WS_URL}" + local timeout=60 + local start_time=$(date +%s) + while ! curl -Isf "${url}" >/dev/null; do if ! jcmd $JVM_MAIN_CLASS VM.uptime >&/dev/null; then echo "$JVM_MAIN_CLASS failed" + if [[ -f "$LOG" ]]; then cat "$LOG"; fi exit 1 fi + + local current_time=$(date +%s) + if (( current_time - start_time > timeout )); then + echo "Timed out waiting for $url" + if [[ -f "$LOG" ]]; then cat "$LOG"; fi + exit 1 + fi + sleep 1 done printf ' ok\n\n' From 29c6f594afbf3a3cb621148d84139d37de6d0abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 10:18:06 +0100 Subject: [PATCH 08/21] Add a .keep to gretty test webapp directory The server cannot start without it --- agent/test/gretty-tomcat/src/main/webapp/.keep | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 agent/test/gretty-tomcat/src/main/webapp/.keep diff --git a/agent/test/gretty-tomcat/src/main/webapp/.keep b/agent/test/gretty-tomcat/src/main/webapp/.keep new file mode 100644 index 00000000..e69de29b From 853fbb6132648f383b250919eaa630ddbe3022f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 10:23:59 +0100 Subject: [PATCH 09/21] feat(agent): Add option to exclude specific hook classes 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=` or an environment variable `APPMAP_HOOKS_EXCLUDE=`. The agent's `ClassFileTransformer` now checks this exclusion list during hook processing, preventing the instrumentation of specified hook classes. --- .../java/com/appland/appmap/config/Properties.java | 2 ++ .../appmap/transform/ClassFileTransformer.java | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/agent/src/main/java/com/appland/appmap/config/Properties.java b/agent/src/main/java/com/appland/appmap/config/Properties.java index 0d4163bd..5068b702 100644 --- a/agent/src/main/java/com/appland/appmap/config/Properties.java +++ b/agent/src/main/java/com/appland/appmap/config/Properties.java @@ -35,6 +35,8 @@ public class Properties { public static final Boolean RecordingRequests = resolveProperty("appmap.recording.requests", true); public static final String[] IgnoredPackages = resolveProperty("appmap.recording.ignoredPackages", new String[] {"java.", "jdk.", "sun."}); + public static final String[] ExcludedHooks = + resolveProperty("appmap.hooks.exclude", new String[0]); public static final String DefaultConfigFile = "appmap.yml"; diff --git a/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java b/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java index 2a84a138..774a610d 100644 --- a/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java +++ b/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java @@ -153,7 +153,21 @@ private Hook[] getHooks(String methodId) { return methodHooks != null ? methodHooks : sortedUnkeyedHooks; } + private boolean isExcludedHook(String className) { + for (String excluded : Properties.ExcludedHooks) { + if (className.equals(excluded)) { + return true; + } + } + return false; + } + private void processClass(CtClass ctClass) { + if (isExcludedHook(ctClass.getName())) { + logger.debug("excluding hook class {}", ctClass.getName()); + return; + } + boolean traceClass = tracePrefix == null || ctClass.getName().startsWith(tracePrefix); if (traceClass) { From 61aa0d056364ab6f72dd8a827e657c7e136e05e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 12:38:24 +0100 Subject: [PATCH 10/21] build(test): Add Oracle JDBC integration tests - Add Oracle JDBC driver dependency to agent/test/jdbc - Add OracleRepositoryTests and application-oracle.properties - Configure GitHub Actions to run Oracle service for integration tests - Add docker-compose.yml for local Oracle testing - Update jdbc.bats to include Oracle test case --- .github/workflows/build-and-test.yml | 15 ++++++++ agent/test/jdbc/build.gradle | 7 ++++ agent/test/jdbc/docker-compose.yml | 10 ++++++ agent/test/jdbc/jdbc.bats | 8 +++++ .../OracleRepositoryTests.java | 36 +++++++++++++++++++ .../resources/application-oracle.properties | 6 ++++ 6 files changed, 82 insertions(+) create mode 100644 agent/test/jdbc/docker-compose.yml create mode 100644 agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java create mode 100644 agent/test/jdbc/src/test/resources/application-oracle.properties diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index e0266407..49825eec 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -31,6 +31,18 @@ jobs: annotation/build/libs/*.jar test-suite: + services: + oracle: + image: docker.io/gvenzl/oracle-free:slim-faststart + ports: + - 1521:1521 + env: + ORACLE_PASSWORD: oracle + options: >- + --health-cmd healthcheck.sh + --health-interval 10s + --health-timeout 5s + --health-retries 5 strategy: matrix: java: ['17', '11', '8'] @@ -106,5 +118,8 @@ jobs: env: BATS_LIB_PATH: ${{ steps.setup-bats.outputs.lib-path }} TERM: xterm + ORACLE_URL: jdbc:oracle:thin:@localhost:1521 + ORACLE_USERNAME: system + ORACLE_PASSWORD: oracle working-directory: ./agent run: bin/test_run diff --git a/agent/test/jdbc/build.gradle b/agent/test/jdbc/build.gradle index ea106d16..57946f05 100644 --- a/agent/test/jdbc/build.gradle +++ b/agent/test/jdbc/build.gradle @@ -17,6 +17,7 @@ repositories { dependencies { implementation 'org.springframework.boot:spring-boot-starter-data-jpa' runtimeOnly 'com.h2database:h2' + runtimeOnly 'com.oracle.database.jdbc:ojdbc8:21.9.0.0' testImplementation 'org.springframework.boot:spring-boot-starter-test' } @@ -24,6 +25,12 @@ def appmapJar = "$System.env.AGENT_JAR" test { useJUnitPlatform() + if (System.env.ORACLE_URL) { + inputs.property("oracleUrl", System.env.ORACLE_URL) + } + if (System.env.AGENT_JAR) { + inputs.file(System.env.AGENT_JAR) + } jvmArgs += [ "-javaagent:${appmapJar}", "-Dappmap.config.file=appmap.yml", diff --git a/agent/test/jdbc/docker-compose.yml b/agent/test/jdbc/docker-compose.yml new file mode 100644 index 00000000..a2ed3ffa --- /dev/null +++ b/agent/test/jdbc/docker-compose.yml @@ -0,0 +1,10 @@ +# This docker-compose file is used for local, manual execution of the Oracle JDBC integration tests. +# It starts a standalone Oracle database for testing purposes. +version: '3.8' +services: + oracle: + image: docker.io/gvenzl/oracle-free:slim-faststart + ports: + - "1521:1521" + environment: + ORACLE_PASSWORD: oracle diff --git a/agent/test/jdbc/jdbc.bats b/agent/test/jdbc/jdbc.bats index 71bb7c26..a76c7062 100644 --- a/agent/test/jdbc/jdbc.bats +++ b/agent/test/jdbc/jdbc.bats @@ -33,5 +33,13 @@ setup() { assert_json_eq '.metadata.test_failure.message' 'expected: but was: ' } +# Requires a running Oracle instance. +# Locally: docker-compose up -d (in agent/test/jdbc) +# CI: Service is configured in .github/workflows/build-and-test.yml +@test "oracle integration test" { + run ./gradlew -q test --tests 'OracleRepositoryTests' + assert_success +} + diff --git a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java new file mode 100644 index 00000000..fc2429d4 --- /dev/null +++ b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java @@ -0,0 +1,36 @@ +package com.example.accessingdatajpa; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager; +import org.springframework.test.context.ActiveProfiles; + +@DataJpaTest +@ActiveProfiles("oracle") +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@EnabledIfEnvironmentVariable(named = "ORACLE_URL", matches = ".*") +public class OracleRepositoryTests { + + @Autowired + private TestEntityManager entityManager; + + @Autowired + private CustomerRepository customers; + + @Test + public void testFindByLastName() { + Customer customer = new Customer("Oracle", "User"); + entityManager.persist(customer); + + List findByLastName = customers.findByLastName(customer.getLastName()); + + assertThat(findByLastName).extracting(Customer::getLastName).containsOnly(customer.getLastName()); + } +} diff --git a/agent/test/jdbc/src/test/resources/application-oracle.properties b/agent/test/jdbc/src/test/resources/application-oracle.properties new file mode 100644 index 00000000..d38c8ab0 --- /dev/null +++ b/agent/test/jdbc/src/test/resources/application-oracle.properties @@ -0,0 +1,6 @@ +spring.datasource.url=${ORACLE_URL:jdbc:oracle:thin:@localhost:1521} +spring.datasource.username=${ORACLE_USERNAME:system} +spring.datasource.password=${ORACLE_PASSWORD:oracle} +spring.datasource.driver-class-name=oracle.jdbc.OracleDriver +spring.jpa.database-platform=org.hibernate.dialect.Oracle12cDialect +spring.jpa.hibernate.ddl-auto=create-drop From d0b80312471d39ff1fc6c2cc869a5c0a04ca8665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 17:33:07 +0100 Subject: [PATCH 11/21] fix: Enforce UTF-8 for AppMap I/O and add encoding regression tests 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. --- .../hooks/remoterecording/ServletRequest.java | 7 +- .../com/appland/appmap/record/Recording.java | 5 +- .../appmap/record/RecordingSession.java | 13 ++-- agent/test/encoding/ReadFullyTest.java | 47 ++++++++++++++ agent/test/encoding/UnicodeTest.java | 42 ++++++++++++ agent/test/encoding/appmap.yml | 3 + agent/test/encoding/encoding.bats | 64 +++++++++++++++++++ agent/test/encoding/encoding_test.cp1252 | 5 ++ 8 files changed, 172 insertions(+), 14 deletions(-) create mode 100644 agent/test/encoding/ReadFullyTest.java create mode 100644 agent/test/encoding/UnicodeTest.java create mode 100644 agent/test/encoding/appmap.yml create mode 100755 agent/test/encoding/encoding.bats create mode 100644 agent/test/encoding/encoding_test.cp1252 diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/remoterecording/ServletRequest.java b/agent/src/main/java/com/appland/appmap/process/hooks/remoterecording/ServletRequest.java index 25bdd7ec..a058fdc3 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/remoterecording/ServletRequest.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/remoterecording/ServletRequest.java @@ -2,6 +2,7 @@ import java.io.IOException; import java.io.PrintWriter; +import java.nio.charset.StandardCharsets; import com.appland.appmap.record.Recording; import com.appland.appmap.reflect.HttpServletRequest; @@ -29,8 +30,8 @@ public void setStatus(int status) { } public void writeJson(String responseJson) throws IOException { - res.setContentType("application/json"); - res.setContentLength(responseJson.length()); + res.setContentType("application/json; charset=UTF-8"); + res.setContentLength(responseJson.getBytes(StandardCharsets.UTF_8).length); res.setStatus(HttpServletResponse.SC_OK); PrintWriter writer = res.getWriter(); @@ -39,7 +40,7 @@ public void writeJson(String responseJson) throws IOException { } public void writeRecording(Recording recording) throws IOException { - res.setContentType("application/json"); + res.setContentType("application/json; charset=UTF-8"); res.setContentLength(recording.size()); recording.readFully(true, res.getWriter()); } diff --git a/agent/src/main/java/com/appland/appmap/record/Recording.java b/agent/src/main/java/com/appland/appmap/record/Recording.java index 649b37d2..b66daacb 100644 --- a/agent/src/main/java/com/appland/appmap/record/Recording.java +++ b/agent/src/main/java/com/appland/appmap/record/Recording.java @@ -5,11 +5,12 @@ import java.io.File; import java.io.FileInputStream; -import java.io.FileReader; import java.io.IOException; import java.io.InputStream; +import java.io.InputStreamReader; import java.io.Reader; import java.io.Writer; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -100,7 +101,7 @@ public Path moveTo(Path targetPath) { } public void readFully(boolean delete, Writer writer) throws IOException { - try (final Reader reader = new FileReader(this.file)) { + try (final Reader reader = new InputStreamReader(new FileInputStream(this.file), StandardCharsets.UTF_8)) { char[] buffer = new char[2048]; int bytesRead; while ((bytesRead = reader.read(buffer)) != -1) { diff --git a/agent/src/main/java/com/appland/appmap/record/RecordingSession.java b/agent/src/main/java/com/appland/appmap/record/RecordingSession.java index 583177fd..ef4d6fe0 100644 --- a/agent/src/main/java/com/appland/appmap/record/RecordingSession.java +++ b/agent/src/main/java/com/appland/appmap/record/RecordingSession.java @@ -1,12 +1,7 @@ package com.appland.appmap.record; -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.RandomAccessFile; -import java.io.Writer; +import java.io.*; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; @@ -98,7 +93,7 @@ public synchronized Recording checkpoint() { public void write(int b) throws IOException { raf.write(b); } - }); + }, StandardCharsets.UTF_8); raf.seek(targetPath.toFile().length()); if ( eventReceived ) { @@ -162,7 +157,7 @@ void start() { try { this.tmpPath = Files.createTempFile(null, ".appmap.json"); this.tmpPath.toFile().deleteOnExit(); - this.serializer = AppMapSerializer.open(new FileWriter(this.tmpPath.toFile())); + this.serializer = AppMapSerializer.open(new OutputStreamWriter(new FileOutputStream(this.tmpPath.toFile()), StandardCharsets.UTF_8)); } catch (IOException e) { this.tmpPath = null; this.serializer = null; diff --git a/agent/test/encoding/ReadFullyTest.java b/agent/test/encoding/ReadFullyTest.java new file mode 100644 index 00000000..68b20c55 --- /dev/null +++ b/agent/test/encoding/ReadFullyTest.java @@ -0,0 +1,47 @@ +package test.pkg; + +import com.appland.appmap.config.AppMapConfig; +import com.appland.appmap.record.Recording; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.FileSystems; + +public class ReadFullyTest { + public static void main(String[] args) { + try { + runTest(); + } catch (Exception e) { + e.printStackTrace(); + System.exit(1); + } + } + + public static void runTest() throws IOException { + // Initialize AppMapConfig + AppMapConfig.initialize(FileSystems.getDefault()); + + // 1. Create a dummy AppMap file with known UTF-8 content + String content = "Check: \u26A0\uFE0F \u041F\u0440\u0438\u0432\u0435\u0442"; + File tempFile = File.createTempFile("readfully", ".appmap.json"); + tempFile.deleteOnExit(); + + try (Writer fw = new OutputStreamWriter(new FileOutputStream(tempFile), StandardCharsets.UTF_8)) { + fw.write(content); + } + + // 2. Create a Recording object pointing to it + Recording recording = new Recording("test", tempFile); + + // 3. Call readFully and write to stdout using UTF-8 + // This validates that readFully correctly reads the UTF-8 file bytes into characters + // regardless of the system's default encoding (which we will set to something else in BATS). + Writer stdoutWriter = new OutputStreamWriter(System.out, StandardCharsets.UTF_8); + recording.readFully(false, stdoutWriter); + stdoutWriter.flush(); + } +} diff --git a/agent/test/encoding/UnicodeTest.java b/agent/test/encoding/UnicodeTest.java new file mode 100644 index 00000000..7bb58615 --- /dev/null +++ b/agent/test/encoding/UnicodeTest.java @@ -0,0 +1,42 @@ +package test.pkg; + +import java.nio.file.Files; +import java.nio.file.Paths; +import java.io.IOException; +import java.io.UnsupportedEncodingException; + +public class UnicodeTest { + public static String echo(String input) { + return input; + } + + public static byte[] echoBytes(byte[] input) { + return input.clone(); + } + + public static void main(String[] args) { + try { + runTest(); + } catch (IOException e) { + e.printStackTrace(); + // exit 1 + System.exit(1); + } + } + + public static void runTest() throws IOException { + byte[] allBytes = Files.readAllBytes(Paths.get("encoding_test.cp1252")); + + String allString = new String(allBytes, "Cp1252"); + String echoedString = echo(allString); + + // print out the echoed string + System.out.println(echoedString); + + byte[] echoedBytes = echoBytes(allBytes); + // print out the echoed bytes as hex + for (byte b : echoedBytes) { + System.out.printf("%02X ", b); + } + } +} diff --git a/agent/test/encoding/appmap.yml b/agent/test/encoding/appmap.yml new file mode 100644 index 00000000..1b191a20 --- /dev/null +++ b/agent/test/encoding/appmap.yml @@ -0,0 +1,3 @@ +name: encoding +packages: +- path: test.pkg diff --git a/agent/test/encoding/encoding.bats b/agent/test/encoding/encoding.bats new file mode 100755 index 00000000..bebe918b --- /dev/null +++ b/agent/test/encoding/encoding.bats @@ -0,0 +1,64 @@ +#!/usr/bin/env bats +# shellcheck disable=SC2164 + +load '../helper' + +sep="$JAVA_PATH_SEPARATOR" +AGENT_JAR="$(find_agent_jar)" +java_cmd="java -cp ${BATS_TEST_DIRNAME}/build -javaagent:'${AGENT_JAR}'" + +setup() { + cd "${BATS_TEST_DIRNAME}" + + mkdir -p build + # Compile tests. Output to test/encoding so package structure 'pkg' works. + # We need to compile both UnicodeTest.java and pkg/Target.java. + javac -d ./build UnicodeTest.java + + # Compile ReadFullyTest, requiring the agent jar on the classpath + javac -cp "${AGENT_JAR}" -d ./build ReadFullyTest.java + + rm -rf "${BATS_TEST_DIRNAME}/tmp/appmap" + _configure_logging +} + +@test "AppMap file encoding with Windows-1252" { + # Run with windows-1252 encoding. + # We assert that the generated file is valid UTF-8 and contains the correct characters, + # even though the JVM default encoding is Windows-1252. + local cmd="${java_cmd} -Dfile.encoding=windows-1252 -Dappmap.recording.auto=true test.pkg.UnicodeTest" + [[ $BATS_VERBOSE_RUN == 1 ]] && echo "cmd: $cmd" >&3 + + eval "$cmd" + + # Verify the output file exists — it should be the only AppMap file generated, with random name + # so glob for tmp/appmap/java/*.appmap.json + appmap_file=$(ls tmp/appmap/java/*.appmap.json) + [ -f "$appmap_file" ] + + # Verify it is valid JSON + jq . "$appmap_file" > /dev/null + + # Verify it is valid UTF-8 + iconv -f UTF-8 -t UTF-8 "$appmap_file" > /dev/null + + # Verify it contains the expected Unicode characters + grep -q "Euro: €, Accent: é, Quote: „" "$appmap_file" +} + +@test "Recording.readFully works with Windows-1252 default encoding" { + # Run ReadFullyTest with windows-1252 default encoding. + # We also need to add the agent jar to the classpath so it can find the Recording class. + local cmd="java -cp ${BATS_TEST_DIRNAME}/build${sep}${AGENT_JAR} -Dfile.encoding=windows-1252 test.pkg.ReadFullyTest" + [[ $BATS_VERBOSE_RUN == 1 ]] && echo "cmd: $cmd" >&3 + + run eval "$cmd" + + [ "$status" -eq 0 ] + [[ "$output" == *"Check: ⚠️ Привет"* ]] +} + +teardown() { + rm -rf tmp + rm -rf build +} diff --git a/agent/test/encoding/encoding_test.cp1252 b/agent/test/encoding/encoding_test.cp1252 new file mode 100644 index 00000000..10b7f7d6 --- /dev/null +++ b/agent/test/encoding/encoding_test.cp1252 @@ -0,0 +1,5 @@ +Euro: , Accent: , Quote: +---SEPARATOR--- +:7ǞI +---BINARY--- +ޭ \ No newline at end of file From b43ea43ec07379a875c5b13a8f9fe85d0337d4d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 22 Dec 2025 17:42:32 +0100 Subject: [PATCH 12/21] ci: cancel in-progress build and test workflow runs Add concurrency configuration to the 'build-and-test' workflow to cancel currently running jobs when a new push is made to the same branch. This optimizes CI resource usage by preventing redundant builds. --- .github/workflows/build-and-test.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 49825eec..93a8dc36 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -1,6 +1,10 @@ name: Build and test on: [push] +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: build-and-check: name: Build and check From b88697f6043c165139783db648d78efc5c0a572b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 29 Dec 2025 15:28:55 +0100 Subject: [PATCH 13/21] fix: Handle empty 'exclude' field in appmap.yml This commit resolves a NullPointerException that occurred when the `exclude` field within a package definition in `appmap.yml` was present but empty (e.g., `exclude:`). The Jackson YAML parser would deserialize this as `null`, leading to a crash when its length was accessed. The fix involves two parts: 1. Modifying `AppMapConfig.java` to add a null check in the stream reduction logic for `p.exclude`, treating a null array as having a length of zero. 2. Initializing the `exclude` field as an empty array in the `AppMapPackage` constructor using `@JsonCreator`, ensuring it's never null during deserialization. A new regression test case, `loadEmptyExcludeField`, has been added to `AppMapConfigTest.java` to verify that an empty `exclude` field is now handled correctly and does not cause a crash. The test ensures that the `exclude` array is non-null and has a length of zero, confirming the intended behavior. --- .../com/appland/appmap/config/AppMapConfig.java | 2 +- .../com/appland/appmap/config/AppMapPackage.java | 11 +++++++++++ .../appland/appmap/config/AppMapConfigTest.java | 15 ++++++++++++++- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java b/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java index cc49f313..67b6bc6f 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java +++ b/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java @@ -145,7 +145,7 @@ static AppMapConfig load(Path configFile, boolean mustExist) { int count = singleton.packages.length; count = Arrays.stream(singleton.packages).map(p -> p.exclude).reduce(count, - (acc, e) -> acc += e.length, Integer::sum); + (acc, e) -> acc += e == null ? 0 : e.length, Integer::sum); int pattern_threshold = Properties.PatternThreshold; if (count > pattern_threshold) { diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java index b5021987..5fdd42ce 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java +++ b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java @@ -21,6 +21,17 @@ public class AppMapPackage { public boolean shallow = false; public Boolean allMethods = true; + @JsonCreator + public AppMapPackage(@JsonProperty("path") String path, + @JsonProperty("exclude") String[] exclude, + @JsonProperty("shallow") Boolean shallow, + @JsonProperty("allMethods") Boolean allMethods) { + this.path = path; + this.exclude = exclude == null ? new String[] {} : exclude; + this.shallow = shallow != null && shallow; + this.allMethods = allMethods == null ? true : allMethods; + } + public static class LabelConfig { private Pattern className = null; diff --git a/agent/src/test/java/com/appland/appmap/config/AppMapConfigTest.java b/agent/src/test/java/com/appland/appmap/config/AppMapConfigTest.java index f655ac4d..fef83891 100644 --- a/agent/src/test/java/com/appland/appmap/config/AppMapConfigTest.java +++ b/agent/src/test/java/com/appland/appmap/config/AppMapConfigTest.java @@ -118,6 +118,19 @@ public void loadPackagesKeyWithScalarValue() throws Exception { String actualErr = tapSystemErr(() -> AppMapConfig.load(configFile, false)); assertTrue(actualErr.contains("AppMap: encountered syntax error in appmap.yml")); } -} + @Test + public void loadEmptyExcludeField() throws Exception { + Path configFile = tmpdir.resolve("appmap.yml"); + final String contents = "name: test\npackages:\n- path: com.example\n exclude:\n"; + Files.write(configFile, contents.getBytes()); + + AppMapConfig config = AppMapConfig.load(configFile, false); + assertNotNull(config); + assertEquals(1, config.packages.length); + assertEquals("com.example", config.packages[0].path); + assertNotNull(config.packages[0].exclude); + assertEquals(0, config.packages[0].exclude.length); + } +} From 4a43f7488826b843a97f1270658df04fec83b9fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Wed, 31 Dec 2025 12:25:31 +0100 Subject: [PATCH 14/21] fix: Do not require SQLException class in SQL hooks The agent was encountering a NoClassDefFoundError for java.sql.SQLException in some environments (e.g., Oracle UCP) due to class loading issues. - Removed direct import of java.sql.SQLException in SqlQuery.java. - Changed catch blocks in getDbName methods to catch Throwable instead of SQLException to broaden exception handling and prevent crashes when SQLException is not directly available. - Added regression test SqlQuerySQLExceptionAvailabilityTest to reproduce the environment where SQLException is missing and verify the fix. --- .../appmap/process/hooks/SqlQuery.java | 5 +- .../SqlQuerySQLExceptionAvailabilityTest.java | 130 ++++++++++++++++++ 2 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 agent/src/test/java/com/appland/appmap/process/hooks/SqlQuerySQLExceptionAvailabilityTest.java diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java index 343ee265..ef72e868 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java @@ -2,7 +2,6 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; -import java.sql.SQLException; import java.sql.Statement; import com.appland.appmap.output.v1.Event; @@ -55,7 +54,7 @@ private static String getDbName(Connection c) { } dbname = metadata.getDatabaseProductName(); - } catch (SQLException e) { + } catch (Throwable e) { Logger.println("WARNING, failed to get database name"); e.printStackTrace(System.err); } @@ -74,7 +73,7 @@ private static String getDbName(Statement s) { } dbname = getDbName(s.getConnection()); - } catch (SQLException e) { + } catch (Throwable e) { Logger.println("WARNING, failed to get statement's connection"); e.printStackTrace(System.err); } diff --git a/agent/src/test/java/com/appland/appmap/process/hooks/SqlQuerySQLExceptionAvailabilityTest.java b/agent/src/test/java/com/appland/appmap/process/hooks/SqlQuerySQLExceptionAvailabilityTest.java new file mode 100644 index 00000000..50be6ca9 --- /dev/null +++ b/agent/src/test/java/com/appland/appmap/process/hooks/SqlQuerySQLExceptionAvailabilityTest.java @@ -0,0 +1,130 @@ +package com.appland.appmap.process.hooks; + +import org.junit.jupiter.api.Test; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Method; +import java.sql.Connection; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.mockito.Mockito.mock; + +/** + * Regression test for a {@link NoClassDefFoundError} involving {@link java.sql.SQLException}. + *

+ * In certain environments (e.g., specific configurations of Oracle UCP or custom container classloaders), + * {@code java.sql.SQLException} might not be visible to the classloader responsible for loading + * {@code com.appland.appmap.process.hooks.SqlQuery}. This can lead to a crash when the agent attempts + * to handle SQL events. + *

+ * The crash manifests as: + * 

+ * Caused by: com.example.operation.flow.FlowException: java/sql/SQLException
+ * ...
+ * Caused by: java.lang.NoClassDefFoundError: java/sql/SQLException
+ *     at com.appland.appmap.process.hooks.SqlQuery.getDbName(SqlQuery.java:76)
+ *     at com.appland.appmap.process.hooks.SqlQuery.recordSql(SqlQuery.java:89)
+ *     at com.appland.appmap.process.hooks.SqlQuery.executeQuery(SqlQuery.java:172)
+ *
+ *

+ * This test reproduces the environment by using a custom {@link ClassLoader} that explicitly + * throws {@link ClassNotFoundException} when {@code java.sql.SQLException} is requested. + * It verifies that {@code SqlQuery} can be loaded and executed without triggering the error. + */ +public class SqlQuerySQLExceptionAvailabilityTest { + + @Test + public void testSqlQueryResilienceToMissingSQLException() throws Exception { + // 1. Create a RestrictedClassLoader that hides java.sql.SQLException + ClassLoader restrictedLoader = new RestrictedClassLoader(this.getClass().getClassLoader()); + + // 2. Load the SqlQuery class using the restricted loader. + // This forces the verifier to check dependencies of SqlQuery using our restricted loader. + // If SqlQuery explicitly catches or references SQLException in a way that requires resolution, + // this (or the method invocation below) should fail. + String sqlQueryClassName = "com.appland.appmap.process.hooks.SqlQuery"; + Class sqlQueryClass = restrictedLoader.loadClass(sqlQueryClassName); + + // 3. Invoke a method that triggers the problematic code path (getDbName). + // We choose recordSql(Event, Connection, String) which calls getDbName(Connection). + Method recordSqlMethod = sqlQueryClass.getMethod("recordSql", + com.appland.appmap.output.v1.Event.class, + java.sql.Connection.class, + String.class + ); + + // Prepare arguments + com.appland.appmap.output.v1.Event mockEvent = mock(com.appland.appmap.output.v1.Event.class); + try (Connection mockConnection = mock(Connection.class)) { + // Ensure getMetaData() throws an exception (simulating a failure), but we catch Throwable now. + // Note: We can't easily throw SQLException here because it's checked, and we're in a context + // where we claim it doesn't exist? Actually, the test code here runs in the normal classloader, + // so we CAN throw it. The question is how SqlQuery handles it. + // However, if SqlQuery references SQLException in its bytecode, loading/verification fails before execution. + + // Let's just run it. The mere act of loading and verifying the method is the primary test. + // Executing it ensures JIT/runtime verification passes too. + + assertDoesNotThrow(() -> { + recordSqlMethod.invoke(null, mockEvent, mockConnection, "SELECT 1"); + }, "SqlQuery should not fail even if java.sql.SQLException is missing"); + } + } + + /** + * A ClassLoader that throws ClassNotFoundException for java.sql.SQLException + * and forces re-definition of SqlQuery to ensure it's loaded by this loader. + */ + private static class RestrictedClassLoader extends ClassLoader { + + public RestrictedClassLoader(ClassLoader parent) { + super(parent); + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + String forbiddenClassName = "java.sql.SQLException"; + if (forbiddenClassName.equals(name)) { + throw new ClassNotFoundException("Simulated missing class: " + name); + } + + // If it's the target class, we want to define it ourselves to ensure + // this classloader (and its restrictions) is used for verification. + String targetClassName = "com.appland.appmap.process.hooks.SqlQuery"; + if (targetClassName.equals(name)) { + // Check if already loaded + Class loaded = findLoadedClass(name); + if (loaded != null) { + return loaded; + } + + try { + byte[] bytes = loadClassBytes(name); + return defineClass(name, bytes, 0, bytes.length); + } catch (IOException e) { + throw new ClassNotFoundException("Failed to read bytes for " + name, e); + } + } + + // For everything else, delegate to parent + return super.loadClass(name); + } + + private byte[] loadClassBytes(String className) throws IOException { + String resourceName = "/" + className.replace('.', '/') + ".class"; + try (InputStream is = getClass().getResourceAsStream(resourceName)) { + if (is == null) { + throw new IOException("Resource not found: " + resourceName); + } + ByteArrayOutputStream stream = new ByteArrayOutputStream(); + byte[] buffer = new byte[4096]; + int bytesRead; + while ((bytesRead = is.read(buffer)) != -1) { + stream.write(buffer, 0, bytesRead); + } + return stream.toByteArray(); + } + } + } +} From 474af6a6497b3c7deeacacc831b191e5db351650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Fri, 2 Jan 2026 13:55:29 +0100 Subject: [PATCH 15/21] fix: More robust matching of excludes --- .../appland/appmap/config/AppMapPackage.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java index 5fdd42ce..f8beffb8 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java +++ b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java @@ -120,20 +120,23 @@ public LabelConfig find(FullyQualifiedName canonicalName) { } /** - * Returns whether or not the canonical name is explicitly excluded - * - * @param canonicalName the canonical name of the class/method to be checked + * Checks whether the behavior is explicitly excluded + * + * @param behavior the behavior to be checked + * @return {@code true} if the behavior is excluded */ public Boolean excludes(CtBehavior behavior) { - FullyQualifiedName fqn = null; + final String fqClass = behavior.getDeclaringClass().getName(); + String candidateName = null; for (String exclusion : this.exclude) { - if (behavior.getDeclaringClass().getName().startsWith(exclusion)) { + if (fqClass.startsWith(exclusion)) { return true; } else { - if (fqn == null) { - fqn = new FullyQualifiedName(behavior); + if (candidateName == null) { + candidateName = fqClass + "." + behavior.getName(); } - if (fqn.toString().startsWith(exclusion)) { + + if (candidateName.startsWith(exclusion.replace('#', '.'))) { return true; } } From 22f095a61b7aba1cfcaa19ea79e2fea04f63b473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Fri, 2 Jan 2026 14:34:06 +0100 Subject: [PATCH 16/21] fix(jdbc): Fix PreparedStatement hooking and ArgumentArray validation - Fix `ArgumentArraySystem` validation to correctly count runtime parameters, resolving a `CompileError` when hooking methods with mismatched signatures (e.g. `PreparedStatement.executeUpdate`). - Refactor `SqlQuery` hooks to support `PreparedStatement` and `CallableStatement`. - Introduce a `WeakHashMap` cache in `SqlQuery` to store SQL strings for PreparedStatements upon creation. - Consolidate `execute`, `executeQuery`, and `executeUpdate` hooks using `@ArgumentArray` to handle both `Statement` (with SQL arg) and `PreparedStatement` (cached SQL) in a single implementation. - Remove redundant return hooks that were generating extra events. - Reorganize the code per-method for readability and maintainability. - Add regression tests in `OracleRepositoryTests` for `PreparedStatement` and `CallableStatement`. --- .../com/appland/appmap/output/v1/Value.java | 5 + .../appmap/process/hooks/SqlQuery.java | 317 ++++++------------ agent/test/jdbc/jdbc.bats | 8 +- .../OracleRepositoryTests.java | 40 +++ 4 files changed, 149 insertions(+), 221 deletions(-) mode change 100644 => 100755 agent/test/jdbc/jdbc.bats diff --git a/agent/src/main/java/com/appland/appmap/output/v1/Value.java b/agent/src/main/java/com/appland/appmap/output/v1/Value.java index c486e87c..f3a63b80 100644 --- a/agent/src/main/java/com/appland/appmap/output/v1/Value.java +++ b/agent/src/main/java/com/appland/appmap/output/v1/Value.java @@ -90,6 +90,11 @@ public Value setName(String name) { return this; } + + public String getName() { + return name; + } + /** * Sets the "kind" field. * @return {@code this} diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java index ef72e868..eafbe5ea 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java @@ -3,6 +3,9 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.Statement; +import java.util.Collections; +import java.util.Map; +import java.util.WeakHashMap; import com.appland.appmap.output.v1.Event; import com.appland.appmap.record.Recorder; @@ -17,12 +20,10 @@ * configuration. */ @Unique("sql_query") +@SuppressWarnings("unused") public class SqlQuery { private static final Recorder recorder = Recorder.getInstance(); - - // ================================================================================================ - // Calls - // ================================================================================================ + private static final Map statementSql = Collections.synchronizedMap(new WeakHashMap<>()); public static void recordSql(Event event, String databaseType, String sql) { event.setSqlQuery(databaseType, sql); @@ -30,6 +31,25 @@ public static void recordSql(Event event, String databaseType, String sql) { recorder.add(event); } + public static void recordSql(Event event, Connection c, String sql) { + recordSql(event, getDbName(c), sql); + } + + public static void recordSql(Event event, Statement s, String sql) { + recordSql(event, getDbName(s), sql); + } + + private static void recordSql(Event event, Statement s, Object[] args) { + String sql = statementSql.get(s); + if (sql == null && args.length > 0 && args[0] instanceof String) { + sql = (String) args[0]; + } + if (sql == null) { + sql = "[unknown sql]"; + } + recordSql(event, s, sql); + } + private static boolean isMock(Object o) { final Class c = o.getClass(); final Package p = c.getPackage(); @@ -80,174 +100,34 @@ private static String getDbName(Statement s) { return dbname; } - public static void recordSql(Event event, Connection c, String sql) { - recordSql(event, getDbName(c), sql); - } - - public static void recordSql(Event event, Statement s, String sql) { - recordSql(event, getDbName(s), sql); - } + // ================================================================================================ + // nativeSQL + // ================================================================================================ @HookClass("java.sql.Connection") public static void nativeSQL(Event event, Connection c, String sql) { recordSql(event, c, sql); } - @HookClass("java.sql.Connection") - public static void prepareCall(Event event, Connection c, String sql) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareCall(Event event, Connection c, String sql, int resultSetType, int resultSetConcurrency) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareCall(Event event, Connection c, String sql, int resultSetType, int resultSetConcurrency, - int resultSetHoldability) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, int autoGeneratedKeys) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, int[] columnIndexes) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, int resultSetType, - int resultSetConcurrency) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, int resultSetType, - int resultSetConcurrency, int resultSetHoldability) { - recordSql(event, c, sql); - } - - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, String[] columnNames) { - recordSql(event, c, sql); - } - - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void addBatch(Event event, Statement s, String sql) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void execute(Event event, Statement s, String sql) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void execute(Event event, Statement s, String sql, int autoGeneratedKeys) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void execute(Event event, Statement s, String sql, int[] columnIndexes) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void execute(Event event, Statement s, String sql, String[] columnNames) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void executeQuery(Event event, Statement s, String sql) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void executeUpdate(Event event, Statement s, String sql) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void executeUpdate(Event event, Statement s, String sql, int autoGeneratedKeys) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void executeUpdate(Event event, Statement s, String sql, int[] columnIndexes) { - recordSql(event, s, sql); - } - - @HookClass("java.sql.Statement") - public static void executeUpdate(Event event, Statement s, String sql, String[] columnNames) { - recordSql(event, s, sql); - } - - // ================================================================================================ - // Returns - // ================================================================================================ - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) public static void nativeSQL(Event event, Connection c, Object returnValue, String sql) { recorder.add(event); } - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareCall(Event event, Connection c, Object returnValue, String sql) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareCall(Event event, Connection c, Object returnValue, String sql, int resultSetType, - int resultSetConcurrency) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareCall(Event event, Connection c, Object returnValue, String sql, int resultSetType, - int resultSetConcurrency, int resultSetHoldability) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, - int autoGeneratedKeys) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, int[] columnIndexes) { - recorder.add(event); - } - - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, int resultSetType, - int resultSetConcurrency) { + @ArgumentArray + @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void nativeSQL(Event event, Connection c, Throwable exception, Object[] args) { + event.setException(exception); recorder.add(event); } - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, int resultSetType, - int resultSetConcurrency, int resultSetHoldability) { - recorder.add(event); - } + // ================================================================================================ + // addBatch + // ================================================================================================ - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, String[] columnNames) { - recorder.add(event); + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void addBatch(Event event, Statement s, String sql) { + recordSql(event, s, sql); } @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) @@ -255,108 +135,111 @@ public static void addBatch(Event event, Statement s, Object returnValue, String recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void execute(Event event, Statement s, Object returnValue, String sql) { + @ArgumentArray + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void addBatch(Event event, Statement s, Throwable exception, Object[] args) { + event.setException(exception); recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void execute(Event event, Statement s, Object returnValue, String sql, int autoGeneratedKeys) { - recorder.add(event); - } + // ================================================================================================ + // execute + // ================================================================================================ - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void execute(Event event, Statement s, Object returnValue, String sql, int[] columnIndexes) { - recorder.add(event); + @ArgumentArray + @HookClass("java.sql.Statement") + public static void execute(Event event, Statement s, Object[] args) { + recordSql(event, s, args); } + @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) { recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void executeQuery(Event event, Statement s, Object returnValue, String sql) { + @ArgumentArray + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void execute(Event event, Statement s, Throwable exception, Object[] args) { + event.setException(exception); recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void executeUpdate(Event event, Statement s, Object returnValue, String sql) { - recorder.add(event); - } + // ================================================================================================ + // executeQuery + // ================================================================================================ - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void executeUpdate(Event event, Statement s, Object returnValue, String sql, int autoGeneratedKeys) { - recorder.add(event); + @ArgumentArray + @HookClass("java.sql.Statement") + public static void executeQuery(Event event, Statement s, Object[] args) { + recordSql(event, s, args); } + @ArgumentArray @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void executeUpdate(Event event, Statement s, Object returnValue, String sql, int[] columnIndexes) { + public static void executeQuery(Event event, Statement s, Object returnValue, Object[] args) { recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void executeUpdate(Event event, Statement s, Object returnValue, String sql, String[] columnNames) { + @ArgumentArray + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void executeQuery(Event event, Statement s, Throwable exception, Object[] args) { + event.setException(exception); recorder.add(event); } // ================================================================================================ - // Exceptions + // executeUpdate // ================================================================================================ - /* - * Many of the methods below are overloaded. However, the hook implementations - * don't make use of the arguments passed to the original method. So, take - * advantage of ArgumentArray's "feature" that causes it to match all - * overloaded mehods by name, and have the hook apply to each of them. - */ - @ArgumentArray - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void nativeSQL(Event event, Connection c, Throwable exception, Object[] args) { - event.setException(exception); - recorder.add(event); - } - - @ArgumentArray - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void prepareCall(Event event, Connection c, Throwable exception, Object[] args) { - event.setException(exception); - recorder.add(event); + @HookClass("java.sql.Statement") + public static void executeUpdate(Event event, Statement s, Object[] args) { + recordSql(event, s, args); } @ArgumentArray - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void prepareStatement(Event event, Connection c, Throwable exception, Object[] args) { - event.setException(exception); + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void executeUpdate(Event event, Statement s, Object returnValue, Object[] args) { recorder.add(event); } @ArgumentArray @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void addBatch(Event event, Statement s, Throwable exception, Object[] args) { + public static void executeUpdate(Event event, Statement s, Throwable exception, Object[] args) { event.setException(exception); recorder.add(event); } - @ArgumentArray - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void execute(Event event, Statement s, Throwable exception, Object[] args) { - event.setException(exception); - recorder.add(event); - } + // ================================================================================================ + // prepareCall + // ================================================================================================ @ArgumentArray - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void executeQuery(Event event, Statement s, Throwable exception, Object[] args) { - event.setException(exception); - recorder.add(event); + @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) + public static void prepareCall(Event event, Connection c, Object returnValue, Object[] args) { + if (returnValue != null) { + String sql = "[unknown sql]"; + if (args.length > 0 && args[0] instanceof String) { + sql = (String) args[0]; + } + statementSql.put((Statement) returnValue, sql); + } } + // ================================================================================================ + // prepareStatement + // ================================================================================================ + @ArgumentArray - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void executeUpdate(Event event, Statement s, Throwable exception, Object[] args) { - event.setException(exception); - recorder.add(event); + @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) + public static void prepareStatement(Event event, Connection c, Object returnValue, Object[] args) { + if (returnValue != null) { + String sql = "[unknown sql]"; + if (args.length > 0 && args[0] instanceof String) { + sql = (String) args[0]; + } + statementSql.put((Statement) returnValue, sql); + } } -} +} \ No newline at end of file diff --git a/agent/test/jdbc/jdbc.bats b/agent/test/jdbc/jdbc.bats old mode 100644 new mode 100755 index a76c7062..b376494a --- a/agent/test/jdbc/jdbc.bats +++ b/agent/test/jdbc/jdbc.bats @@ -3,7 +3,7 @@ load '../helper' setup_file() { - cd test/jdbc + cd "$BATS_TEST_DIR" || true _configure_logging ./gradlew -q clean @@ -19,9 +19,9 @@ setup() { output="$(<./tmp/appmap/junit/com_example_accessingdatajpa_CustomerRepositoryTests_testFindFromBogusTable.appmap.json)" assert_json_eq '.metadata.test_status' succeeded - assert_json_eq '.events | length' 6 - assert_json_eq '.events[3].exceptions | length' 1 - assert_json_eq '.events[3].exceptions[0].class' org.h2.jdbc.JdbcSQLSyntaxErrorException + assert_json_eq '.events | length' 4 + assert_json_eq '.events[2].exceptions | length' 3 + assert_json_eq '.events[2].exceptions[2].class' org.h2.jdbc.JdbcSQLSyntaxErrorException } @test "failing test" { diff --git a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java index fc2429d4..f0090cea 100644 --- a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java +++ b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java @@ -2,8 +2,14 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; import java.util.List; +import javax.sql.DataSource; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; import org.springframework.beans.factory.annotation.Autowired; @@ -24,6 +30,40 @@ public class OracleRepositoryTests { @Autowired private CustomerRepository customers; + @Autowired + private DataSource dataSource; + + @Test + public void testJDBC() throws SQLException { + try (Connection connection = dataSource.getConnection()) { + try (PreparedStatement statement = connection.prepareStatement("SELECT 1 FROM DUAL")) { + statement.execute(); + statement.execute(); + } + try (CallableStatement statement = connection.prepareCall("begin null; end;")) { + statement.execute(); + } + } + } + + @Test + public void testPreparedStatement() throws SQLException { + try (Connection connection = dataSource.getConnection(); + PreparedStatement preparedStatement = connection.prepareStatement("SELECT * FROM DUAL WHERE DUMMY = ?")) { + preparedStatement.setString(1, "X"); + preparedStatement.execute(); + } + } + + @Test + public void testCallableStatement() throws SQLException { + try (Connection connection = dataSource.getConnection(); + CallableStatement callableStatement = connection.prepareCall("begin :1 := 1; end;")) { + callableStatement.registerOutParameter(1, java.sql.Types.INTEGER); + callableStatement.execute(); + } + } + @Test public void testFindByLastName() { Customer customer = new Customer("Oracle", "User"); From 73daf93980db2c169758ac4f90a912a1cee73613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Fri, 2 Jan 2026 14:36:16 +0100 Subject: [PATCH 17/21] fix: Don't throw when loading logging config fails --- .../appmap/util/tinylog/AppMapConfigurationLoader.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/agent/src/main/java/com/appland/appmap/util/tinylog/AppMapConfigurationLoader.java b/agent/src/main/java/com/appland/appmap/util/tinylog/AppMapConfigurationLoader.java index defb275e..d99cb64a 100644 --- a/agent/src/main/java/com/appland/appmap/util/tinylog/AppMapConfigurationLoader.java +++ b/agent/src/main/java/com/appland/appmap/util/tinylog/AppMapConfigurationLoader.java @@ -16,7 +16,7 @@ public class AppMapConfigurationLoader implements ConfigurationLoader { @Override - public Properties load() throws IOException { + public Properties load() { Properties properties = new Properties(); final File localConfigFile = new File("appmap-log.local.properties"); final String[] configFiles = {"appmap-log.properties", localConfigFile.getName()}; @@ -28,6 +28,8 @@ public Properties load() throws IOException { if (stream != null) { properties.load(stream); } + } catch (IOException e) { + InternalLogger.log(Level.ERROR, e, "Failed to load " + configFile + " from classloader " + cl); } } } From 5ee0d506891b38dd7769f35781d6bd93eb1d295c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 5 Jan 2026 16:15:27 +0100 Subject: [PATCH 18/21] wip: More comprehensive JDBC tests --- agent/test/jdbc/build.gradle | 8 +- agent/test/jdbc/jdbc.bats | 45 +++- agent/test/jdbc/regenerate_jdbc_snapshots.sh | 52 +++++ ...pa_PureJDBCTests_testCallableStatement.sql | 0 ...ssingdatajpa_PureJDBCTests_testExecute.sql | 2 + ...ccessingdatajpa_PureJDBCTests_testJDBC.sql | 0 ...a_PureJDBCTests_testMultipleExecutions.sql | 3 + ...pa_PureJDBCTests_testPreparedStatement.sql | 0 ...reJDBCTests_testPreparedStatementBatch.sql | 0 ...CTests_testPreparedStatementLargeBatch.sql | 0 ...tajpa_PureJDBCTests_testStatementBatch.sql | 0 ...ssingdatajpa_PureJDBCTests_testUpdates.sql | 0 ...datajpa_PureJDBCTests_testexecuteQuery.sql | 0 .../OracleRepositoryTests.java | 39 +--- .../accessingdatajpa/PureJDBCTests.java | 201 ++++++++++++++++++ 15 files changed, 306 insertions(+), 44 deletions(-) create mode 100755 agent/test/jdbc/regenerate_jdbc_snapshots.sh create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testCallableStatement.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testExecute.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testJDBC.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testMultipleExecutions.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatement.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementBatch.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementLargeBatch.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testStatementBatch.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testUpdates.sql create mode 100644 agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testexecuteQuery.sql create mode 100644 agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java diff --git a/agent/test/jdbc/build.gradle b/agent/test/jdbc/build.gradle index 57946f05..ef47f6c8 100644 --- a/agent/test/jdbc/build.gradle +++ b/agent/test/jdbc/build.gradle @@ -33,9 +33,9 @@ test { } jvmArgs += [ "-javaagent:${appmapJar}", - "-Dappmap.config.file=appmap.yml", - "-Djava.util.logging.config.file=${System.env.JUL_CONFIG}" - // "-Dappmap.debug=true", - // "-Dappmap.debug.file=../../build/log/jdbc-appmap.log" + "-Dappmap.config.file=appmap.yml", + "-Djava.util.logging.config.file=${System.env.JUL_CONFIG}", + "-Dappmap.debug=true", + // "-Dappmap.debug.file=../../build/log/jdbc-appmap.log" ] } diff --git a/agent/test/jdbc/jdbc.bats b/agent/test/jdbc/jdbc.bats index b376494a..3dd0bc44 100755 --- a/agent/test/jdbc/jdbc.bats +++ b/agent/test/jdbc/jdbc.bats @@ -1,5 +1,7 @@ #!/usr/bin/env bats +# To regenerate the SQL snapshots, run ./regenerate_jdbc_snapshots.sh from this directory. + load '../helper' setup_file() { @@ -13,7 +15,7 @@ setup() { rm -rf tmp/appmap } -@test "successful test" { +@test "h2 successful test" { run ./gradlew -q test --tests 'CustomerRepositoryTests.testFindFromBogusTable' assert_success @@ -24,7 +26,7 @@ setup() { assert_json_eq '.events[2].exceptions[2].class' org.h2.jdbc.JdbcSQLSyntaxErrorException } -@test "failing test" { +@test "h2 failing test" { run ./gradlew -q test --tests 'CustomerRepositoryTests.testFails' assert_failure @@ -36,10 +38,47 @@ setup() { # Requires a running Oracle instance. # Locally: docker-compose up -d (in agent/test/jdbc) # CI: Service is configured in .github/workflows/build-and-test.yml -@test "oracle integration test" { +@test "oracle jpa test" { run ./gradlew -q test --tests 'OracleRepositoryTests' assert_success + + map_file="tmp/appmap/junit/com_example_accessingdatajpa_OracleRepositoryTests_testFindByLastName.appmap.json" + [ -f "$map_file" ] + output="$(<"$map_file")" + assert_json_eq '.metadata.test_status' succeeded + event_count=$(echo "$output" | jq '.events | length') + if [ "$event_count" -le 0 ]; then + echo "Expected event count to be greater than 0, but it was $event_count" + return 1 + fi } +@test "oracle pure jdbc test suite (snapshot)" { + run ./gradlew -q test --tests 'PureJDBCTests' + assert_success + + # Verify that the list of generated appmaps corresponds to the list of snapshots. + appmap_list=$(ls tmp/appmap/junit/com_example_accessingdatajpa_PureJDBCTests_*.appmap.json | xargs -n 1 basename | sed 's/\.appmap\.json$//' | sort) + snapshot_list=$(ls snapshots/*.sql | xargs -n 1 basename | sed 's/\.sql$//' | sort) + + run diff -u <(echo "$appmap_list") <(echo "$snapshot_list") + assert_success "Mismatch between generated AppMaps and snapshots" + + for f in tmp/appmap/junit/com_example_accessingdatajpa_PureJDBCTests_*.appmap.json; do + snapshot_file="snapshots/$(basename "$f" .appmap.json).sql" + [ -f "$snapshot_file" ] || { echo "Snapshot file not found: $snapshot_file"; return 1; } + + new_output_file=$(mktemp) + jq -r '.events[] | select(.sql_query) | .sql_query.sql' "$f" > "$new_output_file" + + run diff -u "$snapshot_file" "$new_output_file" + assert_success "Snapshot mismatch for $(basename "$f")" + + rm "$new_output_file" + done +} + + + diff --git a/agent/test/jdbc/regenerate_jdbc_snapshots.sh b/agent/test/jdbc/regenerate_jdbc_snapshots.sh new file mode 100755 index 00000000..3fa088d4 --- /dev/null +++ b/agent/test/jdbc/regenerate_jdbc_snapshots.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env bash + +set -eo pipefail + +# This script regenerates the SQL snapshots for the PureJDBCTests. +# It should be run from the agent/test/jdbc directory. + +# Check if ORACLE_URL is set +if [[ -z "${ORACLE_URL:-}" ]]; then + echo "ERROR: ORACLE_URL environment variable is not set." >&2 + echo "Please set ORACLE_URL to your Oracle database connection string, e.g.:" >&2 + echo " export ORACLE_URL=\"jdbc:oracle:thin:@localhost:1521\"" >&2 + exit 1 +fi + +echo "INFO: Running PureJDBCTests to generate new AppMaps..." + +# Source helper.bash to get _find_agent_jar function +# Set BATS_TEST_DIR so helper.bash can locate files correctly +export BATS_TEST_DIR="$(pwd)" +source ../helper.bash + +find_agent_jar +if [[ -z "$AGENT_JAR" ]]; then + echo "ERROR: Agent JAR not found by helper.bash. Please ensure the agent is built." >&2 + exit 1 +fi + +export ORACLE_URL +export AGENT_JAR +# JAVA_HOME is handled by gradlew wrapper + +# Run the tests to generate fresh AppMaps +./gradlew -q test --tests 'PureJDBCTests' + +echo "INFO: Regenerating raw SQL snapshots..." + +SNAPSHOT_DIR="$(pwd)/snapshots" +APPMAP_DIR="$(pwd)/tmp/appmap/junit" + +# Clear old snapshots +rm -f "$SNAPSHOT_DIR"/* + +# Generate new raw SQL snapshots +for f in "$APPMAP_DIR"/com_example_accessingdatajpa_PureJDBCTests_*.appmap.json; do + if [ -f "$f" ]; then + snapshot_name=$(basename "$f" .appmap.json).sql + jq -r '.events[] | select(.sql_query) | .sql_query.sql' "$f" > "$SNAPSHOT_DIR/$snapshot_name" + fi +done + +echo "INFO: Snapshots regenerated successfully in $SNAPSHOT_DIR" diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testCallableStatement.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testCallableStatement.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testExecute.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testExecute.sql new file mode 100644 index 00000000..5754c13a --- /dev/null +++ b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testExecute.sql @@ -0,0 +1,2 @@ +INSERT INTO customer (id, first_name, last_name) VALUES (5000, 'Exec', 'Test') +SELECT * FROM customer where id = 5000 diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testJDBC.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testJDBC.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testMultipleExecutions.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testMultipleExecutions.sql new file mode 100644 index 00000000..3200c2ac --- /dev/null +++ b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testMultipleExecutions.sql @@ -0,0 +1,3 @@ +SELECT 1 FROM DUAL +SELECT 1 FROM DUAL +SELECT 1 FROM DUAL diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatement.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatement.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementBatch.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementBatch.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementLargeBatch.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testPreparedStatementLargeBatch.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testStatementBatch.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testStatementBatch.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testUpdates.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testUpdates.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testexecuteQuery.sql b/agent/test/jdbc/snapshots/com_example_accessingdatajpa_PureJDBCTests_testexecuteQuery.sql new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java index f0090cea..1c0456b0 100644 --- a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java +++ b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java @@ -2,14 +2,9 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.sql.CallableStatement; -import java.sql.Connection; -import java.sql.PreparedStatement; import java.sql.SQLException; import java.util.List; - import javax.sql.DataSource; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; import org.springframework.beans.factory.annotation.Autowired; @@ -33,37 +28,6 @@ public class OracleRepositoryTests { @Autowired private DataSource dataSource; - @Test - public void testJDBC() throws SQLException { - try (Connection connection = dataSource.getConnection()) { - try (PreparedStatement statement = connection.prepareStatement("SELECT 1 FROM DUAL")) { - statement.execute(); - statement.execute(); - } - try (CallableStatement statement = connection.prepareCall("begin null; end;")) { - statement.execute(); - } - } - } - - @Test - public void testPreparedStatement() throws SQLException { - try (Connection connection = dataSource.getConnection(); - PreparedStatement preparedStatement = connection.prepareStatement("SELECT * FROM DUAL WHERE DUMMY = ?")) { - preparedStatement.setString(1, "X"); - preparedStatement.execute(); - } - } - - @Test - public void testCallableStatement() throws SQLException { - try (Connection connection = dataSource.getConnection(); - CallableStatement callableStatement = connection.prepareCall("begin :1 := 1; end;")) { - callableStatement.registerOutParameter(1, java.sql.Types.INTEGER); - callableStatement.execute(); - } - } - @Test public void testFindByLastName() { Customer customer = new Customer("Oracle", "User"); @@ -71,6 +35,7 @@ public void testFindByLastName() { List findByLastName = customers.findByLastName(customer.getLastName()); - assertThat(findByLastName).extracting(Customer::getLastName).containsOnly(customer.getLastName()); + assertThat(findByLastName).extracting(Customer::getLastName) + .containsOnly(customer.getLastName()); } } diff --git a/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java new file mode 100644 index 00000000..b80cac67 --- /dev/null +++ b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/PureJDBCTests.java @@ -0,0 +1,201 @@ +package com.example.accessingdatajpa; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.CallableStatement; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +@EnabledIfEnvironmentVariable(named = "ORACLE_URL", matches = ".*") +@Execution(ExecutionMode.SAME_THREAD) +public class PureJDBCTests { + + private Connection connection; + + @BeforeEach + public void setUp() throws SQLException { + String oracleUrl = System.getenv("ORACLE_URL"); + String oracleUsername = System.getenv("ORACLE_USERNAME"); + if (oracleUsername == null) { + oracleUsername = "system"; + } + String oraclePassword = System.getenv("ORACLE_PASSWORD"); + if (oraclePassword == null) { + oraclePassword = "oracle"; + } + + connection = DriverManager.getConnection(oracleUrl, oracleUsername, oraclePassword); + + try (Statement statement = connection.createStatement()) { + statement.execute("CREATE TABLE customer (id NUMBER(19,0) NOT NULL, first_name VARCHAR2(255 CHAR), last_name VARCHAR2(255 CHAR), PRIMARY KEY (id))"); + } + } + + @AfterEach + public void tearDown() throws SQLException { + try (Statement statement = connection.createStatement()) { + statement.execute("DROP TABLE customer"); + } + if (connection != null && !connection.isClosed()) { + connection.close(); + } + } + + @Test + public void testJDBC() throws SQLException { + try (PreparedStatement statement = connection.prepareStatement("SELECT 1 FROM DUAL")) { + statement.execute(); + statement.execute(); + } + try (CallableStatement statement = connection.prepareCall("begin null; end; --foobar")) { + statement.execute(); + } + } + + @Test + public void testPreparedStatement() throws SQLException { + try ( + PreparedStatement preparedStatement = connection + .prepareStatement("SELECT * FROM DUAL WHERE DUMMY = ?")) { + preparedStatement.setString(1, "X"); + preparedStatement.execute(); + } + } + + @Test + public void testCallableStatement() throws SQLException { + try ( + CallableStatement callableStatement = connection.prepareCall("begin :1 := 1; end;")) { + callableStatement.registerOutParameter(1, java.sql.Types.INTEGER); + callableStatement.execute(); + } + } + + @Test + public void testStatementBatch() throws SQLException { + try (Statement statement = connection.createStatement()) { + statement.addBatch( + "INSERT INTO customer (id, first_name, last_name) VALUES (1000, 'John', 'Doe')"); + statement.addBatch( + "INSERT INTO customer (id, first_name, last_name) VALUES (1001, 'Jane', 'Doe')"); + int[] updateCounts = statement.executeBatch(); + assertArrayEquals(new int[] {1, 1}, updateCounts); + + statement.clearBatch(); + statement.addBatch( + "INSERT INTO customer (id, first_name, last_name) VALUES (1002, 'Foo', 'Bar')"); + updateCounts = statement.executeBatch(); + assertArrayEquals(new int[] {1}, updateCounts); + } + } + + @Test + public void testPreparedStatementBatch() throws SQLException { + try (PreparedStatement statement = connection + .prepareStatement("INSERT INTO customer (id, first_name, last_name) VALUES (?, ?, ?)")) { + statement.setLong(1, 2000); + statement.setString(2, "John"); + statement.setString(3, "Smith"); + statement.addBatch(); + + statement.setLong(1, 2001); + statement.setString(2, "Jane"); + statement.setString(3, "Smith"); + statement.addBatch(); + + int[] updateCounts = statement.executeBatch(); + assertArrayEquals(new int[] {1, 1}, updateCounts); + } + } + + @Test + public void testPreparedStatementLargeBatch() throws SQLException { + try (PreparedStatement statement = connection + .prepareStatement("INSERT INTO customer (id, first_name, last_name) VALUES (?, ?, ?)")) { + statement.setLong(1, 3000); + statement.setString(2, "Big"); + statement.setString(3, "Batch"); + statement.addBatch(); + + long[] updateCounts = statement.executeLargeBatch(); + assertArrayEquals(new long[] {1}, updateCounts); + } + } + + @Test + public void testUpdates() throws SQLException { + try (PreparedStatement statement = connection + .prepareStatement("INSERT INTO customer (id, first_name, last_name) VALUES (?, ?, ?)")) { + statement.setLong(1, 4000); + statement.setString(2, "Test"); + statement.setString(3, "User1"); + int updateCount = statement.executeUpdate(); + assertEquals(1, updateCount); + } + + try (PreparedStatement statement = connection + .prepareStatement("UPDATE customer SET last_name = ? WHERE first_name = ?")) { + statement.setString(1, "User2"); + statement.setString(2, "Test"); + long largeUpdateCount = statement.executeLargeUpdate(); + assertEquals(1L, largeUpdateCount); + } + } + + @Test + public void testExecute() throws SQLException { + // With update + try (PreparedStatement statement = connection.prepareStatement( + "INSERT INTO customer (id, first_name, last_name) VALUES (5000, 'Exec', 'Test')")) { + boolean result = statement.execute(); + assertFalse(result); // false if it is an update count or there are no results + assertEquals(1, statement.getUpdateCount()); + } + + // With query + try (PreparedStatement statement = connection + .prepareStatement("SELECT * FROM customer where id = 5000")) { + boolean result = statement.execute(); + assertTrue(result); // true if the result is a ResultSet + try (ResultSet rs = statement.getResultSet()) { + assertTrue(rs.next()); + assertEquals("Exec", rs.getString("first_name")); + } + } + } + + @Test + public void testMultipleExecutions() throws SQLException { + try (PreparedStatement ps = connection.prepareStatement("SELECT 1 FROM DUAL")) { + for (int i = 0; i < 3; i++) { + ps.execute(); + } + } + } + + @Test + public void testexecuteQuery() throws SQLException { + try (Statement statement = connection.createStatement()) { + statement.execute("INSERT INTO customer (id, first_name, last_name) VALUES (6000, 'first', 'last')"); + } + + try (Statement statement = connection.createStatement(); + ResultSet rs = statement.executeQuery("select * from customer")) { + assertTrue(rs.next()); + assertEquals("first", rs.getString("first_name")); + } + } +} From 00afec47035e88d845806e83fc89869932838be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 5 Jan 2026 16:16:04 +0100 Subject: [PATCH 19/21] Add more SQL handling --- .../appmap/process/hooks/SqlQuery.java | 96 +++++++++++++++---- agent/test/jdbc/jdbc.bats | 2 +- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java index eafbe5ea..827d18c0 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java @@ -24,6 +24,7 @@ public class SqlQuery { private static final Recorder recorder = Recorder.getInstance(); private static final Map statementSql = Collections.synchronizedMap(new WeakHashMap<>()); + private static final Map> statementBatchSql = Collections.synchronizedMap(new WeakHashMap<>()); public static void recordSql(Event event, String databaseType, String sql) { event.setSqlQuery(databaseType, sql); @@ -101,47 +102,87 @@ private static String getDbName(Statement s) { } // ================================================================================================ - // nativeSQL + // addBatch // ================================================================================================ - @HookClass("java.sql.Connection") - public static void nativeSQL(Event event, Connection c, String sql) { - recordSql(event, c, sql); + @HookClass(value = "java.sql.PreparedStatement", methodEvent = MethodEvent.METHOD_RETURN) + public static void addBatch(Event event, Statement s) { + String sql = statementSql.get(s); + if (sql != null) { + statementBatchSql.computeIfAbsent(s, k -> new java.util.ArrayList<>()).add(sql); + } } - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void nativeSQL(Event event, Connection c, Object returnValue, String sql) { + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void addBatch(Event event, Statement s, String sql) { + statementBatchSql.computeIfAbsent(s, k -> new java.util.ArrayList<>()).add(sql); + } + + // ================================================================================================ + // clearBatch + // ================================================================================================ + + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void clearBatch(Event event, Statement s) { + statementBatchSql.remove(s); + } + + // ================================================================================================ + // executeBatch + // ================================================================================================ + + @HookClass("java.sql.Statement") + public static void executeBatch(Event event, Statement s) { + recordSqlBatch(event, s); + } + + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void executeBatch(Event event, Statement s, Object returnValue) { recorder.add(event); } - @ArgumentArray - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void nativeSQL(Event event, Connection c, Throwable exception, Object[] args) { + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void executeBatch(Event event, Statement s, Throwable exception) { event.setException(exception); recorder.add(event); } // ================================================================================================ - // addBatch + // executeLargeBatch // ================================================================================================ - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void addBatch(Event event, Statement s, String sql) { - recordSql(event, s, sql); + @HookClass("java.sql.Statement") + public static void executeLargeBatch(Event event, Statement s) { + recordSqlBatch(event, s); } @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) { recorder.add(event); } - @ArgumentArray @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void addBatch(Event event, Statement s, Throwable exception, Object[] args) { + public static void executeLargeBatch(Event event, Statement s, Throwable exception) { event.setException(exception); recorder.add(event); } + private static void recordSqlBatch(Event event, Statement s) { + // According to the JDBC spec, calling executeBatch clears the batch + // on the statement. So, we'll remove our copy of it. + java.util.List sqls = statementBatchSql.remove(s); + String sqlToRecord; + + if (sqls != null && !sqls.isEmpty()) { + // In order to represent the batch as a single query, we'll join + // the SQL statements together. + sqlToRecord = String.join(";\n", sqls); + } else { + sqlToRecord = "[empty batch]"; + } + recordSql(event, s, sqlToRecord); + } + // ================================================================================================ // execute // ================================================================================================ @@ -211,6 +252,29 @@ public static void executeUpdate(Event event, Statement s, Throwable exception, recorder.add(event); } + // ================================================================================================ + // executeLargeUpdate + // ================================================================================================ + + @ArgumentArray + @HookClass("java.sql.Statement") + public static void executeLargeUpdate(Event event, Statement s, Object[] args) { + recordSql(event, s, args); + } + + @ArgumentArray + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void executeLargeUpdate(Event event, Statement s, Object returnValue, Object[] args) { + recorder.add(event); + } + + @ArgumentArray + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void executeLargeUpdate(Event event, Statement s, Throwable exception, Object[] args) { + event.setException(exception); + recorder.add(event); + } + // ================================================================================================ // prepareCall // ================================================================================================ diff --git a/agent/test/jdbc/jdbc.bats b/agent/test/jdbc/jdbc.bats index 3dd0bc44..e18f6747 100755 --- a/agent/test/jdbc/jdbc.bats +++ b/agent/test/jdbc/jdbc.bats @@ -5,7 +5,7 @@ load '../helper' setup_file() { - cd "$BATS_TEST_DIR" || true + cd "$BATS_TEST_DIRNAME" || exit 1 _configure_logging ./gradlew -q clean From 2a60a207267b6fc6cd4978832227424145ccde74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Mon, 5 Jan 2026 17:32:38 +0100 Subject: [PATCH 20/21] feat(agent): Optimize class exclusion with PrefixTrie Replaced the linear search for class exclusions in `AppMapPackage` with a `PrefixTrie` for `O(M)` lookup performance, where M is the length of the class name. This significantly improves performance, especially with large exclusion lists. Exclusion patterns can now be specified relative to the package path in `appmap.yml`, improving configuration clarity. Backward compatibility is maintained by supporting both relative and fully qualified exclusion patterns. The original `exclude` array was preserved for debugging and logging purposes to prevent breaking existing functionality in `AppMapConfig`. --- .../appland/appmap/config/AppMapPackage.java | 62 ++++++++++++------- .../com/appland/appmap/util/PrefixTrie.java | 58 +++++++++++++++++ 2 files changed, 96 insertions(+), 24 deletions(-) create mode 100644 agent/src/main/java/com/appland/appmap/util/PrefixTrie.java diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java index f8beffb8..d9dd3ec9 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java +++ b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java @@ -11,6 +11,8 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.appland.appmap.util.PrefixTrie; + import javassist.CtBehavior; public class AppMapPackage { private static final TaggedLogger logger = AppMapConfig.getLogger(null); @@ -20,6 +22,7 @@ public class AppMapPackage { public String[] exclude = new String[] {}; public boolean shallow = false; public Boolean allMethods = true; + private final PrefixTrie excludeTrie = new PrefixTrie(); @JsonCreator public AppMapPackage(@JsonProperty("path") String path, @@ -29,7 +32,20 @@ public AppMapPackage(@JsonProperty("path") String path, this.path = path; this.exclude = exclude == null ? new String[] {} : exclude; this.shallow = shallow != null && shallow; - this.allMethods = allMethods == null ? true : allMethods; + this.allMethods = allMethods == null || allMethods; + + if (exclude != null) { + final String packagePrefix = this.path + "."; + for (String exclusion : exclude) { + if (exclusion.startsWith(packagePrefix)) { + // Absolute path, strip the package path and add the rest + this.excludeTrie.insert(exclusion.substring(packagePrefix.length())); + } else { + // Relative path, add as-is + this.excludeTrie.insert(exclusion); + } + } + } } public static class LabelConfig { @@ -77,7 +93,7 @@ public boolean matches(String className, String methodName) { /** * Check if a class/method is included in the configuration. - * + * * @param canonicalName the canonical name of the class/method to be checked * @return {@code true} if the class/method is included in the configuration. {@code false} if it * is not included or otherwise explicitly excluded. @@ -119,6 +135,14 @@ public LabelConfig find(FullyQualifiedName canonicalName) { return null; } + private String getRelativeClassName(String fqcn) { + final String packagePrefix = this.path + "."; + if (fqcn.startsWith(packagePrefix)) { + return fqcn.substring(packagePrefix.length()); + } + return fqcn; + } + /** * Checks whether the behavior is explicitly excluded * @@ -126,32 +150,22 @@ public LabelConfig find(FullyQualifiedName canonicalName) { * @return {@code true} if the behavior is excluded */ public Boolean excludes(CtBehavior behavior) { - final String fqClass = behavior.getDeclaringClass().getName(); - String candidateName = null; - for (String exclusion : this.exclude) { - if (fqClass.startsWith(exclusion)) { - return true; - } else { - if (candidateName == null) { - candidateName = fqClass + "." + behavior.getName(); - } - - if (candidateName.startsWith(exclusion.replace('#', '.'))) { - return true; - } - } + String fqClass = behavior.getDeclaringClass().getName(); + String relativeClassName = getRelativeClassName(fqClass); + if (this.excludeTrie.startsWith(relativeClassName)) { + return true; } - return false; + // Also check method-specific exclusions + String methodName = behavior.getName(); + String relativeMethodName = String.format("%s.%s", relativeClassName, methodName) + .replace('#', '.'); + return this.excludeTrie.startsWith(relativeMethodName); } public Boolean excludes(FullyQualifiedName canonicalName) { - for (String exclusion : this.exclude) { - if (canonicalName.toString().startsWith(exclusion)) { - return true; - } - } - - return false; + String fqcn = canonicalName.toString(); + String relativeName = getRelativeClassName(fqcn); + return this.excludeTrie.startsWith(relativeName); } } diff --git a/agent/src/main/java/com/appland/appmap/util/PrefixTrie.java b/agent/src/main/java/com/appland/appmap/util/PrefixTrie.java new file mode 100644 index 00000000..031b0382 --- /dev/null +++ b/agent/src/main/java/com/appland/appmap/util/PrefixTrie.java @@ -0,0 +1,58 @@ +package com.appland.appmap.util; + +import java.util.HashMap; +import java.util.Map; + +/** + * A simple Trie (Prefix Tree) for efficient prefix-based string matching. + * This is used to check if a class name matches any of the exclusion patterns. + */ +public class PrefixTrie { + private static class TrieNode { + Map children = new HashMap<>(); + boolean isEndOfWord = false; + } + + private final TrieNode root; + + public PrefixTrie() { + root = new TrieNode(); + } + + /** + * Inserts a word into the Trie. + * @param word The word to insert. + */ + public void insert(String word) { + TrieNode current = root; + for (char ch : word.toCharArray()) { + current = current.children.computeIfAbsent(ch, c -> new TrieNode()); + } + current.isEndOfWord = true; + } + + /** + * Checks if any prefix of the given word exists in the Trie. + * For example, if "java." is in the Trie, this will return true for "java.lang.String". + * @param word The word to check. + * @return {@code true} if a prefix of the word is found in the Trie, {@code false} otherwise. + */ + public boolean startsWith(String word) { + TrieNode current = root; + for (int i = 0; i < word.length(); i++) { + char ch = word.charAt(i); + current = current.children.get(ch); + if (current == null) { + return false; // No prefix match + } + if (current.isEndOfWord) { + // We've found a stored pattern that is a prefix of the word. + // e.g., Trie has "java." and word is "java.lang.String" + return true; + } + } + // The word itself is a prefix or an exact match for a pattern in the Trie + // e.g., Trie has "java.lang" and word is "java.lang" + return current.isEndOfWord; + } +} From 029b7a084ae627448e98838fc0e6be5cab8195c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Rzepecki?= Date: Tue, 6 Jan 2026 17:46:59 +0100 Subject: [PATCH 21/21] Don't require Statement class in SqlQuery hooks In some situations the class is not accessible, which causes crashes like Caused by: java.lang.NoClassDefFoundError: java/sql/Statement at com.appland.appmap.process.hooks.SqlQuery.prepareStatement(SqlQuery.java:306) --- .../java/com/appland/appmap/process/hooks/SqlQuery.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java index 827d18c0..9fde322f 100644 --- a/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java +++ b/agent/src/main/java/com/appland/appmap/process/hooks/SqlQuery.java @@ -23,8 +23,8 @@ @SuppressWarnings("unused") public class SqlQuery { private static final Recorder recorder = Recorder.getInstance(); - private static final Map statementSql = Collections.synchronizedMap(new WeakHashMap<>()); - private static final Map> statementBatchSql = Collections.synchronizedMap(new WeakHashMap<>()); + private static final Map statementSql = Collections.synchronizedMap(new WeakHashMap<>()); + private static final Map> statementBatchSql = Collections.synchronizedMap(new WeakHashMap<>()); public static void recordSql(Event event, String databaseType, String sql) { event.setSqlQuery(databaseType, sql); @@ -287,7 +287,7 @@ public static void prepareCall(Event event, Connection c, Object returnValue, Ob if (args.length > 0 && args[0] instanceof String) { sql = (String) args[0]; } - statementSql.put((Statement) returnValue, sql); + statementSql.put(returnValue, sql); } } @@ -303,7 +303,7 @@ public static void prepareStatement(Event event, Connection c, Object returnValu if (args.length > 0 && args[0] instanceof String) { sql = (String) args[0]; } - statementSql.put((Statement) returnValue, sql); + statementSql.put(returnValue, sql); } } } \ No newline at end of file