From 2d5b5834ec4d43ab1e7844f47f0f0f07d50aa8af Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Tue, 5 May 2026 08:44:44 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20feat(config):=20hot-reload=20Phase?= =?UTF-8?q?=201=20=E2=80=94=20logging.level=20(ADR-0023)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the first phase of ADR-0023 selective hot-reloading. Adds viper.WatchConfig wiring + an OnConfigChange handler that re-unmarshals the Config struct on file changes and applies the hot-reloadable subset. Phase 1 reloadable field: logging.level — re-applied via SetupLogging on every change. The remaining 3 fields listed in ADR-0023 (api.v2_enabled, telemetry sampler type/ratio, auth.jwt.ttl) follow the same pattern and will land in subsequent phase PRs without further infrastructure work. Changes: - pkg/config/config.go : Config struct gets unexported viper + reloadMu fields; new WatchAndApply(ctx) method starts the watcher and stops it on context cancel. Defensive: no-op when no config file is in use. - pkg/server/server.go Run() : calls WatchAndApply(rootCtx) so the watcher stops on graceful shutdown. - pkg/config/config_hot_reload_test.go (new) : 3 unit tests covering end-to-end reload, no-config-file no-op, nil-viper no-op. Race detector clean. - adr/0023-config-hot-reloading.md : Status → Phase 1 Implemented; remaining fields explicitly Proposed for follow-up phases. Verifier verdict: APPROVE. Race detector passes. Full BDD suite still green. The @flaky scenario in features/config/config_hot_reloading.feature remains @flaky for now — activating it requires reliable cross-process file-watching behaviour which is sensitive to filesystem semantics on the CI runner; the unit test exercises the same code path deterministically. --- adr/0023-config-hot-reloading.md | 3 +- go.mod | 2 +- pkg/config/config.go | 65 ++++++++++++++++++++++ pkg/config/config_hot_reload_test.go | 83 ++++++++++++++++++++++++++++ pkg/server/server.go | 4 ++ 5 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 pkg/config/config_hot_reload_test.go diff --git a/adr/0023-config-hot-reloading.md b/adr/0023-config-hot-reloading.md index aa9a2d0..2440445 100644 --- a/adr/0023-config-hot-reloading.md +++ b/adr/0023-config-hot-reloading.md @@ -1,8 +1,9 @@ # Config Hot Reloading Strategy -**Status:** Proposed +**Status:** Phase 1 Implemented (2026-05-05 — `logging.level` hot-reloadable via `Config.WatchAndApply` in `pkg/config/config.go`, wired in `pkg/server/server.go Run`. Remaining fields — `api.v2_enabled`, telemetry sampler, `auth.jwt.ttl` — Proposed for follow-up phases following the same pattern.) **Authors:** Gabriel Radureau, AI Agent **Date:** 2026-04-05 +**Last Updated:** 2026-05-05 ## Context and Problem Statement diff --git a/go.mod b/go.mod index c85af0c..3da2dcf 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.1 require ( github.com/cucumber/godog v0.15.1 + github.com/fsnotify/fsnotify v1.9.0 github.com/go-chi/chi/v5 v5.2.5 github.com/go-playground/locales v0.14.1 github.com/go-playground/universal-translator v0.18.1 @@ -37,7 +38,6 @@ require ( github.com/cucumber/messages/go/v21 v21.0.1 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/gabriel-vasile/mimetype v1.4.13 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect diff --git a/pkg/config/config.go b/pkg/config/config.go index 0db2f11..a5ae6d9 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,11 +1,14 @@ package config import ( + "context" "fmt" "os" "strings" + "sync" "time" + "github.com/fsnotify/fsnotify" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/viper" @@ -29,6 +32,15 @@ type Config struct { Database DatabaseConfig `mapstructure:"database"` RateLimit RateLimitConfig `mapstructure:"rate_limit"` Cache CacheConfig `mapstructure:"cache"` + + // viper is the underlying configuration source. Kept (unexported, + // mapstructure:"-") so hot-reload can re-unmarshal on file changes — + // see WatchAndApply (ADR-0023 selective hot-reload). + viper *viper.Viper `mapstructure:"-"` + + // reloadMu serialises Unmarshal during hot-reload so a partial mutation + // can't be observed mid-flight by getter calls. + reloadMu sync.RWMutex `mapstructure:"-"` } // ServerConfig holds server-related configuration @@ -299,6 +311,9 @@ func LoadConfig() (*Config, error) { return nil, fmt.Errorf("config unmarshal error: %w", err) } + // Keep the viper instance for hot-reload (ADR-0023). + config.viper = v + // Setup logging based on configuration (level, output file, time format). // The JSON/console format was already applied at the top of LoadConfig via // peekJSONLogging, so SetupLogging only needs to handle the remaining knobs. @@ -590,3 +605,53 @@ func (c *Config) setupLogOutput() { log.Logger = log.Output(file) log.Trace().Str("output", output).Msg("Logging to file") } + +// WatchAndApply starts watching the config file for changes and applies the +// hot-reloadable subset on every change (ADR-0023 selective hot-reload). +// +// Phase 1 (this PR) reloads: +// - logging.level — re-applies via SetupLogging on every change. +// +// The other fields listed in ADR-0023 (api.v2_enabled, telemetry sampler, +// auth.jwt.ttl) remain restart-only until their handlers land in a follow-up. +// +// Stops when ctx is cancelled. Safe to call once at server startup. +// If the config file is absent (ConfigFileNotFoundError at load time), this +// becomes a no-op and logs a single warning. +func (c *Config) WatchAndApply(ctx context.Context) { + if c.viper == nil { + log.Warn().Msg("Config hot-reload disabled: no viper instance attached") + return + } + if c.viper.ConfigFileUsed() == "" { + log.Info().Msg("Config hot-reload disabled: no config file in use (env-only or defaults)") + return + } + + c.viper.OnConfigChange(func(in fsnotify.Event) { + log.Info().Str("event", in.Op.String()).Str("file", in.Name).Msg("Config file changed, reloading hot-reloadable fields") + c.reloadMu.Lock() + defer c.reloadMu.Unlock() + + if err := c.viper.Unmarshal(c); err != nil { + log.Error().Err(err).Msg("Hot-reload: failed to unmarshal new config, keeping previous values") + return + } + + // Apply hot-reloadable fields. Order matters: logging first so the + // rest of the reload is logged at the right level. + c.SetupLogging() + log.Info().Str("logging_level", c.GetLogLevel()).Msg("Hot-reload applied (logging.level)") + }) + c.viper.WatchConfig() + + log.Info().Str("file", c.viper.ConfigFileUsed()).Msg("Config hot-reload watcher started (ADR-0023 Phase 1)") + + // Stop the watcher on context cancel — viper has no public Stop method, + // so we just clear the callback to make further events no-ops. + go func() { + <-ctx.Done() + c.viper.OnConfigChange(func(_ fsnotify.Event) {}) + log.Info().Msg("Config hot-reload watcher stopped") + }() +} diff --git a/pkg/config/config_hot_reload_test.go b/pkg/config/config_hot_reload_test.go new file mode 100644 index 0000000..0e829a7 --- /dev/null +++ b/pkg/config/config_hot_reload_test.go @@ -0,0 +1,83 @@ +package config + +import ( + "context" + "os" + "path/filepath" + "testing" + "time" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// loadFromFile is a helper that mimics LoadConfig() for a specific file path +// without going through the env-prefix and singleton machinery — keeps the +// test hermetic. +func loadFromFile(t *testing.T, path string) *Config { + t.Helper() + v := viper.New() + v.SetConfigFile(path) + v.SetConfigType("yaml") + v.SetDefault("logging.level", "info") + require.NoError(t, v.ReadInConfig()) + + c := &Config{viper: v} + require.NoError(t, v.Unmarshal(c)) + return c +} + +// TestWatchAndApply_LoggingLevel proves the hot-reload pipe end-to-end: +// write a new logging.level to the watched file, the OnConfigChange handler +// re-unmarshals, and the in-memory Config reflects the new value. +func TestWatchAndApply_LoggingLevel(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(path, []byte("logging:\n level: info\n"), 0644)) + + c := loadFromFile(t, path) + assert.Equal(t, "info", c.GetLogLevel()) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c.WatchAndApply(ctx) + + // Mutate the file. fsnotify needs a real write event; rewrite atomically. + require.NoError(t, os.WriteFile(path, []byte("logging:\n level: debug\n"), 0644)) + + // Poll for up to 2s waiting for the in-memory level to flip. + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + c.reloadMu.RLock() + level := c.GetLogLevel() + c.reloadMu.RUnlock() + if level == "debug" { + return + } + time.Sleep(20 * time.Millisecond) + } + c.reloadMu.RLock() + defer c.reloadMu.RUnlock() + t.Fatalf("logging level did not hot-reload to debug: still %q", c.GetLogLevel()) +} + +// TestWatchAndApply_NoFileNoOp confirms the watcher is a safe no-op when no +// config file is in use (env-only / defaults) — important so production +// containers without a mounted config.yaml don't crash. +func TestWatchAndApply_NoFileNoOp(t *testing.T) { + c := &Config{viper: viper.New()} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c.WatchAndApply(ctx) // should return without panicking +} + +// TestWatchAndApply_NilViperNoOp confirms the watcher tolerates a Config +// constructed without the viper field (e.g. tests that build a Config{} +// manually — same defensive code path as production but exercised explicitly). +func TestWatchAndApply_NilViperNoOp(t *testing.T) { + c := &Config{} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + c.WatchAndApply(ctx) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 2cdbe60..1282c88 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -708,6 +708,10 @@ func (s *Server) Run() error { s.userService.StartJWTSecretCleanupLoop(rootCtx, s.config.GetJWTSecretCleanupInterval()) } + // Start config hot-reload watcher (ADR-0023 Phase 1: logging.level only). + // Stops automatically on rootCtx cancellation. + s.config.WatchAndApply(rootCtx) + // Create HTTP server log.Trace().Str("address", s.config.GetServerAddress()).Msg("Server running")