From 33559ea031dd0179bbc8702c407c4e9491fdb221 Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Sat, 29 Jul 2017 07:31:33 +0100 Subject: [PATCH 1/5] Switch to kingpin in log --- log/log.go | 74 +++++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 45 deletions(-) diff --git a/log/log.go b/log/log.go index 1321741ad..54f152ac0 100644 --- a/log/log.go +++ b/log/log.go @@ -14,7 +14,6 @@ package log import ( - "flag" "fmt" "io" "io/ioutil" @@ -25,26 +24,11 @@ import ( "strconv" "strings" + "gopkg.in/alecthomas/kingpin.v2" + "github.com/sirupsen/logrus" ) -type levelFlag string - -// String implements flag.Value. -func (f levelFlag) String() string { - return fmt.Sprintf("%q", origLogger.Level.String()) -} - -// Set implements flag.Value. -func (f levelFlag) Set(level string) error { - l, err := logrus.ParseLevel(level) - if err != nil { - return err - } - origLogger.Level = l - return nil -} - // setSyslogFormatter is nil if the target architecture does not support syslog. var setSyslogFormatter func(logger, string, string) error @@ -55,38 +39,38 @@ func setJSONFormatter() { origLogger.Formatter = &logrus.JSONFormatter{} } -type logFormatFlag url.URL - -// String implements flag.Value. -func (f logFormatFlag) String() string { - u := url.URL(f) - return fmt.Sprintf("%q", u.String()) +type loggerSettings struct { + level string + format string } -// Set implements flag.Value. -func (f logFormatFlag) Set(format string) error { - return baseLogger.SetFormat(format) +func (s *loggerSettings) apply(ctx *kingpin.ParseContext) error { + err := baseLogger.SetLevel(s.level) + if err != nil { + return err + } + err = baseLogger.SetFormat(s.format) + return err } func init() { - AddFlags(flag.CommandLine) -} - -// AddFlags adds the flags used by this package to the given FlagSet. That's -// useful if working with a custom FlagSet. The init function of this package -// adds the flags to flag.CommandLine anyway. Thus, it's usually enough to call -// flag.Parse() to make the logging flags take effect. -func AddFlags(fs *flag.FlagSet) { - fs.Var( - levelFlag(origLogger.Level.String()), - "log.level", - "Only log messages with the given severity or above. Valid levels: [debug, info, warn, error, fatal]", - ) - fs.Var( - logFormatFlag(url.URL{Scheme: "logger", Opaque: "stderr"}), - "log.format", - `Set the log target and format. Example: "logger:syslog?appname=bob&local=7" or "logger:stdout?json=true"`, - ) + AddFlags(kingpin.CommandLine) +} + +// AddFlags adds the flags used by this package to the Kingpin application. That's +// useful if working with a custom application. The init function of this package +// adds the flags to the default instance anyway. Thus, it's usually enough to call +// kingpin.Parse() to make the logging flags take effect. +func AddFlags(a *kingpin.Application) { + s := loggerSettings{} + kingpin.Flag("log.level", "Only log messages with the given severity or above. Valid levels: [debug, info, warn, error, fatal]"). + Default(origLogger.Level.String()). + StringVar(&s.level) + defaultFormat := url.URL{Scheme: "logger", Opaque: "stderr"} + kingpin.Flag("log.format", `Set the log target and format. Example: "logger:syslog?appname=bob&local=7" or "logger:stdout?json=true"`). + Default(defaultFormat.String()). + StringVar(&s.format) + a.Action(s.apply) } // Logger is the interface for loggers used in the Prometheus components. From e1c8772d083ee69ce07a4bf93b76131b7786b300 Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Sun, 30 Jul 2017 08:43:01 +0100 Subject: [PATCH 2/5] Remove init adding log flags --- log/log.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/log/log.go b/log/log.go index 54f152ac0..ac74b1030 100644 --- a/log/log.go +++ b/log/log.go @@ -53,14 +53,6 @@ func (s *loggerSettings) apply(ctx *kingpin.ParseContext) error { return err } -func init() { - AddFlags(kingpin.CommandLine) -} - -// AddFlags adds the flags used by this package to the Kingpin application. That's -// useful if working with a custom application. The init function of this package -// adds the flags to the default instance anyway. Thus, it's usually enough to call -// kingpin.Parse() to make the logging flags take effect. func AddFlags(a *kingpin.Application) { s := loggerSettings{} kingpin.Flag("log.level", "Only log messages with the given severity or above. Valid levels: [debug, info, warn, error, fatal]"). From a9c68e6d3eb961017c427ee8c6aadd9089cf3a33 Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Sun, 30 Jul 2017 10:04:54 +0100 Subject: [PATCH 3/5] Re-add AddFlags doc string --- log/log.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/log/log.go b/log/log.go index ac74b1030..b8e3216d3 100644 --- a/log/log.go +++ b/log/log.go @@ -53,6 +53,8 @@ func (s *loggerSettings) apply(ctx *kingpin.ParseContext) error { return err } +// AddFlags adds the flags used by this package to the Kingpin application. +// To use the default Kingpin application, call AddFlags(kingpin.CommandLine) func AddFlags(a *kingpin.Application) { s := loggerSettings{} kingpin.Flag("log.level", "Only log messages with the given severity or above. Valid levels: [debug, info, warn, error, fatal]"). From 2cbc42c09a9b873fb57b89b13fff0bb26a509a2d Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Sun, 30 Jul 2017 10:29:16 +0100 Subject: [PATCH 4/5] Fix log test regexp --- log/log_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/log/log_test.go b/log/log_test.go index 2cd2b18e2..f63b4417f 100644 --- a/log/log_test.go +++ b/log/log_test.go @@ -32,7 +32,7 @@ func TestFileLineLogging(t *testing.T) { Debug("This debug-level line should not show up in the output.") Infof("This %s-level line should show up in the output.", "info") - re := `^time=".*" level=info msg="This info-level line should show up in the output." source="log_test.go:33" \n$` + re := `^time=".*" level=info msg="This info-level line should show up in the output." source="log_test.go:33"\n$` if !regexp.MustCompile(re).Match(buf.Bytes()) { t.Fatalf("%q did not match expected regex %q", buf.String(), re) } From 8ba51016a21456f1649877d7079f416d69eb3948 Mon Sep 17 00:00:00 2001 From: Calle Pettersson Date: Mon, 31 Jul 2017 10:30:31 +0100 Subject: [PATCH 5/5] Fix import order --- log/log.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/log/log.go b/log/log.go index b8e3216d3..04e906c07 100644 --- a/log/log.go +++ b/log/log.go @@ -24,9 +24,8 @@ import ( "strconv" "strings" - "gopkg.in/alecthomas/kingpin.v2" - "github.com/sirupsen/logrus" + "gopkg.in/alecthomas/kingpin.v2" ) // setSyslogFormatter is nil if the target architecture does not support syslog.