From cb9e579248045aa267b17302e2ab2ac22608ac67 Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Wed, 3 Feb 2021 20:57:57 +1100 Subject: [PATCH] Correctly map log levels and add spaces for *ln() methods --- zapgrpc/zapgrpc.go | 51 +++++++++++++++-- zapgrpc/zapgrpc_test.go | 123 +++++++++++++++++++++++++++++++++------- 2 files changed, 149 insertions(+), 25 deletions(-) diff --git a/zapgrpc/zapgrpc.go b/zapgrpc/zapgrpc.go index 24b673849..19aeb356f 100644 --- a/zapgrpc/zapgrpc.go +++ b/zapgrpc/zapgrpc.go @@ -26,6 +26,26 @@ import ( "go.uber.org/zap/zapcore" ) +const ( + // See https://github.com/grpc/grpc-go/blob/v1.35.0/grpclog/loggerv2.go#L77-L86 + + grpcLvlInfo = 0 + grpcLvlWarn = 1 + grpcLvlError = 2 + grpcLvlFatal = 3 +) + +var ( + // grpc2zapLvl maps gRPC log levels to zap log levels. + // See https://pkg.go.dev/go.uber.org/zap@v1.16.0/zapcore#Level + grpc2zapLvl = map[int]zapcore.Level{ + grpcLvlInfo: zapcore.InfoLevel, + grpcLvlWarn: zapcore.WarnLevel, + grpcLvlError: zapcore.ErrorLevel, + grpcLvlFatal: zapcore.FatalLevel, + } +) + // An Option overrides a Logger's default configuration. type Option interface { apply(*Logger) @@ -90,7 +110,7 @@ func (l *Logger) Printf(format string, args ...interface{}) { // Println implements grpclog.Logger. // Deprecated: use Info(). func (l *Logger) Println(args ...interface{}) { - l.Print(args...) + l.Print(addSpaces(args)...) } // Info implements grpclog.LoggerV2. @@ -100,7 +120,7 @@ func (l *Logger) Info(args ...interface{}) { // Infoln implements grpclog.LoggerV2. func (l *Logger) Infoln(args ...interface{}) { - l.delegate.Info(args...) + l.delegate.Info(addSpaces(args)...) } // Infof implements grpclog.LoggerV2. @@ -115,7 +135,7 @@ func (l *Logger) Warning(args ...interface{}) { // Warningln implements grpclog.LoggerV2. func (l *Logger) Warningln(args ...interface{}) { - l.delegate.Warn(args...) + l.delegate.Warn(addSpaces(args)...) } // Warningf implements grpclog.LoggerV2. @@ -130,7 +150,7 @@ func (l *Logger) Error(args ...interface{}) { // Errorln implements grpclog.LoggerV2. func (l *Logger) Errorln(args ...interface{}) { - l.delegate.Error(args...) + l.delegate.Error(addSpaces(args)...) } // Errorf implements grpclog.LoggerV2. @@ -149,7 +169,7 @@ func (l *Logger) Fatal(args ...interface{}) { // Fatalln implements grpclog.LoggerV2. func (l *Logger) Fatalln(args ...interface{}) { - l.Fatal(args...) + l.Fatal(addSpaces(args)...) } // Fatalf implements grpclog.LoggerV2. @@ -163,5 +183,24 @@ func (l *Logger) Fatalf(format string, args ...interface{}) { // V implements grpclog.LoggerV2. func (l *Logger) V(level int) bool { - return l.delegate.Desugar().Core().Enabled(zapcore.Level(level)) + return l.delegate.Desugar().Core().Enabled(grpc2zapLvl[level]) +} + +// addSpaces always adds spaces between arguments like https://golang.org/pkg/fmt/#Println +func addSpaces(args []interface{}) []interface{} { + l := len(args) + if l == 0 || l == 1 { + return args + } + res := make([]interface{}, 0, l+l-1) + first := true + for _, arg := range args { + if first { + first = false + res = append(res, arg) + } else { + res = append(res, " ", arg) + } + } + return res } diff --git a/zapgrpc/zapgrpc_test.go b/zapgrpc/zapgrpc_test.go index ed827bf7f..055c2153c 100644 --- a/zapgrpc/zapgrpc_test.go +++ b/zapgrpc/zapgrpc_test.go @@ -21,6 +21,7 @@ package zapgrpc import ( + "fmt" "testing" "go.uber.org/zap" @@ -33,87 +34,171 @@ import ( func TestLoggerInfoExpected(t *testing.T) { checkMessages(t, zapcore.DebugLevel, nil, zapcore.InfoLevel, []string{ "hello", - "world", + "hello world", + "", "foo", + "foo bar", "hello", - "world", + "hello world", + "", "foo", + "foo bar", }, func(logger *Logger) { logger.Info("hello") - logger.Infof("world") + logger.Infof("%s world", "hello") + logger.Infoln() logger.Infoln("foo") + logger.Infoln("foo", "bar") logger.Print("hello") - logger.Printf("world") + logger.Printf("%s world", "hello") + logger.Println() logger.Println("foo") + logger.Println("foo", "bar") }) } func TestLoggerDebugExpected(t *testing.T) { checkMessages(t, zapcore.DebugLevel, []Option{WithDebug()}, zapcore.DebugLevel, []string{ "hello", - "world", + "hello world", + "", "foo", + "foo bar", }, func(logger *Logger) { logger.Print("hello") - logger.Printf("world") + logger.Printf("%s world", "hello") + logger.Println() logger.Println("foo") + logger.Println("foo", "bar") }) } func TestLoggerDebugSuppressed(t *testing.T) { checkMessages(t, zapcore.InfoLevel, []Option{WithDebug()}, zapcore.DebugLevel, nil, func(logger *Logger) { logger.Print("hello") - logger.Printf("world") + logger.Printf("%s world", "hello") + logger.Println() logger.Println("foo") + logger.Println("foo", "bar") }) } func TestLoggerWarningExpected(t *testing.T) { checkMessages(t, zapcore.DebugLevel, nil, zapcore.WarnLevel, []string{ "hello", - "world", + "hello world", + "", "foo", + "foo bar", }, func(logger *Logger) { logger.Warning("hello") - logger.Warningf("world") + logger.Warningf("%s world", "hello") + logger.Warningln() logger.Warningln("foo") + logger.Warningln("foo", "bar") }) } func TestLoggerErrorExpected(t *testing.T) { checkMessages(t, zapcore.DebugLevel, nil, zapcore.ErrorLevel, []string{ "hello", - "world", + "hello world", + "", "foo", + "foo bar", }, func(logger *Logger) { logger.Error("hello") - logger.Errorf("world") + logger.Errorf("%s world", "hello") + logger.Errorln() logger.Errorln("foo") + logger.Errorln("foo", "bar") }) } func TestLoggerFatalExpected(t *testing.T) { checkMessages(t, zapcore.DebugLevel, nil, zapcore.FatalLevel, []string{ "hello", - "world", + "hello world", + "", "foo", + "foo bar", }, func(logger *Logger) { logger.Fatal("hello") - logger.Fatalf("world") + logger.Fatalf("%s world", "hello") + logger.Fatalln() logger.Fatalln("foo") + logger.Fatalln("foo", "bar") }) } func TestLoggerVTrueExpected(t *testing.T) { - checkLevel(t, zapcore.DebugLevel, true, func(logger *Logger) bool { - return logger.V(0) - }) + enabled := map[zapcore.Level][]int{ + zapcore.DebugLevel: { + grpcLvlInfo, grpcLvlWarn, grpcLvlError, grpcLvlFatal, + }, + zapcore.InfoLevel: { + grpcLvlInfo, grpcLvlWarn, grpcLvlError, grpcLvlFatal, + }, + zapcore.WarnLevel: { + grpcLvlWarn, grpcLvlError, grpcLvlFatal, + }, + zapcore.ErrorLevel: { + grpcLvlError, grpcLvlFatal, + }, + zapcore.DPanicLevel: { + grpcLvlFatal, + }, + zapcore.PanicLevel: { + grpcLvlFatal, + }, + zapcore.FatalLevel: { + grpcLvlFatal, + }, + } + for zapLvl, grpcLvls := range enabled { + for _, grpcLvl := range grpcLvls { + t.Run(fmt.Sprintf("%s %d", zapLvl, grpcLvl), func(t *testing.T) { + checkLevel(t, zapLvl, true, func(logger *Logger) bool { + return logger.V(grpcLvl) + }) + }) + } + } } func TestLoggerVFalseExpected(t *testing.T) { - checkLevel(t, zapcore.WarnLevel, false, func(logger *Logger) bool { - return logger.V(0) - }) + disabled := map[zapcore.Level][]int{ + zapcore.DebugLevel: { + // everything is enabled, nothing is disabled + }, + zapcore.InfoLevel: { + // everything is enabled, nothing is disabled + }, + zapcore.WarnLevel: { + grpcLvlInfo, + }, + zapcore.ErrorLevel: { + grpcLvlInfo, grpcLvlWarn, + }, + zapcore.DPanicLevel: { + grpcLvlInfo, grpcLvlWarn, grpcLvlError, + }, + zapcore.PanicLevel: { + grpcLvlInfo, grpcLvlWarn, grpcLvlError, + }, + zapcore.FatalLevel: { + grpcLvlInfo, grpcLvlWarn, grpcLvlError, + }, + } + for zapLvl, grpcLvls := range disabled { + for _, grpcLvl := range grpcLvls { + t.Run(fmt.Sprintf("%s %d", zapLvl, grpcLvl), func(t *testing.T) { + checkLevel(t, zapLvl, false, func(logger *Logger) bool { + return logger.V(grpcLvl) + }) + }) + } + } } func checkLevel(