From 560419217c4e7597ae93f94253179029b071c36f Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sat, 25 Jul 2026 00:59:28 +0900 Subject: [PATCH] fix: config warnings honor logging.format/level (#113) parse() emitted deprecation and unknown-tier warnings through the default text handler before setupLogging() installed the operator-configured one, so they ignored logging.format/level and broke JSON log parsers. Buffer log records during config load and replay them through the real handler once it is installed, filtering by the configured level. Keeps the config package untouched; startup ordering is fixed in main. Fixes #113 --- .changes/unreleased/Fixed-113.yaml | 4 +++ cmd/devcloud/buffer.go | 43 ++++++++++++++++++++++++++ cmd/devcloud/buffer_test.go | 49 ++++++++++++++++++++++++++++++ cmd/devcloud/main.go | 11 +++++++ 4 files changed, 107 insertions(+) create mode 100644 .changes/unreleased/Fixed-113.yaml create mode 100644 cmd/devcloud/buffer.go create mode 100644 cmd/devcloud/buffer_test.go diff --git a/.changes/unreleased/Fixed-113.yaml b/.changes/unreleased/Fixed-113.yaml new file mode 100644 index 0000000..8973b11 --- /dev/null +++ b/.changes/unreleased/Fixed-113.yaml @@ -0,0 +1,4 @@ +kind: Fixed +body: Config-time warnings (deprecated `dashboard` key, unknown `DEVCLOUD_SERVICES` tier) now honor `logging.format`/`logging.level` instead of always printing as plain text before the logger is configured +custom: + Issue: "113" diff --git a/cmd/devcloud/buffer.go b/cmd/devcloud/buffer.go new file mode 100644 index 0000000..2399c32 --- /dev/null +++ b/cmd/devcloud/buffer.go @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "log/slog" +) + +// bufferHandler is a slog.Handler that captures every record instead of +// emitting it, so log calls made before the operator-configured handler is +// installed (notably config.parse() warnings) can be replayed afterwards and +// honor logging.format / logging.level. See flushTo. +// +// ponytail: single-threaded startup, add a mutex if buffered logging ever +// goes concurrent. +type bufferHandler struct { + records []slog.Record +} + +// Enabled buffers all levels; the real handler filters on replay in flushTo. +func (h *bufferHandler) Enabled(context.Context, slog.Level) bool { return true } + +func (h *bufferHandler) Handle(_ context.Context, r slog.Record) error { + h.records = append(h.records, r.Clone()) + return nil +} + +// WithAttrs/WithGroup return the handler unchanged: config-time logging uses +// plain slog.Warn/Info without attr or group chaining, so nothing is lost. +func (h *bufferHandler) WithAttrs([]slog.Attr) slog.Handler { return h } +func (h *bufferHandler) WithGroup(string) slog.Handler { return h } + +// flushTo replays buffered records through dst, honoring dst's level filter, +// then clears the buffer. +func (h *bufferHandler) flushTo(dst slog.Handler) { + for _, r := range h.records { + if dst.Enabled(context.Background(), r.Level) { + _ = dst.Handle(context.Background(), r) + } + } + h.records = nil +} diff --git a/cmd/devcloud/buffer_test.go b/cmd/devcloud/buffer_test.go new file mode 100644 index 0000000..ee5e036 --- /dev/null +++ b/cmd/devcloud/buffer_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "context" + "log/slog" + "strings" + "testing" +) + +// TestBufferHandler_FlushHonorsFormatAndLevel verifies that records buffered +// before the real handler is installed are replayed as JSON and filtered by +// the destination handler's level — the two acceptance criteria from #113. +func TestBufferHandler_FlushHonorsFormatAndLevel(t *testing.T) { + buf := &bufferHandler{} + log := slog.New(buf) + log.Info("config file not found") + log.Warn("dashboard key deprecated", "token", "tierX") + + var out bytes.Buffer + dst := slog.NewJSONHandler(&out, &slog.HandlerOptions{Level: slog.LevelWarn}) + buf.flushTo(dst) + + got := out.String() + if strings.Contains(got, "config file not found") { + t.Errorf("info record should be dropped at level=warn, got: %s", got) + } + if !strings.Contains(got, `"dashboard key deprecated"`) || !strings.Contains(got, `"token":"tierX"`) { + t.Errorf("warn record (with attrs) should be replayed as JSON, got: %s", got) + } + + // flush clears the buffer so a second flush is a no-op. + out.Reset() + buf.flushTo(dst) + if out.Len() != 0 { + t.Errorf("second flush should emit nothing, got: %s", out.String()) + } +} + +// TestBufferHandler_EnabledBuffersAllLevels confirms buffering does not drop +// records below the eventual threshold; the level filter applies only on flush. +func TestBufferHandler_EnabledBuffersAllLevels(t *testing.T) { + buf := &bufferHandler{} + if !buf.Enabled(context.Background(), slog.LevelDebug) { + t.Fatal("bufferHandler must buffer all levels, including debug") + } +} diff --git a/cmd/devcloud/main.go b/cmd/devcloud/main.go index 3031d0e..da866d3 100644 --- a/cmd/devcloud/main.go +++ b/cmd/devcloud/main.go @@ -32,6 +32,12 @@ func main() { cfgPath := flag.String("config", "", "Path to config file (optional; uses ./devcloud.yaml if present, else embedded defaults)") flag.Parse() + // Buffer log records emitted while loading config (parse() deprecation and + // unknown-tier warnings, etc.) so they can be replayed through the + // operator-configured handler below and honor logging.format / level. + buf := &bufferHandler{} + slog.SetDefault(slog.New(buf)) + var ( cfg *config.Config err error @@ -44,11 +50,16 @@ func main() { cfg, err = config.LoadOrDefault("devcloud.yaml") } if err != nil { + // Load failed, so there is no logging config to honor; fall back to the + // default handler, flush any buffered warnings, then surface the error. + setupLogging(config.LoggingConfig{}) + buf.flushTo(slog.Default().Handler()) slog.Error("failed to load config", "error", err) os.Exit(1) } setupLogging(cfg.Logging) + buf.flushTo(slog.Default().Handler()) registry := plugin.DefaultRegistry