Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .agents/skills/perf-review/references/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ Format: **pattern** — *expensive when (the interprocedural condition to trace)
5. **Polymorphic dispatch on a hot path** — *a hot call site becomes polymorphic enough to defeat the runtime's inlining/devirtualization (real for JIT runtimes — JVM/.NET/V8; AOT/interpreted differ)* — **flag-as-measure** ("may defeat devirtualization; verify on the target runtime") — SEV-2/3 — fix: keep hot call sites mono/bi-morphic; specialize.
6. **FFI / native-boundary crossing on a hot path** *(central to the shared-core effort)* — *a native crossing per-span/per-item (not batched), or transporting strings/objects rather than primitives/IDs* — flag-with-confidence (boundary cost is mechanism-determined; runtime-specific pinning → addendum) — SEV-1/2 (SEV-1 if it blocks/pins under concurrency) — fix: batch (one per flush, not per item); transport interned IDs not strings; keep crossings off the hot/concurrency path.
7. **Escape / allocation-elision defeated** *(Java/Go/.NET/V8 all have a version)* — *a refactor makes a previously-local object escape (stored, returned, captured by a closure, passed to a virtual/non-inlined call) → silent heap allocation on a hot path* — **flag-as-measure** ("may now escape and allocate; verify with an allocation profiler") — SEV-2/3 — fix: keep it local; avoid the escaping store/capture.
8. **`@NoEscape` field-storage violation** — *a field (instance or static, directly or as a generic type argument) declared with an `@NoEscape`-annotated type (`datadog.trace.api.function.NoEscape`), **or** initialized directly from a call to an `@NoEscape`-annotated method — with no comment at the declaration justifying the retention*. The type form marks a concrete type (e.g. `SubSequence`); the method form exists for a return value whose concrete type can't itself carry the annotation (an anonymous class or lambda implementing a JDK interface). The annotation's own javadoc carries a self-contained "Checker contract" section (trigger / not-a-trigger / violation example / compliant example, both forms) written so this can be checked from the diff alone, with no other context needed. The underlying rule is "should", not "must" (RFC-2119 sense): a trigger is a presumptive finding, not an automatic failure — compliant if the declaration carries either a `// Retained on purpose: <reason>`-style comment, or `@SuppressPerfContract(value = NoEscape.class, reason = "...")` (`datadog.perfcontract.SuppressPerfContract` — the generic exemption mechanism shared by every perf-contract marker, not `@NoEscape`-specific), or a named canned-exception annotation itself meta-annotated `@SuppressPerfContract` with `NoEscape.class` among its `value` (e.g. a hypothetical `@Borrowed`) — treat the meta-annotation's own `reason` as satisfying the requirement. Don't flag a `SuppressPerfContract`/canned-exception use as non-compliant just for being terse; that mirrors the annotation's own Checker contract. **flag-with-confidence** — SEV-2/3 (SEV-1 if the annotated type/method shares backing storage with something large, per its own javadoc). Current wearers: `SubSequence`, `Maybe` (type form; see J7 below for `SubSequence`'s specific retention-vs-transient discriminator). **No lint enforces this yet — the AI reviewer is the only check, so this stays here (not under deterministic-lint candidates below) until a checker lands and it can migrate down.**
9. **`@StaticLifetime` field-lifetime violation** — *a field annotated `@StaticLifetime` (`datadog.perfcontract.StaticLifetime`), **or** an unannotated field whose declared type is listed in the contract's `@ImpliedFor` (`datadog.perfcontract.ImpliedFor`; read the types from the `@ImpliedFor` on `StaticLifetime` itself, subtypes included), that is neither `static final` nor a `final` instance field declared on a class annotated `@Singleton` (`datadog.perfcontract.Singleton`)*. Declaration-scan only, same shape as the `@NoEscape` field-storage check above — it does not attempt to prove a field is actually reused enough to be worth caching, and it does not catch an unannotated per-instance cache of a non-implied type; it only guards fields that opt in or whose type implies the contract. A deliberate per-instance holder of an implied type is compliant with `@SuppressPerfContract(value = StaticLifetime.class, reason = "...")`. `@Singleton`, applied to a class, is a trusted, unverified declaration (v1 posture — the same stance the other perf-contract markers take toward `static final` itself) that the class has exactly one process-wide instance; declaring it satisfies `@StaticLifetime` for that class's `final` instance fields. **flag-with-confidence** — SEV-2/3, motivated by a real production defect (a per-request-constructed cache that never actually amortized anything — see the annotation's own javadoc for the full trigger/accepted-form table). Refines the "per-call cache / expensive-object creation that should be static/once" deterministic-lint candidate below by giving that pattern's opted-in subset an explicit annotation and trigger; the unannotated general case is still that line's job. **No lint enforces this yet — the AI reviewer is the only check, so this stays here (not under deterministic-lint candidates below) until a checker lands and it can migrate down.**
8. **`@NoEscape` field-storage violation** — *a field (instance or static, directly or as a generic type argument) declared with an `@NoEscape`-annotated type (`datadog.trace.api.function.NoEscape`), **or** initialized directly from a call to an `@NoEscape`-annotated method — with no comment at the declaration justifying the retention*. The type form marks a concrete type (e.g. `SubSequence`); the method form exists for a return value whose concrete type can't itself carry the annotation (an anonymous class or lambda implementing a JDK interface). The annotation's own javadoc carries a self-contained "Checker contract" section (trigger / not-a-trigger / violation example / compliant example, both forms) written so this can be checked from the diff alone, with no other context needed. The underlying rule is "should", not "must" (RFC-2119 sense): a trigger is a presumptive finding, not an automatic failure — compliant if the declaration carries either a `// Retained on purpose: <reason>`-style comment, or `@SuppressPerfContract(value = NoEscape.class, reason = "...")` (`datadog.contract.SuppressPerfContract` — the generic exemption mechanism shared by every perf-contract marker, not `@NoEscape`-specific), or a named canned-exception annotation itself meta-annotated `@SuppressPerfContract` with `NoEscape.class` among its `value` (e.g. a hypothetical `@Borrowed`) — treat the meta-annotation's own `reason` as satisfying the requirement. Don't flag a `SuppressPerfContract`/canned-exception use as non-compliant just for being terse; that mirrors the annotation's own Checker contract. **flag-with-confidence** — SEV-2/3 (SEV-1 if the annotated type/method shares backing storage with something large, per its own javadoc). Current wearers: `SubSequence`, `Maybe` (type form; see J7 below for `SubSequence`'s specific retention-vs-transient discriminator). **No lint enforces this yet — the AI reviewer is the only check, so this stays here (not under deterministic-lint candidates below) until a checker lands and it can migrate down.**
9. **`@StaticLifetime` field-lifetime violation** — *a field annotated `@StaticLifetime` (`datadog.contract.StaticLifetime`), **or** an unannotated field whose declared type is listed in the contract's `@ImpliedFor` (`datadog.contract.ImpliedFor`; read the types from the `@ImpliedFor` on `StaticLifetime` itself, subtypes included), that is neither `static final` nor a `final` instance field declared on a class annotated `@Singleton` (`datadog.contract.Singleton`)*. Declaration-scan only, same shape as the `@NoEscape` field-storage check above — it does not attempt to prove a field is actually reused enough to be worth caching, and it does not catch an unannotated per-instance cache of a non-implied type; it only guards fields that opt in or whose type implies the contract. A deliberate per-instance holder of an implied type is compliant with `@SuppressPerfContract(value = StaticLifetime.class, reason = "...")`. `@Singleton`, applied to a class, is a trusted, unverified declaration (v1 posture — the same stance the other perf-contract markers take toward `static final` itself) that the class has exactly one process-wide instance; declaring it satisfies `@StaticLifetime` for that class's `final` instance fields. **flag-with-confidence** — SEV-2/3, motivated by a real production defect (a per-request-constructed cache that never actually amortized anything — see the annotation's own javadoc for the full trigger/accepted-form table). Refines the "per-call cache / expensive-object creation that should be static/once" deterministic-lint candidate below by giving that pattern's opted-in subset an explicit annotation and trigger; the unannotated general case is still that line's job. **No lint enforces this yet — the AI reviewer is the only check, so this stays here (not under deterministic-lint candidates below) until a checker lands and it can migrate down.**

## Deterministic-lint candidates (DON'T spend AI budget — make these real lints)
Fixed-signature, mechanically checkable:
Expand Down
3 changes: 1 addition & 2 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@
/internal-api/src/*/*/datadog/trace/api/endpoint/ @DataDog/apm-java
/internal-api/src/*/*/datadog/trace/api/env/ @DataDog/apm-java
/internal-api/src/*/*/datadog/trace/api/function/ @DataDog/apm-java
/internal-api/src/*/*/datadog/apicontract/ @DataDog/apm-java
/internal-api/src/*/*/datadog/perfcontract/ @DataDog/apm-java
/internal-api/src/*/*/datadog/contract/ @DataDog/apm-java
/internal-api/src/*/*/datadog/trace/api/intake/ @DataDog/apm-java
/internal-api/src/*/*/datadog/trace/api/logging/ @DataDog/apm-java
/internal-api/src/*/*/datadog/trace/api/metrics/ @DataDog/apm-java
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import datadog.communication.serialization.StreamingBuffer;
import datadog.communication.serialization.ValueWriter;
import datadog.communication.serialization.WritableFormatter;
import datadog.perfcontract.StaticLifetime;
import datadog.perfcontract.SuppressPerfContract;
import datadog.contract.StaticLifetime;
import datadog.contract.SuppressPerfContract;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
import java.nio.BufferOverflowException;
import java.nio.ByteBuffer;
Expand Down
2 changes: 1 addition & 1 deletion gradle/forbiddenApiFilters/instrumentation.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
datadog.trace.bootstrap.instrumentation.api.AgentSpan#setServiceName(java.lang.String)

# TagMap.internals() skips checks the tracer core has already made; it is @Restricted to the core
# (see datadog.apicontract.Restricted). Instrumentation uses TagMap's checked methods.
# (see datadog.contract.Restricted). Instrumentation uses TagMap's checked methods.
@defaultMessage TagMap.internals() is restricted to the tracer core; use TagMap's checked methods instead.
datadog.trace.api.TagMap#internals()
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.perfcontract;
package datadog.contract;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.perfcontract;
package datadog.contract;

import datadog.trace.api.function.NoEscape;
import java.lang.annotation.Documented;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.apicontract;
package datadog.contract;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.perfcontract;
package datadog.contract;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.perfcontract;
package datadog.contract;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package datadog.perfcontract;
package datadog.contract;

import java.lang.annotation.Annotation;
import java.lang.annotation.Documented;
Expand Down
2 changes: 1 addition & 1 deletion internal-api/src/main/java/datadog/trace/api/TagMap.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package datadog.trace.api;

import datadog.apicontract.Restricted;
import datadog.contract.Restricted;
import datadog.trace.api.function.NoEscape;
import datadog.trace.api.function.TriConsumer;
import datadog.trace.api.internal.VisibleForTesting;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package datadog.trace.api.function;

import datadog.perfcontract.PerfContract;
import datadog.perfcontract.SuppressPerfContract;
import datadog.contract.PerfContract;
import datadog.contract.SuppressPerfContract;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package datadog.trace.api.function;

import datadog.perfcontract.PerfContract;
import datadog.contract.PerfContract;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Inherited;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package datadog.trace.api.function;

import datadog.perfcontract.PerfContract;
import datadog.contract.PerfContract;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
Expand Down
Loading