From 74eda41ec555403ca0329cef1775aa7fa923228b Mon Sep 17 00:00:00 2001 From: Paul Wells Date: Tue, 22 Sep 2026 00:25:57 -0700 Subject: [PATCH 1/2] logger: resolve component levels without allocating ResolveComponentLevel split the component on "." and rejoined the remaining parts on every lookup, so resolving rtc.room.track allocated a slice and a string per level. Walk backwards from the end of the string instead, reslicing to the last "." after each miss. The map lookups take those substrings directly, so the walk no longer allocates. What remains is ParseZapLevel, whose []byte conversion escapes into zapcore's UnmarshalText. --- logger/config.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/logger/config.go b/logger/config.go index 75c77477e..8b7571192 100644 --- a/logger/config.go +++ b/logger/config.go @@ -83,12 +83,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 } From d77b4a5b1c25296d4be8b02543cbbdee0e100946 Mon Sep 17 00:00:00 2001 From: Paul Wells Date: Tue, 22 Sep 2026 00:25:57 -0700 Subject: [PATCH 2/2] logger: implement yaml.Marshaler on Config Marshaling a Config read its fields alongside whatever Update was writing. MarshalYAML now snapshots them under the same lock. The snapshot needs a type without the mutex: the usual `type config Config` cast either returns before the encoder reads the fields, which defers the read back outside the lock, or copies the mutex to avoid that. So the yaml-visible fields are mirrored on configYAML, and TestConfigYAMLFields fails if the two drift. ComponentLevels is cloned because the encoder walks the returned value after the lock is released. --- .changeset/logger-config-marshal-yaml.md | 6 +++ logger/config.go | 37 ++++++++++++++++ logger/config_test.go | 56 ++++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 .changeset/logger-config-marshal-yaml.md 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 8b7571192..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 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)) +}