Replace perf-review skill with multi-perspective dd-apm-sdk-review

.agents/dd-apm-sdk-review-overrides/repo-context.md

@@ -0,0 +1,12 @@
+# Repo context — dd-trace-java
+
+Read only by the orchestrator (Step 0 of `SKILL.md`), not by individual reviewers. Repo-specific; not part of the shared core. This whole `.agents/dd-apm-sdk-review-overrides/` folder is owned by this repo — edit it freely, unlike `.agents/skills/dd-apm-sdk-review/`, which is a verbatim copy of the shared core.
+
+## Related skills in this repo
+
+The other skills in this repo author or review specific things; this one is the general multi-perspective push gate. Cite them as authoritative for their own area, do not invoke them, and note they must not invoke this skill either:
+
+- `techdebt` — duplication / unnecessary complexity / dead-code review, run before marking a PR ready.
+- `review-groovy-migration`, `migrate-groovy-to-java` — Groovy→Java test migration tooling and its review pass.
+- `apm-integrations` — instrumentation authoring.
+- `migrate-junit-source-to-tabletest` — test-source migration tooling.

.agents/dd-apm-sdk-review-overrides/reviewers/conventions.md

@@ -0,0 +1,56 @@
+Override for `reviewers/conventions.md` (in the core skill folder) — read that file first, then this.
+
+# Codebase conventions — dd-trace-java specifics
+
+## The repo's stated rules
+
+Start at **AGENTS.md § "Key documentation"** — that table is the index. Open the linked file for the topic under review; do not restate it here.
+
+Also not in that table, and in scope for this lens:
+
+- `.editorconfig` and `gradle/spotless.gradle` — the mechanically enforced format (google-java-format via Spotless). Human-facing write-up is **CONTRIBUTING.md § "Automatic code formatting"** and **§ "Static imports"**.
+- `.github/pull_request_template.md` — PR body contract.
+- `.github/CODEOWNERS` — new paths need an owner when this repo's existing pattern would assign one.
+- `metadata/supported-configurations.json` — the config/integration registry CI validates (`validate_supported_configurations_v2_local_file` in `.gitlab-ci.yml`).
+- `.agents/skills/apm-integrations/SKILL.md` (+ `references/`) — instrumentation authoring, including integration-name registration and the Groovy-test exception. Cite it; do not invoke it (see `.agents/dd-apm-sdk-review-overrides/repo-context.md`).
+
+Bootstrap / advice constraints in **AGENTS.md § "Critical constraints"** belong to the design lens, not this one.
+
+## Mechanical checks — run these, don't eyeball them
+
+Check-mode only. Anything that would rewrite files is the author's to run; if a check fails, report it.
+
+Read **AGENTS.md § "Code conventions"** and **CONTRIBUTING.md § "Automatic code formatting"** for the rules, then run the check against the changed modules:
+
+```bash
+./gradlew spotlessCheck                          # whole repo
+./gradlew :path:to:module:spotlessCheck          # prefer this when the diff is scoped
+# Do NOT run spotlessApply.
+```
+
+There is no eslint / `tsc` equivalent. Spotless *does* cover Markdown, but only under `gradle/spotless.gradle`'s `format 'markdown'` target: root-level `*.md`, `.github/**/*.md`, `src/**/*.md`, and `application/**/*.md`. Markdown outside those paths — e.g. under `.agents/skills/**` — is not covered; `.editorconfig` is what applies there. If Gradle or the JDK is missing, report `NOT VERIFIED (<reason>)` rather than eyeballing format.
+
+## Config options — registration path
+
+Read **docs/add_new_configurations.md**. It owns the steps, the files, source priority, and the `supported-configurations.json` schema. Do not restate them from memory; open that doc and check the diff against it.
+
+Only the parts that doc does not state as a severity:
+
+- A new `DD_*` / `dd.*` read that is missing from `metadata/supported-configurations.json` is a CI failure (`validate_supported_configurations_v2_local_file`), not a nit — Blocking.
+- Integration *names* (the strings passed to `super(...)` / `instrumentationNames()`) also need entries there. That shape is in `.agents/skills/apm-integrations/references/supported-configurations.md`, not in `add_new_configurations.md`.
+
+## Instrumentations and tests
+
+- New instrumentation: **docs/add_new_instrumentation.md** (Gradle include, layout, class/package naming) plus **docs/how_instrumentations_work.md § "Naming"** and **§ "Files/Directories"**. Missing `:dd-java-agent:instrumentation:…` include in `settings.gradle.kts` is silent non-build — P0.
+- Tests: **docs/how_to_test.md** (and **docs/how_to_test_with_junit.md** when the change is JUnit). **AGENTS.md § "Code conventions"** is the one-line summary; the how-to is the spec.
+- New `.groovy` test files are blocked by CI unless the PR has `tag: override groovy enforcement`. Instrumentation tests are the intended exception — see `.agents/skills/apm-integrations/SKILL.md`.
+
+## Commit and PR hygiene
+
+Read **CONTRIBUTING.md § "Pull request guidelines"** (draft-first, title, labels, merge queue) and **AGENTS.md § "PR conventions"** (adds `tag: ai generated`). Those own the rules.
+
+Only the parts not stated there:
+
+- There is no `pr-title.yml` (or equivalent) that rejects a title. The title is a house rule plus changelog input, not a CI gate — flag a bad title, do not invent a missing-linter finding.
+- There is no changelog file: the PR title is the release note. Audit the title and `tag: no release notes` rather than asking for a CHANGELOG entry.
+- No in-repo rule mandates `gh --repo` flags or a fork-vs-branch policy; do not invent one.

