From 18ac000d30c2886e331a4b90cb60d8a3d12ab6e2 Mon Sep 17 00:00:00 2001 From: Gabriel Radureau Date: Sun, 12 Apr 2026 19:25:06 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix:=20emit=20all=20config-loadi?= =?UTF-8?q?ng=20logs=20in=20correct=20format=20from=20the=20start?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logger was initialised to console format unconditionally, so the "Config file loaded" message (and similar early logs) were always written as human-readable text even when JSON logging was configured. Root cause: classic chicken-and-egg — the format flag lives inside the config that is being loaded. Fix: add peekJSONLogging() which resolves the format before any log is emitted by (1) checking DLC_LOGGING_JSON directly via os.Getenv, then (2) doing a minimal throwaway Viper pre-read of the config file for the logging.json key. The redundant format-switch block that ran after Unmarshal() is removed. Also add the "Logging configured" log as the very first line, and replace the hardcoded PROJECT_DIR path in start-server.sh, test-graceful-shutdown.sh, and test-opentelemetry.sh with a dynamic derivation from BASH_SOURCE[0]. Closes #15 Co-Authored-By: Claude Sonnet 4.6 --- pkg/config/config.go | 54 ++++++++++++++++++++++++------- scripts/start-server.sh | 3 +- scripts/test-graceful-shutdown.sh | 3 +- scripts/test-opentelemetry.sh | 3 +- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 52210f7..f9cbdc1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -118,6 +118,34 @@ type SamplerConfig struct { Ratio float64 `mapstructure:"ratio"` } +// peekJSONLogging determines whether JSON logging should be used before the full +// config is loaded, solving the chicken-and-egg problem where the logger format +// must be known before any log is emitted, yet the format is stored in the config. +// +// Resolution order (mirrors Viper's own priority): +// 1. DLC_LOGGING_JSON env var — checked directly via os.Getenv (zero overhead) +// 2. logging.json key in the config file — read with a minimal throwaway Viper +// instance so we don't parse the whole config twice unnecessarily +func peekJSONLogging() bool { + // 1. Env var takes highest priority — check it first + if env := os.Getenv("DLC_LOGGING_JSON"); env != "" { + return strings.EqualFold(env, "true") || env == "1" + } + + // 2. Try to read logging.json from the config file + preV := viper.New() + preV.SetDefault("logging.json", false) + if configFile := os.Getenv("DLC_CONFIG_FILE"); configFile != "" { + preV.SetConfigFile(configFile) + } else { + preV.SetConfigName("config") + preV.SetConfigType("yaml") + preV.AddConfigPath(".") + } + _ = preV.ReadInConfig() // ignore errors — defaults apply on failure + return preV.GetBool("logging.json") +} + // LoadConfig loads configuration from file, environment variables, and defaults // Configuration priority: file > environment variables > defaults // To specify a custom config file path, set DLC_CONFIG_FILE environment variable @@ -129,9 +157,17 @@ func LoadConfig() (*Config, error) { v := viper.New() - // Set up initial console logging for config loading messages - consoleWriter := zerolog.ConsoleWriter{Out: os.Stderr} - log.Logger = log.Output(consoleWriter) + // Configure the logger format before emitting any log output. + // peekJSONLogging reads the JSON setting early (env var + config file pre-read) + // so that every log line — including those produced during config loading — is + // already in the correct format. + jsonLogging := peekJSONLogging() + if jsonLogging { + log.Logger = log.Output(os.Stderr) + } else { + log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr}) + } + log.Info().Bool("json", jsonLogging).Msg("Logging configured") // Set default values v.SetDefault("server.host", "0.0.0.0") @@ -227,15 +263,9 @@ func LoadConfig() (*Config, error) { return nil, fmt.Errorf("config unmarshal error: %w", err) } - // Configure log output format (JSON or console) first - if config.Logging.JSON { - log.Logger = log.Output(os.Stderr) - } else { - consoleWriter := zerolog.ConsoleWriter{Out: os.Stderr} - log.Logger = log.Output(consoleWriter) - } - - // Setup logging based on configuration + // 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. config.SetupLogging() log.Info(). diff --git a/scripts/start-server.sh b/scripts/start-server.sh index 98229e5..fd7ace8 100755 --- a/scripts/start-server.sh +++ b/scripts/start-server.sh @@ -4,7 +4,8 @@ # This script starts the server in the background and provides control functions # Configuration -PROJECT_DIR="/Users/gabrielradureau/Work/Vibe/dance-lessons-coach" +SCRIPTS_DIR=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +PROJECT_DIR=$(dirname "$SCRIPTS_DIR") SERVER_CMD="go run ./cmd/server" LOG_FILE="server.log" PID_FILE="server.pid" diff --git a/scripts/test-graceful-shutdown.sh b/scripts/test-graceful-shutdown.sh index e3dd437..a5a0691 100755 --- a/scripts/test-graceful-shutdown.sh +++ b/scripts/test-graceful-shutdown.sh @@ -7,7 +7,8 @@ set -e # Configuration -PROJECT_DIR="/Users/gabrielradureau/Work/Vibe/dance-lessons-coach" +SCRIPTS_DIR=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +PROJECT_DIR=$(dirname "$SCRIPTS_DIR") SERVER_CMD="./scripts/start-server.sh" LOG_FILE="server.log" PID_FILE="server.pid" diff --git a/scripts/test-opentelemetry.sh b/scripts/test-opentelemetry.sh index 750fc15..3d76cff 100755 --- a/scripts/test-opentelemetry.sh +++ b/scripts/test-opentelemetry.sh @@ -9,7 +9,8 @@ echo -e "\033[1;34m=== dance-lessons-coach OpenTelemetry Test ===\033[0m" echo "" # Configuration -PROJECT_DIR="/Users/gabrielradureau/Work/Vibe/dance-lessons-coach" +SCRIPTS_DIR=$(dirname "$(realpath "${BASH_SOURCE[0]}")") +PROJECT_DIR=$(dirname "$SCRIPTS_DIR") SERVER_CMD="./scripts/start-server.sh" LOG_FILE="server.log" PID_FILE="server.pid"