Skip to content

Commit 7846d7c

Browse files
authored
fix(logger): add mapstructure tags to logging config structs (#2186)
* fix(logger): add mapstructure tags to logging config structs Viper uses mapstructure (not yaml/json tags) for unmarshaling config into Go structs. Without mapstructure tags, fields with underscore keys (file_output, default_level, module_levels) and renamed fields (modules → ModuleOutputs) silently stayed at zero values. applyConfigDefaults() then filled in defaults with empty Level fields, falling back to "info" and suppressing all debug logs. Also fixes a Viper default key mismatch in defaults.go where "max_backups" was used instead of "max_rotated_files". * fix(conf): update sample config to use max_rotated_files key The sample config.yaml still used the old `max_backups` key which doesn't match the `max_rotated_files` mapstructure tag, causing the value to be silently ignored by Viper. --------- Co-authored-by: tphakala <tphakala@users.noreply.github.com>
1 parent 87b0893 commit 7846d7c

File tree

5 files changed

+264
-29
lines changed

5 files changed

+264
-29
lines changed

internal/conf/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1305,7 +1305,7 @@ type Settings struct {
13051305
ValidationWarnings []string `yaml:"-" json:"validationWarnings,omitempty"` // Configuration validation warnings for telemetry
13061306

13071307
// Logging configuration
1308-
Logging logger.LoggingConfig `json:"logging"` // centralized logging configuration
1308+
Logging logger.LoggingConfig `json:"logging" mapstructure:"logging"` // centralized logging configuration
13091309

13101310
Main struct {
13111311
Name string `json:"name"` // name of BirdNET-Go node, can be used to identify source of notes

internal/conf/config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ logging:
2626
path: logs/birdnet.log # default log file path
2727
max_size: 100 # maximum size in MB before rotation
2828
max_age: 30 # maximum age in days to keep old logs
29-
max_backups: 10 # maximum number of old log files to keep
29+
max_rotated_files: 10 # maximum number of old log files to keep
3030
compress: true # compress rotated logs
3131
level: info # log level for file output
3232

internal/conf/defaults.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func setDefaultConfig() {
2121
viper.SetDefault("logging.file_output.level", "info")
2222
viper.SetDefault("logging.file_output.max_size", 100)
2323
viper.SetDefault("logging.file_output.max_age", 30)
24-
viper.SetDefault("logging.file_output.max_backups", 10)
24+
viper.SetDefault("logging.file_output.max_rotated_files", 10)
2525
viper.SetDefault("logging.file_output.compress", true)
2626

2727
// Per-module log files
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
package conf
2+
3+
import (
4+
"os"
5+
"strings"
6+
"testing"
7+
8+
"github.com/spf13/viper"
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"github.com/tphakala/birdnet-go/internal/logger"
12+
)
13+
14+
// TestViperUnmarshalLoggingConfig verifies that Viper correctly unmarshals
15+
// logging configuration including module outputs with underscore-separated
16+
// keys. This is a regression test for missing mapstructure tags that caused
17+
// module-level debug settings to be silently ignored.
18+
func TestViperUnmarshalLoggingConfig(t *testing.T) {
19+
t.Parallel()
20+
21+
configContent := `
22+
logging:
23+
default_level: debug
24+
debug_webhooks: true
25+
timezone: UTC
26+
console:
27+
enabled: true
28+
level: warn
29+
file_output:
30+
enabled: true
31+
path: logs/app.log
32+
level: debug
33+
max_size: 50
34+
max_age: 7
35+
max_rotated_files: 5
36+
compress: true
37+
modules:
38+
imageprovider:
39+
enabled: true
40+
file_path: logs/imageprovider.log
41+
level: debug
42+
console_also: true
43+
max_size: 25
44+
max_age: 14
45+
max_rotated_files: 3
46+
audio:
47+
enabled: true
48+
file_path: logs/audio.log
49+
level: trace
50+
module_levels:
51+
analysis: warn
52+
ebird: error
53+
`
54+
55+
v := viper.New()
56+
v.SetConfigType("yaml")
57+
58+
err := v.ReadConfig(strings.NewReader(configContent))
59+
require.NoError(t, err)
60+
61+
var settings Settings
62+
err = v.Unmarshal(&settings, viper.DecodeHook(DurationDecodeHook()))
63+
require.NoError(t, err)
64+
65+
cfg := settings.Logging
66+
67+
// Verify top-level fields
68+
assert.Equal(t, "debug", cfg.DefaultLevel, "DefaultLevel should be unmarshaled from default_level")
69+
assert.True(t, cfg.DebugWebhooks, "DebugWebhooks should be unmarshaled from debug_webhooks")
70+
assert.Equal(t, "UTC", cfg.Timezone)
71+
72+
// Verify console config
73+
require.NotNil(t, cfg.Console, "Console should not be nil")
74+
assert.True(t, cfg.Console.Enabled)
75+
assert.Equal(t, "warn", cfg.Console.Level)
76+
77+
// Verify file output config (underscore keys)
78+
require.NotNil(t, cfg.FileOutput, "FileOutput should not be nil (mapped from file_output)")
79+
assert.True(t, cfg.FileOutput.Enabled)
80+
assert.Equal(t, "logs/app.log", cfg.FileOutput.Path)
81+
assert.Equal(t, "debug", cfg.FileOutput.Level)
82+
assert.Equal(t, 50, cfg.FileOutput.MaxSize, "MaxSize should be unmarshaled from max_size")
83+
assert.Equal(t, 7, cfg.FileOutput.MaxAge, "MaxAge should be unmarshaled from max_age")
84+
assert.Equal(t, 5, cfg.FileOutput.MaxRotatedFiles, "MaxRotatedFiles should be unmarshaled from max_rotated_files")
85+
assert.True(t, cfg.FileOutput.Compress)
86+
87+
// Verify module outputs (the critical fix — "modules" maps to ModuleOutputs)
88+
require.NotNil(t, cfg.ModuleOutputs, "ModuleOutputs should not be nil (mapped from modules)")
89+
require.Contains(t, cfg.ModuleOutputs, "imageprovider")
90+
require.Contains(t, cfg.ModuleOutputs, "audio")
91+
92+
imgMod := cfg.ModuleOutputs["imageprovider"]
93+
assert.True(t, imgMod.Enabled)
94+
assert.Equal(t, "logs/imageprovider.log", imgMod.FilePath, "FilePath should be unmarshaled from file_path")
95+
assert.Equal(t, "debug", imgMod.Level, "Module level should be 'debug' — this was the root cause of missing debug logs")
96+
assert.True(t, imgMod.ConsoleAlso, "ConsoleAlso should be unmarshaled from console_also")
97+
assert.Equal(t, 25, imgMod.MaxSize)
98+
assert.Equal(t, 14, imgMod.MaxAge)
99+
assert.Equal(t, 3, imgMod.MaxRotatedFiles)
100+
101+
audioMod := cfg.ModuleOutputs["audio"]
102+
assert.True(t, audioMod.Enabled)
103+
assert.Equal(t, "logs/audio.log", audioMod.FilePath)
104+
assert.Equal(t, "trace", audioMod.Level)
105+
106+
// Verify module levels (separate from module outputs)
107+
require.NotNil(t, cfg.ModuleLevels, "ModuleLevels should not be nil (mapped from module_levels)")
108+
assert.Equal(t, "warn", cfg.ModuleLevels["analysis"])
109+
assert.Equal(t, "error", cfg.ModuleLevels["ebird"])
110+
}
111+
112+
// TestViperUnmarshalLoggingDefaults verifies that Viper SetDefault values
113+
// for logging modules are correctly unmarshaled with mapstructure tags.
114+
func TestViperUnmarshalLoggingDefaults(t *testing.T) {
115+
t.Parallel()
116+
117+
v := viper.New()
118+
119+
// Set defaults the same way setDefaultConfig does
120+
v.SetDefault("logging.default_level", "info")
121+
v.SetDefault("logging.console.enabled", true)
122+
v.SetDefault("logging.console.level", "info")
123+
v.SetDefault("logging.file_output.enabled", true)
124+
v.SetDefault("logging.file_output.path", "logs/application.log")
125+
v.SetDefault("logging.file_output.level", "info")
126+
v.SetDefault("logging.file_output.max_size", 100)
127+
v.SetDefault("logging.modules.imageprovider.enabled", true)
128+
v.SetDefault("logging.modules.imageprovider.file_path", "logs/imageprovider.log")
129+
v.SetDefault("logging.modules.imageprovider.level", "debug")
130+
v.SetDefault("logging.module_levels.analysis", "warn")
131+
132+
var settings Settings
133+
err := v.Unmarshal(&settings, viper.DecodeHook(DurationDecodeHook()))
134+
require.NoError(t, err)
135+
136+
cfg := settings.Logging
137+
138+
assert.Equal(t, "info", cfg.DefaultLevel)
139+
140+
require.NotNil(t, cfg.FileOutput)
141+
assert.Equal(t, "logs/application.log", cfg.FileOutput.Path)
142+
assert.Equal(t, 100, cfg.FileOutput.MaxSize)
143+
144+
require.NotNil(t, cfg.ModuleOutputs, "ModuleOutputs must be populated from logging.modules.* defaults")
145+
require.Contains(t, cfg.ModuleOutputs, "imageprovider")
146+
assert.Equal(t, "debug", cfg.ModuleOutputs["imageprovider"].Level,
147+
"Module level from defaults should be 'debug'")
148+
assert.Equal(t, "logs/imageprovider.log", cfg.ModuleOutputs["imageprovider"].FilePath)
149+
150+
require.NotNil(t, cfg.ModuleLevels, "ModuleLevels must be populated from logging.module_levels.* defaults")
151+
assert.Equal(t, "warn", cfg.ModuleLevels["analysis"])
152+
}
153+
154+
// TestLoggingModuleLevelAppliedToLogger verifies the end-to-end flow:
155+
// module level from config reaches the CentralLogger and produces a
156+
// logger that actually enables debug messages.
157+
func TestLoggingModuleLevelAppliedToLogger(t *testing.T) {
158+
t.Parallel()
159+
160+
logPath := t.TempDir() + "/test.log"
161+
cfg := &logger.LoggingConfig{
162+
DefaultLevel: "info",
163+
Timezone: "UTC",
164+
Console: &logger.ConsoleOutput{
165+
Enabled: false, // disable console to avoid noise
166+
},
167+
ModuleOutputs: map[string]logger.ModuleOutput{
168+
"testmod": {
169+
Enabled: true,
170+
FilePath: logPath,
171+
Level: "debug",
172+
},
173+
},
174+
}
175+
176+
cl, err := logger.NewCentralLogger(cfg)
177+
require.NoError(t, err)
178+
defer func() { require.NoError(t, cl.Close()) }()
179+
180+
log := cl.Module("testmod")
181+
require.NotNil(t, log)
182+
183+
// Debug should not be filtered — the module is set to debug level
184+
log.Debug("test debug message", logger.String("key", "value"))
185+
log.Info("test info message")
186+
187+
// Flush and read the log file to verify debug entries appear
188+
require.NoError(t, cl.Flush())
189+
190+
content, err := os.ReadFile(logPath)
191+
require.NoError(t, err)
192+
193+
assert.Contains(t, string(content), "DEBUG", "Log file should contain DEBUG entries when module level is debug")
194+
assert.Contains(t, string(content), "test debug message")
195+
assert.Contains(t, string(content), "INFO")
196+
assert.Contains(t, string(content), "test info message")
197+
}
198+
199+
// TestLoggingModuleLevelInfoFiltersDebug verifies that info-level modules
200+
// correctly filter out debug messages.
201+
func TestLoggingModuleLevelInfoFiltersDebug(t *testing.T) {
202+
t.Parallel()
203+
204+
logPath := t.TempDir() + "/test.log"
205+
cfg := &logger.LoggingConfig{
206+
DefaultLevel: "info",
207+
Timezone: "UTC",
208+
Console: &logger.ConsoleOutput{
209+
Enabled: false,
210+
},
211+
ModuleOutputs: map[string]logger.ModuleOutput{
212+
"testmod": {
213+
Enabled: true,
214+
FilePath: logPath,
215+
Level: "info",
216+
},
217+
},
218+
}
219+
220+
cl, err := logger.NewCentralLogger(cfg)
221+
require.NoError(t, err)
222+
defer func() { require.NoError(t, cl.Close()) }()
223+
224+
log := cl.Module("testmod")
225+
log.Debug("should not appear")
226+
log.Info("should appear")
227+
228+
require.NoError(t, cl.Flush())
229+
230+
content, err := os.ReadFile(logPath)
231+
require.NoError(t, err)
232+
233+
assert.NotContains(t, string(content), "should not appear", "Debug messages should be filtered at info level")
234+
assert.Contains(t, string(content), "should appear")
235+
}

internal/logger/config.go

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,49 @@ package logger
22

33
// LoggingConfig represents logging configuration
44
type LoggingConfig struct {
5-
Level string `yaml:"level" json:"level"` // debug, info, warn, error (deprecated, use DefaultLevel)
6-
Timezone string `yaml:"timezone" json:"timezone"` // "Local", "UTC", or IANA timezone name like "Europe/Helsinki"
7-
File string `yaml:"file" json:"file"` // optional log file path (deprecated, use FileOutput)
8-
DebugWebhooks bool `yaml:"debug_webhooks" json:"debug_webhooks"` // if true, logs full webhook details (headers, body, etc.)
9-
DefaultLevel string `yaml:"default_level" json:"default_level"` // default log level for all modules
10-
Console *ConsoleOutput `yaml:"console" json:"console"` // console output configuration
11-
FileOutput *FileOutput `yaml:"file_output" json:"file_output"` // file output configuration
12-
ModuleOutputs map[string]ModuleOutput `yaml:"modules" json:"modules"` // per-module output configuration
13-
ModuleLevels map[string]string `yaml:"module_levels" json:"module_levels"` // per-module log levels
5+
Level string `yaml:"level" json:"level" mapstructure:"level"` // debug, info, warn, error (deprecated, use DefaultLevel)
6+
Timezone string `yaml:"timezone" json:"timezone" mapstructure:"timezone"` // "Local", "UTC", or IANA timezone name like "Europe/Helsinki"
7+
File string `yaml:"file" json:"file" mapstructure:"file"` // optional log file path (deprecated, use FileOutput)
8+
DebugWebhooks bool `yaml:"debug_webhooks" json:"debug_webhooks" mapstructure:"debug_webhooks"` // if true, logs full webhook details (headers, body, etc.)
9+
DefaultLevel string `yaml:"default_level" json:"default_level" mapstructure:"default_level"` // default log level for all modules
10+
Console *ConsoleOutput `yaml:"console" json:"console" mapstructure:"console"` // console output configuration
11+
FileOutput *FileOutput `yaml:"file_output" json:"file_output" mapstructure:"file_output"` // file output configuration
12+
ModuleOutputs map[string]ModuleOutput `yaml:"modules" json:"modules" mapstructure:"modules"` // per-module output configuration
13+
ModuleLevels map[string]string `yaml:"module_levels" json:"module_levels" mapstructure:"module_levels"` // per-module log levels
1414
}
1515

1616
// ConsoleOutput represents console logging configuration.
1717
// Console output uses human-readable text format without timestamps.
1818
// Timestamps are omitted following the Twelve-Factor App methodology;
1919
// the execution environment (journald, Docker) adds them automatically.
2020
type ConsoleOutput struct {
21-
Enabled bool `yaml:"enabled" json:"enabled"` // enable console output
22-
Level string `yaml:"level" json:"level"` // log level for console output
21+
Enabled bool `yaml:"enabled" json:"enabled" mapstructure:"enabled"` // enable console output
22+
Level string `yaml:"level" json:"level" mapstructure:"level"` // log level for console output
2323
}
2424

2525
// FileOutput represents file logging configuration.
2626
// File output uses JSON format with RFC3339 timestamps for machine parsing
2727
// and log aggregation systems (ELK, Loki, Splunk, etc.).
2828
type FileOutput struct {
29-
Enabled bool `yaml:"enabled" json:"enabled"` // enable file output
30-
Path string `yaml:"path" json:"path"` // log file path
31-
MaxSize int `yaml:"max_size" json:"max_size"` // maximum size in MB before rotation (0 = disabled)
32-
MaxAge int `yaml:"max_age" json:"max_age"` // maximum age in days to keep rotated logs (0 = no limit)
33-
MaxRotatedFiles int `yaml:"max_rotated_files" json:"max_rotated_files"` // maximum number of rotated log files to keep (0 = no limit)
34-
Compress bool `yaml:"compress" json:"compress"` // compress rotated logs with gzip
35-
Level string `yaml:"level" json:"level"` // log level for file output
29+
Enabled bool `yaml:"enabled" json:"enabled" mapstructure:"enabled"` // enable file output
30+
Path string `yaml:"path" json:"path" mapstructure:"path"` // log file path
31+
MaxSize int `yaml:"max_size" json:"max_size" mapstructure:"max_size"` // maximum size in MB before rotation (0 = disabled)
32+
MaxAge int `yaml:"max_age" json:"max_age" mapstructure:"max_age"` // maximum age in days to keep rotated logs (0 = no limit)
33+
MaxRotatedFiles int `yaml:"max_rotated_files" json:"max_rotated_files" mapstructure:"max_rotated_files"` // maximum number of rotated log files to keep (0 = no limit)
34+
Compress bool `yaml:"compress" json:"compress" mapstructure:"compress"` // compress rotated logs with gzip
35+
Level string `yaml:"level" json:"level" mapstructure:"level"` // log level for file output
3636
}
3737

3838
// ModuleOutput represents per-module output configuration
3939
type ModuleOutput struct {
40-
Enabled bool `yaml:"enabled" json:"enabled"` // enable module-specific output
41-
FilePath string `yaml:"file_path" json:"file_path"` // dedicated file path for this module
42-
Level string `yaml:"level" json:"level"` // log level override for this module
43-
ConsoleAlso bool `yaml:"console_also" json:"console_also"` // also log to console
44-
MaxSize int `yaml:"max_size" json:"max_size"` // maximum size in MB before rotation (0 = use FileOutput default)
45-
MaxAge int `yaml:"max_age" json:"max_age"` // maximum age in days (0 = use FileOutput default)
46-
MaxRotatedFiles int `yaml:"max_rotated_files" json:"max_rotated_files"` // maximum number of rotated files (0 = use FileOutput default)
47-
Compress *bool `yaml:"compress,omitempty" json:"compress,omitempty"` // compress rotated logs (nil = use FileOutput default)
40+
Enabled bool `yaml:"enabled" json:"enabled" mapstructure:"enabled"` // enable module-specific output
41+
FilePath string `yaml:"file_path" json:"file_path" mapstructure:"file_path"` // dedicated file path for this module
42+
Level string `yaml:"level" json:"level" mapstructure:"level"` // log level override for this module
43+
ConsoleAlso bool `yaml:"console_also" json:"console_also" mapstructure:"console_also"` // also log to console
44+
MaxSize int `yaml:"max_size" json:"max_size" mapstructure:"max_size"` // maximum size in MB before rotation (0 = use FileOutput default)
45+
MaxAge int `yaml:"max_age" json:"max_age" mapstructure:"max_age"` // maximum age in days (0 = use FileOutput default)
46+
MaxRotatedFiles int `yaml:"max_rotated_files" json:"max_rotated_files" mapstructure:"max_rotated_files"` // maximum number of rotated files (0 = use FileOutput default)
47+
Compress *bool `yaml:"compress,omitempty" json:"compress,omitempty" mapstructure:"compress"` // compress rotated logs (nil = use FileOutput default)
4848
}
4949

5050
// Default values for logging configuration.

0 commit comments

Comments
 (0)