diff --git a/AGENTS.md b/AGENTS.md index 078f5c9..fc10bbf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -348,6 +348,83 @@ Actual function names: `bindProductionAuth`, `bindProductionBlog`, `bindProducti --- +## Instrumentation conventions + +**Symbols (in `core-shared/instrumentation/symbols.ts`):** +- `INSTRUMENTATION_SYMBOLS.TRACER` — bound to `ITracer` (`NoopTracer` / `SentryTracer`) +- `INSTRUMENTATION_SYMBOLS.LOGGER` — bound to `ILogger` (`NoopLogger` / `SentryLogger`) + +**Repository constructor signature (every feature):** + +```ts +constructor( + config: SanitizedConfig, + tracer: ITracer = new NoopTracer(), + logger: ILogger = new NoopLogger(), +) +``` + +**Repository method body (every public async method):** + +```ts +return this.tracer.startSpan( + { name: ".", op: "repository", attributes: { /* ... */ } }, + async (span) => { + try { + const result = await /* payload op */; + span.setAttribute("count", /* ... */); + return result; + } catch (err) { + this.logger.captureException(err, { + tags: { feature: "", repo: "", method: "" }, + }); + span.setStatus("error", err instanceof Error ? err.message : String(err)); + throw err; + } + }, +); +``` + +**Use case + controller spans (applied at DI bind time):** + +```ts +const wrappedUC = withSpan( + tracer, + { name: "blog.getArticles", op: "use-case" }, + getArticlesUseCase(repo), +); +const wrappedCtrl = withSpan( + tracer, + { name: "blog.getArticles", op: "controller" }, + getArticlesController(wrappedUC), +); +``` + +**Capture rules:** + +| Layer | Captures | Doesn't capture | +|---|---|---| +| Repository | Infra/Payload errors that originate here | Bubbled errors | +| Use case | Business-rule violations originated in this body | Errors from repos (already captured) | +| Controller | `InputParseError` from safeParse failure | Anything else | +| `defineErrorMiddleware` | Nothing — maps domain → TRPCError only | — | + +**Boundary rule (eslint-enforced, R40):** +Feature packages MUST NOT `import "@sentry/*"`. Allowlist: +- `**/instrumentation/sentry/**` (core-shared) +- `**/instrumentation/di/bind-sentry-instrumentation.{ts,test.ts}` +- `**/setup/no-sentry.{ts,js}` + the test guard +- `apps/*/instrumentation*.{ts,mjs,js}` +- `apps/*/next.config.{mjs,ts,js}` +- `apps/*/vite.config.{ts,mjs,js}` + +**Test rules:** +- Default to `NoopTracer` / `NoopLogger` (constructor defaults) +- Assert spans/captures by injecting `RecordingTracer` / `RecordingLogger` from `@repo/core-testing/instrumentation` +- Real `@sentry/*` SDK MUST NOT initialize during tests (guarded by `core-testing/setup/no-sentry.ts`) + +--- + ## Specification & Guides - **Vertical Feature Spec** — `docs/architecture/vertical-feature-spec.md` — full design, rationale, decision log diff --git a/CLAUDE.md b/CLAUDE.md index 6024dbb..32a517f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,6 +50,12 @@ Turborepo + pnpm monorepo organized by vertical features. Each feature (`auth`, - **Payload repositories via constructor** — Feature packages receive Payload config at constructor time, not as a direct dependency - **Three binding modes per feature** — Each feature exports two binders: `./di/bind-production` (real Payload) and `./di/bind-dev-seed` (populated mock). The app's `bindAll()` dispatcher in `apps/web-next/src/server/bind-production.ts` picks one by env: `USE_DEV_SEED="true"` → dev seed; `NODE_ENV="production"` → production; otherwise → dev seed (developer default so `pnpm dev` boots without Payload). Dev seed lives in `src/__seeds__/dev.ts` as a lazy `buildDev()` function that uses the feature's existing factory - **App bootstrap** — Each app calls `bindAll()` from a server entry point (page server component, route handler) before resolving any feature controller. The dispatcher is idempotent +- **Instrumentation lives in `core-shared/instrumentation/`** — Two interfaces (`ITracer`, `ILogger`), three implementations (`NoopTracer`/`NoopLogger`, `SentryTracer`/`SentryLogger`, and `RecordingTracer`/`RecordingLogger` from `core-testing`). Feature packages MUST NOT import `@sentry/*` directly (R40, eslint-enforced) +- **Spans applied at DI bind time** — Use cases + controllers wrapped via `withSpan(tracer, { name: ".", op: "use-case" }, factory(...))` inside `bind-production` / `bind-dev-seed`. Repository methods emit explicit `tracer.startSpan({ name: ".", op: "repository", attributes: {...} }, ...)` (R41, R42) +- **Capture at throw sites only** — Repository catch blocks call `this.logger.captureException(err, { tags: { feature, repo, method } })`; use cases capture errors they originate; the tRPC error middleware does NOT capture (R43, R44) +- **PII handling is non-negotiable** — `sendDefaultPii: false` everywhere (R31, CI grep gate); replay default-masks all text/inputs/media (R34, R35, allowlist starts empty); `Sentry.setUser({ id })` only — no email/username (R36); `beforeSend` + `beforeSendTransaction` scrubbers strip emails/passwords/tokens/cookies/auth/IPs (R32, R33) +- **Three apps, three Sentry projects** — `WEB_NEXT_SENTRY_DSN`, `CMS_SENTRY_DSN`, `WEB_TANSTACK_SENTRY_DSN`. Browser DSNs use `NEXT_PUBLIC_` (web-next) and `VITE_` (web-tanstack) prefixes +- **Instrumentation binding is orthogonal to repo binding** — `bindAll()`'s Rule 0 (DSN → Sentry vs Noop) is independent of `USE_DEV_SEED` / `NODE_ENV`. Run `pnpm dev` with `WEB_NEXT_SENTRY_DSN` set to test the integration locally ## MCP Servers diff --git a/docs/architecture/vertical-feature-spec.md b/docs/architecture/vertical-feature-spec.md index aa4de1d..3793eaa 100644 --- a/docs/architecture/vertical-feature-spec.md +++ b/docs/architecture/vertical-feature-spec.md @@ -683,3 +683,27 @@ Big-bang refactor executed as 11 internal phases; each phase ends with a verific ## 15. Post-approval next step Invoke the `superpowers:writing-plans` skill to produce a detailed, executable implementation plan based on the 11-phase sequencing in §12. Each phase becomes a plan section with concrete file-by-file steps, verification commands, and commit-message templates. + +--- + +## 16. Instrumentation & error capture (Plan 10) + +**Spec:** `docs/superpowers/specs/2026-05-06-instrumentation-sentry-design.md` (R31–R55). + +**File additions per feature:** + +- `infrastructure/repositories/.repository.ts` — constructor takes `(config, tracer, logger)` with Noop defaults; every public method's body is wrapped in `tracer.startSpan(...)` and any `catch` block calls `logger.captureException(err, { tags: { feature, repo, method } })` before re-throwing. +- `infrastructure/repositories/.repository.mock.ts` — same constructor/wrapping shape (no catch — mocks don't originate infra errors). +- `di/bind-production.ts` — signature `(config, tracer, logger)`. Binds TRACER + LOGGER to the feature container; constructs the real repo with tracer/logger; wraps every use case + controller via `withSpan` at bind time. +- `di/bind-dev-seed.ts` — signature `(tracer, logger)`. Same wrapping as bind-production but with the populated mock. + +**Required exports (per feature root):** unchanged. + +**Public surface impact:** none for `./` (contracts) and `./ui`. The `./di/bind-production` and `./di/bind-dev-seed` subpaths now have new signatures — any consumer outside the app dispatcher is unaffected (the dispatcher is the only consumer per ADR-008). + +**Test patterns:** + +- **Direct injection** of `RecordingTracer` / `RecordingLogger` from `@repo/core-testing/instrumentation` for span/capture assertions. +- **Contract suite span assertions** — every repo's contract suite (`__contracts__/*-repository.contract.ts`) includes a `span emission (R50)` describe block enumerating one assertion per method. + +**Tradeoff:** every public repo method gains ~6 lines of `tracer.startSpan(...)` boilerplate. Worth the per-method visibility in production traces; if it ever proves excessive, a `withRepoSpan` helper can collapse the wrapping.