Harden @Strategy/@StrategyConsumer annotations

[dougqh]

What Does This Do

Follow-up review pass on the @Strategy/@StrategyConsumer annotations (internal-api/src/main/java/datadog/trace/api/function/) and their one real production wearer, FlatHashtable:

• Switch retention SOURCE → CLASS on both annotations, matching @NoEscape's existing rationale: a classfile-only checker on a dependent module still needs to see the marker on a type/method declared elsewhere, which SOURCE retention hides.
• Drop @Inherited from @Strategy — it does nothing here: CLASS retention already makes it invisible to core reflection, and @Inherited never applies when the annotated type is used on an interface (4 of 5 production sites are interfaces).
• Document the non-capturing lambda constant as an accepted, if weaker, alternative to a concrete-typed static final field — grounded in the JIT's per-call-site inline-cache/type-profiling, not global class-hierarchy analysis.
• Add a "Checker contract" Javadoc section to both annotations (trigger / not-a-trigger / violation example / compliant example / out-of-scope), modeled on @NoEscape's existing pattern, so a future static checker or an AI reviewer can apply the rule without re-deriving it from prose.
• Add @Strategy.DynamicDispatch, a new nested exemption annotation for a field/parameter that deliberately stays on dynamic dispatch, and apply it to FlatHashtable.StrategyHashIterator#hashStrat.
• Mark D1#getOrCreate / D2#getOrCreate @StrategyConsumer — both call straight through to a @Strategy-typed create strategy but weren't marked.

Motivation

These two annotations pre-dated the "checker contract" documentation pattern established by @NoEscape, and pre-dated any exemption channel for deliberate exceptions. This pass brings them into line with that precedent and closes gaps found during an 8-lens dd-apm-sdk-review audit (Coherence, Correctness, Security, Design, Performance, Maintainability, Conventions, Cross-SDK) of the annotations and their usage in FlatHashtable.

Additional Notes

Two findings from the audit are deliberately not addressed in this PR:

• No production example yet exercises @Strategy's PARAMETER target — expected to land with upcoming product uses, not manufactured here.
• A potential hash-flooding (unseeded-hash algorithmic-complexity) concern in FlatHashtable is out of scope for an annotation-review PR and will be tracked separately.

Contributor Checklist

• Format the title according to the contribution guidelines
• Assign the type: and (comp: or inst:) labels in addition to any other useful labels
• Avoid using close, fix, or any linking keywords when referencing an issue
• Update the CODEOWNERS file on source file addition, migration, or deletion
• Update public documentation with any new configuration flags or behaviors
• Once approved, use merge queue to merge the PR

Jira ticket: [none]

🤖 Generated with Claude Code

[dd-octo-sts]

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite	Status
Startup	🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results

Scenario	Candidate	master	Δ (95% CI of mean)
startup:insecure-bank:iast:Agent	14.82 s	14.80 s	[-0.8%; +1.0%] (no difference)
startup:insecure-bank:tracing:Agent	13.73 s	13.73 s	[-0.9%; +0.9%] (no difference)
startup:petclinic:appsec:Agent	17.60 s	17.44 s	[+0.1%; +1.7%] (maybe worse)
startup:petclinic:iast:Agent	17.37 s	17.11 s	[-2.9%; +5.9%] (no difference)
startup:petclinic:profiling:Agent	17.50 s	17.34 s	[-0.4%; +2.2%] (no difference)
startup:petclinic:sca:Agent	17.61 s	17.55 s	[-0.6%; +1.3%] (no difference)
startup:petclinic:tracing:Agent	16.56 s	16.67 s	[-1.4%; +0.2%] (no difference)

Commit: abde608c · CI Pipeline · Benchmarking Platform UI

---

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

[pr-commenter]

LLM Validation

LLM Validation Gate — dd-apm-sdk-review

🔴 FAIL

