diff --git a/.changeset/logger-config-marshal-yaml.md b/.changeset/logger-config-marshal-yaml.md new file mode 100644 index 000000000..64d162246 --- /dev/null +++ b/.changeset/logger-config-marshal-yaml.md @@ -0,0 +1,6 @@ +--- +"github.com/livekit/protocol": patch +"@livekit/protocol": patch +--- + +`logger.Config` implements `yaml.Marshaler`, so marshaling a `*Config` snapshots it under the same lock `Update` takes instead of reading fields alongside a concurrent update. diff --git a/logger/config.go b/logger/config.go index 75c77477e..dae27f4a8 100644 --- a/logger/config.go +++ b/logger/config.go @@ -15,6 +15,7 @@ package logger import ( + "maps" "strings" "sync" @@ -50,6 +51,42 @@ type Config struct { type ConfigObserver func(*Config) error +// configYAML mirrors Config's yaml-visible fields so MarshalYAML can snapshot them +// under the lock; Config itself cannot be copied out. TestConfigYAMLFields guards drift. +type configYAML struct { + JSON bool `yaml:"json,omitempty"` + Level string `yaml:"level,omitempty"` + Sample bool `yaml:"sample,omitempty"` + + ComponentLevels map[string]string `yaml:"component_levels,omitempty"` + + SampleInitial int `yaml:"sample_initial,omitempty"` + SampleInterval int `yaml:"sample_interval,omitempty"` + + ItemSampleSeconds int `yaml:"item_sample_seconds,omitempty"` + ItemSampleInitial int `yaml:"item_sample_initial,omitempty"` + ItemSampleInterval int `yaml:"item_sample_interval,omitempty"` +} + +// The encoder reads the returned value after the lock is released, so the map is cloned +// rather than shared with a config that Update may replace. +func (c *Config) MarshalYAML() (any, error) { + c.lock.Lock() + defer c.lock.Unlock() + + return configYAML{ + JSON: c.JSON, + Level: c.Level, + Sample: c.Sample, + ComponentLevels: maps.Clone(c.ComponentLevels), + SampleInitial: c.SampleInitial, + SampleInterval: c.SampleInterval, + ItemSampleSeconds: c.ItemSampleSeconds, + ItemSampleInitial: c.ItemSampleInitial, + ItemSampleInterval: c.ItemSampleInterval, + }, nil +} + func (c *Config) Update(o *Config) error { c.lock.Lock() c.JSON = o.JSON @@ -83,12 +120,14 @@ func (c *Config) ResolveComponentLevel(component string) (zapcore.Level, bool) { c.lock.Lock() defer c.lock.Unlock() - parts := strings.Split(component, ".") - for len(parts) > 0 { - if lvl, ok := c.ComponentLevels[strings.Join(parts, ".")]; ok { + for { + if lvl, ok := c.ComponentLevels[component]; ok { return ParseZapLevel(lvl), true } - parts = parts[:len(parts)-1] + i := strings.LastIndexByte(component, '.') + if i < 0 { + return ParseZapLevel(c.Level), true + } + component = component[:i] } - return ParseZapLevel(c.Level), true } diff --git a/logger/config_test.go b/logger/config_test.go index 55fb4054f..64f9d796a 100644 --- a/logger/config_test.go +++ b/logger/config_test.go @@ -2,10 +2,12 @@ package logger import ( "io" + "reflect" "testing" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" + "gopkg.in/yaml.v3" "github.com/livekit/protocol/logger/zaputil" ) @@ -23,3 +25,57 @@ func TestConfigResolveComponentLevel(t *testing.T) { require.True(t, ok) require.Equal(t, zapcore.InfoLevel, lvl) } + +func TestConfigYAMLFields(t *testing.T) { + conf, snap := reflect.TypeFor[Config](), reflect.TypeFor[configYAML]() + + var i int + for _, f := range reflect.VisibleFields(conf) { + if !f.IsExported() { + continue + } + require.Less(t, i, snap.NumField(), "configYAML is missing %s", f.Name) + + s := snap.Field(i) + require.Equal(t, f.Name, s.Name) + require.Equal(t, f.Type, s.Type) + require.Equal(t, f.Tag.Get("yaml"), s.Tag.Get("yaml")) + i++ + } + require.Equal(t, snap.NumField(), i, "configYAML has fields Config does not") +} + +func TestConfigMarshalYAML(t *testing.T) { + conf := &Config{ + JSON: true, + Level: "debug", + Sample: true, + ComponentLevels: map[string]string{"rtc.room": "debug"}, + SampleInitial: 5, + } + + b, err := yaml.Marshal(conf) + require.NoError(t, err) + require.Equal(t, `json: true +level: debug +sample: true +component_levels: + rtc.room: debug +sample_initial: 5 +`, string(b)) + + var out Config + require.NoError(t, yaml.Unmarshal(b, &out)) + require.Equal(t, conf.ComponentLevels, out.ComponentLevels) + require.Equal(t, conf.Level, out.Level) + require.Equal(t, conf.SampleInitial, out.SampleInitial) + + // The encoder walks the snapshot after MarshalYAML returns, so a component level + // dropped in the meantime must not change what was marshaled. + delete(conf.ComponentLevels, "rtc.room") + require.Contains(t, string(b), "rtc.room: debug") + + b, err = yaml.Marshal(&Config{}) + require.NoError(t, err) + require.Equal(t, "{}\n", string(b)) +}