feat(observability): emit a PR report from the quota review, and correct the control it asks for

domains/observability/knowledge/span-sub-sampling.md

@@ -77,4 +77,6 @@ Removing ~90% of volume before the sample multiplies headroom — a higher sub-r
 
 ## Kill Switch
 
-Ship every always-on span family with an env disable flag (PR #39891: `SENTRY_DISTRIBUTED_TRACING_DISABLED` returns the messenger un-wrapped). It turns a future emergency cut into a config flip instead of a cherry-pick.
+**For anything the sampler sees, the per-name rate is the kill switch** — a remote name-to-rate map pins a transaction to zero at the next Sentry init, on every build that reads the flag, with no release. Do not add a bespoke env flag to get that.
+
+An env flag still earns its place for what the rate cannot reach. A span handed an explicit parent is a child span, and no `tracesSampler` call is made for it. And a flag can remove the wrapping itself rather than sample its output: PR #39891's `SENTRY_DISTRIBUTED_TRACING_DISABLED` returns the messenger un-wrapped, so the hot path pays nothing at all — a different thing from emitting fewer spans.

domains/observability/skills/sentry-quota/repos/metamask-extension.md

@@ -52,6 +52,9 @@ gh pr diff <n> --repo MetaMask/metamask-extension \
 - **Gate location differs by repo.** Extension spans go through `trace()` — gate at the call site or in the wrapper. Core controller spans go through the injected callback — gate in the package's trace util or the callback so every consumer (extension, mobile) inherits the cap.
 - **`BackgroundRpc` / `MessengerCall`** (the `TraceName` tail) are the already-gated wrapper spans from [PR #39891](https://github.com/MetaMask/metamask-extension/pull/39891) — the reference implementation of the Tier-2 sub-sample pattern and the `SENTRY_DISTRIBUTED_TRACING_DISABLED` kill-switch.
 - **Tier-0 fix path is a core PR + a patch on the extension release branch.** Controller instrumentation originates in `MetaMask/core`; the release branch is where the cherry-pick lands. The sev-1 blocker goes on the in-flight release milestone — e.g. [issue #43211](https://github.com/MetaMask/metamask-extension/issues/43211) ("Assets Controller Sentry Instrumentation exceeding quota").
+- **Which control the per-name rate is.** `sentry-traces-sampler.ts` resolves a rate in this order: the remote `sentry.transactionSampleRates` map, then `DEFAULT_TRANSACTION_SAMPLE_RATES`, then the parent's decision, then the default — and every non-zero result is capped by the remote `sentry.tracesSampleRate`, which doubles as the ceiling. A per-name entry pins its rate regardless of the parent, so a throttled name cannot ride in on a sampled parent. `AssetsDataSourceTiming` and `AssetsUpdatePipeline` are seeded at `0` and are the worked example.
+- **What the remote map does not reach.** `sentry-remote-rates.ts` reads the flag from persisted `RemoteFeatureFlagController` state (merged with `_flags.remoteFeatureFlags.sentry` manifest overrides) **once at Sentry init**, so a published rate takes effect at the next init, not mid-session — and only in a build that ships this module. Adding a *name* inside `transactionSampleRates` is a value change; adding a new top-level rate key needs the LaunchDarkly `variationJsonSchema` updated first, since it is written with `additionalProperties: false`.
+- **Which spans the sampler sees.** `trace()` sets `forceTransaction` only when the request carries no `parentContext` and a span is already active, so a `trace()` call given an explicit parent stays a child span and no `tracesSampler` call is made for it. Those are the spans for which the call-site gate is the only control. [MetaMask-planning#7546](https://github.com/MetaMask/MetaMask-planning/issues/7546) (make `parentContext` explicit) removes that ambient fallback, so a span declaring no parent will root its own trace instead of grafting onto whatever was active — still a transaction the sampler sees, but drawing its own decision at its own rate rather than inheriting a parent's. Re-read this bullet when it lands. `shouldSampleWrappers` also reads a remote rate (`sentry.wrapperSampleRate`), so the gate is tunable without a release as well; what it buys over a per-name rate is per-trace coherence and never building the span.
 - **Spotting the culprit first:** `sentry-mcp-queries` → Volume Estimation (`span.op` aggregate `count()`) ranks span contributors by span count; this skill takes over once you have the offending span name. A span-count ranking is not a ranking by billed volume: retention differs per name, so no single factor rescales it, and on a plan metered in transactions it can invert.
 
 ## Requests Already Recorded as `http.client` Spans

domains/observability/skills/sentry-quota/skill.md

@@ -1,7 +1,7 @@
 ---
 maturity: experimental
 name: sentry-quota
-description: Catch quota-risky Sentry span instrumentation in code and PRs — fan-out × ungated × no-kill-switch — before it blows the span budget
+description: Catch quota-risky Sentry span instrumentation in code and PRs — fan-out × ungated × not in the sampler's rate table — before it blows the span budget
 ---
 
 # Sentry Span Quota Guard
@@ -19,7 +19,7 @@ Find and fix custom Sentry span instrumentation that blows the project span budg
 
 - Reading the live span counts themselves — that's `sentry-mcp-queries` (Volume Estimation).
 - Product-analytics events (Segment / `trackEvent`) — that's `instrumentation`, with data domain `knowledge/segment-governance.md` for Segment governance.
-- The span is already behind a per-trace sample gate **and** a kill-switch — already mitigated.
+- The span is already named in the sampler's rate table and, where it is a span family on a hot path, gated at the call site — already mitigated.
 - Error volume. Errors are metered separately from spans and transactions, so no change here moves the error quota.
 
 ## Breach Triad
@@ -30,7 +30,7 @@ A custom span is a quota risk when these stack. The first three together are the
 |---|---|---|
 | **Fan-out** | span created in a loop / `.map` / `.forEach` / per-asset / per-account / per-chain / poller, or a `trace()` callback that awaits requests | N spans per trace, not 1. Each request inside an active span is an automatic `http.client` child |
 | **Always-on** | no `tracesSampleRate` sub-rate, no hash gate before the span | every qualifying call emits |
-| **No kill-switch** | not guarded by an env flag | disabling needs a release, not a config flip |
+| **No per-name rate** | the transaction name is absent from the sampler's rate table | cutting it needs a release, because a runtime throttle can only act on a name it holds |
 | Hot path | data-source / update-pipeline / network callback, not a discrete user action | high call frequency |
 
 Low fan-out + discrete user action + already gated = fine. Don't flag healthy spans.
@@ -60,11 +60,60 @@ Low fan-out + discrete user action + already gated = fine. Don't flag healthy sp
 ### Mitigate
 Pick the lowest tier that stops the bleed.
 
+## PR Report
+
+What the review emits, not a form the author fills in. Every field but the last is derivable from the diff plus one Sentry query; the last is the author's, and it is the term the estimate is most sensitive to.
+
+| # | Field | Source |
+|---|---|---|
+| 1 | Names this diff adds or changes | diff |
+| 2 | What starts each one, and what stops it | diff + repo |
+| 3 | Every span that will be a child of it | diff + SDK config |
+| 4 | What this measures that those spans do not | field 3 |
+| 5 | Whether the names already emit | one query, 90 days |
+| 6 | Which rate resolves, and where it lives | sampler source |
+| 7 | Whether a call-site gate is warranted | fields 2 and 3 |
+| 8 | Attributes, and the cardinality of each dynamic key | diff |
+| 9 | Reach | **the author** |
+
+1. **Names.** Every new `TraceName`, every new `trace(` site, and any `trace` passed as an *argument* into an existing call — the third has no `trace(` in the diff to find.
+2. **Triggers.** What starts it and what stops it: a discrete user action, a page load, a poller, a selector, a loop. Follow mounts, subscriptions and inits, not only `trace(` sites, because a change can start a traced process with no span anywhere in its diff. "Nothing stops it" is a finding, not a blank.
+3. **Children, enumerated rather than described.** A `trace()` callback holds its span active, so every request inside becomes an `http.client` child except those `shouldCreateSpanForRequest` drops, plus any nested traced call (`knowledge/auto-instrumentation.md`). This list is what makes one span more than one span, and it is what field 4 is answered against.
+4. **What it adds.** Where the new span wraps a request that already carries an `http.client` span with a duration, say what the new name measures that the existing one does not. A timing span over an interval already recorded is a rename of existing data.
+5. **Already emitting** — workflow step 2. Lead the verdict with the current count.
+6. **Rate resolution**, in order: a remote per-name rate, then a build-time per-name rate, then the enclosing trace's decision, then the global default. **A span started inside a sampled trace inherits that decision and the global default never applies.** Say whether the per-name entry is in *this* PR; a rate that ships in a later ref does not cap this one.
+7. **Gate** — see the next section. It is not a question about a kill switch.
+8. **Attributes.** Span `data`, not scope tags. For every dynamic key, the number of distinct values it can take.
+9. **Reach.** The share of sessions that reach the trigger. No static analysis reaches it, so the estimate ships as a range across it with the assumption written down, never folded into a single number.
+
+The report closes on
+
+```
+spans  =  occurrences x spans per occurrence x the rate that applies
+occurrences  =  reach x frequency x duration
+```
+
+with spans per occurrence from field 3 and the rate from field 6.
+
+## Which Control To Ask For
+
+**The per-name rate is the kill switch.** A remote name-to-rate map pins any transaction to zero at runtime, with no release, and it wins over the build-time table. So do not ask an author to build an env flag — ask that the name ship in the rate table alongside the code that emits it. Two limits belong in the report:
+
+- **It reaches only builds that read it.** A rate published today does nothing for an installed build shipped before the sampler, which is why the release inbound filter (Tier 1) is a separate instrument rather than a redundant one.
+- **It only sees transactions.** A span handed an explicit parent is a child span and the sampler is never called for it, so there the call-site gate is the only control.
+
+**A call-site gate is for the two things a rate cannot do**, and asking for both by reflex is how a review spends its credit on redundancy:
+
+- **Per-trace coherence.** The sampler draws independently per name, so a per-name rate leaves a trace showing some of its spans and not others. A deterministic hash on the trace id keeps a whole family together, which is what makes the waterfall readable (`knowledge/span-sub-sampling.md`). Never `Math.random()` per span.
+- **Not creating the span at all**, rather than dropping it at send — the difference that matters on a hot path where building the span is itself the cost.
+
+Warranted for a span family on a hot path. Not warranted for a single named transaction the rate table can cap.
+
 ## Mitigation Ladder
 
 | Tier | When | Action |
 |---|---|---|
-| **0 — Immediate** | a span fans out and is actively breaching on the live release | disable the `trace()` call at source (or env-guard it) + **cherry-pick to the release branch** + file a sev-1 release blocker on the in-flight release milestone |
+| **0 — Immediate** | a span fans out and is actively breaching on the live release | publish the name at rate `0` in the remote per-name rate map — no release, effective at the next Sentry init on every build that reads the flag. Falls back to disabling the `trace()` call at source and **cherry-picking to the release branch** for a child span the sampler never sees, or for builds predating the sampler. Either way, file a sev-1 release blocker on the in-flight release milestone |
 | **1 — Release containment** | spike concentrated in an old, already-patched release with lingering users. A sampler fix in a newer build does not change that release's rates unless it reads its rate remotely | Sentry **inbound filter** dropping `release:<bad>` spans + force-update. The only dashboard action. Filters target a whole release, not one span — don't filter a release you still want data from. There is no inbound filter by transaction name, only a fixed health-check one. Filtered events do not consume quota, so confirm the drop in the filtered outcomes (`stats_v2` grouped by `reason`), not in Explore. It is not instant: one recorded release filter took 3.8 days from filing to taking effect |
 | **2 — Durable** | the span is justified long-term but ungated | deterministic `traceId`-hash sub-sample gate before the span (`span-sub-sampling`) |
 | **3 — Wrong tool** | the metric needs full fidelity; sampling loses the signal | move the metric off trace spans — they are the wrong substrate for always-on high-cardinality metrics. Segment is the usual target, but its events can ship unregistered, with no CI check and no billing review (data domain `knowledge/segment-governance.md`), so it is not a free lunch |
@@ -83,7 +132,8 @@ Tier 0 + 1 stop the bleed; Tier 2 is the follow-up so the metric returns.
 | "No grep hits, so it's safe" | The culprit may be on a release ref not checked out — verify the version/ref |
 | Disable the span on `main` only | Cherry-pick to the active release branch — `main` alone leaves the live release breaching |
 | Treat "move to Segment" as free | Segment events ship without CI governance or billing review (data domain `knowledge/segment-governance.md`) |
-| Ship new always-on instrumentation with no kill-switch | Add an env disable flag on day one — turns a future cut into a config flip, not a cherry-pick |
+| Ship a new transaction whose name is not in the sampler's rate table | The name is the unit a runtime throttle acts on. In the table on day one, a later cut is a flag value; absent from it, a cut is a release |
+| Ask the author for a bespoke env kill-switch | The per-name rate is the kill switch. Ask for a call-site gate only where per-trace coherence, or not building the span at all, is the point |
 | An optional `trace?` param passes review because it emits nothing | It is a dormant fan-out — it detonates when any caller supplies the argument. Remove the *param*, not just the argument, so one line can't re-arm it. |
 | Disable one entry point of a multi-path change | One change can reach the backend by more than one path (a controller callback *and* a selector param). Audit every entry point it added, not just the one that fired. |
 | Read a span that "fires N million times" as one triggered too often | A total is transactions × spans per transaction. Check spans per trace before blaming the trigger: fan-out multiplies the count with no change in how often the trigger fires |