diff --git a/docs/superpowers/specs/2026-05-06-instrumentation-sentry-design.md b/docs/superpowers/specs/2026-05-06-instrumentation-sentry-design.md new file mode 100644 index 0000000..c37e1d8 --- /dev/null +++ b/docs/superpowers/specs/2026-05-06-instrumentation-sentry-design.md @@ -0,0 +1,474 @@ +# Instrumentation & Sentry Logging — Design Spec + +**Date:** 2026-05-06 +**Status:** Approved (autonomous execution authorized) +**Author:** Claude Opus 4.7 (1M context) +**Reviewer:** Danijel +**Extends:** ADRs 008 (per-feature DI), 011 (TDD foundation), 012 (Lazar conformance), 013 (input/output unification). +**Inspired by:** Lazar Nikolov's `nextjs-clean-architecture` reference repo (`Sentry.startSpan` + `Sentry.captureException` pattern) — adapted for a multi-app monorepo with per-feature DI containers and a strict `core-shared`-only vendor boundary. + +--- + +## 1. Goal + +Add distributed tracing and error capture to the monorepo with three properties: + +1. **Vendor-agnostic interfaces.** Feature packages MUST NOT import from `@sentry/*` anywhere. They depend only on a `Tracer` and `Logger` interface defined in `core-shared`. Sentry is one implementation behind that interface, swappable like any other DI binding. +2. **Full-depth visibility.** Traces span every meaningful boundary — tRPC procedure → controller → use case → repository — so a slow request can be diagnosed without re-reading code or adding ad-hoc timers. +3. **Explicit, audit-ready PII handling.** Replay, breadcrumbs, and event payloads default-mask everything; a small allowlist opts in to unmask. PII handling is a top-level design concern (R31–R38), not a configuration footnote. + +Three production-facing apps are in scope: `web-next`, `cms`, `web-tanstack`. (Storybook is excluded — dev tool only.) Each gets its own Sentry project, its own DSN env var, and its own `instrumentation.ts` / `instrumentation-client.ts`. + +The success criterion is operational: when a user reports a slow page or a silent failure, the engineer should be able to open Sentry, find the trace from the user's session, see the full nested span tree (request → procedure → controller → use case → repository → Payload op) with attributes documenting *what* each layer was doing, and know within seconds where the problem lives. + +--- + +## 2. Non-goals + +- **No general-purpose structured logging pipeline.** This spec covers *exception capture* and *distributed tracing*. A `console.log → Sentry` forwarder, a pino setup, or a log-aggregation backend is not in v1. +- **No custom OpenTelemetry exporter or independent OTel backend.** Sentry's built-in tracer is sufficient. We do not stand up Tempo/Jaeger/Honeycomb in parallel. +- **No `@sentry/profiling-node` profiling.** CPU profiling is a future addition, not v1. +- **No `Sentry.showReportDialog()` user-facing error reporter.** Out of scope; can be added per-app later. +- **No PII allowlist seeding.** Per R34, the replay unmask allowlist starts empty. Adding selectors is a follow-up PR with justification, not part of this spec's execution. +- **No retroactive change to existing capture call sites.** There are none today (clean slate). We do not refactor existing error-handling code beyond what the new pattern requires. +- **No instrumentation for `apps/storybook`.** Storybook is a development tool; instrumenting it adds noise without operational value. +- **No replacement of `defineErrorMiddleware`.** The middleware's contract (map domain errors → `TRPCError`) is unchanged. It does NOT gain capture responsibility (R43). + +--- + +## 3. Rules (the contract — RFC 2119 language) + +These rules extend the post-Plan-9 architecture (R1–R30 from the I/O-unification spec). Numbering continues from R31. + +### 3.A — PII rules (highest priority — non-negotiable) + +**R31 — `sendDefaultPii` MUST be `false`.** Every `Sentry.init()` call across all three apps and both server/browser SDKs MUST set `sendDefaultPii: false`. An ESLint rule (or a CI grep step) MUST fail the build if a `sendDefaultPii: true` literal appears anywhere in the repo. + +**R32 — `beforeSend` scrubber MUST run on every event.** A shared scrubber function (in `core-shared/src/instrumentation/sentry/scrub.ts`) MUST deep-walk every event payload and strip values whose key contains (case-insensitive substring match) any of: `email`, `password`, `token`, `cookie`, `authorization`, `set-cookie`, `x-api-key`, `apikey`, `api_key`, `secret`. Substring (not exact) match is required so derived keys like `userEmail`, `accessToken`, `apiKey` are also caught. The scrubber MUST also redact IPv4 and IPv6 addresses found in string values (replaced with `"[redacted-ip]"`). Every `Sentry.init()` MUST pass this scrubber to `beforeSend`. + +**R33 — `beforeSendTransaction` scrubber MUST sanitize URLs.** A shared transaction scrubber MUST strip the values of query-string params whose keys contain (case-insensitive substring match) any of: `token`, `email`, `password`, `key`, `sig`, `signature`, `access_token`, `accesstoken`, `secret`. The scrubber MUST act on `event.request.url`, `event.transaction`, and any nested URL fields, replacing matched values with `"[redacted]"`. Every `Sentry.init()` MUST pass this scrubber to `beforeSendTransaction`. + +**R34 — Replay MUST default-mask everything.** `replayIntegration({ maskAllText: true, maskAllInputs: true, blockAllMedia: true })` MUST be the only configuration shape. An "unmask allowlist" (CSS selector list, configured per-app in `instrumentation-client.ts`) MAY pass selectors to `unmask` / `unblock` options, but the allowlist MUST start empty. Every entry added later MUST have an inline comment justifying why the content is non-PII. + +**R35 — Replay-mask flags are not configurable.** `maskAllText`, `maskAllInputs`, `blockAllMedia` MUST always be `true`. They are set as literal `true` in the init code, not derived from env vars. Code review MUST reject any PR that parameterizes these flags. + +**R36 — User context is opaque IDs only.** `Sentry.setUser({ id })` MUST be the only call site shape. The `SentryLogger.setUser` adapter MUST strip any keys other than `id` (e.g., `email`, `username`, `ip_address`) before forwarding to the SDK, and MUST emit a `console.warn` in dev when stripping occurs (so misuse is visible during development). The wrapper MUST NOT throw — production code paths MUST NOT break because someone accidentally passed `email`. + +**R37 — Sample rates from env; replay session rate defaults to `0.0`.** `tracesSampleRate` MUST come from `SENTRY_TRACES_SAMPLE_RATE` (default `1.0` in dev, `0.1` in prod). `replaysSessionSampleRate` MUST default to `0.0` (no recording of healthy sessions). `replaysOnErrorSampleRate` MUST default to `1.0` (record every errored session). Production overrides MAY be set via env, but defaults MUST favor privacy. + +**R38 — Each app MUST have a PII scrubber test.** Per app, a vitest test MUST construct a synthetic event with `email`, `password`, `cookie`, IP, and a query string with `?token=secret`, run it through both scrubbers, and assert all PII fields are redacted. Test file: `apps//src/__tests__/sentry-pii-scrubber.test.ts`. + +### 3.B — Architecture & boundary rules + +**R39 — Tracer/Logger live in `core-shared/src/instrumentation/`.** Two interface files (`tracer.interface.ts`, `logger.interface.ts`) define `ITracer`, `ISpan`, `ILogger`, `CaptureContext`, `SpanOpts`, `Breadcrumb`. Three implementations live alongside: `noop-tracer.ts`, `noop-logger.ts`, and `sentry/sentry-tracer.ts` + `sentry/sentry-logger.ts`. **Within `packages/`** (feature packages, core packages), the `core-shared/src/instrumentation/sentry/` subfolder is the only location permitted to `import` from `@sentry/*`. Apps (which need `withSentryConfig` from `@sentry/nextjs` and `@sentry/vite-plugin`) have a separate, narrowly-scoped allowlist in R40 — they MUST NOT make runtime SDK calls (e.g. `Sentry.captureException`, `Sentry.startSpan`) anywhere; only build-time wrappers and the dedicated `instrumentation.ts` / `instrumentation-client.ts` files may import from `@sentry/*`. + +**R40 — Boundary rule enforced by ESLint.** A `no-restricted-imports` rule MUST forbid `@sentry/*` imports outside this allowlist: +- `packages/core-shared/src/instrumentation/sentry/**` (the adapter implementations) +- `apps/*/instrumentation.ts` and `apps/*/instrumentation-client.ts` (Next.js / TanStack server-boot hooks) +- `apps/*/next.config.{mjs,ts}` and `apps/*/sentry.*.config.{ts,mjs,js}` (build-time wrappers like `withSentryConfig`) +- `apps/web-tanstack/vite.config.{ts,mjs}` (for `@sentry/vite-plugin`) + +Feature packages, controllers, use cases, and repositories MUST NOT import Sentry directly. The rule MUST be in `packages/core-eslint` so it applies repo-wide. + +**R41 — Use case + controller spans applied via `withSpan` at DI binding time.** Use case and controller factory bodies MUST NOT call `tracer.startSpan` directly. Instead, `bindProductionX` and `bindDevSeedX` MUST wrap the resolved factory result with `withSpan(tracer, { name, op }, fn)` from `core-shared/instrumentation/with-span.ts` before passing it to `.toConstantValue(...)`. This keeps factory code identical to today. + +**R42 — Repository methods MUST emit explicit spans.** Every public method on every concrete repository implementation (real Payload + mock) MUST wrap its body in `tracer.startSpan({ name: ".", op: "repository", attributes: {...} }, async (span) => { ... })`. Mock repositories use `NoopTracer` by default but the `startSpan` call MUST be present, so mock and real share the same span emission shape (verifiable in contract tests). + +**R43 — `captureException` is throw-site only.** `Sentry.captureException` (via `ILogger.captureException`) MUST be called only at the layer that *originates* the error: + +| Layer | Captures | Doesn't capture | +|---|---|---| +| Repository | Infra/Payload errors that originate here | Errors bubbling from below | +| Use case | Business-rule violations originated in this body | Errors from repos (already captured) | +| Controller | `InputParseError` from `safeParse` failure | Anything else | +| `defineErrorMiddleware` | **Nothing** | Continues to map domain → `TRPCError` only | + +**R44 — `SentryLogger.captureException` MUST guard against double-report.** The adapter MUST check for a non-enumerable `__sentryReported` property on the error object; if present, return early. Otherwise, call `Sentry.captureException`, then mark the error via `Object.defineProperty(err, "__sentryReported", { value: true })`. Call sites MUST NOT manage this flag manually. + +**R45 — Span name format is fixed.** Use case spans: `.` (e.g. `"blog.getArticles"`, `"auth.signIn"`). Controller spans: same name with `op: "controller"`. Repository spans: `.` (e.g. `"articles.findAll"`, `"users.create"`). The convention MUST be consistent across features for searchability in Sentry. + +**R46 — Each app has its own DSN; no cross-app reuse.** `WEB_NEXT_SENTRY_DSN`, `CMS_SENTRY_DSN`, `WEB_TANSTACK_SENTRY_DSN` MUST be three distinct Sentry projects. The browser DSNs (`NEXT_PUBLIC_WEB_NEXT_SENTRY_DSN`, `VITE_WEB_TANSTACK_SENTRY_DSN`) point at the same projects as their server counterparts (one project covers an app's server + browser). + +**R47 — Instrumentation binding mode is orthogonal to repo binding mode.** `bindAll()` MUST evaluate "use Sentry vs. use Noop" as a separate rule from "use real Payload vs. dev seed." Concretely: +- DSN set → `bindSentryInstrumentation` +- DSN unset → `bindNoopInstrumentation` + +This rule is independent of `USE_DEV_SEED` and `NODE_ENV`. A developer running `pnpm dev` with `SENTRY_DSN` set in `.env.local` gets dev seed repos *and* real Sentry capture — useful for testing the integration itself. + +**R48 — NoopTracer/NoopLogger MUST be the default everywhere DSN is absent.** This applies regardless of `NODE_ENV`. Production deployments without a DSN configured (e.g. preview environments) MUST function correctly with no instrumentation, not crash. + +### 3.C — Testing rules + +**R49 — Tests MUST use Noop or Recording, never real Sentry.** The vitest setup in `core-testing/src/vitest.setup.ts` MUST bind `NoopTracer` + `NoopLogger` by default. Tests asserting on capture/span calls MUST inject a `RecordingLogger` or `RecordingTracer` directly via the factory function (consistent with the existing direct-injection test pattern, R27 from Plan 9). The `@sentry/*` packages MUST NOT initialize during `pnpm test` — verified by a build-time check that no process under vitest has `Sentry.getCurrentHub().getClient()` returning a real client. + +**R50 — Repository contract suites assert on span shape.** `defineContractSuite` (in `core-testing`) MUST gain an optional `expectSpan` helper. For every repository method tested, the suite asserts a span was emitted with the expected `name`, `op: "repository"`, and a documented set of attributes. This catches drift where a repo method is added but its span is forgotten. + +### 3.D — Operational rules + +**R51 — Replay allowlist additions require code-review justification.** Any PR that adds a CSS selector to the replay `unmask` or `unblock` allowlist MUST include an inline code comment (above the entry) explaining why the content is verifiably non-PII. PR template SHOULD prompt reviewers to confirm. + +**R52 — Source map upload uses a build-time-only token.** `SENTRY_AUTH_TOKEN` MUST be present only at build time (CI environment). It MUST NOT be exposed to runtime or to client bundles. `withSentryConfig` MUST be configured with `silent: process.env.CI !== "true"` so missing-token warnings are visible in CI logs but suppressed locally. + +**R53 — Release tag is the git SHA.** Every `Sentry.init()` MUST set `release: process.env.VERCEL_GIT_COMMIT_SHA ?? execSync("git rev-parse HEAD").toString().trim()`. This ensures source-map symbolication works correctly across deploys. + +**R54 — Refactor changelog at `docs/superpowers/refactor-logs/2026-05-06-instrumentation-sentry.md`.** Following the pattern of Plans 8 and 9 (R30 of Plan 9), every commit landing as part of this work MUST be tracked in the changelog with task number, file paths, and rationale. + +**R55 — ADR-014 documents the decisions.** A new `docs/decisions/adr-014-instrumentation-sentry.md` MUST be added in the final commit, recording: vendor-agnostic interface in `core-shared`; per-feature DI binding for tracer/logger; full-depth instrumentation; throw-site capture; PII rules; per-app DSNs; orthogonal binding mode. + +--- + +## 4. Architecture + +### 4.1 The instrumentation subsystem in `core-shared` + +``` +packages/core-shared/src/instrumentation/ +├── index.ts — re-exports interfaces + Noop impls +├── tracer.interface.ts — ITracer, ISpan, SpanOpts, AttributeValue +├── logger.interface.ts — ILogger, CaptureContext, Breadcrumb +├── noop-tracer.ts — pass-through ITracer +├── noop-logger.ts — pass-through ILogger +├── with-span.ts — higher-order wrapper for DI bind-time +├── symbols.ts — TRACER, LOGGER (DI symbols) +├── sentry/ +│ ├── sentry-tracer.ts — adapter: ITracer → @sentry/nextjs +│ ├── sentry-logger.ts — adapter: ILogger → @sentry/nextjs +│ ├── scrub.ts — beforeSend + beforeSendTransaction +│ ├── init-server.ts — Sentry.init() helper for server runtime +│ ├── init-client.ts — Sentry.init() helper for browser runtime +│ └── pii-fields.ts — regex constants for R32/R33 +├── di/ +│ ├── bind-noop-instrumentation.ts — binds NoopTracer + NoopLogger +│ └── bind-sentry-instrumentation.ts — binds SentryTracer + SentryLogger +└── *.test.ts — unit tests per file +``` + +### 4.2 Interface shapes + +```ts +// tracer.interface.ts +export type AttributeValue = string | number | boolean | null; + +export type SpanOpts = { + name: string; + op?: "use-case" | "controller" | "repository" | "service" | string; + attributes?: Record; +}; + +export interface ISpan { + setAttribute(key: string, value: AttributeValue): void; + setStatus(status: "ok" | "error", message?: string): void; +} + +export interface ITracer { + startSpan(opts: SpanOpts, fn: (span: ISpan) => Promise): Promise; +} + +// logger.interface.ts +export type Breadcrumb = { + category: string; + message: string; + level?: "info" | "warning" | "error"; + data?: Record; +}; + +export type CaptureContext = { + tags?: Record; + extras?: Record; + fingerprint?: string[]; +}; + +export interface ILogger { + captureException(err: unknown, ctx?: CaptureContext): void; + captureMessage(msg: string, level?: "info" | "warning" | "error", ctx?: CaptureContext): void; + addBreadcrumb(b: Breadcrumb): void; + setUser(user: { id: string } | null): void; +} +``` + +### 4.3 The `withSpan` higher-order wrapper (DI binding-time use case + controller spans) + +```ts +// core-shared/src/instrumentation/with-span.ts +export function withSpan( + tracer: ITracer, + opts: SpanOpts | ((args: Args) => SpanOpts), + fn: (...args: Args) => Promise, +): (...args: Args) => Promise { + return (...args) => { + const resolved = typeof opts === "function" ? opts(args) : opts; + return tracer.startSpan(resolved, () => fn(...args)); + }; +} +``` + +Used in feature `bind-production.ts` like: + +```ts +// packages/blog/src/di/bind-production.ts (excerpt) +const tracer = container.get(SHARED_SYMBOLS.TRACER); +const articlesRepo = new PayloadArticlesRepository(config, tracer); // repo gets tracer for explicit spans +const wrappedUseCase = withSpan(tracer, { name: "blog.getArticles", op: "use-case" }, + getArticlesUseCase(articlesRepo)); +container.bind(BLOG_SYMBOLS.GetArticlesUseCase).toConstantValue(wrappedUseCase); + +const wrappedController = withSpan(tracer, { name: "blog.getArticles", op: "controller" }, + getArticlesController(wrappedUseCase)); +container.bind(BLOG_SYMBOLS.GetArticlesController).toConstantValue(wrappedController); +``` + +### 4.4 Repository explicit spans (R42) + +Every public method on every repo wraps its body. Concrete shape: + +```ts +// packages/blog/src/infrastructure/repositories/articles.repository.ts (excerpt) +async findAll(input: FindAllInput): Promise { + return this.tracer.startSpan( + { name: "articles.findAll", op: "repository", attributes: { collection: "articles", limit: input.limit ?? null } }, + async (span) => { + try { + const result = await this.payload.find({ collection: "articles", limit: input.limit, ... }); + span.setAttribute("count", result.docs.length); + return result.docs.map(toArticle); + } catch (err) { + this.logger.captureException(err, { tags: { feature: "blog", repo: "articles", method: "findAll" } }); + span.setStatus("error", err instanceof Error ? err.message : String(err)); + throw err; + } + }, + ); +} +``` + +The mock repository (`articles.repository.mock.ts`) uses the same shape but with the `NoopTracer` injected by default — the `startSpan` call still runs, but it's a pass-through. This keeps mock and real symmetric (verifiable via R50). + +### 4.5 The `bindAll()` dispatcher with the orthogonal instrumentation rule + +```ts +// apps/web-next/src/server/bind-production.ts (excerpt — post-spec) +import { bindNoopInstrumentation } from "@repo/core-shared/di/bind-noop-instrumentation"; +import { bindSentryInstrumentation } from "@repo/core-shared/di/bind-sentry-instrumentation"; + +export async function bindAll(): Promise { + if (bound) return; + bound = true; + + // Rule 0: instrumentation — DSN presence decides, independent of repo mode + const dsn = process.env.WEB_NEXT_SENTRY_DSN; + if (dsn) { + await bindSentryInstrumentation(container, { dsn, environment: ..., release: ... }); + } else { + bindNoopInstrumentation(container); + } + + // Rules 1-3: repo mode (existing logic — unchanged; bound flag already set above) + if (process.env.USE_DEV_SEED === "true") return bindAllDevSeed(); + if (process.env.NODE_ENV === "production") return bindAllProduction(); + return bindAllDevSeed(); +} +``` + +The instrumentation step runs *first* so feature binders (`bindProductionBlog`, `bindDevSeedBlog`, etc.) can resolve `TRACER` and `LOGGER` from the container during their own setup. + +### 4.6 Per-app integration files + +#### `apps/web-next/` + +- **`instrumentation.ts`** (Next.js convention; runs on server boot) + ```ts + export async function register() { + if (process.env.NEXT_RUNTIME === "nodejs" || process.env.NEXT_RUNTIME === "edge") { + const { initSentryServer } = await import("@repo/core-shared/instrumentation/sentry/init-server"); + initSentryServer({ dsn: process.env.WEB_NEXT_SENTRY_DSN, app: "web-next" }); + } + } + ``` +- **`instrumentation-client.ts`** (Next.js 15+ browser hook) + ```ts + import { initSentryClient } from "@repo/core-shared/instrumentation/sentry/init-client"; + initSentryClient({ dsn: process.env.NEXT_PUBLIC_WEB_NEXT_SENTRY_DSN, app: "web-next" }); + ``` +- **`next.config.mjs`** wrapped with `withSentryConfig({ silent: process.env.CI !== "true", authToken: process.env.SENTRY_AUTH_TOKEN, ... })`. +- **`src/__tests__/sentry-pii-scrubber.test.ts`** (R38). + +#### `apps/cms/` + +Server-only Payload host. Same pattern minus `instrumentation-client.ts` and the browser DSN. `cms/src/instrumentation.ts` plus `withSentryConfig` on the Next config that hosts Payload's admin UI. + +#### `apps/web-tanstack/` + +TanStack Start uses Vite. The init pattern moves to TanStack's own server entry hook (`src/server.ts`) and a Vite plugin for source-map upload (`@sentry/vite-plugin`). Browser SDK init lives in the client entry. Env var prefix is `VITE_` for client. + +### 4.7 Env vars (declared in `turbo.json` `globalEnv`) + +| Var | Scope | Purpose | +|---|---|---| +| `WEB_NEXT_SENTRY_DSN` | Server runtime | web-next server SDK init | +| `NEXT_PUBLIC_WEB_NEXT_SENTRY_DSN` | Client bundle | web-next browser SDK init | +| `CMS_SENTRY_DSN` | Server runtime | cms server SDK init | +| `WEB_TANSTACK_SENTRY_DSN` | Server runtime | web-tanstack server SDK init | +| `VITE_WEB_TANSTACK_SENTRY_DSN` | Client bundle | web-tanstack browser SDK init | +| `SENTRY_AUTH_TOKEN` | Build only | Source-map upload (CI only; never bundled) | +| `SENTRY_TRACES_SAMPLE_RATE` | Server + client | Default `1.0` dev, `0.1` prod | +| `SENTRY_ENVIRONMENT` | Server + client | Auto-derived: `VERCEL_ENV ?? NODE_ENV` | + +All eight MUST be added to `turbo.json` `globalEnv`. + +--- + +## 5. Data flow with spans + capture + +For a single tRPC request `blog.getArticles({ limit: 10 })`: + +``` +1. Browser: user clicks → trpc.blog.getArticles.useQuery({ limit: 10 }) + → Sentry browser SDK: HTTP transaction span starts +2. Network: POST /api/trpc/blog.getArticles (sentry-trace + baggage headers carry parent context) +3. Server: @sentry/nextjs HTTP integration: server transaction starts (parent = browser transaction) +4. tRPC: auto-instrumented procedure span "blog.getArticles" (from tRPC integration) +5. Middleware: defineErrorMiddleware sets active scope tags { feature: "blog", procedure: "getArticles" } +6. Controller (DI-wrapped): span "blog.getArticles" (op="controller") + ├─ inputSchema.safeParse({ limit: 10 }) — no span (too granular per R45) + └─ Use case (DI-wrapped): span "blog.getArticles" (op="use-case") + └─ Repository (explicit per R42): span "articles.findAll" (op="repository", attributes={ collection: "articles", limit: 10 }) + └─ Payload Local API (auto-instrumented by Sentry): span for the DB query + └─ presenter(result) — no span + ← returns view-model +7. Middleware exit: success → tRPC serializes response +8. Server: transaction ends; sent to Sentry per sample rate +9. Browser: transaction continues until response handled; sent to browser-side Sentry +``` + +**Failure variant** (Payload throws on step 6's repo call): +- Repo `catch` block calls `this.logger.captureException(err, { tags: { feature: "blog", repo: "articles", method: "findAll" } })` (R43). +- `SentryLogger.captureException` checks `err.__sentryReported` — false; sends to Sentry; sets `__sentryReported = true` (R44). +- Repo span gets `setStatus("error", message)`; re-throws. +- Use case span propagates error status (no capture — already captured). +- Controller span propagates. +- `defineErrorMiddleware` catches, looks up `[Ctor, code]` map, throws `TRPCError(code, { cause: err })`. **Does not capture again** (R43). +- tRPC sends error response to client; client SDK records the failed transaction. + +Result: one event in Sentry, with the full nested span tree as context, attributes documenting *what* the repo was doing, and the error fingerprinted by class + procedure. + +--- + +## 6. Migration order + +Each step is one commit, TDD-d. Final ordering goes into the plan. + +1. **Scaffold.** Spec (this doc) + refactor log (`docs/superpowers/refactor-logs/2026-05-06-instrumentation-sentry.md`) + initial empty ADR-014 stub. +2. **Interfaces + Noop.** `core-shared/src/instrumentation/{tracer,logger}.interface.ts`, `noop-tracer.ts`, `noop-logger.ts`, `symbols.ts`, `with-span.ts`, `index.ts`. Unit tests for Noop and `withSpan`. +3. **Sentry adapters + scrubbers.** `core-shared/src/instrumentation/sentry/{sentry-tracer,sentry-logger,scrub,pii-fields,init-server,init-client}.ts`. Unit tests for `SentryTracer.startSpan` (mocking Sentry's exported `startSpan`), `SentryLogger.captureException` double-report guard, `scrub` against fixture events. +4. **DI binders + bindAll dispatcher rule.** `core-shared/src/di/bind-{noop,sentry}-instrumentation.ts`. Update `apps/web-next/src/server/bind-production.ts` `bindAll()` with the orthogonal rule. Tests asserting (a) DSN absent → Noop bound; (b) DSN present → Sentry bound; (c) instrumentation choice independent of `USE_DEV_SEED`/`NODE_ENV`. +5. **Test infrastructure (`core-testing`).** `RecordingTracer`, `RecordingLogger`, `bindRecordingInstrumentation`. `vitest.setup.ts` updated to bind Noop by default. Self-tests for Recording. +6. **Per-feature wiring — blog (pilot).** Update `bind-production.ts` + `bind-dev-seed.ts` to wrap use-case + controller factories with `withSpan`. Update every method on `PayloadArticlesRepository` and `MockArticlesRepository` to use `tracer.startSpan` (R42). Use case + repo + controller tests updated for direct-injection of `RecordingLogger`/`RecordingTracer` where assertions matter. +7. **Per-feature wiring — auth, marketing-pages, navigation, media.** Same pattern as step 6, one feature per commit. +8. **Contract suite span assertions.** Add `expectSpan` helper to `defineContractSuite`; update every repo contract test to assert span shape (R50). +9. **App integration — web-next.** `instrumentation.ts`, `instrumentation-client.ts`, `next.config.mjs` `withSentryConfig`, `src/__tests__/sentry-pii-scrubber.test.ts`, env vars in `.env.example` and `turbo.json` `globalEnv`. +10. **App integration — cms.** +11. **App integration — web-tanstack.** Vite-specific (different env prefix, `@sentry/vite-plugin`). +12. **ESLint boundary rule.** `no-restricted-imports` blocking `@sentry/*` outside the allowed paths (R40). CI grep step for `sendDefaultPii: true` (R31). +13. **Doc updates.** CLAUDE.md, AGENTS.md, vertical-feature-spec.md, tdd-workflow.md, testing-strategy.md, dependency-flow.md additions for the new symbols, binding rule, throw-site-capture rule, PII discipline. +14. **HTML updates.** Add §07 "Tracing & error capture" to `docs/architecture/data-flow-explainer.html` (trace tree visualization, capture rule table, PII rules summary). Update `docs/architecture/di-explainer.html` with the new TRACER/LOGGER symbols and the orthogonal binding rule. +15. **ADR-014 final.** Document decisions, alternatives considered, consequences. + +--- + +## 7. Testing strategy + +### Unit tests (vitest) +- `NoopTracer.startSpan` returns the function result; `ISpan` methods are no-ops. +- `withSpan` calls `tracer.startSpan(opts, () => fn(...args))` and returns the result. +- `SentryLogger.captureException` calls `Sentry.captureException` once, marks `__sentryReported`, second call returns early. +- `SentryTracer.startSpan` delegates to Sentry's exported `startSpan`, passing through the function and result. +- `scrub.beforeSend` strips email/password/cookie/auth/IP from a fixture event (R32). +- `scrub.beforeSendTransaction` strips `?token=`/`?email=`/`?password=`/`?key=` from URLs (R33). + +### DI tests +- `bindAll()` rule independence (R47): four matrix cases verifying repo mode and instrumentation mode are independent. +- `bindNoopInstrumentation` populates `TRACER` + `LOGGER` symbols. +- `bindSentryInstrumentation` initializes Sentry with the configured DSN and binds the SDK-backed adapters. + +### Contract tests +- Every repo contract suite gains `expectSpan` assertions per method (R50). + +### Integration tests (per app) +- `apps//src/__tests__/sentry-pii-scrubber.test.ts` (R38) — synthetic event with PII fields → run scrubbers → assert all redacted. + +### What is **not** tested +- Actual Sentry transport (we do not stand up a mock Sentry server). The adapter delegating correctly is the test boundary. +- Replay flag enforcement at runtime (R35) — tests assert the init code passes the literal flags; we trust the SDK to honor them. + +--- + +## 8. Out of scope (deferred — explicit non-goals) + +- Structured logging pipeline (pino, log forwarding) — separate effort if needed. +- `@sentry/profiling-node` profiling integration — future addition. +- Sentry Dashboards / Alert rules / on-call routing config — operational concern, lives in Sentry org settings, not in repo. +- `Sentry.showReportDialog()` — not in v1. +- Replay allowlist content — empty at v1; populated incrementally with per-PR justification (R34, R51). +- `apps/storybook` instrumentation — out of scope; storybook is a dev tool. +- Migration of any existing capture/log code — there is none today (clean slate). +- Performance budgets / bundle-size CI gates for the SDK additions — separate concern; this spec only documents expected size impact (~60KB browser SDK with replay, ~250KB peak when replay activates on error). + +--- + +## 9. Risks & mitigations + +| Risk | Mitigation | +|---|---| +| Per-repo-method span boilerplate becomes noisy across ~30 methods. | Boilerplate is uniform (always wraps the body in `tracer.startSpan` with the same pattern); a shared `withRepoSpan` helper MAY be added if duplication exceeds tolerance, but starting explicit per R42 keeps every span visible at the call site. | +| `__sentryReported` flag pollutes error objects. | Property is non-enumerable (won't appear in JSON.stringify or spread); only checked inside `SentryLogger`. Acceptable cost for double-report prevention. | +| Replay bundle size hits performance budget. | Default `replaysSessionSampleRate: 0.0` means replay code is loaded but recordings are only captured on errored sessions. Peak bundle impact is acknowledged in §8; not gated by this spec. | +| Developer accidentally sets `sendDefaultPii: true` during a debugging session. | R31 ESLint/CI rule fails the build. PR can't merge. | +| Sentry SDK initializes during `pnpm test`. | `vitest.setup.ts` binds NoopTracer/NoopLogger; SDK is never imported in test paths because feature code uses interface-only imports (R39). Verified by R49. | +| Three Sentry projects to administer (vs. one). | Accepted cost — better quotas, cleaner alert routing, no cross-app noise. | +| Build-time `SENTRY_AUTH_TOKEN` exposure risk. | Token is server-only env, never `NEXT_PUBLIC_`/`VITE_` prefixed; `withSentryConfig` strips it from runtime bundles by design. | +| Vendor lock-in to Sentry. | Mitigated by the interface boundary (R39, R40). Swapping to OTel + a different exporter means writing one new adapter in `core-shared/instrumentation//`; feature code does not change. | + +--- + +## 10. Acceptance criteria + +This spec is satisfied when: +- All R31–R55 rules are implemented and verifiable. +- `pnpm build` succeeds in all three apps with and without DSN env vars set. +- `pnpm test` passes; no Sentry SDK initialization in any test process. +- Every repo method has an explicit `tracer.startSpan` call (R42). +- Every use-case + controller is wrapped via `withSpan` at DI bind time (R41). +- All PII scrubber tests pass (R38). +- ESLint boundary rule rejects a synthetic `import "@sentry/nextjs"` outside permitted paths (R40). +- The CI grep for `sendDefaultPii: true` returns no matches across the repo (R31). +- `docs/architecture/data-flow-explainer.html` has §07 documenting the trace tree, capture rules, and PII rules. +- `docs/architecture/di-explainer.html` documents the new TRACER/LOGGER symbols and orthogonal binding rule. +- ADR-014 is committed and explains the design decisions. +- Refactor log at `docs/superpowers/refactor-logs/2026-05-06-instrumentation-sentry.md` is complete. +- A live test against a real Sentry project verifies: (a) a slow request produces a nested trace; (b) a thrown infra error produces exactly one captured event with the documented tags/attributes; (c) replay records an errored session with all text masked; (d) PII-bearing events are visibly redacted in the Sentry UI. + +--- + +## 11. Decision log + +- **Tracer/Logger interface in `core-shared` vs. direct Sentry imports in features.** Chose interface (R39) — preserves the architecture's vendor-isolation principle (use cases don't know who Payload is; they shouldn't know who Sentry is). +- **Full Lazar depth (procedure + controller + use-case + repository spans) vs. procedure-only.** Chose full depth — explicit per-repo `startSpan` boilerplate is the price of admission for trace-driven debugging without ad-hoc timers. +- **Throw-site capture vs. middleware capture.** Chose throw-site (R43) — avoids noisy capture of expected domain errors (input validation, unauthenticated, etc.) and gives capture calls richer context (the repo knows the collection/op; the middleware doesn't). +- **`__sentryReported` flag for double-report prevention** vs. centralized capture only at one layer. Chose flag (R44) — allows multi-layer capture if/when a use case wants to capture its own throws *and* lets repos capture, without coordinating call sites. +- **Three Sentry projects vs. one with environment tags.** Chose three (R46) — separate alert routing, separate quotas, cleaner per-app debugging UX. +- **Optional dev Sentry vs. always-Noop in dev.** Chose optional (R47/R48) — DSN-presence-toggles-mode is the simplest rule, and it lets developers test the integration locally. +- **Replay default-mask + empty allowlist** vs. configurable mask flags. Chose mandatory mask (R34, R35) — privacy posture must be the default; opt-out requires per-selector justification (R51). +- **`sendDefaultPii: false` enforced via build-time check** vs. trust developers. Chose enforcement (R31) — too-easy-to-forget settings get forgotten; build-time check makes the policy unbreakable. + +--- + +## 12. References + +- ADR-008 — per-feature DI containers +- ADR-011 — TDD foundation +- ADR-012 — Lazar pattern conformance +- ADR-013 — input/output unification +- `docs/superpowers/specs/2026-05-06-input-output-unification-design.md` (R1–R30, the rule numbering this spec extends) +- Lazar Nikolov's `nextjs-clean-architecture` reference repo (Sentry pattern source) +- Sentry docs — `@sentry/nextjs` instrumentation hooks, `replayIntegration` config, `beforeSend` API +- Sentry docs — Distributed tracing across HTTP boundaries (sentry-trace + baggage headers)