From 2ed3eaac9e0195c84b022938d7dbc957f8b2943f Mon Sep 17 00:00:00 2001 From: djmil Date: Wed, 3 Jun 2026 19:31:20 +0000 Subject: [PATCH] pkg/logger: io.Writer injection, Flush(), -log-dump flag Co-Authored-By: Claude Sonnet 4.6 --- cmd/app/config.go | 10 +++---- cmd/app/main.go | 13 +++++---- pkg/logger/logger.go | 59 ++++++++++++++++++--------------------- pkg/logger/logger_test.go | 2 +- pkg/result/result.go | 11 ++++++++ 5 files changed, 52 insertions(+), 43 deletions(-) diff --git a/cmd/app/config.go b/cmd/app/config.go index 945f307..7360c89 100644 --- a/cmd/app/config.go +++ b/cmd/app/config.go @@ -19,8 +19,8 @@ type AppConfig struct { // LoggerConfig controls logging behavior. type LoggerConfig struct { - Level string // debug | info | warn | error - DebugFile string // non-empty enables debug mode: writes full JSON trace to this path + Level string // debug | info | warn | error + LogDump string // non-empty enables debug mode: writes full JSON trace to this path } // Greeter config for internal/greeter/Service. @@ -42,7 +42,7 @@ func parseArgs() *Config { port := flag.Int("port", 8080, "listen port") env := flag.String("env", "dev", "environment: dev | staging | prod") level := flag.String("log-level", "info", "log level: debug | info | warn | error") - debugLog := flag.String("debug-log", "", "write full debug trace to file (enables debug mode)") + debugLog := flag.String("log-dump", "", "write full debug trace to file (enables debug mode)") flag.Parse() @@ -52,8 +52,8 @@ func parseArgs() *Config { Env: *env, }, Logger: LoggerConfig{ - Level: *level, - DebugFile: *debugLog, + Level: *level, + LogDump: *debugLog, }, Greeter: GreeterConfig{ Name: *name, diff --git a/cmd/app/main.go b/cmd/app/main.go index 114b98c..f6aa4aa 100644 --- a/cmd/app/main.go +++ b/cmd/app/main.go @@ -6,6 +6,7 @@ package main import ( "fmt" + "io" "os" "path/filepath" @@ -22,9 +23,13 @@ type app struct { } func newApp(cfg *Config) *app { - // NewCLI auto-detects: human text on a terminal, JSON when piped. - // Pass -debug-log FILE to enable debug mode (writes full trace to file). - log := logger.NewCLI(cfg.Logger.Level, cfg.Logger.DebugFile).Expect("create logger") + // Open the debug writer if requested. The OS closes it on exit. + var debugOut io.Writer + if cfg.Logger.LogDump != "" { + debugOut = result.Of(os.Create(cfg.Logger.LogDump)).Expect("enable logs dump") // #nosec G304 — CLI flag + } + + log := logger.NewCLI(cfg.Logger.Level, debugOut).Expect("create logger") log.Debug("config", "port", cfg.App.Port, "level", cfg.Logger.Level, "env", cfg.App.Env) if cfg.App.Env == "dev" { log.Warn("dev mode — not for production") @@ -55,8 +60,6 @@ func main() { } func (a *app) run() { - defer a.log.Close() - // Warm up the greeter with a few names before the real call. // In debug mode this produces repeated identical debug lines from the greeter, // demonstrating how the deduplication counter collapses them in-place. diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 6ede106..5bb180e 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -11,26 +11,27 @@ // Two constructors write to stderr: // // - New(level) — JSON, 12-factor compatible; use for headless services. -// - NewCLI(level, debugFile) — auto-detects: human text on a terminal, JSON when piped. -// Providing a non-empty debugFile enables debug mode: DEBUG writes to that -// file as JSON; the screen always shows INFO and above. +// - NewCLI(level, debugOut) — auto-detects: human text on a terminal, JSON when piped. +// Passing a non-nil debugOut enables debug mode: screen shows debug messages +// and the writer receives a full JSON trace. Any io.Writer is accepted — +// a file, a bytes.Buffer, os.Stdout, a network connection. The caller owns +// the writer and is responsible for closing it. // -// Typical use in a CLI application: +// Typical CLI use: // -// log := logger.NewCLI("info", "").Expect("create logger") -// log.Info("server started", "port", 8080) +// // Normal mode: +// log := logger.NewCLI("info", nil).Expect("create logger") // -// // child logger with request-scoped fields: -// req := log.WithField("request_id", rid) -// req.Info("start") -// req.Info("end") +// // Debug mode — full trace to a file: +// f, err := os.Create(path) // caller controls the file; #nosec G304 if path is a CLI flag +// if err != nil { ... } +// defer f.Close() +// log := logger.NewCLI("info", f).Expect("create logger") // // In tests that need to assert on log content, use NewWriter with a buffer: // // var buf bytes.Buffer // log := logger.NewWriter(&buf, "debug").Expect("create logger") -// // ... exercise code ... -// // assert on buf.String() package logger import ( @@ -63,18 +64,16 @@ func New(level string) result.Expect[*Logger] { // NewCLI creates a logger for programs invoked by a human operator. // // When stderr is a terminal it operates in one of two modes: -// - Normal (debugFile == ""): human-readable text, INFO and above on screen. -// - Debug (debugFile != ""): same screen output, plus a full JSON trace of -// DEBUG and above written to debugFile for post-run investigation. +// - Normal (debugOut == nil): human-readable text, INFO and above on screen. +// - Debug (debugOut != nil): same screen output, plus full JSON trace written +// to debugOut. level="debug" also activates debug mode without a writer. // // When stderr is not a terminal (piped or redirected), NewCLI behaves like -// New: JSON at the given level to stderr; debugFile is ignored. -// -// Passing a level above "info" when running in a terminal is unusual — -// NewCLI will log a warning because interactive mode always shows INFO and above. +// New: JSON at the given level to stderr; debugOut is ignored. // +// The caller owns debugOut and is responsible for closing it when done. // Valid levels: debug, info, warn, error. -func NewCLI(level, debugFile string) result.Expect[*Logger] { +func NewCLI(level string, debugOut io.Writer) result.Expect[*Logger] { lvl := parseLevel(level) if lvl.Err() != nil { return result.Errw[*Logger](lvl.Err(), "parse log level") @@ -85,20 +84,16 @@ func NewCLI(level, debugFile string) result.Expect[*Logger] { return result.Ok(&Logger{slog.New(h)}) } - // debugFile activates debug mode regardless of level; level="debug" also activates it. + // debugOut or level="debug" activates debug mode. screenLevel := slog.LevelInfo - if debugFile != "" || lvl.Value() <= slog.LevelDebug { + if debugOut != nil || lvl.Value() <= slog.LevelDebug { screenLevel = slog.LevelDebug } screen := newHumanHandler(os.Stderr, screenLevel) var h slog.Handler = screen - if debugFile != "" { - f, err := os.Create(debugFile) - if err != nil { - return result.Errw[*Logger](err, "create debug log") - } - dump := slog.NewJSONHandler(f, &slog.HandlerOptions{Level: slog.LevelDebug}) + if debugOut != nil { + dump := slog.NewJSONHandler(debugOut, &slog.HandlerOptions{Level: slog.LevelDebug}) h = multiHandler{screen, dump} } @@ -124,10 +119,10 @@ func NewWriter(w io.Writer, level string) result.Expect[*Logger] { return result.Ok(&Logger{slog.New(h)}) } -// Close flushes any pending debug output. Call defer log.Close() at the entry -// point so a debug line that was never followed by an INFO/WARN/ERROR record -// still gets its trailing newline before the process exits. -func (l *Logger) Close() { +// Flush writes any pending debug output. Only needed if a debug loop is the +// very last operation before exit with no subsequent INFO/WARN/ERROR record. +// Most programs never need this — an INFO at the end commits the line naturally. +func (l *Logger) Flush() { type flusher interface{ flush() } if f, ok := l.Handler().(flusher); ok { f.flush() diff --git a/pkg/logger/logger_test.go b/pkg/logger/logger_test.go index 2deae81..3ca6e2d 100644 --- a/pkg/logger/logger_test.go +++ b/pkg/logger/logger_test.go @@ -22,7 +22,7 @@ func TestNewCLI(t *testing.T) { for _, tc := range tests { t.Run(tc.level, func(t *testing.T) { // In tests stderr is not a terminal — NewCLI uses JSON path. - r := logger.NewCLI(tc.level, "") + r := logger.NewCLI(tc.level, nil) if tc.wantErr { testutil.ResultErr(t, r) return diff --git a/pkg/result/result.go b/pkg/result/result.go index 36c5876..716a7eb 100644 --- a/pkg/result/result.go +++ b/pkg/result/result.go @@ -119,7 +119,18 @@ func (r Expect[T]) Unwrap() (T, error) { // [Run] call as a normal Go error. A stack trace is captured at this call site // when [CaptureStack] is true. // +// msg should express intent — what the code was trying to accomplish — not +// the mechanism. This produces error messages that read as "intent: cause" +// rather than "operation: cause", keeping failure context meaningful to the +// reader without leaking implementation details. +// +// // Good — expresses intent: // data := Parse(raw).Expect("parse user input") +// log := logger.NewCLI(level, out).Expect("create logger") +// +// // Avoid — describes the mechanism, not the goal: +// data := Parse(raw).Expect("call Parse()") +// log := logger.NewCLI(level, out).Expect("call NewCLI") func (r Expect[T]) Expect(msg string) T { if r.err != nil { exitGoroutine(&stackError{