From 2bec173784452a4f75d88b53d6dc1fb3016f8fdd Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 5 May 2026 14:50:26 +0200 Subject: [PATCH 1/2] Improve agentic instrumentation changes and generation --- .claude/skills/add-apm-integrations/SKILL.md | 14 ++++++++++++++ AGENTS.md | 2 ++ 2 files changed, 16 insertions(+) diff --git a/.claude/skills/add-apm-integrations/SKILL.md b/.claude/skills/add-apm-integrations/SKILL.md index e29b93ef3a2..2b8b79486ce 100644 --- a/.claude/skills/add-apm-integrations/SKILL.md +++ b/.claude/skills/add-apm-integrations/SKILL.md @@ -78,6 +78,19 @@ Conventions to enforce: - Declare `contextStore()` entries if context stores are needed (key class → value class) - Keep method matchers as narrow as possible (name, parameter types, visibility) +### Must NOT do in InstrumenterModule + +- **Do not extract one-shot method return values into static constants.** + Methods like `triggerClasses()`, `contextStore()`, `classLoaderMatcher()`, and `methodAdvice()` + are called **once** by `AgentInstaller` / the framework wiring. Extracting their return value + into a `private static final` constant provides no performance benefit and needlessly bloats + the constant pool of the instrumentation class. + + ❌ `private static final String[] TRIGGER_CLASSES = new String[]{"com.example.Foo"};` + `public String[] triggerClasses() { return TRIGGER_CLASSES; }` + + ✅ `public String[] triggerClasses() { return new String[]{"com.example.Foo"}; }` + ## Step 6 – Write the Decorator - Extend the most specific available base decorator: @@ -200,6 +213,7 @@ Output this checklist and confirm each item is satisfied: - [ ] `helperClassNames()` lists ALL referenced helpers (including inner, anonymous, and enum synthetic classes) - [ ] Advice methods are `static` with `@Advice.OnMethodEnter` / `@Advice.OnMethodExit` annotations - [ ] `suppress = Throwable.class` on enter/exit (unless the hooked method is a constructor) +- [ ] No static constants holding return values of one-shot instrumenter methods (`triggerClasses()`, `contextStore()`, etc.) - [ ] No logger field in the Advice class or InstrumenterModule class - [ ] No `inline=false` left in production code - [ ] No `java.util.logging.*` / `java.nio.*` / `javax.management.*` in bootstrap path diff --git a/AGENTS.md b/AGENTS.md index 2494b519b5b..4df13a44ca3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,6 +61,8 @@ docs/ Developer documentation (see below) - **Test frameworks**: Always use JUnit 5. **Do not write new Groovy / Spock tests** and migrate the existing one to JUnit 5 if it is written in Groovy. - **Forked tests**: Use `ForkedTest` suffix when tests need a separate JVM - **Flaky tests**: Annotate with `@Flaky` — they are skipped in CI by default +- **Instrumentation one-shot methods**: Never extract the return values of `triggerClasses()`, `contextStore()`, `classLoaderMatcher()`, or `methodAdvice()` into static constants. These are called once by the framework — extracting to a constant adds constant-pool bloat with no benefit. +- **SLF4J logging**: Pass objects directly to logger calls (`LOGGER.debug("{}", cls)`), not the result of calling methods on them (`LOGGER.debug("{}", cls.getName())`). SLF4J evaluates the argument lazily (via `toString()`) only when the log level is active. ## PR conventions From 9a84d8c4b8eefb48f3e6473605d5915b9efb049c Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Tue, 5 May 2026 15:08:41 +0200 Subject: [PATCH 2/2] suggestions --- AGENTS.md | 1 - 1 file changed, 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 4df13a44ca3..524a42f14dd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,6 @@ docs/ Developer documentation (see below) - **Forked tests**: Use `ForkedTest` suffix when tests need a separate JVM - **Flaky tests**: Annotate with `@Flaky` — they are skipped in CI by default - **Instrumentation one-shot methods**: Never extract the return values of `triggerClasses()`, `contextStore()`, `classLoaderMatcher()`, or `methodAdvice()` into static constants. These are called once by the framework — extracting to a constant adds constant-pool bloat with no benefit. -- **SLF4J logging**: Pass objects directly to logger calls (`LOGGER.debug("{}", cls)`), not the result of calling methods on them (`LOGGER.debug("{}", cls.getName())`). SLF4J evaluates the argument lazily (via `toString()`) only when the log level is active. ## PR conventions