diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index e0266407..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 @@ -31,6 +35,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 +122,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/src/main/java/com/appland/appmap/Agent.java b/agent/src/main/java/com/appland/appmap/Agent.java index 3de302b7..3de6be8c 100644 --- a/agent/src/main/java/com/appland/appmap/Agent.java +++ b/agent/src/main/java/com/appland/appmap/Agent.java @@ -72,8 +72,11 @@ 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()); + + 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); @@ -163,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); @@ -214,13 +223,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/src/main/java/com/appland/appmap/config/AppMapConfig.java b/agent/src/main/java/com/appland/appmap/config/AppMapConfig.java index 846d962a..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) { @@ -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(); } } 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..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,31 @@ 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, + @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 || 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 { @@ -66,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. @@ -108,36 +135,37 @@ 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; + } + /** - * 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; - for (String exclusion : this.exclude) { - if (behavior.getDeclaringClass().getName().startsWith(exclusion)) { - return true; - } else { - if (fqn == null) { - fqn = new FullyQualifiedName(behavior); - } - if (fqn.toString().startsWith(exclusion)) { - 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/config/Properties.java b/agent/src/main/java/com/appland/appmap/config/Properties.java index 5c4c168e..5068b702 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); @@ -30,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/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/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 343ee265..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 @@ -2,8 +2,10 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; -import java.sql.SQLException; 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; @@ -18,12 +20,11 @@ * 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<>()); + private static final Map> statementBatchSql = Collections.synchronizedMap(new WeakHashMap<>()); public static void recordSql(Event event, String databaseType, String sql) { event.setSqlQuery(databaseType, sql); @@ -31,6 +32,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(); @@ -55,7 +75,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,290 +94,216 @@ 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); } 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); - } - - @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); - } + // ================================================================================================ + // addBatch + // ================================================================================================ - @HookClass("java.sql.Connection") - public static void prepareStatement(Event event, Connection c, String sql, String[] columnNames) { - 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.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); + statementBatchSql.computeIfAbsent(s, k -> new java.util.ArrayList<>()).add(sql); } // ================================================================================================ - // Returns + // clearBatch // ================================================================================================ - @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.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void clearBatch(Event event, Statement s) { + statementBatchSql.remove(s); } - @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); - } + // ================================================================================================ + // executeBatch + // ================================================================================================ - @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("java.sql.Statement") + public static void executeBatch(Event event, Statement s) { + recordSqlBatch(event, s); } - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql) { + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void executeBatch(Event event, Statement s, Object returnValue) { 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) { + @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); } - @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); - } + // ================================================================================================ + // executeLargeBatch + // ================================================================================================ - @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) { - recorder.add(event); + @HookClass("java.sql.Statement") + public static void executeLargeBatch(Event event, Statement s) { + recordSqlBatch(event, s); } - @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) { + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) + public static void executeLargeBatch(Event event, Statement s, Object returnValue) { recorder.add(event); } - @HookClass(value = "java.sql.Connection", methodEvent = MethodEvent.METHOD_RETURN) - public static void prepareStatement(Event event, Connection c, Object returnValue, String sql, String[] columnNames) { + @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_EXCEPTION) + public static void executeLargeBatch(Event event, Statement s, Throwable exception) { + event.setException(exception); recorder.add(event); } - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void addBatch(Event event, Statement s, Object returnValue, String sql) { - 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; - @HookClass(value = "java.sql.Statement", methodEvent = MethodEvent.METHOD_RETURN) - public static void execute(Event event, Statement s, Object returnValue, String sql) { - recorder.add(event); + 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); } - @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("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 nativeSQL(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.Connection", methodEvent = MethodEvent.METHOD_EXCEPTION) - public static void prepareCall(Event event, Connection c, Throwable exception, Object[] args) { + @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); } + // ================================================================================================ + // executeLargeUpdate + // ================================================================================================ + @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); - recorder.add(event); + @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_EXCEPTION) - public static void addBatch(Event event, Statement s, Throwable exception, Object[] args) { - event.setException(exception); + @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 execute(Event event, Statement s, Throwable exception, Object[] args) { + public static void executeLargeUpdate(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(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(returnValue, sql); + } } -} +} \ No newline at end of file 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/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java b/agent/src/main/java/com/appland/appmap/transform/ClassFileTransformer.java index 34764ffc..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) { @@ -189,9 +203,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 +211,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 +283,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 +308,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 +323,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 3ff0e28e..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; @@ -73,7 +74,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 -> { @@ -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, 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; + } +} 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); } } } 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); + } +} 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(); + } + } + } +} 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 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..6d821876 --- /dev/null +++ b/agent/test/gretty-tomcat/gretty-tomcat.bats @@ -0,0 +1,36 @@ +#!/usr/bin/env bats + +load '../helper' + +setup_file() { + _require_java_version 11 + + 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/gretty-tomcat/src/main/webapp/.keep b/agent/test/gretty-tomcat/src/main/webapp/.keep new file mode 100644 index 00000000..e69de29b diff --git a/agent/test/helper.bash b/agent/test/helper.bash index e11df0b5..f52df480 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,*/*' "${@}" } @@ -154,11 +161,24 @@ check_ws_running() { } wait_for_ws() { - while ! curl -Isf "${WS_URL}" >/dev/null; do + 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' @@ -296,4 +316,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 +} diff --git a/agent/test/jdbc/build.gradle b/agent/test/jdbc/build.gradle index ea106d16..ef47f6c8 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,11 +25,17 @@ 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", - "-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/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 old mode 100644 new mode 100755 index 71bb7c26..e18f6747 --- a/agent/test/jdbc/jdbc.bats +++ b/agent/test/jdbc/jdbc.bats @@ -1,9 +1,11 @@ #!/usr/bin/env bats +# To regenerate the SQL snapshots, run ./regenerate_jdbc_snapshots.sh from this directory. + load '../helper' setup_file() { - cd test/jdbc + cd "$BATS_TEST_DIRNAME" || exit 1 _configure_logging ./gradlew -q clean @@ -13,18 +15,18 @@ setup() { rm -rf tmp/appmap } -@test "successful test" { +@test "h2 successful test" { run ./gradlew -q test --tests 'CustomerRepositoryTests.testFindFromBogusTable' assert_success 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" { +@test "h2 failing test" { run ./gradlew -q test --tests 'CustomerRepositoryTests.testFails' assert_failure @@ -33,5 +35,50 @@ 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 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 new file mode 100644 index 00000000..1c0456b0 --- /dev/null +++ b/agent/test/jdbc/src/test/java/com/example/accessingdatajpa/OracleRepositoryTests.java @@ -0,0 +1,41 @@ +package com.example.accessingdatajpa; + +import static org.assertj.core.api.Assertions.assertThat; + +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; +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; + + @Autowired + private DataSource dataSource; + + @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/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")); + } + } +} 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