Fix zap levels to align with logr

Author: bharathi-tenneti

pkg/log/zap/flags.go

@@ -29,16 +29,15 @@ import (
 )
 
 var levelStrings = map[string]zapcore.Level{
-	"debug":  zap.DebugLevel,
-	"-1":     zap.DebugLevel,
-	"info":   zap.InfoLevel,
-	"0":      zap.InfoLevel,
-	"error":  zap.ErrorLevel,
-	"2":      zap.ErrorLevel,
-	"dpanic": zap.DPanicLevel,
-	"panic":  zap.PanicLevel,
-	"warn":   zap.WarnLevel,
-	"fatal":  zap.FatalLevel,
+	"debug": zap.DebugLevel,
+	"info":  zap.InfoLevel,
+	"error": zap.ErrorLevel,
+}
+
+var stackLevelStrings = map[string]zapcore.Level{
+	"disabled": zap.FatalLevel,
+	"info":     zap.InfoLevel,
+	"error":    zap.ErrorLevel,
 }
 
 type encoderFlag struct {
@@ -94,14 +93,15 @@ func (ev *levelFlag) Set(flagValue string) error {
 		if err != nil {
 			return fmt.Errorf("invalid log level \"%s\"", flagValue)
 		}
-		if logLevel > 0 {
+		if logLevel >= 0 {
 			intLevel := -1 * logLevel
 			ev.setFunc(zap.NewAtomicLevelAt(zapcore.Level(int8(intLevel))))
 		} else {
 			return fmt.Errorf("invalid log level \"%s\"", flagValue)
 		}
+	} else {
+		ev.setFunc(zap.NewAtomicLevelAt(level))
 	}
-	ev.setFunc(zap.NewAtomicLevelAt(level))
 	ev.value = flagValue
 	return nil
 }
@@ -122,7 +122,7 @@ type stackTraceFlag struct {
 var _ pflag.Value = &stackTraceFlag{}
 
 func (ev *stackTraceFlag) Set(flagValue string) error {
-	level, validLevel := levelStrings[strings.ToLower(flagValue)]
+	level, validLevel := stackLevelStrings[strings.ToLower(flagValue)]
 	if !validLevel {
 		return fmt.Errorf("invalid stacktrace level \"%s\"", flagValue)
 	}

pkg/log/zap/zap_test.go

@@ -19,11 +19,14 @@ package zap
 import (
 	"bytes"
 	"encoding/json"
+	"flag"
 	"io/ioutil"
+	"os"
 
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"go.uber.org/zap/zapcore"
 	kapi "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
@@ -296,3 +299,135 @@ var _ = Describe("Zap logger setup", func() {
 		})
 	})
 })
+
+var _ = Describe("Zap log level flag options setup", func() {
+	var (
+		fromFlags Options
+		fs        flag.FlagSet
+	)
+
+	BeforeEach(func() {
+		fromFlags = Options{}
+		fs = *flag.NewFlagSet(os.Args[0], flag.ExitOnError) //flags are now reset
+	})
+
+	Context("with  zap-log-level options provided", func() {
+		It("Should set info/debug zap-log-level flags.", func() {
+			args := []string{"--zap-log-level=debug"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info("info text")
+			logger.V(1).Info("debug 1 text")
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring("info text"))
+			Expect(string(outRaw)).Should(ContainSubstring("debug 1 text"))
+
+		})
+
+		It("Should set error zap-log-level flags.", func() {
+			args := []string{"--zap-log-level=error"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info("info text")
+			logger.V(1).Info("debug 1 text")
+
+			outRaw := logOut.Bytes()
+
+			Expect(outRaw).To(BeEmpty())
+		})
+
+		It("Should set level 1 debug zap-log-level flags.", func() {
+			args := []string{"--zap-log-level=1"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info("info text")
+			logger.V(1).Info("debug 1 text")
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring("info text"))
+			Expect(string(outRaw)).Should(ContainSubstring("debug 1 text"))
+		})
+	})
+
+	Context("with  zap-log-level  with increased verbosity.", func() {
+		It("Should set Level 2 increased verbosity for zap-log-level flags.", func() {
+			args := []string{"--zap-log-level=2"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info("info text")
+			logger.V(1).Info("debug 1 text")
+			logger.V(2).Info("debug 2 text")
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring("info text"))
+			Expect(string(outRaw)).Should(ContainSubstring("debug 1 text"))
+			Expect(string(outRaw)).Should(ContainSubstring("debug 2 text"))
+
+		})
+
+	})
+
+	Context("with  zap-stacktrace-level options provided", func() {
+
+		It("Should set info stacktrace for zap-stacktrace-level flags.", func() {
+			args := []string{"--zap-stacktrace-level=info", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.StacktraceLevel.Enabled(zapcore.InfoLevel)).To(BeTrue())
+		})
+
+		It("Should set error stacktrace  for zap-stacktrace-level flags.", func() {
+			args := []string{"--zap-stacktrace-level=error", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.StacktraceLevel.Enabled(zapcore.ErrorLevel)).To(BeTrue())
+		})
+
+		It("Should disable stacktrace for zap-stacktrace-level flags.", func() {
+			args := []string{"--zap-stacktrace-level=disabled", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.StacktraceLevel.Enabled(zapcore.FatalLevel)).To(BeTrue())
+		})
+	})
+
+})