Merge branch 'master' into ygree/llmobs-systest-fixes

Author: ygree

.claude/skills/add-apm-integrations/SKILL.md

@@ -0,0 +1,232 @@
+---
+name: add-apm-integrations
+description: Write a new library instrumentation end-to-end. Use when the user ask to add a new APM integration or a library instrumentation.
+context: fork
+allowed-tools:
+  - Bash
+  - Read
+  - Write
+  - Edit
+  - Glob
+  - Grep
+---
+
+Write a new APM end-to-end integration for dd-trace-java, based on library instrumentations, following all project conventions.
+
+## Step 1 – Read the authoritative docs and sync this skill (mandatory, always first)
+
+Before writing any code, read all three files in full:
+
+1. [`docs/how_instrumentations_work.md`](docs/how_instrumentations_work.md) — full reference (types, methods, advice, helpers, context stores, decorators)
+2. [`docs/add_new_instrumentation.md`](docs/add_new_instrumentation.md) — step-by-step walkthrough
+3. [`docs/how_to_test.md`](docs/how_to_test.md) — test types and how to run them
+
+These files are the single source of truth. Reference them while implementing.
+
+**After reading the docs, sync this skill with them:**
+
+Compare the content of the three docs against the rules encoded in Steps 2–11 of this skill file. Look for:
+- Patterns, APIs, or conventions described in the docs but absent or incorrect here
+- Steps that are out of date relative to the current docs (e.g. renamed methods, new base classes)
+- Advice constraints or test requirements that have changed
+
+For every discrepancy found, edit this file (`.claude/skills/apm-integrations/SKILL.md`) to correct it using the
+`Edit` tool before continuing. Keep changes targeted: fix what diverged, add what is missing, remove what is wrong.
+Do not touch content that already matches the docs.
+
+## Step 2 – Clarify the task
+
+If the user has not already provided all of the following, ask before proceeding:
+
+- **Framework name** and **minimum supported version** (e.g. `okhttp-3.0`)
+- **Target class(es) and method(s)** to instrument (fully qualified class names preferred)
+- **Target system**: one of `Tracing`, `Profiling`, `AppSec`, `Iast`, `CiVisibility`, `Usm`, `ContextTracking`
+- **Whether this is a bootstrap instrumentation** (affects allowed imports)
+
+## Step 3 – Find a reference instrumentation
+
+Search `dd-java-agent/instrumentation/` for a structurally similar integration:
+- Same target system
+- Comparable type-matching strategy (single type, hierarchy, known types)
+
+Read the reference integration's `InstrumenterModule`, Advice, Decorator, and test files to understand the established
+pattern before writing new code. Use it as a template.
+
+## Step 4 – Set up the module
+
+1. Create directory: `dd-java-agent/instrumentation/$framework/$framework-$minVersion/`
+2. Under it, create the standard Maven source layout:
+   - `src/main/java/` — instrumentation code
+   - `src/test/groovy/` — Spock tests
+3. Create `build.gradle` with:
+   - `compileOnly` dependencies for the target framework
+   - `testImplementation` dependencies for tests
+   - `muzzle { pass { } }` directives (see Step 9)
+4. Register the new module in `settings.gradle.kts` in **alphabetical order**
+
+## Step 5 – Write the InstrumenterModule
+
+Conventions to enforce:
+
+- Add `@AutoService(InstrumenterModule.class)` annotation — required for auto-discovery
+- Extend the correct `InstrumenterModule.*` subclass (never the bare abstract class)
+- Implement the **narrowest** `Instrumenter` interface possible:
+  - Prefer `ForSingleType` > `ForKnownTypes` > `ForTypeHierarchy`
+- Add `classLoaderMatcher()` if a sentinel class identifies the framework on the classpath
+- Declare **all** helper class names in `helperClassNames()`:
+  - Include inner classes (`Foo$Bar`), anonymous classes (`Foo$1`), and enum synthetic classes
+- Declare `contextStore()` entries if context stores are needed (key class → value class)
+- Keep method matchers as narrow as possible (name, parameter types, visibility)
+
+## Step 6 – Write the Decorator
+
+- Extend the most specific available base decorator:
+  - `HttpClientDecorator`, `DatabaseClientDecorator`, `ServerDecorator`, `MessagingClientDecorator`, etc.
+- One `public static final DECORATE` instance
+- Define `UTF8BytesString` constants for the component name and operation name
+- Keep all tag/naming/error logic here — not in the Advice class
+- Override `spanType()`, `component()`, `spanKind()` as appropriate
+
+## Step 7 – Write the Advice class (highest-risk step)
+
+### Must do
+
+- Advice methods **must** be `static`
+- Annotate enter: `@Advice.OnMethodEnter(suppress = Throwable.class)`
+- Annotate exit: `@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)`
+  - **Exception**: do NOT use `suppress` when hooking a constructor
+- Use `@Advice.Local("...")` for values shared between enter and exit (span, scope)
+- Use the correct parameter annotations:
+  - `@Advice.This` — the receiver object
+  - `@Advice.Argument(N)` — a method argument by index
+  - `@Advice.Return` — the return value (exit only)
+  - `@Advice.Thrown` — the thrown exception (exit only)
+  - `@Advice.Enter` — the return value of the enter method (exit only)
+- Use `CallDepthThreadLocalMap` to guard against recursive instrumentation of the same method
+
+### Span lifecycle (in order)
+
+Enter method:
+1. `AgentSpan span = startSpan(DECORATE.operationName(), ...)`
+2. `DECORATE.afterStart(span)` + set domain-specific tags
+3. `AgentScope scope = activateSpan(span)` — return or store via `@Advice.Local`
+
+Exit method:
+4. `DECORATE.onError(span, throwable)` — only if throwable is non-null
+5. `DECORATE.beforeFinish(span)`
+6. `span.finish()`
+7. `scope.close()`
+
+### Must NOT do
+
+- **No logger fields** in the Advice class or the Instrumentation class (loggers only in helpers/decorators)
+- **No code in the Advice constructor** — it is never called
+- **Do not use lambdas in advice methods** — they create synthetic classes that will be missing from helper declarations
+- **No references** to other methods in the same Advice class or in the InstrumenterModule class
+- **No `InstrumentationContext.get()`** outside of Advice code
+- **No `inline=false`** in production code (only for debugging; must be removed before committing)
+- **No `java.util.logging.*`, `java.nio.*`, or `javax.management.*`** in bootstrap instrumentations
+
+## Step 8 – Add SETTER/GETTER adapters (if applicable)
+
+For context propagation to and from upstream services, like HTTP headers,
+implement `AgentPropagation.Setter` / `AgentPropagation.Getter` adapters that wrap the framework's specific header API.
+Place them in the helpers package, declare them in `helperClassNames()`.
+
+## Step 9 – Write tests
+
+Cover all mandatory test types:
+
+### 1. Instrumentation test (mandatory)
+
+- Spock spec extending `InstrumentationSpecification`
+- Place in `src/test/groovy/`
+- Verify: spans created, tags set, errors propagated, resource names correct
+- Use `TEST_WRITER.waitForTraces(N)` for assertions
+- Use `runUnderTrace("root") { ... }` for synchronous code
+
+For tests that need a separate JVM, suffix the test class with `ForkedTest` and run via the `forkedTest` task.
+
+### 2. Muzzle directives (mandatory)
+
+In `build.gradle`, add `muzzle` blocks:
+```groovy
+muzzle {
+  pass {
+    group = "com.example"
+    module = "framework"
+    versions = "[$minVersion,)"
+    assertInverse = true  // ensures versions below $minVersion fail muzzle
+  }
+}
+```
+
+### 3. Latest dependency test (mandatory)
+
+Use the `latestDepTestLibrary` helper in `build.gradle` to pin the latest available version. Run with:
+```bash
+./gradlew :dd-java-agent:instrumentation:$framework-$version:latestDepTest
+```
+
+### 4. Smoke test (optional)
+
+Add a smoke test in `dd-smoke-tests/` only if the framework warrants a full end-to-end demo-app test.
+
+## Step 10 – Build and verify
+
+Run these commands in order and fix any failures before proceeding:
+
+```bash
+./gradlew :dd-java-agent:instrumentation:$framework-$version:muzzle
+./gradlew :dd-java-agent:instrumentation:$framework-$version:test
+./gradlew :dd-java-agent:instrumentation:$framework-$version:latestDepTest
+./gradlew spotlessCheck
+```
+
+**If muzzle fails:** check for missing helper class names in `helperClassNames()`.
+
+**If tests fail:** verify span lifecycle order (start → activate → error → finish → close), helper registration,
+and `contextStore()` map entries match actual usage.
+
+**If spotlessCheck fails:** run `./gradlew spotlessApply` to auto-format, then re-check.
+
+## Step 11 – Checklist before finishing
+
+Output this checklist and confirm each item is satisfied:
+
+- [ ] `settings.gradle.kts` entry added in alphabetical order
+- [ ] `build.gradle` has `compileOnly` deps and `muzzle` directives with `assertInverse = true`
+- [ ] `@AutoService(InstrumenterModule.class)` annotation present on the module class
+- [ ] `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 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
+- [ ] Span lifecycle order is correct: startSpan → afterStart → activateSpan (enter); onError → beforeFinish → finish → close (exit)
+- [ ] Muzzle passes
+- [ ] Instrumentation tests pass
+- [ ] `latestDepTest` passes
+- [ ] `spotlessCheck` passes
+
+## Step 12 – Retrospective: update this skill with what was learned
+
+After the instrumentation is complete (or abandoned), review the full session and improve this skill for future use.
+
+**Collect lessons from four sources:**
+
+1. **Build/test failures** — did any Gradle task fail with an error that this skill did not anticipate or gave wrong
+   guidance for? (e.g. a muzzle failure that wasn't caused by missing helpers, a test pattern that didn't work)
+2. **Docs vs. skill gaps** — did Step 1's sync miss anything? Did you consult the docs for something not captured here?
+3. **Reference instrumentation insights** — did the reference integration use a pattern, API, or convention not
+   reflected in any step of this skill?
+4. **User corrections** — did the user correct an output, override a decision, or point out a mistake?
+
+**For each lesson identified**, edit this file (`.claude/skills/apm-integrations/SKILL.md`) using the `Edit` tool:
+- Wrong rule → fix it in place
+- Missing rule → add it to the most relevant step
+- Wrong failure guidance → update the relevant "If X fails" section in Step 10
+- Misleading or obsolete content → remove it
+
+Keep each change minimal and targeted. Do not rewrite sections that worked correctly.
+After editing, confirm to the user which improvements were made to the skill.

.claude/skills/migrate-groovy-to-java/SKILL.md

@@ -1,6 +1,9 @@
 ---
 name: migrate-groovy-to-java
-description: migrate test groovy files to java
+description: >
+  Converts Spock/Groovy test files in a Gradle module to equivalent JUnit 5 Java tests.
+  Use when asked to "migrate groovy", "convert groovy to java", "g2j", or when a module
+  has .groovy test files that need to be replaced with .java equivalents.
 ---
 
 Migrate test Groovy files to Java using JUnit 5
@@ -13,11 +16,45 @@ Migrate test Groovy files to Java using JUnit 5
 
 When converting Groovy code to Java code, make sure that:
 - The Java code generated is compatible with JDK 8
-- When translating Spock tests, favor using `@CsvSource` with `|` delimiters
-- When using `@MethodSource`, name the arguments method by appending `Arguments` using camelCase to the test method name (e.g. `testMethodArguments`)
-- Ensure parameterized test names are human-readable (i.e. no hashcodes); instead add a description string as the first `Arguments.of(...)` value or index the test case
+- When translating Spock tests, prefer `@TableTest` for data rows that are naturally tabular. See detailed guidance in the "TableTest usage" section.
+- `@TableTest` and `@MethodSource` may be combined on the same `@ParameterizedTest` when most cases are tabular but a few cases require programmatic setup.
+- In combined mode, keep table-friendly cases in `@TableTest`, and put only non-tabular/complex cases in `@MethodSource`.
+- If `@TableTest` is not viable for the test at all, use `@MethodSource` only.
+- For `@MethodSource`, name the arguments method `<testMethodName>Arguments` (camelCase, e.g. `testMethodArguments`) and return `Stream<Arguments>` using `Stream.of(...)` and `arguments(...)` with static import.
+- Ensure parameterized test names are human-readable (i.e. no hashcodes); instead add a description string as the first `Arguments.arguments(...)` value or index the test case
 - When converting tuples, create a light dedicated structure instead to keep the typing system
 - Instead of checking a state and throwing an exception, use JUnit asserts
+- Instead of using `assertTrue(a.equals(b))` or `assertFalse(a.equals(b))`, use `assertEquals(expected, actual)` and `assertNotEquals(unexpected, actual)`
+- Import frequently used types rather than using fully-qualified names inline, to improve readability
 - Do not wrap checked exceptions and throw a Runtime exception; prefer adding a throws clause at method declaration
 - Do not mark local variables `final`
 - Ensure variables are human-readable; avoid single-letter names and pre-define variables that are referenced multiple times
+- When translating Spock `Mock(...)` usage, use `libs.bundles.mockito` instead of writing manual recording/stub implementations
+
+TableTest usage
+  Import: `import org.tabletest.junit.TableTest;`
+
+  JDK 8 rules:
+    - No text blocks.
+    - @TableTest must use String[] annotation array syntax:
+    ```
+    @TableTest({
+      "a | b",
+      "1 | 2"
+    })
+    ```
+
+  Spock `where:` → @TableTest:
+    - First row = header (column names = method parameters).
+    - Add `scenario` column as first column (display name, not a method parameter).
+    - Use `|` delimiter; align columns so pipes line up vertically.
+    - Prefer single quotes for strings with special chars (e.g., `'a|b'`, `'[]'`).
+    - Blank cell = null (object types); `''` = empty string.
+    - Collections: `[a, b]` = List/array, `{a, b}` = Set, `[k: v]` = Map.
+
+  Mixed eligibility:
+    - Prefer combining `@TableTest` + `@MethodSource` on one `@ParameterizedTest` when only some cases are complex.
+    - Use `@MethodSource` only when tabular representation is not practical for the test.
+
+  Do NOT use @TableTest when:
+    - Majority of rows require complex objects or custom converters.

.claude/skills/migrate-junit-source-to-tabletest/SKILL.md

@@ -0,0 +1,79 @@
+---
+name: migrate-junit-source-to-tabletest
+description: Convert JUnit 5 @MethodSource/@CsvSource/@ValueSource parameterized tests to @TableTest (JDK8)
+---
+Goal: Migrate JUnit 5 parameterized tests using @MethodSource/@CsvSource/@ValueSource to @TableTest with minimal churn and passing tests.
+
+Process (do in this order):
+1) Locate targets via Grep (no agent subprocess). Search for: "@ParameterizedTest", "@MethodSource", "@CsvSource", "@ValueSource".
+2) Read all matching files up front (parallel is OK).
+3) Convert eligible tests to @TableTest.
+4) Write each modified file once in full using Write (no incremental per-test edits).
+5) Run module tests once and verify "BUILD SUCCESSFUL". If failed, inspect JUnit XML report.
+
+Import: `import org.tabletest.junit.TableTest;`
+
+JDK 8 rules:
+- No text blocks.
+- @TableTest must use String[] annotation array syntax:
+  ```
+  @TableTest({
+   "a | b",
+   "1 | 2"
+  })
+  ```
+
+Table formatting rules (mandatory):
+- Always include a header row (parameter names).
+- Always add a "scenario" column; using common sense for naming; scenario is NOT a method parameter.
+- Use '|' as delimiter.
+- Align columns with spaces so pipes line up vertically.
+- Prefer single quotes for strings requiring quotes (e.g., 'a|b', '[]', '{}', ' ').
+- Use value sets (`{a, b, c}`) instead of matrix-style repetition when only one dimension varies across otherwise-identical rows.
+
+Conversions:
+A) @CsvSource
+- Remove @ParameterizedTest and @CsvSource.
+- If delimiter is '|': rows map directly to @TableTest.
+- If delimiter is ',' (default): replace ',' with '|' in rows.
+
+B) @ValueSource
+- Keep single-parameter tests on `@ValueSource` (and `@NullSource` when null cases are needed).
+- Otherwise convert to @TableTest with header from parameter name.
+- Each value becomes one row.
+- Add "scenario" column using common sense for name.
+
+C) @MethodSource (convert only if values are representable as strings)
+- Convert when argument values are primitives, strings, enums, booleans, nulls, and simple collection literals supported by TableTest:
+  - Array: [a, b, ...]
+  - List: [a, b, ...]
+  - Set: {a, b, ...}
+  - Map: [k: v, ...]
+- `@TableTest` and `@MethodSource` may be combined on the same `@ParameterizedTest` when most cases are tabular but a few cases require programmatic setup.
+- In combined mode, keep table-friendly cases in `@TableTest`, and put only non-tabular/complex cases in `@MethodSource`.
+- If `@TableTest` is not viable for the test at all, use `@MethodSource` only.
+- For `@MethodSource`, name the arguments method `<testMethodName>Arguments` (camelCase, e.g. `testMethodArguments`) and return `Stream<Arguments>` using `Stream.of(...)` and `arguments(...)` with static import.
+- Blank cell = null (non-primitive).
+- '' = empty string.
+- For String params that start with '[' or '{', quote to avoid collection parsing (prefer '[]'/'{}').
+
+D) @TypeConverter
+- Use `@TypeConverter` for symbolic constants used by migrated table rows (e.g. `Long.MAX_VALUE`, `DDSpanId.MAX`).
+- Prefer explicit one-case-one-return mappings.
+- Prefer shared converter utilities (e.g. in `utils/test-utils`) when reuse across modules is likely.
+
+Scenario handling:
+- If MethodSource includes a leading description string OR @ParameterizedTest(name=...) uses {0}, convert that to a scenario column and remove that parameter from method signature.
+
+Cleanup:
+- Delete now-unused @MethodSource provider methods and unused imports.
+
+Mixed eligibility:
+- Prefer combining `@TableTest` + `@MethodSource` on one `@ParameterizedTest` when only some cases are complex.
+
+Do NOT convert when:
+- Most rows require complex builders/mocks.
+
+Test command (exact):
+./gradlew :path:to:module:test --rerun-tasks 2>&1 | tail -20
+- If BUILD FAILED: cat path/to/module/build/test-results/test/TEST-*.xml