rs/zerolog

zerolog.Logger{} should behave the same as zerolog.Nop() logger

Closed this issue · 2 comments

It seems that in practice it behaves almost the same. zerolog.Logger{} does more work though, constructing whole event and then bailing out when it tries to write to nil. I think it would be nice if zerolog.Logger{} would behave exactly the same as zerolog.Nop. I think that Logger.should should probably check if writer is set to non-nil value. Currently it is:

func (l *Logger) should(lvl Level) bool {
	if lvl < l.level || lvl < GlobalLevel() {
		return false
	}
	if l.sampler != nil && !samplingDisabled() {
		return l.sampler.Sample(lvl)
	}
	return true
}

I propose:

func (l *Logger) should(lvl Level) bool {
	if l.w == nil {
		return false
	}
	if lvl < l.level || lvl < GlobalLevel() {
		return false
	}
	if l.sampler != nil && !samplingDisabled() {
		return l.sampler.Sample(lvl)
	}
	return true
}

Zero logger returns false on l.w == nil while nop logger return false on lvl < l.level (because l.level is set to Disabled which holds true for all levels).

rs commented

I agree with that. Do you want to propose a PR?

Done: #606.