From 6287d4ef8ff8ae61e44d6b044fb2df3b7bf953eb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 18:47:33 +0000 Subject: [PATCH 1/6] Initial plan From 5a406a922f2c83e1b495d192788af1807f969775 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 18:50:54 +0000 Subject: [PATCH 2/6] Fix IndexOutOfBoundsException in GitUtil.getRepositoryURL when no remotes exist Agent-Logs-Url: https://github.com/getappmap/appmap-java/sessions/2575e3fc-ec41-4655-ab8e-0d6c7f3670b7 Co-authored-by: kgilpin <86395+kgilpin@users.noreply.github.com> --- agent/src/main/java/com/appland/appmap/util/GitUtil.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/agent/src/main/java/com/appland/appmap/util/GitUtil.java b/agent/src/main/java/com/appland/appmap/util/GitUtil.java index cb42f880..36cd0a52 100644 --- a/agent/src/main/java/com/appland/appmap/util/GitUtil.java +++ b/agent/src/main/java/com/appland/appmap/util/GitUtil.java @@ -99,8 +99,15 @@ public Repository getRepository() { public String getRepositoryURL() { try { List remotes = git.remoteList().call(); + if (remotes.isEmpty()) { + return ""; + } Optional originConfig = remotes.stream().filter(r -> r.getName().equals("origin")).findFirst(); - List uris = originConfig.isPresent() ? originConfig.get().getURIs() : remotes.get(0).getURIs(); + RemoteConfig remote = originConfig.orElseGet(() -> remotes.get(0)); + List uris = remote.getURIs(); + if (uris.isEmpty()) { + return ""; + } return uris.get(0).toASCIIString(); } catch (GitAPIException e) { logger.warn(e); From 5956839f6665ade2bed844455831c56b4f6d7cba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 19:01:56 +0000 Subject: [PATCH 3/6] Return null from getRepositoryURL when no remotes; skip writing repository key in serializer Agent-Logs-Url: https://github.com/getappmap/appmap-java/sessions/9269e403-28ed-4394-be5f-17ca657e822a Co-authored-by: kgilpin <86395+kgilpin@users.noreply.github.com> --- .../java/com/appland/appmap/record/AppMapSerializer.java | 7 +++++-- agent/src/main/java/com/appland/appmap/util/GitUtil.java | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/agent/src/main/java/com/appland/appmap/record/AppMapSerializer.java b/agent/src/main/java/com/appland/appmap/record/AppMapSerializer.java index 7d7959d8..e1732b1c 100644 --- a/agent/src/main/java/com/appland/appmap/record/AppMapSerializer.java +++ b/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"); diff --git a/agent/src/main/java/com/appland/appmap/util/GitUtil.java b/agent/src/main/java/com/appland/appmap/util/GitUtil.java index 36cd0a52..0ad84cb7 100644 --- a/agent/src/main/java/com/appland/appmap/util/GitUtil.java +++ b/agent/src/main/java/com/appland/appmap/util/GitUtil.java @@ -100,19 +100,19 @@ public String getRepositoryURL() { try { List remotes = git.remoteList().call(); if (remotes.isEmpty()) { - return ""; + return null; } Optional originConfig = remotes.stream().filter(r -> r.getName().equals("origin")).findFirst(); RemoteConfig remote = originConfig.orElseGet(() -> remotes.get(0)); List uris = remote.getURIs(); if (uris.isEmpty()) { - return ""; + return null; } return uris.get(0).toASCIIString(); } catch (GitAPIException e) { logger.warn(e); } - return ""; + return null; } public String getBranch() { From 7171e591dbf45c0b204c6434dce522edb2108bcf Mon Sep 17 00:00:00 2001 From: kgilpin Date: Tue, 5 May 2026 21:48:21 -0400 Subject: [PATCH 4/6] fix: record methods with @Labels even if they look trivial Methods annotated with @Labels (or named explicitly under `methods:` in appmap.yml, or labeled there) were being dropped by the getter/setter trivial-method filter before the agent ever consulted the opt-in. They now bypass that filter so explicit user intent always wins. Extract the @Labels-by-name reading from CodeObject into a shared LabelUtil so ConfigCondition can ask the same question during match. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../appland/appmap/config/AppMapPackage.java | 9 ++ .../appland/appmap/output/v1/CodeObject.java | 26 +---- .../process/conditions/ConfigCondition.java | 30 ++++- .../com/appland/appmap/util/LabelUtil.java | 45 ++++++++ .../conditions/ConfigConditionTest.java | 103 ++++++++++++++++++ .../appmap/test/util/MethodBuilder.java | 5 + .../appland/appmap/util/LabelUtilTest.java | 44 ++++++++ .../LineNumberAttributeTestHelper.java | 35 ++++++ 8 files changed, 270 insertions(+), 27 deletions(-) create mode 100644 agent/src/main/java/com/appland/appmap/util/LabelUtil.java create mode 100644 agent/src/test/java/com/appland/appmap/util/LabelUtilTest.java create mode 100644 agent/src/test/java/javassist/bytecode/LineNumberAttributeTestHelper.java diff --git a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java b/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java index 1a6ff8cf..a87e4b1e 100644 --- a/agent/src/main/java/com/appland/appmap/config/AppMapPackage.java +++ b/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 diff --git a/agent/src/main/java/com/appland/appmap/output/v1/CodeObject.java b/agent/src/main/java/com/appland/appmap/output/v1/CodeObject.java index 25b2b3d0..46b9d9d2 100644 --- a/agent/src/main/java/com/appland/appmap/output/v1/CodeObject.java +++ b/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") diff --git a/agent/src/main/java/com/appland/appmap/process/conditions/ConfigCondition.java b/agent/src/main/java/com/appland/appmap/process/conditions/ConfigCondition.java index 5bfef89d..64a6d20c 100644 --- a/agent/src/main/java/com/appland/appmap/process/conditions/ConfigCondition.java +++ b/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 matchResult) { } } - if (!AppMapBehavior.isRecordable(behavior) || ignoreMethod(behavior)) { + if (!AppMapBehavior.isRecordable(behavior)) { return false; } @@ -67,12 +68,31 @@ private boolean doMatch(CtBehavior behavior, Map 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].*"); diff --git a/agent/src/main/java/com/appland/appmap/util/LabelUtil.java b/agent/src/main/java/com/appland/appmap/util/LabelUtil.java new file mode 100644 index 00000000..bdc56213 --- /dev/null +++ b/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; + } + } +} diff --git a/agent/src/test/java/com/appland/appmap/process/conditions/ConfigConditionTest.java b/agent/src/test/java/com/appland/appmap/process/conditions/ConfigConditionTest.java index 4f492846..22e7a55f 100644 --- a/agent/src/test/java/com/appland/appmap/process/conditions/ConfigConditionTest.java +++ b/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 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 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, diff --git a/agent/src/test/java/com/appland/appmap/test/util/MethodBuilder.java b/agent/src/test/java/com/appland/appmap/test/util/MethodBuilder.java index fa2559ac..31e839f5 100644 --- a/agent/src/test/java/com/appland/appmap/test/util/MethodBuilder.java +++ b/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 parameters = new ArrayList(); private List annotations = new ArrayList(); + 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); diff --git a/agent/src/test/java/com/appland/appmap/util/LabelUtilTest.java b/agent/src/test/java/com/appland/appmap/util/LabelUtilTest.java new file mode 100644 index 00000000..0a37142b --- /dev/null +++ b/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())); + } +} diff --git a/agent/src/test/java/javassist/bytecode/LineNumberAttributeTestHelper.java b/agent/src/test/java/javassist/bytecode/LineNumberAttributeTestHelper.java new file mode 100644 index 00000000..840f7a2a --- /dev/null +++ b/agent/src/test/java/javassist/bytecode/LineNumberAttributeTestHelper.java @@ -0,0 +1,35 @@ +package javassist.bytecode; + +/** + * Test helper that creates a {@link LineNumberAttribute} by invoking its package-private + * constructor. Used by tests that synthesize methods via Javassist and need a non-negative + * line number on the resulting bytecode (e.g. so the agent does not treat the method as + * runtime-generated). + */ +public final class LineNumberAttributeTestHelper { + private LineNumberAttributeTestHelper() {} + + /** + * Build a {@code LineNumberTable} with a single entry mapping pc=0 to the given line. + */ + public static LineNumberAttribute singleEntry(ConstPool cp, int line) { + byte[] info = new byte[6]; + // table_length (u2) + info[0] = 0; + info[1] = 1; + // start_pc (u2) = 0 + info[2] = 0; + info[3] = 0; + // line_number (u2) + info[4] = (byte)((line >> 8) & 0xff); + info[5] = (byte)(line & 0xff); + try { + java.lang.reflect.Constructor ctor = + LineNumberAttribute.class.getDeclaredConstructor(ConstPool.class, byte[].class); + ctor.setAccessible(true); + return ctor.newInstance(cp, info); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + } +} From 6e358ae341de0c9379650fd0946233912be90de4 Mon Sep 17 00:00:00 2001 From: kgilpin Date: Tue, 5 May 2026 21:53:44 -0400 Subject: [PATCH 5/6] fix: restore working integrationTest task Two latent regressions were leaving the agent's integration tests silently disabled: 1. The integrationTest Gradle task was missing testClassesDirs, classpath, and useJUnitPlatform(). Gradle 9 stopped wiring those from the test sourceSet for custom Test tasks, so the task ran as NO-SOURCE and reported success without executing anything. 2. Even with the task fixed, premain's agent-jar discovery via Class.getResource only succeeded when the URL protocol was "jar:". In tests, Agent.class is also visible on a classpath directory (build/classes/java/main) and the resource resolved as "file:", so the runtime jar was never extracted and HookFunctions.onMethodCall stayed null. Instrumented methods then NPE'd at the first hook call. Add a fallback that parses -javaagent: out of the JVM input arguments (via ManagementFactory) and verifies the candidate jar's manifest declares the right Premain-Class. The two paths together let the eight existing integration tests in com/appland/appmap/integration actually run and pass. Add :annotation as a test-only dependency so future integration tests can apply @Labels / @NoAppMap to fixture classes. Co-Authored-By: Claude Opus 4.7 (1M context) --- agent/build.gradle | 11 ++ .../main/java/com/appland/appmap/Agent.java | 102 +++++++++++++----- 2 files changed, 89 insertions(+), 24 deletions(-) diff --git a/agent/build.gradle b/agent/build.gradle index c7dceb37..938075f6 100644 --- a/agent/build.gradle +++ b/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 diff --git a/agent/src/main/java/com/appland/appmap/Agent.java b/agent/src/main/java/com/appland/appmap/Agent.java index 3de6be8c..6b0edea0 100644 --- a/agent/src/main/java/com/appland/appmap/Agent.java +++ b/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. + * + *

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 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:[=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 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) From 8ef7646b466cd66061eb79bd7788ae31a77fac13 Mon Sep 17 00:00:00 2001 From: kgilpin Date: Tue, 5 May 2026 22:22:38 -0400 Subject: [PATCH 6/6] test: end-to-end recording test for the labels-bypass fix Drive a fixture class with @Labels-annotated and config-named getters/ setters under the real -javaagent: setup and assert the resulting classMap and events. Confirms that: - @Labels-annotated getter/setter are recorded with their labels - a getter named under "methods:" in appmap.yml is recorded - a plain unlabeled getter is still filtered out Co-Authored-By: Claude Opus 4.7 (1M context) --- agent/appmap.yml | 4 + .../integration/LabelFixtureRecorderTest.java | 122 ++++++++++++++++++ .../test/fixture/labels/LabelFixture.java | 42 ++++++ 3 files changed, 168 insertions(+) create mode 100644 agent/src/test/java/com/appland/appmap/integration/LabelFixtureRecorderTest.java create mode 100644 agent/src/test/java/com/appland/appmap/test/fixture/labels/LabelFixture.java diff --git a/agent/appmap.yml b/agent/appmap.yml index c0380ac0..f8467fa4 100644 --- a/agent/appmap.yml +++ b/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 diff --git a/agent/src/test/java/com/appland/appmap/integration/LabelFixtureRecorderTest.java b/agent/src/test/java/com/appland/appmap/integration/LabelFixtureRecorderTest.java new file mode 100644 index 00000000..13f7a626 --- /dev/null +++ b/agent/src/test/java/com/appland/appmap/integration/LabelFixtureRecorderTest.java @@ -0,0 +1,122 @@ +package com.appland.appmap.integration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.StringWriter; +import java.nio.file.FileSystems; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.alibaba.fastjson.JSON; +import com.appland.appmap.config.AppMapConfig; +import com.appland.appmap.record.Recorder; +import com.appland.appmap.record.Recording; +import com.appland.appmap.test.fixture.labels.LabelFixture; + +/** + * End-to-end check that the {@code @Labels} bypass and config-named bypass take effect when the + * agent instruments real classes. Runs under {@code integrationTest} with the agent attached as + * {@code -javaagent:}. + */ +public class LabelFixtureRecorderTest { + private static final String FX = "com.appland.appmap.test.fixture.labels.LabelFixture"; + + private final Recorder recorder = Recorder.getInstance(); + + @BeforeEach + public void initialize() throws Exception { + AppMapConfig.initialize(FileSystems.getDefault()); + } + + @Test + public void labeledGettersAndSettersAreRecorded() throws IOException { + final LabelFixture fixture = new LabelFixture(); + + Recording recording = recorder.record(() -> { + fixture.getPlain(); + fixture.getSecret(); + fixture.setSecret("updated"); + fixture.getNamedInConfig(); + fixture.describe(); + }); + assertNotNull(recording); + + StringWriter sw = new StringWriter(); + recording.readFully(true, sw); + Map appmap = JSON.parseObject(sw.toString(), Map.class); + + Map> recorded = collectFunctions( + (List)appmap.get("classMap"), ""); + + assertTrue(recorded.containsKey(FX + "#getSecret"), + "@Labels-annotated getter should be recorded; saw " + recorded.keySet()); + assertTrue(recorded.containsKey(FX + "#setSecret"), + "@Labels-annotated setter should be recorded; saw " + recorded.keySet()); + assertTrue(recorded.containsKey(FX + "#getNamedInConfig"), + "Getter named in appmap.yml should be recorded; saw " + recorded.keySet()); + assertTrue(recorded.containsKey(FX + "#describe"), + "Non-trivial method should be recorded; saw " + recorded.keySet()); + + assertFalse(recorded.containsKey(FX + "#getPlain"), + "Plain unlabeled getter should NOT be recorded; saw " + recorded.keySet()); + + assertEquals(java.util.Collections.singletonList("secret"), + recorded.get(FX + "#getSecret").get("labels")); + assertEquals(java.util.Collections.singletonList("mutator"), + recorded.get(FX + "#setSecret").get("labels")); + + Set calledMethods = new HashSet<>(); + for (Object e : (List)appmap.get("events")) { + Map event = (Map)e; + if ("call".equals(event.get("event"))) { + Object cls = event.get("defined_class"); + Object method = event.get("method_id"); + if (cls != null && method != null) { + calledMethods.add(cls + "#" + method); + } + } + } + assertTrue(calledMethods.contains(FX + "#getSecret"), + "Expected call event for getSecret; saw " + calledMethods); + assertTrue(calledMethods.contains(FX + "#setSecret"), + "Expected call event for setSecret; saw " + calledMethods); + assertTrue(calledMethods.contains(FX + "#getNamedInConfig"), + "Expected call event for getNamedInConfig; saw " + calledMethods); + assertFalse(calledMethods.contains(FX + "#getPlain"), + "Plain unlabeled getter should not appear in events; saw " + calledMethods); + } + + /** Walk the classMap tree and collect all "function" leaves keyed by fully qualified name. */ + @SuppressWarnings("unchecked") + private Map> collectFunctions(List nodes, String parent) { + Map> out = new HashMap<>(); + if (nodes == null) { + return out; + } + for (Object n : nodes) { + Map node = (Map)n; + String name = (String)node.get("name"); + String type = (String)node.get("type"); + String qualified; + if ("function".equals(type)) { + Boolean isStatic = (Boolean)node.get("static"); + qualified = parent + (Boolean.TRUE.equals(isStatic) ? "." : "#") + name; + out.put(qualified, node); + } else { + qualified = parent.isEmpty() ? name : parent + "." + name; + } + out.putAll(collectFunctions((List)node.get("children"), qualified)); + } + return out; + } +} diff --git a/agent/src/test/java/com/appland/appmap/test/fixture/labels/LabelFixture.java b/agent/src/test/java/com/appland/appmap/test/fixture/labels/LabelFixture.java new file mode 100644 index 00000000..abc696ff --- /dev/null +++ b/agent/src/test/java/com/appland/appmap/test/fixture/labels/LabelFixture.java @@ -0,0 +1,42 @@ +package com.appland.appmap.test.fixture.labels; + +import com.appland.appmap.annotation.Labels; + +/** + * Fixture exercising the trivial-method filter bypass for explicitly opted-in methods. + * The integration test {@code LabelFixtureRecorderTest} drives every method here and + * inspects the resulting AppMap recording. + */ +public class LabelFixture { + private String value = "initial"; + + /** Plain getter, no opt-in: should be filtered as trivial and NOT appear in the recording. */ + public String getPlain() { + return value; + } + + /** Annotated getter: should bypass the trivial filter and appear with its label. */ + @Labels("secret") + public String getSecret() { + return value; + } + + /** Annotated setter: should bypass the trivial filter and appear with its label. */ + @Labels("mutator") + public void setSecret(String value) { + this.value = value; + } + + /** + * Plain getter named explicitly under {@code methods:} in {@code appmap.yml} — should bypass + * the trivial filter on the strength of the config alone (no annotation). + */ + public String getNamedInConfig() { + return value; + } + + /** Non-trivial method: always recorded via the surrounding exclude-mode package entry. */ + public String describe() { + return "value=" + value; + } +}