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:
Constructor injection. Change every retrofitted middleware to take a
*slog.Loggerparameter:middleware.Recovery(log). Callers passobserv.Logger()at wire time; tests pass a buffered handler. Clean DI, fully decoupled.slog.Default(). The middleware callsslog.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 servicemain.gofiles 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.Printfis "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 inpkg/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 stdliblog/slog's default handler — better thanlog.Printfwas — just without the redactor and the ambient identity tags. - Tests use the documented
slog.SetDefault(...)/t.Cleanup/slog.SetDefault(prev)pattern. Same pattern aspkg/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.Defaultto a non-observ handler keep working. Their panic logs just land through their handler instead ofobserv.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 existingRecovery(). Rejected — a second entry point for the same behaviour is a maintenance tax with no concrete consumer asking for it. Every service currently callsRecovery(); adding an optional second shape is a future refactor, not a retrofit. - Phase in via a deprecation cycle — ship
Recovery()that still useslog.Printffor two minor versions, then flip. Rejected — thelog.Printfoutput 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.FromContextso the middleware picks up any request-scoped logger attached upstream. Considered, and partially adopted: theErrorContextcall path carriesctxso the slog handler can read request-scoped attributes thatobserv.Accessattached earlier. But resolving a full*slog.Loggerfrom context would re-introduce thepkg/observdependency intopkg/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.gouses the sameslog.Defaultconvention and documents it. - ADR-016 — HTTP access middleware composition, including the
OBSERV_ACCESS_RERAISErule. - ADR-006 — Error taxonomy; the panic log is categorised under
internalbyobserv.Access, but the retrofittedRecoverydoes not seterr_categorydirectly because its consumer might pair it with its own error-enrichment middleware.