Removed time encoder flag

kubernetes-sigs · Jul 23, 2020 · 40df381 · 40df381
1 parent 788241a
commit 40df381
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 114 deletions.
diff --git a/pkg/log/zap/flags.go b/pkg/log/zap/flags.go
@@ -70,31 +70,6 @@ func (ev *encoderFlag) Set(flagValue string) error {
 	return nil
 }
 
-type timeEncoderFlag struct {
-	setFunc func(zapcore.TimeEncoder)
-	value   string
-}
-
-var _ flag.Value = &timeEncoderFlag{}
-
-func (ev *timeEncoderFlag) String() string {
-	return ev.value
-}
-
-func (ev *timeEncoderFlag) Type() string {
-	return "timeencoder"
-}
-
-func (ev *timeEncoderFlag) Set(flagValue string) error {
-	var encoder zapcore.TimeEncoder
-	if err := encoder.UnmarshalText([]byte(flagValue)); err != nil {
-		return fmt.Errorf("unable to unmarshal, \"%s\"", flagValue)
-	}
-	ev.setFunc(encoder)
-	ev.value = flagValue
-	return nil
-}
-
 type levelFlag struct {
 	setFunc func(zapcore.LevelEnabler)
 	value   string

diff --git a/pkg/log/zap/zap.go b/pkg/log/zap/zap.go
@@ -121,17 +121,6 @@ func newConsoleEncoder(ecfs ...EncoderConfigOption) zapcore.Encoder {
 	return zapcore.NewConsoleEncoder(encoderConfig)
 }
 
-// TimeEncoder configures how the logger will encode time format.
-// See Options.EncoderConfigOptions
-func TimeEncoder(timeEncoder zapcore.TimeEncoder) func(o *Options) {
-	return func(o *Options) {
-		f := func(ec *zapcore.EncoderConfig) {
-			ec.EncodeTime = timeEncoder
-		}
-		o.EncoderConfigOptions = append(o.EncoderConfigOptions, f)
-	}
-}
-
 // Level sets the the minimum enabled logging level e.g Debug, Info
 // See Options.Level
 func Level(level zapcore.LevelEnabler) func(o *Options) {
@@ -255,7 +244,6 @@ func NewRaw(opts ...Opts) *zap.Logger {
 //  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 (Eg., 'json' or 'console')
-//  zap-time-encoder: Zap time encoding format, defaults to 'epoch'.
 //  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')
@@ -266,14 +254,6 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
 		"Development Mode defaults(encoder=consoleEncoder,logLevel=Debug,stackTraceLevel=Warn). "+
 			"Production Mode defaults(encoder=jsonEncoder,logLevel=Info,stackTraceLevel=Error)")
 
-	// Set TimeEncoder value
-	var timeEncVal timeEncoderFlag
-	timeEncVal.setFunc = func(fromFlag zapcore.TimeEncoder) {
-		// Set o.EncoderConfigOptions with zap-time-encoder
-		TimeEncoder(fromFlag)(o)
-	}
-	fs.Var(&timeEncVal, "zap-time-encoder", "Zap time encoding format, defaults to 'epoch'")
-
 	// Set Encoder value
 	var encVal encoderFlag
 	encVal.setFunc = func(fromFlag NewEncoderFunc) {

diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go
@@ -341,91 +341,50 @@ var _ = Describe("Zap flag options setup", func() {
 
 		})
 	})
-	Context("with direct Options provided, and no Flag values", func() {
 
-		It("Should set dev=true, and default ConsoleEncoder with EpochTime stated in o.addDefaults()", func() {
+	Context("with encoder options provided.", func() {
+
+		It("Should set Console Encoder, with given Nanos TimEncoder option.", func() {
 			logOut := new(bytes.Buffer)
-			log := New(UseDevMode(true), WriteTo(logOut))
+			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 default Epoch Time format
-			Expect(string(outRaw)).Should(ContainSubstring("-"))
 			// Assert for Console Encoder
 			res := map[string]interface{}{}
 			Expect(json.Unmarshal(outRaw, &res)).ToNot(Succeed())
-
-		})
-		It("Should set dev=false, and JSON encoder with EpochNanosTime stated in o.addDefaults()", func() {
-			logOut := new(bytes.Buffer)
-			log := New(UseDevMode(false), TimeEncoder(zapcore.EpochNanosTimeEncoder), WriteTo(logOut))
-			log.Info("This is a test message")
-			outRaw := logOut.Bytes()
-			// Assert for  Epoch Nanos time format
+			// Assert for Epoch Nanos TimeEncoder
 			Expect(string(outRaw)).ShouldNot(ContainSubstring("."))
-			// Assert for JSON Encoder
-			res := map[string]interface{}{}
-			Expect(json.Unmarshal(outRaw, &res)).To(Succeed())
 
 		})
-	})
-
-	Context("with -zap-encoder and -zap-time-encoder options provided.", func() {
-
-		It("Should set JSON Encoder, with given TimEncoder options.", func() {
-			args := []string{"-zap-encoder=json", "-zap-time-encoder=millis"}
-			fromFlags.BindFlags(&fs)
-			if err := fs.Parse(args); err != nil {
-				Expect(err).ToNot(HaveOccurred())
-			}
-
+		It("Should set JSON Encoder, with given Millis TimEncoder option, and MessageKey", func() {
 			logOut := new(bytes.Buffer)
-			log := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
-			log.Info("This is a test message")
-			outRaw := logOut.Bytes()
-
-			// Assert for millis Time format
-			Expect(string(outRaw)).Should(ContainSubstring("."))
-			// Assert for JSON Encoder
-			res := map[string]interface{}{}
-			Expect(json.Unmarshal(outRaw, &res)).To(Succeed())
-		})
-
-		It("Should set Console Encoder, with given TimEncoder options.", func() {
-			args := []string{"-zap-encoder=console", "-zap-time-encoder=nanos"}
-			fromFlags.BindFlags(&fs)
-			if err := fs.Parse(args); err != nil {
-				Expect(err).ToNot(HaveOccurred())
+			f := func(ec *zapcore.EncoderConfig) {
+				ec.MessageKey = "MillisTimeFormat"
+				if err := ec.EncodeTime.UnmarshalText([]byte("millis")); err != nil {
+					Expect(err).ToNot(HaveOccurred())
+				}
 			}
-
-			logOut := new(bytes.Buffer)
-			log := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
-			log.Info("This is a test message")
-			outRaw := logOut.Bytes()
-
-			// Assert for nanos Time format
-			Expect(string(outRaw)).ToNot(ContainSubstring("."))
-			// Assert for CONSOLE Encoder
-			res := map[string]interface{}{}
-			Expect(json.Unmarshal(outRaw, &res)).ToNot(Succeed())
-		})
-
-		It("Should set Console Encoder, with default epoch time format.", func() {
-			args := []string{"-zap-encoder=console", "-zap-time-encoder=dummyyyy"}
-			fromFlags.BindFlags(&fs)
-			if err := fs.Parse(args); err != nil {
-				Expect(err).ToNot(HaveOccurred())
+			opts := func(o *Options) {
+				o.EncoderConfigOptions = append(o.EncoderConfigOptions, f)
 			}
-
-			logOut := new(bytes.Buffer)
-			log := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
+			log := New(UseDevMode(false), WriteTo(logOut), opts)
 			log.Info("This is a test message")
 			outRaw := logOut.Bytes()
-
-			// Assert for default Epoch Time format
-			Expect(string(outRaw)).To(ContainSubstring("."))
-			// Assert for CONSOLE Encoder
+			// Assert for JSON Encoder
 			res := map[string]interface{}{}
-			Expect(json.Unmarshal(outRaw, &res)).ToNot(Succeed())
+			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"))
 		})
 
 	})