From c6b1f076cbd00289e18dd8cbcc5e823cadff4fb4 Mon Sep 17 00:00:00 2001 From: bharathi-tenneti Date: Wed, 22 Apr 2020 10:41:33 -0400 Subject: [PATCH] Added EncoderConfigOptions field in Options struct. --- go.mod | 1 - go.sum | 2 ++ pkg/log/zap/flags.go | 24 ++++--------- pkg/log/zap/zap.go | 53 +++++++++++++++++++++------ pkg/log/zap/zap_test.go | 80 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 29 deletions(-) diff --git a/go.mod b/go.mod index 20fc9fbe4c..654a2737c9 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index a0d0312130..5790b6812f 100644 --- a/go.sum +++ b/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= diff --git a/pkg/log/zap/flags.go b/pkg/log/zap/flags.go index 6dae739a4f..7a7b764f6f 100644 --- a/pkg/log/zap/flags.go +++ b/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,9 +60,9 @@ 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) } @@ -70,22 +70,12 @@ func (ev *encoderFlag) Set(flagValue string) error { 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)] diff --git a/pkg/log/zap/zap.go b/pkg/log/zap/zap.go index 0815e22d62..90d2388366 100644 --- a/pkg/log/zap/zap.go +++ b/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() } } diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go index 049b50316e..1dab4aca4b 100644 --- a/pkg/log/zap/zap_test.go +++ b/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")) + }) + + }) })