docs(spec): instrumentation + Sentry logging design (R31–R55)
Defines the post-Plan-9 instrumentation subsystem: vendor-agnostic Tracer/Logger interfaces in core-shared, full-depth tracing (procedure → controller → use-case → repository) with use-case + controller spans applied via withSpan at DI binding time and explicit tracer.startSpan in every repository method, throw-site error capture with __sentryReported double-report guard, and hard PII rules (sendDefaultPii:false, default-mask replay, beforeSend/beforeSendTransaction scrubbers, opaque user IDs only, build-time CI grep gate). Three apps in scope (web-next, cms, web-tanstack), each with its own Sentry project and DSN. Instrumentation binding is orthogonal to USE_DEV_SEED/NODE_ENV repo binding. Optional dev-mode Sentry: NoopTracer default, real SDK initializes only if DSN env is set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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/<app>/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: "<entity>.<method>", 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: `<feature>.<useCase>` (e.g. `"blog.getArticles"`, `"auth.signIn"`). Controller spans: same name with `op: "controller"`. Repository spans: `<entity>.<method>` (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<string, AttributeValue>;
|
||||
};
|
||||
|
||||
export interface ISpan {
|
||||
setAttribute(key: string, value: AttributeValue): void;
|
||||
setStatus(status: "ok" | "error", message?: string): void;
|
||||
}
|
||||
|
||||
export interface ITracer {
|
||||
startSpan<T>(opts: SpanOpts, fn: (span: ISpan) => Promise<T>): Promise<T>;
|
||||
}
|
||||
|
||||
// logger.interface.ts
|
||||
export type Breadcrumb = {
|
||||
category: string;
|
||||
message: string;
|
||||
level?: "info" | "warning" | "error";
|
||||
data?: Record<string, unknown>;
|
||||
};
|
||||
|
||||
export type CaptureContext = {
|
||||
tags?: Record<string, string>;
|
||||
extras?: Record<string, unknown>;
|
||||
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<Args extends unknown[], R>(
|
||||
tracer: ITracer,
|
||||
opts: SpanOpts | ((args: Args) => SpanOpts),
|
||||
fn: (...args: Args) => Promise<R>,
|
||||
): (...args: Args) => Promise<R> {
|
||||
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<ITracer>(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<Article[]> {
|
||||
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<void> {
|
||||
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/<app>/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/<vendor>/`; 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)
|
||||
Reference in New Issue
Block a user