• 1 blocking case(s) confidently regressed (win-rate below 45% with the CI upper bound below 55%, or the CI entirely below 50%) — these must not regress.
• Advisory: the candidate introduced a bad signal on 1 case(s) that the baseline did not — surfaced for review (see Cases); it does not block on its own.
• Confirmation pass: re-ran 1 case(s) flagged as a confident regression (+8 runs each); 1 reproduced after pooling and still block.

Analysis

Changed instruction file(s): .agents/skills/dd-apm-sdk-review/SKILL.md, .agents/skills/dd-apm-sdk-review/reviewers/_common.md, .agents/skills/dd-apm-sdk-review/reviewers/correctness.md, .agents/skills/dd-apm-sdk-review/reviewers/performance.md, .agents/skills/dd-apm-sdk-review/reviewers/report-template.md, .agents/skills/dd-apm-sdk-review/review-without-harness.md, .agents/skills/dd-apm-sdk-review/reviewers/coherence.md, .agents/skills/dd-apm-sdk-review/reviewers/security.md, .agents/skills/dd-apm-sdk-review/reviewers/design.md, .agents/skills/dd-apm-sdk-review/reviewers/maintainability.md, .agents/skills/dd-apm-sdk-review/reviewers/conventions.md, .agents/skills/dd-apm-sdk-review/reviewers/cross-sdk.md, .agents/dd-apm-sdk-review-overrides/repo-context.md, .agents/dd-apm-sdk-review-overrides/reviewers/security.md, .agents/dd-apm-sdk-review-overrides/reviewers/performance.md, .agents/dd-apm-sdk-review-overrides/reviewers/design.md, .agents/dd-apm-sdk-review-overrides/reviewers/conventions.md, .agents/dd-apm-sdk-review-overrides/reviewers/maintainability.md.

Blocking — 1 case(s) regressed; the gate failed on the worst below.

• java-correctness-mapper-state-leak: won 23% of blind comparisons, quality -12.7. ⚠️ Introduced a new bad signal (advisory).
  • Asked: Run dd-apm-sdk-review on the following change. There is no git repository to inspect — this diff is the entire change set; do not attempt any git commands, go
  • Baseline answer: I have enough to complete the review. The test at TraceMapperV04PayloadTest.java:63-68 is the smoking gun — it manually calls traceMapper.reset() on a dropped trace with a comment claiming "in the real life the mapper is always reset each trace", which is not what production does. Here is the review. --- #
  • Candidate answer: I now have everything needed. I ran the perspectives in single-context sequential mode (expected for a pasted diff — not a capability gap), using the real repo only to verify the contract the diff's comment asserts. Key confirmations: - DDTags.PROCESS_TAGS = "_dd.tags.process" — the process tags family, gated by

Results

• Pairwise win-rate: 52% [43%–61%] — candidate's share of blind comparisons (90% CI; spanning 50% = no clear difference)
• Overall quality: 75.6 → 75.7 (/100, +0.1)
• Bad signals introduced (advisory): 1
• Candidate criteria coverage (advisory): 29/32 (91%) — expected_criteria the candidate met; does not affect the gate
• Candidate bad signals (all, advisory): 1 case(s) — trips on the candidate, including ones the baseline also hit
• Blocking-case regressions: 1

Cases

Case	Mode	Quality Δ	Win-rate (90% CI)	Safety
java-perf-lens-wrong-collection-001	block	-3.3	50% [50%–50%]	ok
java-perf-pipeline-full-review-002	block	+29.6	100% [100%–100%]	ok
java-security-crash-handler-before-trust	block	+2.5	50% [50%–50%]	ok
java-correctness-capture-before-send	block	+14.6	88% [67%–100%]	ok
java-correctness-sqs-queue-name-incomplete	block	+5.0	75% [51%–99%]	ok
java-maintainability-resource-leak-streams	block	+15.0	75% [51%–99%]	ok
java-correctness-span-events-list-only	block	+1.3	75% [51%–99%]	ok
java-correctness-mapper-state-leak 🔴	block	-12.7	23% [10%–35%]	⚠️ bad signal

_{Per-dimension scores, token usage, latency, and estimated cost are in the CI job logs.}