docs(adr): rename ADR-012 — drop Lazar; update title + content + cross-refs

- Rename docs/decisions/adr-012-lazar-conformance.md → adr-012-feature-conventions.md
- Strip "Lazar", "Plan 8/9/10/11", "refactor-logs" refs from all ADRs,
  architecture docs, HTML explainers, and feature/core AGENTS.md files
- Update all incoming links in docs/, packages/*/AGENTS.md, HTML explainers

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-13 10:07:37 +02:00
parent 06da37f723
commit 841655573b
18 changed files with 420 additions and 435 deletions

View File

@@ -1,10 +1,8 @@
# ADR-014 — Instrumentation & Sentry Logging
**Status:** Accepted
**Status (revised):** Superseded by ADR-017 for the implementation layer. The interface decisions (R31R51) remain authoritative.
**Status (revised):** Superseded by ADR-017 for the implementation layer. The interface decisions remain authoritative.
**Date:** 2026-05-06
**Spec:** docs/superpowers/specs/2026-05-06-instrumentation-sentry-design.md
**Plan:** docs/superpowers/plans/2026-05-06-plan-10-instrumentation-sentry.md
## Context
@@ -14,7 +12,7 @@ The monorepo had no distributed tracing or error capture. Production failures su
2. Centralized exception capture so silent failures (especially in CMS mutations) get a permanent record with stack + context.
3. Privacy posture suitable for production: scrubbing for PII, masked replay, no opaque-vs-named user identifiers.
The Lazar Nikolov reference repo (`nextjs-clean-architecture`) demonstrates a Sentry-driven pattern with `Sentry.startSpan` and `Sentry.captureException` calls inline in use cases and repos. We needed to adapt this to:
The reference Clean Architecture repo demonstrates a Sentry-driven pattern with `Sentry.startSpan` and `Sentry.captureException` calls inline in use cases and repos. We needed to adapt this to:
- Three apps (web-next, cms, web-tanstack) — not one.
- Per-feature DI (ADR-008) — not a single container.
@@ -34,11 +32,11 @@ The Lazar Nikolov reference repo (`nextjs-clean-architecture`) demonstrates a Se
**6. ESLint boundary rule (R40).** `no-restricted-imports` blocks `@sentry/*` outside the allowlisted paths: `core-shared/instrumentation/sentry/**`, `instrumentation/di/bind-sentry-instrumentation.{ts,test.ts}`, `core-testing/setup/no-sentry.{ts,test.ts}`, and the apps' `instrumentation*.{ts,mjs}` / `next.config.{mjs}` / `vite.config.{ts}` entries. Allowlist patterns use `**/`-prefix so they match whether ESLint runs from the repo root or from inside a sub-package.
**7. Test-side `RecordingTracer` / `RecordingLogger`** in `core-testing/instrumentation/`. Tests inject them directly into factory functions (consistent with R27 from Plan 9 — direct-injection, not container manipulation). The `core-testing/setup/no-sentry.ts` setup file mocks `@sentry/nextjs`, `@sentry/node`, and `@sentry/react` at the module level, so any code that imports them gets a no-op surface during vitest runs (R49).
**7. Test-side `RecordingTracer` / `RecordingLogger`** in `core-testing/instrumentation/`. Tests inject them directly into factory functions (direct-injection, not container manipulation). The `core-testing/setup/no-sentry.ts` setup file mocks `@sentry/nextjs`, `@sentry/node`, and `@sentry/react` at the module level, so any code that imports them gets a no-op surface during vitest runs.
## Alternatives considered
- **Direct `@sentry/nextjs` imports in features (Lazar's pattern).** Rejected — couples every feature package to a vendor SDK, violating the architecture's vendor-isolation principle.
- **Direct `@sentry/nextjs` imports in features.** Rejected — couples every feature package to a vendor SDK, violating the architecture's vendor-isolation principle.
- **Procedure-only spans (no per-use-case or per-repo spans).** Rejected — would lose the breakdown that makes a slow request diagnosable. The middle path (procedure + use case + controller, no per-repo) was rejected for the same reason at a finer granularity.
- **Capture in `defineErrorMiddleware` only.** Rejected — would noisily report every input-parse / unauthenticated error as a Sentry event, polluting the inbox.
- **Single Sentry project for all apps with environment tags.** Rejected — different alert routing, different quotas. Three projects scale better.
@@ -48,6 +46,7 @@ The Lazar Nikolov reference repo (`nextjs-clean-architecture`) demonstrates a Se
## Consequences
**Positive:**
- End-to-end traces in Sentry with full context.
- One captured event per error (no double-report, no noise from expected domain errors).
- Privacy-by-default replay and scrubbing.
@@ -55,6 +54,7 @@ The Lazar Nikolov reference repo (`nextjs-clean-architecture`) demonstrates a Se
- Tests run against `Recording*` for assertions; `Noop*` by default.
**Negative:**
- Every public repo method gains ~6 lines of `tracer.startSpan(...)` boilerplate. Mitigated by uniform pattern; if it ever proves excessive, a `withRepoSpan` collapse helper can be added.
- `__sentryReported` flag mutates errors. Non-enumerable, so JSON / spread are unaffected; flag is checked only inside `SentryLogger`.
- Three Sentry projects to administer.
@@ -71,7 +71,7 @@ The Lazar Nikolov reference repo (`nextjs-clean-architecture`) demonstrates a Se
## Post-merge follow-up — closing the R44 gap
Plan 10 as merged shipped repository-side capture (R43) but **not** use-case or controller capture (R44). The ADR/AGENTS docs described the intended capture-rules table as if it were the as-shipped state; in fact every `captureException` call site lived in `infrastructure/repositories/*.repository.ts`. A grep proved it: zero call sites in any controller or use-case body. The user spotted the gap.
The initial implementation shipped repository-side capture but **not** use-case or controller capture. The ADR/AGENTS docs described the intended capture-rules table as if it were the as-shipped state; in fact every `captureException` call site lived in `infrastructure/repositories/*.repository.ts`. A grep proved it: zero call sites in any controller or use-case body. The gap was spotted and fixed.
**Fix (post-merge commit):**
@@ -81,12 +81,11 @@ Plan 10 as merged shipped repository-side capture (R43) but **not** use-case or
4. `RecordingLogger.captureException` now also honours the flag, so test assertions about capture counts stay honest.
5. Added `packages/blog/tests/r44-no-double-capture.test.ts` to lock the contract: an error originated in the repo is captured once with repo tags; an error originated in the controller (parse failure) is captured once with controller tags; success paths capture nothing.
**Why use cases also wrap, even though current bodies mostly delegate to the repo:** R44's intent is that *any* throw originated locally — output-schema validation, business-rule errors like `AuthenticationError` in `signInUseCase` — gets captured with use-case tags. The wrapper makes the rule uniform; the flag makes it safe.
**Why use cases also wrap, even though current bodies mostly delegate to the repo:** R44's intent is that _any_ throw originated locally — output-schema validation, business-rule errors like `AuthenticationError` in `signInUseCase` — gets captured with use-case tags. The wrapper makes the rule uniform; the flag makes it safe.
## Related
- ADR-008 — per-feature DI containers
- ADR-011 — TDD foundation
- ADR-012 — Lazar pattern conformance
- ADR-012 — feature conventions
- ADR-013 — input/output unification
- Plan 10 spec (R31R55)