Merge pull request kubernetes-sigs#915 from bharathi-tenneti/zaplogger

 ✨ Added Encoder Config Options field.

Author: k8s-ci-robot

go.mod

@@ -15,7 +15,6 @@ require (
 	github.com/onsi/gomega v1.10.1
 	github.com/prometheus/client_golang v1.7.1
 	github.com/prometheus/client_model v0.2.0
-	github.com/spf13/pflag v1.0.5
 	go.uber.org/zap v1.15.0
 	golang.org/x/time v0.0.0-20200630173020-3af7569d3a1e
 	gomodules.xyz/jsonpatch/v2 v2.1.0

go.sum

@@ -35,6 +35,7 @@ github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbt
 github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
 github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
 github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM=
+github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5VpdgMhJosfJnn5/FoN2SRZ4p7fJNX58YPaU=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
@@ -233,6 +234,7 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V
 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q=
 github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
+github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/konsorten/go-windows-terminal-sequences v1.0.3/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=

pkg/log/zap/flags.go

@@ -19,11 +19,11 @@ limitations under the License.
 package zap
 
 import (
+	"flag"
 	"fmt"
 	"strconv"
 	"strings"
 
-	"github.com/spf13/pflag"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 )
@@ -42,11 +42,11 @@ var stackLevelStrings = map[string]zapcore.Level{
 }
 
 type encoderFlag struct {
-	setFunc func(zapcore.Encoder)
+	setFunc func(NewEncoderFunc)
 	value   string
 }
 
-var _ pflag.Value = &encoderFlag{}
+var _ flag.Value = &encoderFlag{}
 
 func (ev *encoderFlag) String() string {
 	return ev.value
@@ -60,32 +60,22 @@ func (ev *encoderFlag) Set(flagValue string) error {
 	val := strings.ToLower(flagValue)
 	switch val {
 	case "json":
-		ev.setFunc(newJSONEncoder())
+		ev.setFunc(newJSONEncoder)
 	case "console":
-		ev.setFunc(newConsoleEncoder())
+		ev.setFunc(newConsoleEncoder)
 	default:
 		return fmt.Errorf("invalid encoder value \"%s\"", flagValue)
 	}
 	ev.value = flagValue
 	return nil
 }
 
-func newJSONEncoder() zapcore.Encoder {
-	encoderConfig := zap.NewProductionEncoderConfig()
-	return zapcore.NewJSONEncoder(encoderConfig)
-}
-
-func newConsoleEncoder() zapcore.Encoder {
-	encoderConfig := zap.NewDevelopmentEncoderConfig()
-	return zapcore.NewConsoleEncoder(encoderConfig)
-}
-
 type levelFlag struct {
 	setFunc func(zapcore.LevelEnabler)
 	value   string
 }
 
-var _ pflag.Value = &levelFlag{}
+var _ flag.Value = &levelFlag{}
 
 func (ev *levelFlag) Set(flagValue string) error {
 	level, validLevel := levelStrings[strings.ToLower(flagValue)]
@@ -120,7 +110,7 @@ type stackTraceFlag struct {
 	value   string
 }
 
-var _ pflag.Value = &stackTraceFlag{}
+var _ flag.Value = &stackTraceFlag{}
 
 func (ev *stackTraceFlag) Set(flagValue string) error {
 	level, validLevel := stackLevelStrings[strings.ToLower(flagValue)]

pkg/log/zap/zap.go

@@ -30,6 +30,12 @@ import (
 	"go.uber.org/zap/zapcore"
 )
 
+// EncoderConfigOption is a function that can modify a `zapcore.EncoderConfig`.
+type EncoderConfigOption func(*zapcore.EncoderConfig)
+
+// NewEncoderFunc is a function that creates an Encoder using the provided EncoderConfigOptions.
+type NewEncoderFunc func(...EncoderConfigOption) zapcore.Encoder
+
 // New returns a brand new Logger configured with Opts. It
 // uses KubeAwareEncoder which adds Type information and
 // Namespace/Name to the log.
@@ -65,6 +71,22 @@ func Encoder(encoder zapcore.Encoder) func(o *Options) {
 	}
 }
 
+func newJSONEncoder(opts ...EncoderConfigOption) zapcore.Encoder {
+	encoderConfig := zap.NewProductionEncoderConfig()
+	for _, opt := range opts {
+		opt(&encoderConfig)
+	}
+	return zapcore.NewJSONEncoder(encoderConfig)
+}
+
+func newConsoleEncoder(opts ...EncoderConfigOption) zapcore.Encoder {
+	encoderConfig := zap.NewDevelopmentEncoderConfig()
+	for _, opt := range opts {
+		opt(&encoderConfig)
+	}
+	return zapcore.NewConsoleEncoder(encoderConfig)
+}
+
 // Level sets the the minimum enabled logging level e.g Debug, Info
 // See Options.Level
 func Level(level zapcore.LevelEnabler) func(o *Options) {
@@ -99,6 +121,14 @@ type Options struct {
 	// Encoder configures how Zap will encode the output.  Defaults to
 	// console when Development is true and JSON otherwise
 	Encoder zapcore.Encoder
+	// EncoderConfigOptions can modify the EncoderConfig needed to initialize an Encoder.
+	// See https://godoc.org/go.uber.org/zap/zapcore#EncoderConfig for the list of options
+	// that can be configured.
+	// Note that the EncoderConfigOptions are not applied when the Encoder option is already set.
+	EncoderConfigOptions []EncoderConfigOption
+	// NewEncoder configures Encoder using the provided EncoderConfigOptions.
+	// Note that the NewEncoder function is not used when the Encoder option is already set.
+	NewEncoder NewEncoderFunc
 	// DestWritter controls the destination of the log output.  Defaults to
 	// os.Stderr.
 	DestWritter io.Writer
@@ -121,9 +151,8 @@ func (o *Options) addDefaults() {
 	}
 
 	if o.Development {
-		if o.Encoder == nil {
-			encCfg := zap.NewDevelopmentEncoderConfig()
-			o.Encoder = zapcore.NewConsoleEncoder(encCfg)
+		if o.NewEncoder == nil {
+			o.NewEncoder = newConsoleEncoder
 		}
 		if o.Level == nil {
 			lvl := zap.NewAtomicLevelAt(zap.DebugLevel)
@@ -136,9 +165,8 @@ func (o *Options) addDefaults() {
 		o.ZapOpts = append(o.ZapOpts, zap.Development())
 
 	} else {
-		if o.Encoder == nil {
-			encCfg := zap.NewProductionEncoderConfig()
-			o.Encoder = zapcore.NewJSONEncoder(encCfg)
+		if o.NewEncoder == nil {
+			o.NewEncoder = newJSONEncoder
 		}
 		if o.Level == nil {
 			lvl := zap.NewAtomicLevelAt(zap.InfoLevel)
@@ -157,6 +185,9 @@ func (o *Options) addDefaults() {
 				}))
 		}
 	}
+	if o.Encoder == nil {
+		o.Encoder = o.NewEncoder(o.EncoderConfigOptions...)
+	}
 	o.ZapOpts = append(o.ZapOpts, zap.AddStacktrace(o.StacktraceLevel))
 }
 
@@ -182,7 +213,7 @@ func NewRaw(opts ...Opts) *zap.Logger {
 // BindFlags will parse the given flagset for zap option flags and set the log options accordingly
 //  zap-devel: Development Mode defaults(encoder=consoleEncoder,logLevel=Debug,stackTraceLevel=Warn)
 //			  Production Mode defaults(encoder=jsonEncoder,logLevel=Info,stackTraceLevel=Error)
-//  zap-encoder: Zap log encoding ('json' or 'console')
+//  zap-encoder: Zap log encoding (one of '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 'info' or 'error')
@@ -195,10 +226,10 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
 
 	// Set Encoder value
 	var encVal encoderFlag
-	encVal.setFunc = func(fromFlag zapcore.Encoder) {
-		o.Encoder = fromFlag
+	encVal.setFunc = func(fromFlag NewEncoderFunc) {
+		o.NewEncoder = fromFlag
 	}
-	fs.Var(&encVal, "zap-encoder", "Zap log encoding ('json' or 'console')")
+	fs.Var(&encVal, "zap-encoder", "Zap log encoding (one of 'json' or 'console')")
 
 	// Set the Log Level
 	var levelVal levelFlag
@@ -221,10 +252,10 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
 // UseFlagOptions configures the logger to use the Options set by parsing zap option flags from the CLI.
 //  opts := zap.Options{}
 //  opts.BindFlags(flag.CommandLine)
+//  flag.Parse()
 //  log := zap.New(zap.UseFlagOptions(&opts))
 func UseFlagOptions(in *Options) Opts {
 	return func(o *Options) {
 		*o = *in
-		o.addDefaults()
 	}
 }

pkg/log/zap/zap_test.go

@@ -425,4 +425,84 @@ var _ = Describe("Zap log level flag options setup", func() {
 
 	})
 
+	Context("with only -zap-devel flag provided", func() {
+		It("Should set dev=true.", func() {
+			args := []string{"--zap-devel=true"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.Development).To(BeTrue())
+			Expect(out.Encoder).To(BeNil())
+			Expect(out.Level).To(BeNil())
+			Expect(out.StacktraceLevel).To(BeNil())
+			Expect(out.EncoderConfigOptions).To(BeNil())
+		})
+		It("Should set dev=false", func() {
+			args := []string{"--zap-devel=false"}
+			fromFlags.BindFlags(&fs)
+			if err := fs.Parse(args); err != nil {
+				Expect(err).ToNot(HaveOccurred())
+			}
+			out := Options{}
+			UseFlagOptions(&fromFlags)(&out)
+
+			Expect(out.Development).To(BeFalse())
+			Expect(out.Encoder).To(BeNil())
+			Expect(out.Level).To(BeNil())
+			Expect(out.StacktraceLevel).To(BeNil())
+			Expect(out.EncoderConfigOptions).To(BeNil())
+
+		})
+	})
+
+	Context("with encoder options provided programmatically.", func() {
+
+		It("Should set Console Encoder, with given Nanos TimeEncoder option.", func() {
+			logOut := new(bytes.Buffer)
+			f := func(ec *zapcore.EncoderConfig) {
+				if err := ec.EncodeTime.UnmarshalText([]byte("nanos")); err != nil {
+					Expect(err).ToNot(HaveOccurred())
+				}
+			}
+			opts := func(o *Options) {
+				o.EncoderConfigOptions = append(o.EncoderConfigOptions, f)
+			}
+			log := New(UseDevMode(true), WriteTo(logOut), opts)
+			log.Info("This is a test message")
+			outRaw := logOut.Bytes()
+			// Assert for Console Encoder
+			res := map[string]interface{}{}
+			Expect(json.Unmarshal(outRaw, &res)).ToNot(Succeed())
+			// Assert for Epoch Nanos TimeEncoder
+			Expect(string(outRaw)).ShouldNot(ContainSubstring("."))
+
+		})
+		It("Should set JSON Encoder, with given Millis TimeEncoder option, and MessageKey", func() {
+			logOut := new(bytes.Buffer)
+			f := func(ec *zapcore.EncoderConfig) {
+				ec.MessageKey = "MillisTimeFormat"
+				if err := ec.EncodeTime.UnmarshalText([]byte("millis")); err != nil {
+					Expect(err).ToNot(HaveOccurred())
+				}
+			}
+			opts := func(o *Options) {
+				o.EncoderConfigOptions = append(o.EncoderConfigOptions, f)
+			}
+			log := New(UseDevMode(false), WriteTo(logOut), opts)
+			log.Info("This is a test message")
+			outRaw := logOut.Bytes()
+			// Assert for JSON Encoder
+			res := map[string]interface{}{}
+			Expect(json.Unmarshal(outRaw, &res)).To(Succeed())
+			// Assert for Epoch Nanos TimeEncoder
+			Expect(string(outRaw)).Should(ContainSubstring("."))
+			// Assert for MessageKey
+			Expect(string(outRaw)).Should(ContainSubstring("MillisTimeFormat"))
+		})
+
+	})
 })