diff --git a/Readme.md b/Readme.md index 2c0a05f..9f78c46 100644 --- a/Readme.md +++ b/Readme.md @@ -33,6 +33,7 @@ Loggers: `logger.NewFmtLogger`, `logger.NewSlogLogger` (Go 1.21+), and the adapt | `tinyconf.Subscribe(ctx, m, onChange) error` | calls `onChange(prev, next *T)` after the changes of the config or the sub config `T` | | `tinyconf.Current[T](m) *T` | returns the current version of the config or the sub config `T` | | `ReloadStatus() ReloadStatus` | state of watching and reloading for the health checks: the last error, stopped drivers | +| `SetLogger(l Logger)` | replaces the logger, safe for concurrent use; with `WithDeferredLogger()` the records of parsing before it are written to the new logger | `Parse` parses all fields even if some of them have errors, all errors are returned as one error that can be checked with `errors.Is` and `errors.As`. diff --git a/docs/features/logging.md b/docs/features/logging.md index ead4042..82e062b 100644 --- a/docs/features/logging.md +++ b/docs/features/logging.md @@ -61,6 +61,39 @@ type Config struct { Add the tag to the parent, `yaml:"http"`, to read the value from `http.host`. +## Logger from the config + +The level, the format or the output of the logger are often a part of the config, but the config is parsed by the manager that already needs the logger. `WithDeferredLogger` breaks this cycle: the manager keeps the log records in memory, and `SetLogger` writes them to the logger created from the parsed config. + +```go +config, _ := tinyconf.New( + tinyconf.WithDriver(tagDriver), + tinyconf.WithDriver(yamlDriver), + tinyconf.WithDeferredLogger(), // the records are kept until SetLogger +) +conf := &Config{} +_ = config.Register(conf) +err := config.Parse(ctx, conf) + +log := newLogger(conf.Logging) // the logger of the application from its config +config.SetLogger(logger.NewSlogLogger(log)) // the kept records are written to it +if err != nil { + log.Error("config is not parsed", "error", err) + os.Exit(1) +} + +// the logger follows the changes of the config without restart +_ = tinyconf.Subscribe(ctx, config, func(_, next *Logging) { + config.SetLogger(logger.NewSlogLogger(newLogger(*next))) +}) +``` + +* The records of the first `Parse` get to the logger of the application in its format, its level filters them. +* `SetLogger` is safe for concurrent use with `Parse`, reloads and other goroutines. The records written during `SetLogger` wait for the kept ones, so nothing is lost and the order is kept. +* Up to 1000 records are kept, the oldest are dropped, and the logger gets a warning with the count of the dropped records. +* `SetLogger(nil)` disables logging, the kept records are dropped. +* `SetLogger` replaces the logger set by `WithLogger` too, without `WithDeferredLogger` nothing is kept. + ## Bundled loggers ```go diff --git a/docs/features/manager.md b/docs/features/manager.md index e211143..794700e 100644 --- a/docs/features/manager.md +++ b/docs/features/manager.md @@ -17,6 +17,7 @@ config, err := tinyconf.New( |---|---| | `WithDriver(driver)` | adds the driver, can be used many times | | `WithLogger(logger)` | sets the logger, see [Logging](logging.md) | +| `WithDeferredLogger()` | keeps the log records until the logger is set by `SetLogger`, see [Logger from the config](logging.md#logger-from-the-config) | ## Priority of drivers @@ -67,6 +68,7 @@ Every request of the drivers is limited to 30 seconds anyway. The reloads use th | `tinyconf.Subscribe(ctx, m, onChange)` | calls `onChange` after the changes of the config or the sub config, see [Reloading](reloading.md) | | `tinyconf.Current[T](m)` | returns the current version of the config or the sub config, see [Reloading](reloading.md) | | `ReloadStatus()` | state of watching and reloading for the health checks, see [Health checks](reloading.md#health-checks) | +| `SetLogger(logger)` | replaces the logger, safe for concurrent use, see [Logger from the config](logging.md#logger-from-the-config) | ## Errors diff --git a/example/config.yaml b/example/config.yaml index 3b3c86d..d272c26 100644 --- a/example/config.yaml +++ b/example/config.yaml @@ -4,5 +4,6 @@ http: database: host: db.internal logging: - level: info + # the logger is created from this config, debug shows the history of every field + level: error hosts: [a.example.com] diff --git a/example/main.go b/example/main.go index 2dda21c..746ae8c 100644 --- a/example/main.go +++ b/example/main.go @@ -9,6 +9,7 @@ package main import ( "context" "fmt" + "log/slog" "os" "os/signal" "time" @@ -59,9 +60,19 @@ func must[T any](value T, err error) T { return value } +// newLogger creates the logger of the application from its config. +func newLogger(conf Logging) tinyconf.Logger { + var level slog.Level + if err := level.UnmarshalText([]byte(conf.Level)); err != nil { + level = slog.LevelInfo + } + return logger.NewSlogLogger(slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: level}))) +} + func main() { config := must(tinyconf.New( - tinyconf.WithLogger(logger.NewFmtLogger(logger.ERROR)), + // the logger is configured by the config itself, the records of parsing are kept until SetLogger + tinyconf.WithDeferredLogger(), // the next driver overrides the values of the previous one tinyconf.WithDriver(must(tag.New("initial"))), tinyconf.WithDriver(must(yaml.New("config.yaml", yaml.WithOptional()))), @@ -78,6 +89,7 @@ func main() { parseCtx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() err := config.Parse(parseCtx, conf) + config.SetLogger(newLogger(conf.Logging)) if conf.Doc != "" { fmt.Print(config.GenDoc(conf.Doc)) return @@ -99,6 +111,7 @@ func main() { })) must(0, tinyconf.Subscribe(ctx, config, func(prev, next *Logging) { fmt.Printf("log level is changed: %s -> %s\n", prev.Level, next.Level) + config.SetLogger(newLogger(*next)) })) fmt.Println("change config.yaml to see the changes, stop by Ctrl+C") <-ctx.Done() diff --git a/example_slog_test.go b/example_slog_test.go new file mode 100644 index 0000000..6b4f24f --- /dev/null +++ b/example_slog_test.go @@ -0,0 +1,65 @@ +//go:build go1.21 + +package tinyconf_test + +import ( + "context" + "fmt" + "log/slog" + "os" + + "github.com/insei/tinyconf/v2" + "github.com/insei/tinyconf/v2/drivers/tag" + "github.com/insei/tinyconf/v2/logger" +) + +type Logging struct { + Level string `initial:"debug" doc:"log level: debug, info, warn, error"` +} + +type AppConfig struct { + Logging Logging + Port int `initial:"8080"` +} + +// newLogger creates the logger of the application from its config, the time is removed for the stable output. +func newLogger(conf Logging) *slog.Logger { + var level slog.Level + _ = level.UnmarshalText([]byte(conf.Level)) + return slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ + Level: level, + ReplaceAttr: func(groups []string, attr slog.Attr) slog.Attr { + if attr.Key == slog.TimeKey { + return slog.Attr{} + } + return attr + }, + })) +} + +// The logger of the application is configured by the config itself: the records of the parsing are kept +// until the logger is created from the parsed config. +func ExampleWithDeferredLogger() { + config := must(tinyconf.New( + tinyconf.WithDriver(must(tag.New("initial"))), + tinyconf.WithDeferredLogger(), + )) + conf := &AppConfig{} + if err := config.Register(conf); err != nil { + panic(err) + } + err := config.Parse(context.Background(), conf) + + // the kept records are written to the logger, the next records are written directly + config.SetLogger(logger.NewSlogLogger(newLogger(conf.Logging))) + if err != nil { + panic(err) + } + fmt.Println("port:", conf.Port) + // Output: + // level=DEBUG msg=initial config=*tinyconf_test.AppConfig field=Logging.Level value="" + // level=DEBUG msg=initial config=*tinyconf_test.AppConfig field=Port value=0 + // level=DEBUG msg=override config=*tinyconf_test.AppConfig driver=tag field=Logging.Level key=initial value=debug + // level=DEBUG msg=override config=*tinyconf_test.AppConfig driver=tag field=Port key=initial value=8080 + // port: 8080 +} diff --git a/logger.go b/logger.go new file mode 100644 index 0000000..268e9a7 --- /dev/null +++ b/logger.go @@ -0,0 +1,145 @@ +package tinyconf + +import ( + "sync" + "sync/atomic" +) + +// deferredLimit is the maximum number of the records kept by the deferred logger, the oldest records are dropped. +const deferredLimit = 1000 + +// loggerBox stores the logger of the Manager in atomic.Pointer. +type loggerBox struct { + Logger +} + +// managerLogger is the logger of the Manager, it is replaced by SetLogger concurrently with the parsing and reloads. +type managerLogger struct { + mu sync.Mutex + box atomic.Pointer[loggerBox] +} + +var noop Logger = &noopLogger{} + +// logger returns the current logger, noop logger if it is not set. +func (c *Manager) logger() Logger { + if box := c.log.box.Load(); box != nil { + return box.Logger + } + return noop +} + +func (c *Manager) storeLogger(l Logger) { + c.log.box.Store(&loggerBox{Logger: l}) +} + +// SetLogger replaces the logger of the Manager, nil disables logging. +// It is safe for concurrent use: Parse, reloads and other goroutines switch to the new logger. +// The records kept since WithDeferredLogger are written to the logger by the first call, +// so the application can create its logger from the parsed config and get the logs of this parsing too. +func (c *Manager) SetLogger(l Logger) { + if l == nil { + l = noop + } + c.log.mu.Lock() + defer c.log.mu.Unlock() + if deferred, ok := c.logger().(*deferredLogger); ok { + deferred.buf.flush(c, l) + return + } + c.storeLogger(l) +} + +type logLevel uint8 + +const ( + debugLevel logLevel = iota + infoLevel + warnLevel + errorLevel +) + +type logRecord struct { + level logLevel + msg string + with []Field + fields []Field +} + +func (r logRecord) writeTo(l Logger) { + if len(r.with) > 0 { + l = l.With(r.with...) + } + switch r.level { + case debugLevel: + l.Debug(r.msg, r.fields...) + case infoLevel: + l.Info(r.msg, r.fields...) + case warnLevel: + l.Warn(r.msg, r.fields...) + default: + l.Error(r.msg, r.fields...) + } +} + +// deferredBuffer keeps the records of the deferred logger until the logger is set by Manager.SetLogger. +type deferredBuffer struct { + mu sync.Mutex + records []logRecord + dropped int + // manager is set by the flush, the next records are written to the current logger of the manager + manager *Manager +} + +func (b *deferredBuffer) write(record logRecord) { + b.mu.Lock() + if b.manager != nil { + b.mu.Unlock() + // the logger was taken before SetLogger, for example by Parse running concurrently + record.writeTo(b.manager.logger()) + return + } + defer b.mu.Unlock() + if len(b.records) == deferredLimit { + b.records[0] = logRecord{} + b.records = b.records[1:] + b.dropped++ + } + b.records = append(b.records, record) +} + +// flush sets the logger of the manager and writes the kept records to it. +// The records written concurrently wait for the end of the flush, so they are written after the kept ones. +func (b *deferredBuffer) flush(c *Manager, l Logger) { + b.mu.Lock() + defer b.mu.Unlock() + c.storeLogger(l) + if b.dropped > 0 { + l.Warn("log records are dropped before the logger is set", LogField("count", b.dropped)) + } + for _, record := range b.records { + record.writeTo(l) + } + b.records, b.dropped, b.manager = nil, 0, c +} + +// deferredLogger keeps the records in memory until the logger is set by Manager.SetLogger, see WithDeferredLogger. +type deferredLogger struct { + buf *deferredBuffer + with []Field +} + +func (l *deferredLogger) log(level logLevel, msg string, fields []Field) { + l.buf.write(logRecord{level: level, msg: msg, with: l.with, fields: append([]Field(nil), fields...)}) +} + +func (l *deferredLogger) Debug(msg string, fields ...Field) { l.log(debugLevel, msg, fields) } +func (l *deferredLogger) Info(msg string, fields ...Field) { l.log(infoLevel, msg, fields) } +func (l *deferredLogger) Warn(msg string, fields ...Field) { l.log(warnLevel, msg, fields) } +func (l *deferredLogger) Error(msg string, fields ...Field) { l.log(errorLevel, msg, fields) } + +func (l *deferredLogger) With(fields ...Field) Logger { + with := make([]Field, 0, len(l.with)+len(fields)) + with = append(append(with, l.with...), fields...) + return &deferredLogger{buf: l.buf, with: with} +} diff --git a/logger_test.go b/logger_test.go new file mode 100644 index 0000000..7578b78 --- /dev/null +++ b/logger_test.go @@ -0,0 +1,222 @@ +package tinyconf + +import ( + "context" + "fmt" + "strings" + "sync" + "testing" + + "github.com/insei/fmap/v3" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// levelLogger records the messages of all levels with the fields, it is safe for concurrent use. +type levelLogger struct { + fields []Field + mu *sync.Mutex + records *[]string +} + +func newLevelLogger() *levelLogger { + return &levelLogger{mu: &sync.Mutex{}, records: &[]string{}} +} + +func (l *levelLogger) record(level, msg string, fields []Field) { + var record strings.Builder + record.WriteString(level + " " + msg + ":") + for _, field := range append(append([]Field{}, l.fields...), fields...) { + record.WriteString(fmt.Sprintf(" %s=%v", field.Key, field.Value)) + } + l.mu.Lock() + *l.records = append(*l.records, record.String()) + l.mu.Unlock() +} + +func (l *levelLogger) Debug(msg string, fields ...Field) { l.record("debug", msg, fields) } +func (l *levelLogger) Info(msg string, fields ...Field) { l.record("info", msg, fields) } +func (l *levelLogger) Warn(msg string, fields ...Field) { l.record("warn", msg, fields) } +func (l *levelLogger) Error(msg string, fields ...Field) { l.record("error", msg, fields) } + +func (l *levelLogger) With(fields ...Field) Logger { + return &levelLogger{fields: append(append([]Field{}, l.fields...), fields...), mu: l.mu, records: l.records} +} + +func (l *levelLogger) get() []string { + l.mu.Lock() + defer l.mu.Unlock() + return append([]string(nil), *l.records...) +} + +func TestWithDeferredLogger_Parse(t *testing.T) { + type Config struct { + Port int + Host string + } + parse := func(opts ...Option) *Manager { + driver := &pathMockDriver{values: map[string]any{"Port": 9090}, errs: map[string]error{"Host": ErrIncorrectTagSettings}} + c, err := New(append([]Option{WithDriver(driver)}, opts...)...) + require.NoError(t, err) + conf := &Config{} + require.NoError(t, c.Register(conf)) + require.NoError(t, c.Parse(context.Background(), conf)) + return c + } + + // the records of the parsing before SetLogger are the same as with the logger set from the start + expected := newLevelLogger() + parse(WithLogger(expected)) + require.NotEmpty(t, expected.get()) + + deferred := newLevelLogger() + c := parse(WithDeferredLogger()) + assert.Empty(t, deferred.get()) + c.SetLogger(deferred) + assert.Equal(t, expected.get(), deferred.get()) + assert.Same(t, deferred, c.logger()) +} + +func TestDeferredLogger_LevelsAndFields(t *testing.T) { + c, err := New(WithDeferredLogger()) + require.NoError(t, err) + log := c.logger() + parent := log.With(LogField("a", 1)) + child := parent.With(LogField("b", 2)) + fields := []Field{LogField("c", 3)} + log.Debug("debug", fields...) + parent.Info("info", fields...) + child.Warn("warn", fields...) + child.Error("error") + // the fields are copied, the caller can reuse the slice + fields[0] = LogField("changed", 0) + + l := newLevelLogger() + c.SetLogger(l) + assert.Equal(t, []string{ + "debug debug: c=3", + "info info: a=1 c=3", + "warn warn: a=1 b=2 c=3", + "error error: a=1 b=2", + }, l.get()) +} + +func TestDeferredLogger_Limit(t *testing.T) { + c, err := New(WithDeferredLogger()) + require.NoError(t, err) + for i := 0; i < deferredLimit+5; i++ { + c.logger().Info(fmt.Sprint(i)) + } + l := newLevelLogger() + c.SetLogger(l) + records := l.get() + require.Len(t, records, deferredLimit+1) + assert.Equal(t, "warn log records are dropped before the logger is set: count=5", records[0]) + assert.Equal(t, "info 5:", records[1]) + assert.Equal(t, fmt.Sprintf("info %d:", deferredLimit+4), records[deferredLimit]) +} + +func TestDeferredLogger_AfterSetLogger(t *testing.T) { + c, err := New(WithDeferredLogger()) + require.NoError(t, err) + // the logger is taken before SetLogger, like by Parse running concurrently + log := c.logger().With(LogField("field", "Port")) + + first := newLevelLogger() + c.SetLogger(first) + log.Info("late") + assert.Equal(t, []string{"info late: field=Port"}, first.get()) + + // the records go to the current logger of the manager + second := newLevelLogger() + c.SetLogger(second) + log.Info("later") + assert.Equal(t, []string{"info later: field=Port"}, second.get()) + assert.Len(t, first.get(), 1) +} + +func TestManager_SetLogger(t *testing.T) { + c, err := New(WithLogger(newLevelLogger())) + require.NoError(t, err) + l := newLevelLogger() + c.SetLogger(l) + assert.Same(t, l, c.logger()) + + c.SetLogger(nil) + assert.Same(t, noop, c.logger()) + + // nil drops the kept records and disables logging + c, err = New(WithDeferredLogger()) + require.NoError(t, err) + c.logger().Info("dropped") + c.SetLogger(nil) + assert.Same(t, noop, c.logger()) +} + +func TestWithLogger_Order(t *testing.T) { + l := newLevelLogger() + c, err := New(WithDeferredLogger(), WithLogger(l)) + require.NoError(t, err) + assert.Same(t, l, c.logger()) + + c, err = New(WithLogger(l), WithDeferredLogger()) + require.NoError(t, err) + assert.IsType(t, &deferredLogger{}, c.logger()) +} + +func TestManager_SetLogger_Concurrent(t *testing.T) { + type Config struct { + Port int + } + c, err := New(WithDriver(&concurrentDriver{}), WithDeferredLogger()) + require.NoError(t, err) + conf := &Config{} + require.NoError(t, c.Register(conf)) + + // less than deferredLimit with the records of Parse, so nothing is dropped + const writers, records = 4, 100 + var wg sync.WaitGroup + start := make(chan struct{}) + for i := 0; i < writers; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + for j := 0; j < records; j++ { + c.logger().With(LogField("writer", i)).Info("record") + } + }(i) + } + wg.Add(1) + go func() { + defer wg.Done() + <-start + for i := 0; i < 10; i++ { + assert.NoError(t, c.Parse(context.Background(), &Config{})) + } + }() + + l := newLevelLogger() + close(start) + c.SetLogger(l) + c.SetLogger(l) + wg.Wait() + + // no record is lost: kept ones are written by SetLogger, the next ones are written directly + count := 0 + for _, record := range l.get() { + if strings.HasPrefix(record, "info record:") { + count++ + } + } + assert.Equal(t, writers*records, count) +} + +// concurrentDriver returns the value for every field, it is safe for concurrent use. +type concurrentDriver struct{} + +func (concurrentDriver) GenDoc(...*Registered) string { return "" } +func (concurrentDriver) GetName() string { return "concurrent" } +func (concurrentDriver) GetValue(fmap.Field) (*Value, error) { + return &Value{Source: "concurrent", Value: 1}, nil +} diff --git a/option.go b/option.go index 6145e27..42ebc8b 100644 --- a/option.go +++ b/option.go @@ -30,10 +30,26 @@ type loggerOption struct { func (o loggerOption) apply(config *Manager) { if o.logger != nil { - config.log = o.logger + config.storeLogger(o.logger) } } +// WithLogger sets the logger of the Manager, the logger can be replaced later by Manager.SetLogger. func WithLogger(logger Logger) Option { return loggerOption{logger: logger} } + +type deferredLoggerOption struct{} + +func (deferredLoggerOption) apply(config *Manager) { + config.storeLogger(&deferredLogger{buf: &deferredBuffer{}}) +} + +// WithDeferredLogger keeps the log records in memory until the logger is set by Manager.SetLogger, +// then they are written to it. It solves the case of the logger configured by the config itself: +// the config is parsed before the logger exists, and the logs of this parsing are not lost. +// Up to 1000 records are kept, the oldest are dropped. +// The last of WithLogger and WithDeferredLogger is used. +func WithDeferredLogger() Option { + return deferredLoggerOption{} +} diff --git a/option_test.go b/option_test.go index 48714a2..11a6632 100644 --- a/option_test.go +++ b/option_test.go @@ -113,8 +113,11 @@ func TestLoggerOption_apply(t *testing.T) { opt.apply(manager) - if manager.log != tt.expected { - t.Errorf("Expected: %v, but got: %v", tt.expected, manager.log) + if tt.expected == nil { + tt.expected = noop + } + if manager.logger() != tt.expected { + t.Errorf("Expected: %v, but got: %v", tt.expected, manager.logger()) } }) } diff --git a/parser.go b/parser.go index 2e63e86..379e00c 100644 --- a/parser.go +++ b/parser.go @@ -20,7 +20,7 @@ type Registered struct { type Manager struct { drivers []Driver - log Logger + log managerLogger registered map[reflect.Type]*Registered mu sync.RWMutex @@ -266,7 +266,7 @@ func (c *Manager) parse(ctx context.Context, conf any, reload bool) ([]FieldSour if !reload { for _, path := range paths { field := register.Storage.MustFind(path) - c.log.Debug("initial", + c.logger().Debug("initial", LogField("config", confTypeOf.String()), LogField("field", path), LogField("value", getLoggerValue(field, convert.GetFieldValue(field, confParse)))) @@ -285,7 +285,7 @@ func (c *Manager) parse(ctx context.Context, conf any, reload bool) ([]FieldSour err = loader.Load() } if err != nil { - c.log.Error(errorMessage(err, "failed to load"), LogField("driver", d.GetName()), LogField("details", err.Error())) + c.logger().Error(errorMessage(err, "failed to load"), LogField("driver", d.GetName()), LogField("details", err.Error())) errs = append(errs, fmt.Errorf("%s driver: %w", d.GetName(), err)) continue } @@ -294,7 +294,7 @@ func (c *Manager) parse(ctx context.Context, conf any, reload bool) ([]FieldSour for _, d := range drivers { for _, path := range paths { field := register.Storage.MustFind(path) - log := c.log.With( + log := c.logger().With( LogField("config", confTypeOf.String()), LogField("driver", d.GetName()), LogField("field", path)) @@ -424,7 +424,7 @@ func (c *Manager) logChanges(storage fmap.Storage, prev, next any, prefix string fields = append(fields, LogField("key", source.Source)) } fields = append(fields, LogField("value", getLoggerValue(field, value))) - c.log.Debug("override", fields...) + c.logger().Debug("override", fields...) } } @@ -449,7 +449,7 @@ func (c *Manager) GenDoc(driverName string) string { } func New(opts ...Option) (*Manager, error) { - m := &Manager{log: &noopLogger{}, registered: map[reflect.Type]*Registered{}} + m := &Manager{registered: map[reflect.Type]*Registered{}} count := countDrivers(opts...) if count > 0 { m.drivers = make([]Driver, 0, count) @@ -461,6 +461,8 @@ func New(opts ...Option) (*Manager, error) { opt.apply(m) case loggerOption: opt.apply(m) + case deferredLoggerOption: + opt.apply(m) } } return m, nil diff --git a/parser_test.go b/parser_test.go index 228f3dc..1ae1ed6 100644 --- a/parser_test.go +++ b/parser_test.go @@ -216,14 +216,15 @@ func TestNew(t *testing.T) { name string opts []Option expected *Manager + logger Logger }{ { name: "Empty Options", opts: []Option{}, expected: &Manager{ - log: &noopLogger{}, registered: map[reflect.Type]*Registered{}, }, + logger: noop, }, { name: "With Driver Option", @@ -231,12 +232,12 @@ func TestNew(t *testing.T) { WithDriver(&mockDriver{}), }, expected: &Manager{ - log: &noopLogger{}, registered: map[reflect.Type]*Registered{}, drivers: []Driver{ &mockDriver{}, }, }, + logger: noop, }, { name: "With Logger Option", @@ -244,9 +245,9 @@ func TestNew(t *testing.T) { WithLogger(&mockLogger{}), }, expected: &Manager{ - log: &mockLogger{}, registered: map[reflect.Type]*Registered{}, }, + logger: &mockLogger{}, }, { name: "With Multiple Different Options", @@ -255,12 +256,12 @@ func TestNew(t *testing.T) { WithLogger(&mockLogger{}), }, expected: &Manager{ - log: &mockLogger{}, registered: map[reflect.Type]*Registered{}, drivers: []Driver{ &mockDriver{}, }, }, + logger: &mockLogger{}, }, { name: "With Multiple Identical Options", @@ -269,13 +270,13 @@ func TestNew(t *testing.T) { WithDriver(&mockDriver{}), }, expected: &Manager{ - log: &noopLogger{}, registered: map[reflect.Type]*Registered{}, drivers: []Driver{ &mockDriver{}, &mockDriver{}, }, }, + logger: noop, }, } @@ -283,7 +284,9 @@ func TestNew(t *testing.T) { t.Run(test.name, func(t *testing.T) { result, err := New(test.opts...) assert.NoError(t, err) - assert.Equal(t, test.expected, result) + assert.Equal(t, test.expected.drivers, result.drivers) + assert.Equal(t, test.expected.registered, result.registered) + assert.Equal(t, test.logger, result.logger()) }) } } @@ -372,11 +375,10 @@ func TestManager_ParseSubConfig(t *testing.T) { } for _, testCase := range testCases { t.Run(testCase.Name, func(t *testing.T) { - c := &Manager{ + c := withLogger(&Manager{ drivers: testCase.Drivers, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) conf := &Config{Field: FieldConfig{}} assert.NoError(t, c.Register(conf)) assert.NoError(t, c.Parse(context.Background(), testCase.config)) @@ -457,17 +459,16 @@ func TestManager_Parse(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - c := &Manager{ + c := withLogger(&Manager{ drivers: tc.Drivers, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) assert.NoError(t, c.Register(tc.Config)) _ = c.Parse(context.Background(), tc.Config) - logger := c.log.(*testLogger) + logger := c.logger().(*testLogger) assert.Equal(t, tc.ExpectWarnLogged, logger.WarnLogged) assert.Equal(t, tc.ExpectErrorLogged, logger.ErrorLogged) @@ -526,11 +527,10 @@ func TestManager_GenDoc(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := &Manager{ + c := withLogger(&Manager{ drivers: tt.fields.drivers, - log: tt.fields.log, registered: tt.fields.registered, - } + }, tt.fields.log) assert.Equalf(t, tt.want, c.GenDoc(tt.args.driverName), "GenDoc(%v)", tt.args.driverName) }) } @@ -563,11 +563,10 @@ func (md *pathMockDriver) GetValue(field fmap.Field) (*Value, error) { } func newPathMockManager(driver *pathMockDriver) *Manager { - return &Manager{ + return withLogger(&Manager{ drivers: []Driver{driver}, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) } func TestManager_Parse_UncomparableValues(t *testing.T) { @@ -638,7 +637,7 @@ func TestManager_Parse_DriverErrors(t *testing.T) { if testCase.WantError { assert.ErrorIs(t, err, driverErr) assert.Contains(t, err.Error(), "Port") - assert.True(t, c.log.(*testLogger).ErrorLogged) + assert.True(t, c.logger().(*testLogger).ErrorLogged) } else { assert.NoError(t, err) } @@ -825,17 +824,16 @@ func TestManager_Parse_Loader(t *testing.T) { loadErr: loadErr, } loaded := &loaderMockDriver{pathMockDriver: pathMockDriver{values: map[string]any{"Port": 80}}} - c := &Manager{ + c := withLogger(&Manager{ drivers: []Driver{&pathMockDriver{values: map[string]any{"Host": "localhost"}}, loaded, failed}, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) conf := &Config{} assert.NoError(t, c.Register(conf)) err := c.Parse(context.Background(), conf) assert.ErrorIs(t, err, loadErr) - assert.True(t, c.log.(*testLogger).ErrorLogged) + assert.True(t, c.logger().(*testLogger).ErrorLogged) // values of the failed driver are not used assert.Equal(t, Config{Host: "localhost", Port: 80}, *conf) assert.Empty(t, failed.requested) @@ -974,11 +972,10 @@ func TestManager_Sources(t *testing.T) { second := &namedPathMockDriver{name: "second", pathMockDriver: pathMockDriver{ values: map[string]any{"HTTP.Port": 8080}, }} - c := &Manager{ + c := withLogger(&Manager{ drivers: []Driver{first, second}, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) conf := &Config{} assert.NoError(t, c.Register(conf)) assert.Nil(t, c.Sources(conf)) @@ -1045,11 +1042,10 @@ func TestManager_RegistrationOrder(t *testing.T) { "Shared.Value": "shared", "Nested.Shared.Value": "nested", }}} - c := &Manager{ + c := withLogger(&Manager{ drivers: []Driver{driver}, registered: make(map[reflect.Type]*Registered), - log: &testLogger{}, - } + }, &testLogger{}) assert.NoError(t, c.Register(&Third{})) assert.NoError(t, c.Register(&First{})) assert.NoError(t, c.Register(&Second{})) @@ -1158,3 +1154,11 @@ func (md *watchMockDriver) Watch(ctx context.Context, onChange func()) error { } } } + +// withLogger sets the logger of the manager created by the literal in the tests. +func withLogger(m *Manager, l Logger) *Manager { + if l != nil { + m.storeLogger(l) + } + return m +} diff --git a/subscribe.go b/subscribe.go index 4b4335e..879922a 100644 --- a/subscribe.go +++ b/subscribe.go @@ -182,7 +182,7 @@ func (c *Manager) watch(ctx context.Context) { defer wg.Done() // the driver can not watch anymore, other drivers continue if err := watcher.Watch(ctx, notify); err != nil && ctx.Err() == nil { - c.log.Error(errorMessage(err, "failed to watch"), LogField("driver", d.GetName()), LogField("details", err.Error())) + c.logger().Error(errorMessage(err, "failed to watch"), LogField("driver", d.GetName()), LogField("details", err.Error())) c.setStatus(func(s *ReloadStatus) { if s.StoppedDrivers == nil { s.StoppedDrivers = make(map[string]error) @@ -252,7 +252,7 @@ func (c *Manager) reloadConfig(ctx context.Context, typeOf reflect.Type, subscri // the requests of the reload are stopped when watching is stopped sources, err := c.parse(ctx, fresh, true) if err != nil { - c.log.Error(errorMessage(err, "failed to reload"), LogField("config", typeOf.String()), LogField("details", err.Error())) + c.logger().Error(errorMessage(err, "failed to reload"), LogField("config", typeOf.String()), LogField("details", err.Error())) return fmt.Errorf("config %s: %w", typeOf, err) } prev := state.current.Load() @@ -275,7 +275,7 @@ func (c *Manager) reloadConfig(ctx context.Context, typeOf reflect.Type, subscri func (c *Manager) callSubscriber(sub *subscriber, prev, next any) { defer func() { if r := recover(); r != nil { - c.log.Error("subscriber panicked", LogField("config", sub.root.String()), LogField("details", fmt.Sprint(r))) + c.logger().Error("subscriber panicked", LogField("config", sub.root.String()), LogField("details", fmt.Sprint(r))) } }() sub.notify(prev, next)