fix: record methods with @Labels even if they look trivial

agent/appmap.yml

@@ -1,5 +1,9 @@
 name: appmap-java
 packages:
+- path: com.appland.appmap.test.fixture.labels
+  methods:
+  - class: LabelFixture
+    name: getNamedInConfig
 - path: com.appland.appmap.test.fixture
   exclude:
   - com.appland.appmap.test.util.UnhandledExceptionCollection

agent/build.gradle

@@ -82,6 +82,12 @@ dependencies {
   testImplementation 'com.github.marschall:memoryfilesystem:2.6.1'
 
   testImplementation 'org.apache.maven:maven-model:3.9.5'
+
+  // Test-only dependency on the @Labels / @NoAppMap annotations so that
+  // integration tests can apply them to fixture classes. Production code
+  // refers to these annotations by name to avoid the shadow relocation,
+  // so this stays out of the agent jar.
+  testImplementation project(':annotation')
 }
 
 compileJava {
@@ -146,6 +152,11 @@ task integrationTest(type: Test) {
   description = 'Runs integration tests'
   group = 'verification'
 
+  // Gradle 9 no longer infers these from the test sourceSet for custom Test
+  // tasks; without them the task reports NO-SOURCE and silently passes.
+  testClassesDirs = sourceSets.test.output.classesDirs
+  classpath = sourceSets.test.runtimeClasspath
+  useJUnitPlatform()
   include 'com/appland/appmap/integration/**'
 
   dependsOn shadowJar

agent/src/main/java/com/appland/appmap/Agent.java

@@ -4,6 +4,7 @@
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.lang.instrument.Instrumentation;
+import java.lang.management.ManagementFactory;
 import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -14,6 +15,7 @@
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.Enumeration;
+import java.util.List;
 import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 
@@ -162,43 +164,95 @@ private static void startAutoRecording(Runnable logShutdown) {
   }
 
   private static void addAgentJars(String agentArgs, Instrumentation inst) {
+    Path agentJarPath = locateAgentJar();
+    if (agentJarPath != null) {
+      try {
+        JarFile agentJar = new JarFile(agentJarPath.toFile());
+        inst.appendToSystemClassLoaderSearch(agentJar);
 
-    Path agentJarPath = null;
+        setupRuntime(agentJarPath, agentJar, inst);
+      } catch (IOException | SecurityException | IllegalArgumentException e) {
+        logger.error(e, "Failed loading agent jars");
+        System.exit(1);
+      }
+    }
+  }
+
+  /**
+   * Locate the agent jar on disk so its bundled runtime jar can be extracted and added to the
+   * bootstrap class loader.
+   *
+   * <p>Prefer {@code Class.getResource} on this class because it works even when the agent has
+   * been loaded by the bootstrap class loader (where {@code Class.getClassLoader()} returns
+   * null). Fall back to parsing {@code -javaagent:} out of the JVM input arguments when
+   * {@code getResource} resolves to a {@code file:} URL — which happens when this class is
+   * also visible on a classpath directory, e.g. during integration tests where the agent jar
+   * is attached via {@code -javaagent:} but the build also puts {@code build/classes/java/main}
+   * on the test runtime classpath.
+   *
+   * @return the agent jar path, or {@code null} if no jar could be identified
+   */
+  private static Path locateAgentJar() {
     try {
-      Class<Agent> agentClass = Agent.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. 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")) {
+      URL resourceURL = Agent.class.getResource(
+          "/" + Agent.class.getName().replace('.', '/') + ".class");
+      if (resourceURL != null && "jar".equals(resourceURL.getProtocol())) {
         String resourcePath = resourceURL.getPath();
         URL jarURL = new URL(resourcePath.substring(0, resourcePath.indexOf('!')));
         logger.debug("jarURL: {}", jarURL);
-        agentJarPath = Paths.get(jarURL.toURI());
+        return Paths.get(jarURL.toURI());
       }
     } catch (URISyntaxException | MalformedURLException e) {
-      // Doesn't seem like these should ever happen....
       logger.error(e, "Failed getting path to agent jar");
       System.exit(1);
     }
-    if (agentJarPath != null) {
-      try {
-        JarFile agentJar = new JarFile(agentJarPath.toFile());
-        inst.appendToSystemClassLoaderSearch(agentJar);
 
-        setupRuntime(agentJarPath, agentJar, inst);
-      } catch (IOException | SecurityException | IllegalArgumentException e) {
-        logger.error(e, "Failed loading agent jars");
-        System.exit(1);
+    Path fromArgs = agentJarFromJvmArgs();
+    if (fromArgs != null) {
+      logger.debug("agent jar from -javaagent: {}", fromArgs);
+    }
+    return fromArgs;
+  }
+
+  /**
+   * Parse {@code -javaagent:<path>[=options]} out of the JVM input arguments and return the path
+   * if it points at an existing jar that declares {@code Premain-Class: com.appland.appmap.Agent}.
+   *
+   * @return the agent jar path or {@code null} if no matching {@code -javaagent} arg is present
+   */
+  private static Path agentJarFromJvmArgs() {
+    final String prefix = "-javaagent:";
+    List<String> jvmArgs;
+    try {
+      jvmArgs = ManagementFactory.getRuntimeMXBean().getInputArguments();
+    } catch (SecurityException e) {
+      logger.warn(e, "Unable to read JVM input arguments");
+      return null;
+    }
+    for (String arg : jvmArgs) {
+      if (!arg.startsWith(prefix)) {
+        continue;
+      }
+      String spec = arg.substring(prefix.length());
+      int eq = spec.indexOf('=');
+      String pathPart = eq < 0 ? spec : spec.substring(0, eq);
+      Path candidate = Paths.get(pathPart);
+      if (!Files.isRegularFile(candidate)) {
+        continue;
+      }
+      try (JarFile jf = new JarFile(candidate.toFile())) {
+        if (jf.getManifest() == null) {
+          continue;
+        }
+        String premain = jf.getManifest().getMainAttributes().getValue("Premain-Class");
+        if (Agent.class.getName().equals(premain)) {
+          return candidate.toAbsolutePath();
+        }
+      } catch (IOException e) {
+        logger.debug(e, "Skipping unreadable -javaagent jar {}", candidate);
       }
     }
+    return null;
   }
 
   private static void setupRuntime(Path agentJarPath, JarFile agentJar, Instrumentation inst)

agent/src/main/java/com/appland/appmap/config/AppMapPackage.java

@@ -99,6 +99,15 @@ public String[] getLabels() {
       return this.labels;
     }
 
+    /**
+     * @return {@code true} if this config came from an explicit {@code methods:} entry in
+     *         {@code appmap.yml} (i.e. the user named the method directly), rather than from a
+     *         generic include in exclude mode.
+     */
+    public boolean isExplicit() {
+      return this.name != null;
+    }
+
     /**
      * Checks if the given fully qualified name matches this configuration.
      * Supports matching against both simple and fully qualified class names for

agent/src/main/java/com/appland/appmap/output/v1/CodeObject.java

@@ -1,7 +1,5 @@
 package com.appland.appmap.output.v1;
 
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
@@ -12,9 +10,9 @@
 
 import com.alibaba.fastjson.annotation.JSONField;
 import com.appland.appmap.util.GitUtil;
+import com.appland.appmap.util.LabelUtil;
 import com.appland.appmap.util.Logger;
 
-import javassist.CtAppMapClassType;
 import javassist.CtBehavior;
 import javassist.CtClass;
 
@@ -187,25 +185,9 @@ public CodeObject(CtBehavior behavior, String[] labels) {
     final String file = CodeObject.getSourceFilePath(ctclass);
     final int lineno = behavior.getMethodInfo().getLineNumber(0);
 
-    try {
-      // Look for the Labels annotation by class name. If we introduce a
-      // compile-time dependency on Labels.class, it will get relocated by the
-      // shadowing process, and so won't match the annotation the user put on
-      // their method.
-      final String labelsClass = "com.appland.appmap.annotation.Labels";
-      if (behavior.hasAnnotation(labelsClass)) {
-        Object annotation = CtAppMapClassType.getAnnotation(behavior, labelsClass);
-        Method value = annotation.getClass().getMethod("value");
-        labels = (String[])(value.invoke(annotation));
-      }
-    } catch (ClassNotFoundException e) {
-      Logger.println(e);
-    } catch (IllegalAccessException e) {
-      Logger.println(e);
-    } catch (InvocationTargetException e) {
-      Logger.println(e);
-    } catch (NoSuchMethodException e) {
-      Logger.println(e);
+    String[] annotationLabels = LabelUtil.readAnnotationLabels(behavior);
+    if (annotationLabels != null) {
+      labels = annotationLabels;
     }
 
     this.setType("function")

agent/src/main/java/com/appland/appmap/process/conditions/ConfigCondition.java

@@ -11,6 +11,7 @@
 import com.appland.appmap.transform.annotations.AppMapAppMethod;
 import com.appland.appmap.util.AppMapBehavior;
 import com.appland.appmap.util.FullyQualifiedName;
+import com.appland.appmap.util.LabelUtil;
 import com.appland.appmap.util.Logger;
 
 import javassist.CtBehavior;
@@ -57,7 +58,7 @@ private boolean doMatch(CtBehavior behavior, Map<String, Object> matchResult) {
       }
     }
 
-    if (!AppMapBehavior.isRecordable(behavior) || ignoreMethod(behavior)) {
+    if (!AppMapBehavior.isRecordable(behavior)) {
       return false;
     }
 
@@ -67,12 +68,31 @@ private boolean doMatch(CtBehavior behavior, Map<String, Object> matchResult) {
     }
 
     final AppMapPackage.LabelConfig ls = AppMapConfig.get().includes(new FullyQualifiedName(behavior));
-    if (ls != null) {
-      matchResult.put("labels", ls.getLabels());
-      return true;
+    if (ls == null) {
+      return false;
     }
 
-    return false;
+    // Explicit opt-ins override the trivial-method filter:
+    //  - @Labels annotation on the method
+    //  - method named directly under "methods:" in appmap.yml
+    //  - labels attached to the method via appmap.yml
+    if (!isExplicitlyLabeled(behavior, ls) && ignoreMethod(behavior)) {
+      return false;
+    }
+
+    matchResult.put("labels", ls.getLabels());
+    return true;
+  }
+
+  private static boolean isExplicitlyLabeled(CtBehavior behavior, AppMapPackage.LabelConfig ls) {
+    if (LabelUtil.hasLabelAnnotation(behavior)) {
+      return true;
+    }
+    if (ls.isExplicit()) {
+      return true;
+    }
+    String[] configLabels = ls.getLabels();
+    return configLabels != null && configLabels.length > 0;
   }
 
   private static final Pattern SETTER_PATTERN = Pattern.compile("^set[A-Z].*");

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

@@ -185,8 +185,11 @@ private void writeMetadata(GitUtil git, Metadata metadata) throws IOException {
         this.json.writeKey("git");
         this.json.startObject();
         {
-          this.json.writeKey("repository");
-          this.json.writeValue(git.getRepositoryURL());
+          String repositoryURL = git.getRepositoryURL();
+          if (repositoryURL != null) {
+            this.json.writeKey("repository");
+            this.json.writeValue(repositoryURL);
+          }
           this.json.writeKey("branch");
           this.json.writeValue(git.getBranch());
           this.json.writeKey("commit");

agent/src/main/java/com/appland/appmap/util/GitUtil.java

@@ -99,13 +99,20 @@ public Repository getRepository() {
   public String getRepositoryURL() {
     try {
       List<RemoteConfig> remotes = git.remoteList().call();
+      if (remotes.isEmpty()) {
+        return null;
+      }
       Optional<RemoteConfig> originConfig = remotes.stream().filter(r -> r.getName().equals("origin")).findFirst();
-      List<URIish> uris = originConfig.isPresent() ? originConfig.get().getURIs() : remotes.get(0).getURIs();
+      RemoteConfig remote = originConfig.orElseGet(() -> remotes.get(0));
+      List<URIish> uris = remote.getURIs();
+      if (uris.isEmpty()) {
+        return null;
+      }
       return uris.get(0).toASCIIString();
     } catch (GitAPIException e) {
       logger.warn(e);
     }
-    return "";
+    return null;
   }
 
   public String getBranch() {

agent/src/main/java/com/appland/appmap/util/LabelUtil.java

@@ -0,0 +1,45 @@
+package com.appland.appmap.util;
+
+import java.lang.reflect.Method;
+
+import javassist.CtAppMapClassType;
+import javassist.CtBehavior;
+
+/**
+ * Reads the {@code @Labels} annotation from a {@link CtBehavior} by class name, avoiding a
+ * compile-time dependency on {@code com.appland.appmap.annotation.Labels}. The annotation class
+ * gets relocated by the agent's shadowing process, so a direct reference would not match the
+ * annotation the user actually placed on their method.
+ */
+public final class LabelUtil {
+  public static final String LABELS_CLASS = "com.appland.appmap.annotation.Labels";
+
+  private LabelUtil() {}
+
+  public static boolean hasLabelAnnotation(CtBehavior behavior) {
+    try {
+      return behavior.hasAnnotation(LABELS_CLASS);
+    } catch (Exception e) {
+      Logger.println(e);
+      return false;
+    }
+  }
+
+  /**
+   * @return the {@code value()} of the {@code @Labels} annotation on the given behavior, or
+   *         {@code null} if the annotation is not present or cannot be read.
+   */
+  public static String[] readAnnotationLabels(CtBehavior behavior) {
+    try {
+      if (!behavior.hasAnnotation(LABELS_CLASS)) {
+        return null;
+      }
+      Object annotation = CtAppMapClassType.getAnnotation(behavior, LABELS_CLASS);
+      Method value = annotation.getClass().getMethod("value");
+      return (String[])(value.invoke(annotation));
+    } catch (Exception e) {
+      Logger.println(e);
+      return null;
+    }
+  }
+}