fraqtal/agentic-dev
Template
1
0
Fork 0
Code Issues Pull Requests Actions 7 Packages Projects Releases Wiki Activity
Files
4fa108dd4867f6eff398c9481a93df0ce4c48eaa
agentic-dev/docs/superpowers/refactor-logs/2026-05-06-instrumentation-sentry.md
Danijel Martinek f0775d6ecc feat(instrumentation): close R44 gap — throw-site capture for use cases + controllers 
Plan 10 documented R44 (capture at originating-throw layer) but only the
R43 repo leg was wired. captureException had zero call sites in any
controller or use-case body. This commit closes the gap.

Mechanism:
- Extract __sentryReported flag helpers into core-shared/instrumentation/
  reported-flag.ts. SentryLogger switches to importing them; RecordingLogger
  carries an inlined copy (tooling → core boundary disallows the import).
- Add withCapture(logger, tags, fn) higher-order wrapper paralleling
  withSpan. On throw: capture-with-tags, mark, re-throw. Bail if the flag
  was already set — covers the bubbled-from-repo case so each error
  surfaces in the logger exactly once with the inner-most layer's tags.
- Apply withSpan(withCapture(factory)) in every feature's bind-production
  and bind-dev-seed: auth (3 use cases × 3 controllers), blog (3×3),
  marketing-pages (2×2), navigation (1×1), media (3×3). Span is outermost
  so the errored span timing reflects the capture-and-rethrow.
- RecordingLogger.captureException now also honours the flag — test
  capture counts stay honest when both repo and outer layer wrap.

Tests:
- packages/core-shared/src/instrumentation/with-capture.test.ts —
  4 cases covering success, capture-on-throw, mark-on-capture, no-double
  via the flag.
- packages/blog/tests/r44-no-double-capture.test.ts — 3 cases: repo throw
  → 1 capture with repo tags; controller parse fail → 1 capture with
  controller tags; success → 0 captures.

Verification: pnpm test 26/26, pnpm lint 15/15, pnpm typecheck 14/14.

Docs: ADR-014 and the refactor log gain a "Post-merge follow-up" section
recording the gap, the fix, and the underlying lesson (don't describe
intent as shipped state — grep first).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-08 00:28:22 +02:00

8.9 KiB
Raw Blame History

Refactor Log — Instrumentation + Sentry Logging (Plan 10)

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 Branch: feature/instrumentation-sentry

Tasks

  • Task 1 — Scaffold refactor log + ADR-014 stub
  • Task 2 — Tracer interface + ISpan + AttributeValue + SpanOpts
  • Task 3 — NoopTracer
  • Task 4 — Logger interface + NoopLogger + Breadcrumb + CaptureContext
  • Task 5 — withSpan helper
  • Task 6 — Symbols + index barrel
  • Task 7 — SentryTracer adapter
  • Task 8 — SentryLogger adapter (with double-report guard)
  • Task 9 — pii-fields constants + scrub.beforeSend / scrub.beforeSendTransaction
  • Task 10 — init-server helper
  • Task 11 — init-client helper (browser-only)
  • Task 12 — bindNoopInstrumentation + bindSentryInstrumentation
  • Task 13 — apps/web-next bindAll() Rule 0 dispatcher
  • Task 14 — Tests for bindAll() orthogonality (R47)
  • Task 15 — RecordingTracer in core-testing
  • Task 16 — RecordingLogger in core-testing
  • Task 17 — vitest.setup.ts binds Noop by default
  • Task 18 — Blog feature wiring (pilot)
  • Task 19 — Auth feature wiring
  • Task 20 — Marketing-pages feature wiring
  • Task 21 — Navigation feature wiring
  • Task 22 — Media feature wiring
  • Task 23 — defineContractSuite expectSpan helper
  • Task 24 — Update repo contract suites to assert span shape
  • Task 25 — apps/web-next instrumentation files + scrubber test
  • Task 26 — apps/cms instrumentation files + scrubber test
  • Task 27 — apps/web-tanstack instrumentation files + scrubber test
  • Task 28 — ESLint boundary rule (R40) + CI grep gate (R31)
  • Task 29 — turbo.json globalEnv updates
  • Task 30 — Doc updates (CLAUDE.md, AGENTS.md, vertical-feature-spec.md)
  • Task 31 — Doc updates (tdd-workflow.md, testing-strategy.md, dependency-flow.md, core-shared/AGENTS.md)
  • Task 32 — HTML updates (data-flow-explainer §06, di-explainer §08)
  • Task 33 — ADR-014 final + refactor log final

