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

[MajorLift]

Stacked on #173 (record what the Sentry SDK already instruments before adding a span). Field 3 below cites knowledge/auto-instrumentation.md, which lands there, so based on main that citation would dangle.

The skill never said what a review hands back

It said what to look for. So the same scan produced a different shape each time, and the output a reviewer most needs — the list of spans that will be children of the new one — stayed as prose about fan-out.

## PR Report makes that nine fields with their sources. Eight are derivable from the diff plus one Sentry query. The ninth is reach, which no static analysis reaches, so it is marked as the author's and ships as a stated range rather than folded into a single number.

Field 3 is the one worth arguing about: enumerate the children, do not describe them. A trace() callback holds its span active, so every request inside becomes an http.client child unless shouldCreateSpanForRequest drops it. That list is what makes one span more than one span, and it is what field 4 — what does this measure that those spans do not — gets answered against.

The control it asked for was wrong

The breach triad scored a span on whether it was "guarded by an env flag", and the pitfall table told authors to add an env disable flag on day one. For a transaction, a bespoke env flag is superseded, and asking for one spends the review's credit on redundancy.

The remote sentry.transactionSampleRates map pins a name to zero at runtime with no release, and it is consulted ahead of the build-time table — sentry-traces-sampler.ts L116-L126. A per-name entry pins its rate regardless of the parent, so a throttled name cannot ride in on a sampled one.

Two limits on it, both now in the report rather than assumed away. It is read from persisted RemoteFeatureFlagController state once at Sentry init, so a published rate lands at the next init and only in a build shipping that module — sentry-remote-rates.ts L1-L23. That is why the release inbound filter stays a separate instrument at tier 1 rather than a redundant one.

And it only sees transactions. trace() sets forceTransaction only where the request carries no parentContext and a span is already active — trace.ts L588-L597 — so a trace() call given an explicit parent stays a child span and no tracesSampler call is ever made for it. There the call-site gate is the only control.

What the call-site gate is still for

Two things a per-name rate cannot do. The sampler draws independently per name, so a rate leaves a trace showing some of its spans and not others; a deterministic hash on the trace id keeps a family together — wrapper-sampling.ts L23-L38. And it declines to build the span at all, rather than dropping it at send, which is the difference that matters on a hot path.

So the gate is warranted for a span family on a hot path, and not for a single named transaction the rate table can cap. shouldSampleWrappers reads a remote rate too (sentry.wrapperSampleRate), so choosing it does not cost you runtime tunability.

Where the correction landed

The claim was in five places, not one: the breach triad, tier 0 of the mitigation ladder, two rows of the pitfall table, the metamask-extension overlay, and knowledge/span-sub-sampling.md, whose Kill Switch section told every author to ship an env flag with every span family. The env flag keeps the two jobs the rate cannot do — a child span, and removing the wrapping itself rather than sampling its output, which is what SENTRY_DISTRIBUTED_TRACING_DISABLED does.

Checks

node .github/scripts/lint-skill-entry.mjs — 63 skills checked, 0 errors, no new warnings on observability/sentry-quota. The four permalinks above return 200 at that sha, against a bogus-sha control returning 404; fragments are not sent to the server, so that proves the file and not the line range.