.agents/dd-apm-sdk-review-overrides/reviewers/design.md

@@ -0,0 +1,43 @@
+Override for `reviewers/design.md` (in the core skill folder) — read that file first, then this.
+
+# Design — dd-trace-java specifics
+
+## Module map and layer boundaries
+
+Start at **ARCHITECTURE.md § "Codemap"** — it owns the module boundaries and what belongs where; do not restate it from memory. Layering rules it states explicitly, in scope for this lens:
+
+- `dd-trace-core` and `internal-api` "grew organically" and now host multi-product code beyond their original scope. Genuinely product-*agnostic* infrastructure being pulled out of either belongs in `components/`; product-*specific* implementation belongs in `products/`. A new file added to either just because "that's where similar code already lives" is the duplication-of-drift this lens should catch.
+- `components/` must stay bootstrap-safe, product-agnostic, and free of *external* dependencies (see ARCHITECTURE.md § "components/"). A new external dependency, or a product-specific type, landing there is a shape violation — but one `components/*` module depending on another bootstrap-safe `components/*` module (e.g. `native-loader` on `environment`) is normal layering, not a violation.
+- `products/` modules typically follow the `{product}-api` / `{product}-bootstrap` / `{product}-lib` / `{product}-agent` layering, but no existing product implements it exactly: `metrics` has no `-bootstrap`; `feature-flagging` adds an extra `-config` submodule. Don't flag a missing or extra submodule name against this list — the layering shape is aspirational, not enumerable. What *is* a hard rule regardless of which submodules a product has: implementation weight added to a thin/boundary submodule (`-api`, `-bootstrap`, `-config`) instead of `-lib` is a layer violation, not a style choice.
+
+## Public API surface
+
+This repo's public API lives in `dd-trace-api/` (`Tracer`, `GlobalTracer`, `DDTags`, `DDSpanTypes`, the `@Trace` annotation, the `*Config` constant classes) and in `dd-trace-ot/`'s `io.opentracing.Tracer` implementation — see ARCHITECTURE.md § "dd-trace-api/" and § "dd-trace-ot/". A change adding a `public`/`protected` class or method to an exported, externally-accessible type in either is public surface and needs explicit justification; it is forever. A package-private or private addition to a non-exported type (e.g. `OTSpan`, `OTSpanContext`, `TypeConverter` in `dd-trace-ot`) is not externally reachable and does not need this justification. `internal-api/` is internal despite the name — it's fair game to reshape, but check callers across `products/` and `dd-java-agent/` before calling a change there "just internal."
+
+## Configuration surface
+
+Read **docs/add_new_configurations.md** — it owns the registration steps; check the diff against it, don't restate it here. One design-shaped consequence that doc doesn't state: `internal-api`'s split between `Config` and `InstrumenterConfig` exists for a build-time reason, not convenience — GraalVM native-image builds freeze instrumentation-affecting decisions into the binary at build time, so a setting that controls which classes/integrations get instrumented belongs in `InstrumenterConfig`; a setting that's runtime-only (endpoints, service name, sampling rate) belongs in `Config` (see ARCHITECTURE.md § "internal-api/"). Landing a native-image-relevant setting in the wrong one breaks native-image builds silently — flag it even if the config-registration mechanics (which belongs to the conventions lens) are otherwise followed correctly.
+
+## Extension points (instrumentations)
+
+An instrumentation must go through `InstrumenterModule` + the `Instrumenter` type-matching interfaces (`ForSingleType`, `ForKnownTypes`, `ForTypeHierarchy`, `ForBootstrap`) and be discovered via `@AutoService(InstrumenterModule.class)` — see ARCHITECTURE.md § "agent-tooling/" and **docs/add_new_instrumentation.md** / **docs/how_instrumentations_work.md**. A bespoke `ClassFileTransformer` or advice registered outside this mechanism bypasses Muzzle's build-time version-safety checks entirely — that's a P0 shape problem, not a nit, independent of whether the bespoke code works.
+
+## Lifecycle / bootstrap
+
+The bootstrap and advice correctness rules for this code live in **AGENTS.md § "Critical constraints"** and **docs/bootstrap_design_guidelines.md** / **docs/instrumentation_design_guidelines.md** — this lens owns them; do not restate them from memory, open the doc and check the diff against it. (The performance override's "Bootstrap / startup-latency note" covers the same code from the cost angle — that's a different finding on the same lines, not a duplicate.) Respect the ordering in ARCHITECTURE.md § "Startup Sequence": `AgentBootstrap.premain()` must stay tiny and side-effect-free; anything heavier belongs in `Agent.start()` or a product's own `*System.start()`, never in premain-reachable code.
+
+## Cross-cutting mechanisms already in the repo
+
+Before approving a new cross-cutting abstraction, check whether one already exists — see ARCHITECTURE.md § "internal-api/":
+
+- `gateway/` — the Instrumentation Gateway event bus. AppSec and IAST use it to hook the HTTP request lifecycle *without* touching instrumentations directly. A new instrumentation reaching into AppSec/IAST internals directly, instead of publishing through the gateway, is a layering violation.
+- `cache/` — `DDCache`, `FixedSizeCache`, `RadixTreeCache`.
+- `naming/` — span/service naming schemas (v0, v1).
+
+A second bespoke event bus, cache, or naming scheme is a P1 duplication finding at minimum, per the generic file's "Duplication of an existing mechanism" check.
+
+## Not this lens's job
+
+- Config-registration file mechanics (`supported-configurations.json`, the CI validator) — conventions lens.
+- Allocation cost, hot-path multipliers, or JIT behavior of a given shape — performance lens.
+- Instrumentation package/class naming and Gradle layout mechanics — conventions lens (the same docs are cited there too; this file only owns whether the extension *mechanism* chosen is the right one, not how it's named or laid out).

.agents/dd-apm-sdk-review-overrides/reviewers/maintainability.md

@@ -0,0 +1,7 @@
+# Maintainability — dd-trace-java overrides
+
+This repo's release-note policy is defined in [`conventions.md`](./conventions.md), not here — read
+that override for the actual policy text.
+
+There is no repo-specific public-API definition beyond what [`design.md`](./design.md) states; fall
+back to judgment as `reviewers/maintainability.md` (core) instructs.