Skip to content

logger: allocation-free component level resolution, yaml.Marshaler on Config - #1813

Open
paulwe wants to merge 2 commits into
mainfrom
paul/logger-config-yaml
Open

paulwe wants to merge 2 commits into
mainfrom
paul/logger-config-yaml

Conversation

@paulwe

@paulwe paulwe commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Two independent changes to logger.Config.

ResolveComponentLevel without allocations

It 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. It now walks backwards from the end of the string, reslicing to the last "." after each miss, and the map lookups take those substrings directly.

Measured with testing.AllocsPerRun: the walk is 0 allocs. The one that remains is ParseZapLevel, whose []byte(level) conversion escapes because zapcore's UnmarshalText hands the bytes to fmt.Errorf. Killing that means duplicating zapcore's level table in a string switch — not done here.

Edge cases match the old behavior exactly: empty string, leading dot, and trailing dot all resolve as before.

yaml.Marshaler on Config

Marshaling read the fields alongside whatever Update was writing. MarshalYAML now snapshots under the same lock.

The snapshot needs a type without the mutex. The usual type config Config cast doesn't give one: returning (*config)(c) defers the field reads until after the lock is released, and dereferencing to snapshot trips copylocks. Encoding into a yaml.Node inside the lock looked like a way out, but yaml's encoder dereferences the pointer and calls in.Interface() on the struct (encode.go:117), which copies the locked mutex where neither vet nor -race can see it. So the yaml-visible fields are mirrored on configYAML, with TestConfigYAMLFields reflecting over both types to fail if a field or tag drifts.

ComponentLevels is cloned because the encoder walks the returned value after the lock drops.

Note that yaml.v3 does not take the address of struct fields, so a config holding logger.Config by value skips this method and falls back to field reflection — same output, no locked snapshot. Holding *logger.Config gets the lock.

Considered and rejected

Moving the fields into an embedded struct would define them once and collapse Update to a single assignment. Go 1.27 allows promoted fields in composite literals, so Config{Level: "info"} keeps compiling — but only for callers whose own module declares go 1.27; the library's directive doesn't matter. Callers on 1.26 would need to bump or rewrite the literal, and that's 93 sites across ~20 repos here, all still on 1.26. Worth revisiting once the fleet moves, at which point it costs consumers nothing.

Test plan

  • go test -race ./logger/..., go vet ./logger/...
  • Byte-exact marshal output, round-trip through Unmarshal, and a mutate-after-marshal check that the snapshot held
  • Both commits verified green independently

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.
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-bot

changeset-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: d77b4a5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
github.com/livekit/protocol Patch
@livekit/protocol Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants