🐛 Fix allowed levels for --zap-log-levels flag to align with logr levels

pkg/log/zap/flags.go

@@ -29,16 +29,16 @@ 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,
+}
+
+// TODO Add level to disable stacktraces.
+// https://github.com/kubernetes-sigs/controller-runtime/issues/1035
+var stackLevelStrings = map[string]zapcore.Level{
+	"info":  zap.InfoLevel,
+	"error": zap.ErrorLevel,
 }
 
 type encoderFlag struct {
@@ -100,8 +100,9 @@ func (ev *levelFlag) Set(flagValue string) error {
 		} 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 +123,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.go

@@ -181,12 +181,15 @@ func (o *Options) addDefaults() {
 			lvl := zap.NewAtomicLevelAt(zap.ErrorLevel)
 			o.StacktraceLevel = &lvl
 		}
-		o.ZapOpts = append(o.ZapOpts,
-			zap.WrapCore(func(core zapcore.Core) zapcore.Core {
-				return zapcore.NewSampler(core, time.Second, 100, 100)
-			}))
+		// Disable sampling for increased Debug levels. Otherwise, this will
+		// cause index out of bounds errors in the sampling code.
+		if !o.Level.Enabled(zapcore.Level(-2)) {
+			o.ZapOpts = append(o.ZapOpts,
+				zap.WrapCore(func(core zapcore.Core) zapcore.Core {
+					return zapcore.NewSampler(core, time.Second, 100, 100)
+				}))
+		}
 	}
-
 	o.ZapOpts = append(o.ZapOpts, zap.AddStacktrace(o.StacktraceLevel))
 }
 
@@ -215,7 +218,7 @@ func NewRaw(opts ...Opts) *zap.Logger {
 //  zap-encoder: Zap log encoding ('json' or 'console')
 //  zap-log-level:  Zap Level to configure the verbosity of logging. Can be one of 'debug', 'info', 'error',
 //			       or any integer value > 0 which corresponds to custom debug levels of increasing verbosity")
-//  zap-stacktrace-level: Zap Level at and above which stacktraces are captured (one of 'warn' or 'error')
+//  zap-stacktrace-level: Zap Level at and above which stacktraces are captured (one of 'info' or 'error')
 func (o *Options) BindFlags(fs *flag.FlagSet) {
 
 	// Set Development mode value
@@ -245,7 +248,7 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
 		o.StacktraceLevel = fromFlag
 	}
 	fs.Var(&stackVal, "zap-stacktrace-level",
-		"Zap Level at and above which stacktraces are captured (one of 'warn' or 'error')")
+		"Zap Level at and above which stacktraces are captured (one of 'info', 'error').")
 }
 
 // UseFlagOptions configures the logger to use the Options set by parsing zap option flags from the CLI.

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,164 @@ var _ = Describe("Zap logger setup", func() {
 		})
 	})
 })
+
+var _ = Describe("Zap log level flag options setup", func() {
+	var (
+		fromFlags      Options
+		fs             flag.FlagSet
+		logInfoLevel0  = "info text"
+		logDebugLevel1 = "debug 1 text"
+		logDebugLevel2 = "debug 2 text"
+		logDebugLevel3 = "debug 3 text"
+	)
+
+	BeforeEach(func() {
+		fromFlags = Options{}
+		fs = *flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+	})
+
+	Context("with  zap-log-level options provided", func() {
+		It("Should output logs for info and debug zap-log-level.", func() {
+			args := []string{"--zap-log-level=debug"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
+
+		})
+
+		It("Should output only error logs, otherwise empty logs", func() {
+			args := []string{"--zap-log-level=error"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+
+			outRaw := logOut.Bytes()
+
+			Expect(outRaw).To(BeEmpty())
+		})
+
+	})
+
+	Context("with  zap-log-level  with increased verbosity.", func() {
+		It("Should output debug and info log, with default production mode.", func() {
+			args := []string{"--zap-log-level=1"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
+		})
+
+		It("Should output info and debug logs, with development mode.", func() {
+			args := []string{"--zap-log-level=1", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
+		})
+
+		It("Should output info, and debug logs with increased verbosity, and with development mode set to true.", func() {
+			args := []string{"--zap-log-level=3", "--zap-devel=false"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+			logger.V(2).Info(logDebugLevel2)
+			logger.V(3).Info(logDebugLevel3)
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel2))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel3))
+
+		})
+		It("Should output info, and debug logs with increased verbosity, and with production mode set to true.", func() {
+			args := []string{"--zap-log-level=3", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			logOut := new(bytes.Buffer)
+
+			logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			logger.V(0).Info(logInfoLevel0)
+			logger.V(1).Info(logDebugLevel1)
+			logger.V(2).Info(logDebugLevel2)
+			logger.V(3).Info(logDebugLevel3)
+
+			outRaw := logOut.Bytes()
+
+			Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel2))
+			Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel3))
+
+		})
+
+	})
+
+	Context("with  zap-stacktrace-level options provided", func() {
+
+		It("Should output stacktrace at info level, with development mode set to true.", func() {
+			args := []string{"--zap-stacktrace-level=info", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.StacktraceLevel.Enabled(zapcore.InfoLevel)).To(BeTrue())
+		})
+
+		It("Should output stacktrace at error level, with development mode set to true.", func() {
+			args := []string{"--zap-stacktrace-level=error", "--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			err := fs.Parse(args)
+			Expect(err).ToNot(HaveOccurred())
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.StacktraceLevel.Enabled(zapcore.ErrorLevel)).To(BeTrue())
+		})
+
+	})
+
+})