Decisions

ADR-034: Legacy `log.Printf` sites retrofit to `slog.Default`, not an injected logger

`pkg/observ` standardised gyrum Go services on `log/slog`, JSON output, the `service`/`version`/`commit`/`release_track` ambient tags, and the layered redactor. A handful of older middleware in…

#034

ADR-034: Legacy log.Printf sites retrofit to slog.Default, not an injected logger

Status: Accepted Date: 2026-04-21

Context

pkg/observ standardised gyrum Go services on log/slog, JSON output, the service/version/commit/release_track ambient tags, and the layered redactor. A handful of older middleware in gyrum-go/pkg/middleware still use log.Printf directly — recovery today writes log.Printf("PANIC: %v\n%s", rec, debug.Stack()), which produces a free-form text line that the dashboard scrapers have no way to filter by level, route, or panic message. The line is not redacted (it short- circuits the pkg/observ replace-attr hook), and it leaks the full stack in plain text regardless of operator log level.

Phase 2c of the rollout is the moment to fix this — the DB wrapper arrives at the same time, both land on the same VERSION bump, and the observability-standards guide is about to go "GA" across all gyrum services. The question is how.

Two approaches compete:

  1. Constructor injection. Change every retrofitted middleware to take a *slog.Logger parameter: middleware.Recovery(log). Callers pass observ.Logger() at wire time; tests pass a buffered handler. Clean DI, fully decoupled.

  2. slog.Default(). The middleware calls slog.ErrorContext(...) directly. The process-wide default logger is set once at startup (or left as the stdlib default in tests); no parameter to thread; callers do not change their wiring.

Constructor injection is the "textbook" answer, and it's what pkg/observ.Access() actually does internally via observ.FromContext (request-scoped logger). It's also what the SOLID preachers would say is "correct".

But constructor injection has two specific costs in the retrofit direction:

  • Every consumer has to change. middleware.Recovery() is called from dozens of service main.go files across the gyrum fleet. A signature change ripples through every one, including downstream repos that are not synced to every gyrum-go release on the same cadence. The retrofit becomes an all-at-once coordination problem, and if any repo misses the sweep, it fails to compile against the new library version.
  • The retrofit stops being a retrofit. The point of retrofitting log.Printf is "lines that currently land on stderr unstructured should start landing on stderr structured instead, without breaking anyone's wiring". A signature change breaks everyone's wiring.

Conversely, slog.Default already has an established contract in gyrum code: pkg/respond.InternalError calls slog.Error(...) directly and documents the behaviour in its godoc — services that want their internal-error logs routed via observ.Logger() call slog.SetDefault(observ.Logger()) once at startup. Every other slog-default-logging site in the library follows the same contract by default.

Decision

Retrofit pkg/middleware/recovery.go (and future legacy log sites) to call slog.ErrorContext/slog.InfoContext directly. Do not introduce a logger parameter on any existing middleware constructor. Document in each retrofitted function's godoc that the service is expected to call slog.SetDefault(observ.Logger()) once at startup — same convention as pkg/respond.

The retrofitted Recovery() middleware emits:

msg=http.panic.recovered level=ERROR
  panic=<fmt.Sprintf("%v", rec)>
  stack=<debug.Stack() as string>
  method=<r.Method>
  path=<r.URL.Path>

The ctx from r.Context() is passed through so request-scoped attributes (request_id, route) attached by observ.Access upstream flow onto the panic line automatically when the service uses both middlewares together.

Composition with observ.Access: only one of the two should be the terminal panic handler per chain. The default is observ.Access (which also recovers panics). Services that need to chain the two — for example, to run middleware.Recovery as an outer safety net around non-HTTP code that sometimes delegates to HTTP handlers — set OBSERV_ACCESS_RERAISE=1, and place middleware.Recovery OUTSIDE observ.Access in the chain. ADR-016 is the source of truth; this ADR simply notes that the retrofitted middleware.Recovery is now slog-compatible with observ.Access's structured output.

Consequences

  • Zero consumer churn. Every service currently calling middleware.Recovery() keeps compiling. The log line shape changes from free-form text to structured JSON — operators see a better dashboard; developers see no breakage.
  • The slog.SetDefault(observ.Logger()) one-liner becomes an enforced convention. It is documented in pkg/observ/README.md, in the retrofitted middleware's godoc, and in the observability-standards guide. A service that skips it still gets structured text via stdlib log/slog's default handler — better than log.Printf was — just without the redactor and the ambient identity tags.
  • Tests use the documented slog.SetDefault(...) / t.Cleanup / slog.SetDefault(prev) pattern. Same pattern as pkg/respond/respond_test.go::captureSlog. The middleware test added alongside this retrofit uses exactly this shape, so the library has one precedent and one idiom.
  • Third-party services that already route slog.Default to a non-observ handler keep working. Their panic logs just land through their handler instead of observ.Logger(). This is the right failure mode — a service that has deliberately configured a non-default handler has opted in to its own shape, and the library does not second-guess that.
  • The door stays open for a logger parameter later. If a future middleware genuinely needs a non-default logger (per-chain custom enrichment, for instance), adding a variadic option (middleware.Recovery(WithLogger(l))) is additive and breaks no one. We prefer this deferred option over pre-emptively paying coordination cost now.

Alternatives considered

  • Add a logger-accepting RecoveryWithLogger(l) alongside the existing Recovery(). Rejected — a second entry point for the same behaviour is a maintenance tax with no concrete consumer asking for it. Every service currently calls Recovery(); adding an optional second shape is a future refactor, not a retrofit.
  • Phase in via a deprecation cycle — ship Recovery() that still uses log.Printf for two minor versions, then flip. Rejected — the log.Printf output is actively harmful (unredacted stacks, unfilterable in dashboards) and a two-version grace adds no safety that a structured-but-compatible retrofit does not already provide.
  • Route through observ.FromContext so the middleware picks up any request-scoped logger attached upstream. Considered, and partially adopted: the ErrorContext call path carries ctx so the slog handler can read request-scoped attributes that observ.Access attached earlier. But resolving a full *slog.Logger from context would re-introduce the pkg/observ dependency into pkg/middleware, which is currently one-way (obs has no middleware import, middleware has no obs import). The current design preserves the zero-dep direction.

References

  • Source: pkg/middleware/recovery.go
  • Precedent: pkg/respond/respond.go uses the same slog.Default convention and documents it.
  • ADR-016 — HTTP access middleware composition, including the OBSERV_ACCESS_RERAISE rule.
  • ADR-006 — Error taxonomy; the panic log is categorised under internal by observ.Access, but the retrofitted Recovery does not set err_category directly because its consumer might pair it with its own error-enrichment middleware.