diff --git a/pkg/log/zap/flags.go b/pkg/log/zap/flags.go index bd4fa6f1b5..6dae739a4f 100644 --- a/pkg/log/zap/flags.go +++ b/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) } diff --git a/pkg/log/zap/zap.go b/pkg/log/zap/zap.go index f240928c38..05b9b357b6 100644 --- a/pkg/log/zap/zap.go +++ b/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. diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go index 342a02cd38..1d2fddb884 100644 --- a/pkg/log/zap/zap_test.go +++ b/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()) + }) + + }) + +})