Decisions deviated from spec

  • PII key-substring list extended (Task 25). Added "ipaddress" to PII_KEY_SUBSTRINGS so keys like ipAddress trigger full key-level redaction in addition to the existing IPv4/IPv6 string-pattern redaction. The plan's R38 PII-scrubber test expected result.extra.ipAddress === "[redacted]", which the original substring list (without "ipaddress") could not satisfy. Tighter posture than the spec; existing scrub.test.ts continues to pass.

  • vertical-feature-spec.md numbering (Task 30). The plan instructed to add the instrumentation section as "§10". The spec already had 15 sections, so the section was appended as §16 (rather than renumbering §10–§15) to avoid invalidating every cross-reference in the spec.

  • Web-tanstack vite.config.ts modifications skipped (Task 27, Step 6). The app is a placeholder — its package.json has "build": "echo 'placeholder — TanStack Start build configured in later plan'" and there's no vite.config.ts. The @sentry/vite-plugin dep is installed but dormant; it will be wired into the build when TanStack Start build is configured. A minimal src/vite-env.d.ts shims ImportMetaEnv for the client entry's import.meta.env reads.

  • data-flow-explainer.html section numbering (Task 32). The plan asked to renumber the verdict from §06 to §07 and slot the new tracing section in as §06 — done. The <style> block uses dark-paper variables (--paper, --ink-2, --rule) tuned to the existing palette rather than the plan's snippet which used --paper-tone, #f4ede3 (light-mode token). Same visual intent, correct theme.

  • di-explainer.html numbering (Task 32). The plan suggested numbering the new instrumentation section as "§ 0X" (placeholder) and "renumber subsequent sections accordingly". The page's existing sections went up to §07; appending the new instrumentation section as §08 required no renumbering and matches the plan's intent.

Notable surprises

  • @repo/core-shared was not a direct dep of apps/cms or apps/web-tanstack before Plan 10. Both apps had transitive access via feature packages. Adding the deep subpath imports @repo/core-shared/instrumentation/sentry/init-server (etc.) required adding @repo/core-shared as a direct workspace:* dependency of each app — pnpm's strict-peer mode wouldn't resolve the subpath through transitive paths.

  • Subpath exports needed for the dynamic-import pattern. Apps' instrumentation.ts uses await import("@repo/core-shared/instrumentation/sentry/init-server") to keep the Sentry code out of the cold-start boot path. The barrel re-export was sufficient for static imports but not for the dynamic deep paths — added explicit package.json#exports entries for each sentry/init-* and sentry/scrub file.

  • core-shared's peerDependenciesMeta.optional was needed for @sentry/node and @sentry/react so feature packages installing core-shared don't pull in the React/Node SDK transitively. The same packages stay in devDependencies of core-shared so its own typecheck and test runs resolve them locally.

  • Pre-existing lint debt blocked the Task 28 R40 rule from going green. Several issues unrelated to instrumentation surfaced once the rule was active: unused variables not covered by an argsIgnorePattern: "^_" config, process undefined in *.mjs config files (Node globals not declared), and stale eslint-disable directives in marketing-pages. These were each a one-line fix and were bundled into the Task 28 commit since they were the only thing blocking the lint gate.

  • core-testing/setup/no-sentry.test.ts was not in the original allowlist. The file imports @sentry/nextjs to assert that the mock is in place. Added **/setup/no-sentry.test.{ts,js} to the allowlist alongside the source file. Same for **/instrumentation/di/bind-sentry-instrumentation.test.{ts,js} — Task 12's test file lives in instrumentation/di/, not instrumentation/sentry/, so the **/instrumentation/sentry/** allowlist alone wouldn't catch it.

  • ESLint flat-config glob patterns are relative to the eslint command's cwd. When pnpm --filter <pkg> lint runs, ESLint sees file paths relative to the package root, not the repo root. The plan's allowlist patterns (packages/core-shared/src/instrumentation/sentry/**) wouldn't match. Switched to **/-prefixed patterns (**/instrumentation/sentry/**) which match regardless of where ESLint is invoked.

  • Vite/TanStack Start typing not yet present in web-tanstack. The client entry uses import.meta.env.VITE_*, but no Vite types are installed since the build infra is a placeholder. A small src/vite-env.d.ts declares the env shape for now; replace with /// <reference types="vite/client" /> when Vite is wired.

  • Spec deviations are usually a sign of a mis-aligned spec, not a wrong implementation. Three of the deviations above (ipaddress, vertical-feature-spec section number, di-explainer section number) are minor; the vite.config skip and vite-env shim are real architecture decisions worth recording in ADR-014 alongside the major decisions.

Post-merge follow-up — R44 capture leg was missing

After Plan 10 merged to main, the user pushed back on a description I gave of how capture works. A grep captureException packages/*/src showed zero call sites in any controller or use-case body — the R44 leg of "throw-site capture" had been documented but never implemented. Only repos (R43) actually called logger.captureException.

I had described intent as reality both during execution and in the ADR-014 capture-rules table. The lesson is now in feedback_verify_before_claiming.md: when describing what code does — especially in the same session as writing it — grep before asserting.

The fix (one commit on main):

  1. Extracted the __sentryReported flag helpers into core-shared/instrumentation/reported-flag.ts. SentryLogger switched to importing them; RecordingLogger got an inlined copy (boundary rule disallows tooling → core).
  2. Added withCapture(logger, tags, fn) at core-shared/instrumentation/with-capture.ts — parallel to withSpan. On throw: capture, mark, re-throw — but bail if the flag was already set.
  3. Wrapped every use case and controller in every feature's bind-production.ts and bind-dev-seed.ts with withSpan(withCapture(factory)). Span outermost so the errored span's timing reflects the capture-and-rethrow.
  4. RecordingLogger now also honours the flag, so test capture counts are honest across the chain.
  5. Added packages/blog/tests/r44-no-double-capture.test.ts proving: (a) repo-originated error → 1 capture with repo tags; (b) controller parse failure → 1 capture with controller tags; (c) success → 0 captures.

Verification: pnpm test 26/26, pnpm lint 15/15 (warnings only), pnpm typecheck 14/14.