1
0
mirror of https://github.com/rs/zerolog synced 2026-06-08 17:13:30 +00:00

fix: UpdateContext skips Nop and zero-value loggers (#754)

UpdateContext previously used pointer equality (l == disabledLogger) to
detect disabled loggers. This only caught the package-level singleton
returned by Ctx() but missed loggers created via Nop() or zero-value
Logger{}, which could lead to data races when a shared Nop logger was
used from multiple goroutines.

Replace the pointer comparison with a check for nil writer or Disabled
level, consistent with how Logger.should() determines if logging is
active. This prevents unnecessary context mutations on loggers that
will never produce output.

Fixes #643

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Varun Chawla
2026-03-17 05:52:29 -07:00
committed by GitHub
parent d64c9a7138
commit 2f5b8a91be
2 changed files with 40 additions and 2 deletions
+7 -2
View File
@@ -294,7 +294,7 @@ func (l Logger) With() Context {
// Caution: This method is not concurrency safe. // Caution: This method is not concurrency safe.
// Use the With method to create a child logger before modifying the context from concurrent goroutines. // Use the With method to create a child logger before modifying the context from concurrent goroutines.
func (l *Logger) UpdateContext(update func(c Context) Context) { func (l *Logger) UpdateContext(update func(c Context) Context) {
if l == disabledLogger { if l.disabled() {
return return
} }
if cap(l.context) == 0 { if cap(l.context) == 0 {
@@ -508,9 +508,14 @@ func (l *Logger) scratchEvent() *Event {
return newEvent(LevelWriterAdapter{io.Discard}, DebugLevel, l.stack, l.ctx, l.hooks) return newEvent(LevelWriterAdapter{io.Discard}, DebugLevel, l.stack, l.ctx, l.hooks)
} }
// disabled returns true if the logger is a disabled or nop logger.
func (l *Logger) disabled() bool {
return l.w == nil || l.level == Disabled
}
// should returns true if the log event should be logged. // should returns true if the log event should be logged.
func (l *Logger) should(lvl Level) bool { func (l *Logger) should(lvl Level) bool {
if l.w == nil { if l.disabled() {
return false return false
} }
if lvl < l.level || lvl < GlobalLevel() { if lvl < l.level || lvl < GlobalLevel() {
+33
View File
@@ -1214,6 +1214,39 @@ func TestUpdateContextOnDisabledLogger(t *testing.T) {
} }
} }
func TestUpdateContextOnNopLogger(t *testing.T) {
// Nop() creates a new disabled logger that is a different pointer than
// the package-level disabledLogger. UpdateContext should still treat it
// as disabled and skip the update function. See issue #643.
log := Nop()
called := false
log.UpdateContext(func(c Context) Context {
called = true
return c.Str("foo", "bar")
})
if called {
t.Error("UpdateContext should not call update func on a Nop logger")
}
}
func TestUpdateContextOnZeroValueLogger(t *testing.T) {
// A zero-value Logger has a nil writer and should be treated as
// disabled by UpdateContext. See issue #643.
var log Logger
called := false
log.UpdateContext(func(c Context) Context {
called = true
return c.Str("foo", "bar")
})
if called {
t.Error("UpdateContext should not call update func on a zero-value logger")
}
}
func TestLevel_String(t *testing.T) { func TestLevel_String(t *testing.T) {
tests := []struct { tests := []struct {
name string name string