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

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/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;
+    }
+  }
+}

agent/src/test/java/com/appland/appmap/process/conditions/ConfigConditionTest.java

@@ -5,11 +5,16 @@
 import static org.junit.jupiter.api.DynamicContainer.dynamicContainer;
 import static org.junit.jupiter.api.DynamicTest.dynamicTest;
 
+import java.util.HashMap;
+import java.util.Map;
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DynamicNode;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -21,9 +26,12 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import com.appland.appmap.config.AppMapConfig;
+import com.appland.appmap.config.AppMapPackage;
 import com.appland.appmap.test.util.ClassBuilder;
 import com.appland.appmap.test.util.MethodBuilder;
 import com.appland.appmap.util.AppMapClassPool;
+import com.appland.appmap.util.LabelUtil;
 
 import javassist.CtClass;
 
@@ -142,6 +150,101 @@ static Stream<Arguments> notSetters() {
                 .setReturnType("java.lang.Integer").endMethod()));
   }
 
+  @Nested
+  class TrivialFilterBypass {
+    private static final String PKG = "com.appland.testfixture";
+
+    private final ConfigCondition condition = new ConfigCondition();
+    private AppMapPackage[] originalPackages;
+    private int classCounter;
+
+    @BeforeEach
+    public void saveConfig() {
+      originalPackages = AppMapConfig.get().packages;
+    }
+
+    @AfterEach
+    public void restoreConfig() {
+      AppMapConfig.get().packages = originalPackages;
+    }
+
+    private MethodBuilder freshGetter(String methodName) {
+      ClassBuilder cb = new ClassBuilder(PKG + ".Class" + (classCounter++));
+      MethodBuilder mb = cb.beginMethod();
+      mb.setName(methodName)
+          .setBody("return \"x\";");
+      try {
+        mb.setReturnType("java.lang.String");
+      } catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+      return mb;
+    }
+
+    private boolean matches(MethodBuilder mb) {
+      Map<String, Object> result = new HashMap<>();
+      return condition.match(mb.getBehavior(), result);
+    }
+
+    @Test
+    public void plainGetterIsSkippedWithoutLabel() throws Exception {
+      AppMapConfig.get().packages = new AppMapPackage[] {
+          new AppMapPackage(PKG, null, false, null)
+      };
+      MethodBuilder mb = freshGetter("getValue");
+      mb.endMethod();
+      assertFalse(matches(mb));
+    }
+
+    @Test
+    public void getterWithLabelsAnnotationIsRecorded() throws Exception {
+      AppMapConfig.get().packages = new AppMapPackage[] {
+          new AppMapPackage(PKG, null, false, null)
+      };
+      MethodBuilder mb = freshGetter("getSecret");
+      mb.addAnnotation(LabelUtil.LABELS_CLASS).endMethod();
+      assertTrue(matches(mb));
+    }
+
+    @Test
+    public void setterWithLabelsAnnotationIsRecorded() throws Exception {
+      AppMapConfig.get().packages = new AppMapPackage[] {
+          new AppMapPackage(PKG, null, false, null)
+      };
+      ClassBuilder cb = new ClassBuilder(PKG + ".Class" + (classCounter++));
+      MethodBuilder mb = cb.beginMethod();
+      mb.setName("setSecret")
+          .addParameter("java.lang.String", "value")
+          .addAnnotation(LabelUtil.LABELS_CLASS)
+          .endMethod();
+      assertTrue(matches(mb));
+    }
+
+    @Test
+    public void getterExplicitlyNamedInConfigIsRecorded() throws Exception {
+      AppMapPackage.LabelConfig methodConfig = new AppMapPackage.LabelConfig(
+          "Class.*", "getValue", new String[] {});
+      AppMapConfig.get().packages = new AppMapPackage[] {
+          new AppMapPackage(PKG, null, false, new AppMapPackage.LabelConfig[] { methodConfig })
+      };
+      MethodBuilder mb = freshGetter("getValue");
+      mb.endMethod();
+      assertTrue(matches(mb));
+    }
+
+    @Test
+    public void getterWithLabelsAttachedInConfigIsRecorded() throws Exception {
+      AppMapPackage.LabelConfig methodConfig = new AppMapPackage.LabelConfig(
+          "Class.*", "getValue", new String[] { "secret" });
+      AppMapConfig.get().packages = new AppMapPackage[] {
+          new AppMapPackage(PKG, null, false, new AppMapPackage.LabelConfig[] { methodConfig })
+      };
+      MethodBuilder mb = freshGetter("getValue");
+      mb.endMethod();
+      assertTrue(matches(mb));
+    }
+  }
+
   static class ClassBuilderResolver implements ParameterResolver {
       @Override
       public boolean supportsParameter(ParameterContext parameterContext,

agent/src/test/java/com/appland/appmap/test/util/MethodBuilder.java

@@ -15,6 +15,7 @@
 import javassist.bytecode.CodeAttribute;
 import javassist.bytecode.ConstPool;
 import javassist.bytecode.Descriptor;
+import javassist.bytecode.LineNumberAttributeTestHelper;
 import javassist.bytecode.LocalVariableAttribute;
 import javassist.bytecode.annotation.Annotation;
 
@@ -30,6 +31,7 @@ public class MethodBuilder {
   private Integer modifiers = Modifier.PUBLIC;
   private List<ParameterBuilder> parameters = new ArrayList<ParameterBuilder>();
   private List<AnnotationBuilder> annotations = new ArrayList<AnnotationBuilder>();
+  private boolean withLineNumber = true;
   private CtMethod behavior;
 
   public CtMethod getBehavior() {
@@ -249,6 +251,9 @@ private CtMethod build() throws CannotCompileException {
 
     codeAttribute.getAttributes().add(locals);
 
+    if (withLineNumber) {
+      codeAttribute.getAttributes().add(LineNumberAttributeTestHelper.singleEntry(constPool, 1));
+    }
 
     final AnnotationsAttribute annotationAttribute =
           new AnnotationsAttribute(constPool, AnnotationsAttribute.visibleTag);

agent/src/test/java/com/appland/appmap/util/LabelUtilTest.java

@@ -0,0 +1,44 @@
+package com.appland.appmap.util;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import com.appland.appmap.test.util.ClassBuilder;
+import com.appland.appmap.test.util.MethodBuilder;
+
+public class LabelUtilTest {
+  @BeforeAll
+  public static void beforeAll() {
+    AppMapClassPool.acquire(Thread.currentThread().getContextClassLoader());
+  }
+
+  @AfterAll
+  public static void afterAll() throws Exception {
+    AppMapClassPool.release();
+  }
+
+  @Test
+  public void detectsLabelsAnnotationByName() throws Exception {
+    MethodBuilder mb = new ClassBuilder("LabelUtilTest$Labeled").beginMethod();
+    mb.setName("getSecret")
+        .setBody("return \"x\";")
+        .setReturnType("java.lang.String")
+        .addAnnotation(LabelUtil.LABELS_CLASS)
+        .endMethod();
+    assertTrue(LabelUtil.hasLabelAnnotation(mb.getBehavior()));
+  }
+
+  @Test
+  public void unlabeledMethodReturnsFalse() throws Exception {
+    MethodBuilder mb = new ClassBuilder("LabelUtilTest$Unlabeled").beginMethod();
+    mb.setName("getSecret")
+        .setBody("return \"x\";")
+        .setReturnType("java.lang.String")
+        .endMethod();
+    assertFalse(LabelUtil.hasLabelAnnotation(mb.getBehavior()));
+  }
+}