Honor level in spy.Logger (#162)

* spy.Logger: factor out .log

* spy.Logger: respect Level

* spy.Logger: fix comment doc for DFatal

* spy.logger: always handle panic and fatal

* TestSampleCheckPanicFatal: explicate "fatal always checks OK"

spy/logger.go

@@ -94,59 +94,80 @@ func (l *Logger) With(fields ...zap.Field) zap.Logger {
 
 // Check returns a CheckedMessage if logging a particular message would succeed.
 func (l *Logger) Check(lvl zap.Level, msg string) *zap.CheckedMessage {
-	if !(lvl >= l.Level()) {
-		return nil
+	switch lvl {
+	case zap.PanicLevel, zap.FatalLevel:
+		// Panic and Fatal should always cause a panic/exit, even if the level
+		// is disabled.
+		break
+	default:
+		if !(lvl >= l.Level()) {
+			return nil
+		}
 	}
 	return zap.NewCheckedMessage(l, lvl, msg)
 }
 
 // Log writes a message at the specified level.
 func (l *Logger) Log(lvl zap.Level, msg string, fields ...zap.Field) {
-	l.sink.WriteLog(lvl, msg, l.allFields(fields))
+	l.log(lvl, msg, fields)
 }
 
 // Debug logs at the Debug level.
 func (l *Logger) Debug(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.DebugLevel, msg, l.allFields(fields))
+	l.log(zap.DebugLevel, msg, fields)
 }
 
 // Info logs at the Info level.
 func (l *Logger) Info(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.InfoLevel, msg, l.allFields(fields))
+	l.log(zap.InfoLevel, msg, fields)
 }
 
 // Warn logs at the Warn level.
 func (l *Logger) Warn(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.WarnLevel, msg, l.allFields(fields))
+	l.log(zap.WarnLevel, msg, fields)
 }
 
 // Error logs at the Error level.
 func (l *Logger) Error(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.ErrorLevel, msg, l.allFields(fields))
+	l.log(zap.ErrorLevel, msg, fields)
 }
 
 // Panic logs at the Panic level. Note that the spy Logger doesn't actually
 // panic.
 func (l *Logger) Panic(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.PanicLevel, msg, l.allFields(fields))
+	l.log(zap.PanicLevel, msg, fields)
 }
 
 // Fatal logs at the Fatal level. Note that the spy logger doesn't actuall call
 // os.Exit.
 func (l *Logger) Fatal(msg string, fields ...zap.Field) {
-	l.sink.WriteLog(zap.FatalLevel, msg, l.allFields(fields))
+	l.log(zap.FatalLevel, msg, fields)
 }
 
-// DFatal logs at the Fatal level if the development flag is set, and the Fatal
+// DFatal logs at the Fatal level if the development flag is set, and the Error
 // level otherwise.
 func (l *Logger) DFatal(msg string, fields ...zap.Field) {
 	if l.Development {
-		l.sink.WriteLog(zap.FatalLevel, msg, l.allFields(fields))
+		l.log(zap.FatalLevel, msg, fields)
 	} else {
-		l.sink.WriteLog(zap.ErrorLevel, msg, l.allFields(fields))
+		l.log(zap.ErrorLevel, msg, fields)
 	}
 }
 
+func (l *Logger) log(lvl zap.Level, msg string, fields []zap.Field) {
+	switch lvl {
+	case zap.PanicLevel, zap.FatalLevel:
+		// Panic and Fatal should always cause a panic/exit, even if the level
+		// is disabled.
+		break
+	default:
+		if !(lvl >= l.Level()) {
+			return
+		}
+	}
+	l.sink.WriteLog(lvl, msg, l.allFields(fields))
+}
+
 func (l *Logger) allFields(added []zap.Field) []zap.Field {
 	all := make([]zap.Field, 0, len(added)+len(l.context))
 	all = append(all, l.context...)

zwrap/sample_test.go

@@ -179,7 +179,7 @@ func TestSamplerCheckPanicFatal(t *testing.T) {
 
 		assert.Nil(t, sampler.Check(zap.DebugLevel, "foo"), "Expected a nil CheckedMessage at disabled log levels.")
 		for i := 0; i < 5; i++ {
-			if cm := sampler.Check(level, "sample"); cm.OK() {
+			if cm := sampler.Check(level, "sample"); assert.True(t, cm.OK(), "expected fatal level to always be OK") {
 				cm.Write(zap.Int("iter", i))
 			}
 		}