From 02c0ef4c4a13d6e6dc972161d3d33a6c9dd129d8 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Sun, 10 Apr 2022 23:24:31 +0200 Subject: [PATCH 1/7] feat: allow to configure level from string --- level/doc.go | 11 +++ level/example_test.go | 21 ++++++ level/level.go | 61 +++++++++++++++- level/level_test.go | 163 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 253 insertions(+), 3 deletions(-) diff --git a/level/doc.go b/level/doc.go index 505d307..f943d50 100644 --- a/level/doc.go +++ b/level/doc.go @@ -7,6 +7,17 @@ // logger = level.NewFilter(logger, level.AllowInfo()) // <-- // logger = log.With(logger, "ts", log.DefaultTimestampUTC) // +// It's also possible to configure log level from a string (for instance from +// a flag, environment variable or configuration file). +// +// fs := flag.NewFlagSet("example") +// lvl := fs.String("log-level", "", `"debug", "info", "warn" or "error"`) +// +// var logger log.Logger +// logger = log.NewLogfmtLogger(os.Stderr) +// logger = level.NewFilter(logger, level.Allow(level.ParseDefault(*lvl, level.InfoValue()))) // <-- +// logger = log.With(logger, "ts", log.DefaultTimestampUTC) +// // Then, at the callsites, use one of the level.Debug, Info, Warn, or Error // helper methods to emit leveled log events. // diff --git a/level/example_test.go b/level/example_test.go index 5f59f21..c751b3e 100644 --- a/level/example_test.go +++ b/level/example_test.go @@ -2,6 +2,7 @@ package level_test import ( "errors" + "flag" "os" "github.com/go-kit/log" @@ -37,3 +38,23 @@ func Example_filtered() { // level=error caller=example_test.go:32 err="bad data" // level=info caller=example_test.go:33 event="data saved" } + +func Example_parsed() { + fs := flag.NewFlagSet("example", flag.ExitOnError) + lvl := fs.String("log-level", "", `"debug", "info", "warn" or "error"`) + fs.Parse([]string{"-log-level", "info"}) + + // Set up logger with level filter. + logger := log.NewLogfmtLogger(os.Stdout) + logger = level.NewFilter(logger, level.Allow(level.ParseDefault(*lvl, level.DebugValue()))) + logger = log.With(logger, "caller", log.DefaultCaller) + + // Use level helpers to log at different levels. + level.Error(logger).Log("err", errors.New("bad data")) + level.Info(logger).Log("event", "data saved") + level.Debug(logger).Log("next item", 17) // filtered + + // Output: + // level=error caller=example_test.go:53 err="bad data" + // level=info caller=example_test.go:54 event="data saved" +} diff --git a/level/level.go b/level/level.go index c94756c..f5b312d 100644 --- a/level/level.go +++ b/level/level.go @@ -1,6 +1,15 @@ package level -import "github.com/go-kit/log" +import ( + "errors" + "strings" + + "github.com/go-kit/log" +) + +var ( + ErrInvalidLevelString = errors.New("invalid level string.") +) // Error returns a logger that includes a Key/ErrorValue pair. func Error(logger log.Logger) log.Logger { @@ -66,6 +75,26 @@ func (l *logger) Log(keyvals ...interface{}) error { // Option sets a parameter for the leveled logger. type Option func(*logger) +// Allow allows to configure the log level with a Value. +func Allow(v Value) Option { + if v != nil { + switch v.String() { + case debugValue.name: + return AllowDebug() + + case infoValue.name: + return AllowInfo() + + case warnValue.name: + return AllowWarn() + + case errorValue.name: + return AllowError() + } + } + return AllowNone() +} + // AllowAll is an alias for AllowDebug. func AllowAll() Option { return AllowDebug() @@ -100,6 +129,36 @@ func allowed(allowed level) Option { return func(l *logger) { l.allowed = allowed } } +// Parse returns a Value from a level string. Allowed values are "debug", "info", "warn" and "error". +// Levels are normalized via strings.TrimSpace and strings.ToLower +func Parse(level string) (Value, error) { + switch strings.TrimSpace(strings.ToLower(level)) { + case debugValue.name: + return debugValue, nil + + case infoValue.name: + return infoValue, nil + + case warnValue.name: + return warnValue, nil + + case errorValue.name: + return errorValue, nil + + default: + return nil, ErrInvalidLevelString + } +} + +// ParseDefault returns a Value from a level string or the default if the level is not valid. +func ParseDefault(level string, def Value) Value { + if v, err := Parse(level); err != nil { + return def + } else { + return v + } +} + // ErrNotAllowed sets the error to return from Log when it squelches a log // event disallowed by the configured Allow[Level] option. By default, // ErrNotAllowed is nil; in this case the log event is squelched with no diff --git a/level/level_test.go b/level/level_test.go index 035fa5d..07b22bf 100644 --- a/level/level_test.go +++ b/level/level_test.go @@ -12,11 +12,51 @@ import ( ) func TestVariousLevels(t *testing.T) { + testCases := []struct { name string allowed level.Option want string }{ + { + "Allow - Debug", + level.Allow(level.DebugValue()), + strings.Join([]string{ + `{"level":"debug","this is":"debug log"}`, + `{"level":"info","this is":"info log"}`, + `{"level":"warn","this is":"warn log"}`, + `{"level":"error","this is":"error log"}`, + }, "\n"), + }, + { + "Allow - Info", + level.Allow(level.InfoValue()), + strings.Join([]string{ + `{"level":"info","this is":"info log"}`, + `{"level":"warn","this is":"warn log"}`, + `{"level":"error","this is":"error log"}`, + }, "\n"), + }, + { + "Allow - Warn", + level.Allow(level.WarnValue()), + strings.Join([]string{ + `{"level":"warn","this is":"warn log"}`, + `{"level":"error","this is":"error log"}`, + }, "\n"), + }, + { + "Allow - Error", + level.Allow(level.ErrorValue()), + strings.Join([]string{ + `{"level":"error","this is":"error log"}`, + }, "\n"), + }, + { + "Allow - nil", + level.Allow(nil), + strings.Join([]string{}, "\n"), + }, { "AllowAll", level.AllowAll(), @@ -147,7 +187,7 @@ func TestLevelContext(t *testing.T) { logger = log.With(logger, "caller", log.DefaultCaller) level.Info(logger).Log("foo", "bar") - if want, have := `level=info caller=level_test.go:149 foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `level=info caller=level_test.go:184 foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } @@ -163,7 +203,7 @@ func TestContextLevel(t *testing.T) { logger = level.NewFilter(logger, level.AllowAll()) level.Info(logger).Log("foo", "bar") - if want, have := `caller=level_test.go:165 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `caller=level_test.go:200 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } @@ -233,3 +273,122 @@ func TestInjector(t *testing.T) { t.Errorf("wrong level value: got %#v, want %#v", got, want) } } + +func TestParse(t *testing.T) { + testCases := []struct { + name string + level string + want level.Value + wantErr bool + }{ + { + name: "Debug", + level: "debug", + want: level.DebugValue(), + wantErr: false, + }, + { + name: "Info", + level: "info", + want: level.InfoValue(), + wantErr: false, + }, + { + name: "Warn", + level: "warn", + want: level.WarnValue(), + wantErr: false, + }, + { + name: "Error", + level: "error", + want: level.ErrorValue(), + wantErr: false, + }, + { + name: "Case Insensitive", + level: "ErRoR", + want: level.ErrorValue(), + wantErr: false, + }, + { + name: "Trimmed", + level: " Error ", + want: level.ErrorValue(), + wantErr: false, + }, + { + name: "Invalid", + level: "invalid", + want: nil, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := level.Parse(tc.level) + if (err != nil) != tc.wantErr { + t.Errorf("got unexpected error %#v", err) + } + + if got != tc.want { + t.Errorf("wrong value: got=%#v, want=%#v", got, tc.want) + } + }) + } +} + +func TestParseDefault(t *testing.T) { + testCases := []struct { + name string + level string + want level.Value + }{ + { + name: "Debug", + level: "debug", + want: level.DebugValue(), + }, + { + name: "Info", + level: "info", + want: level.InfoValue(), + }, + { + name: "Warn", + level: "warn", + want: level.WarnValue(), + }, + { + name: "Error", + level: "error", + want: level.ErrorValue(), + }, + { + name: "Case Insensitive", + level: "ErRoR", + want: level.ErrorValue(), + }, + { + name: "Trimmed", + level: " Error ", + want: level.ErrorValue(), + }, + { + name: "Invalid", + level: "invalid", + want: level.InfoValue(), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := level.ParseDefault(tc.level, level.InfoValue()) + + if got != tc.want { + t.Errorf("wrong value: got=%#v, want=%#v", got, tc.want) + } + }) + } +} From ad6e4ef9f2e7d8c62fc3c9a9ef4e0f9810308080 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 11 Apr 2022 02:15:39 +0200 Subject: [PATCH 2/7] fix staticcheck violation --- level/level.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/level/level.go b/level/level.go index f5b312d..40f6239 100644 --- a/level/level.go +++ b/level/level.go @@ -8,7 +8,7 @@ import ( ) var ( - ErrInvalidLevelString = errors.New("invalid level string.") + ErrInvalidLevelString = errors.New("invalid level string") ) // Error returns a logger that includes a Key/ErrorValue pair. From 197b8819fc1f407bc5fcb35f3b3c988d54d120e4 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Mon, 11 Apr 2022 03:06:20 +0200 Subject: [PATCH 3/7] fix UTs --- level/example_test.go | 4 ++-- level/level_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/level/example_test.go b/level/example_test.go index c751b3e..d3d3e16 100644 --- a/level/example_test.go +++ b/level/example_test.go @@ -35,8 +35,8 @@ func Example_filtered() { level.Debug(logger).Log("next item", 17) // filtered // Output: - // level=error caller=example_test.go:32 err="bad data" - // level=info caller=example_test.go:33 event="data saved" + // level=error caller=example_test.go:33 err="bad data" + // level=info caller=example_test.go:34 event="data saved" } func Example_parsed() { diff --git a/level/level_test.go b/level/level_test.go index 07b22bf..94bdb91 100644 --- a/level/level_test.go +++ b/level/level_test.go @@ -187,7 +187,7 @@ func TestLevelContext(t *testing.T) { logger = log.With(logger, "caller", log.DefaultCaller) level.Info(logger).Log("foo", "bar") - if want, have := `level=info caller=level_test.go:184 foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `level=info caller=level_test.go:189 foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } @@ -203,7 +203,7 @@ func TestContextLevel(t *testing.T) { logger = level.NewFilter(logger, level.AllowAll()) level.Info(logger).Log("foo", "bar") - if want, have := `caller=level_test.go:200 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `caller=level_test.go:205 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } From 9a046be189851130faee577a91226ee07d00125e Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Wed, 13 Apr 2022 08:45:03 +0200 Subject: [PATCH 4/7] fix: apply suggested changes --- level/doc.go | 8 ++++---- level/level.go | 38 +++++++++++++++++++------------------- level/level_test.go | 15 +++++++-------- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/level/doc.go b/level/doc.go index f943d50..fd681dc 100644 --- a/level/doc.go +++ b/level/doc.go @@ -7,11 +7,11 @@ // logger = level.NewFilter(logger, level.AllowInfo()) // <-- // logger = log.With(logger, "ts", log.DefaultTimestampUTC) // -// It's also possible to configure log level from a string (for instance from -// a flag, environment variable or configuration file). +// It's also possible to configure log level from a string. For instance from +// a flag, environment variable or configuration file. // -// fs := flag.NewFlagSet("example") -// lvl := fs.String("log-level", "", `"debug", "info", "warn" or "error"`) +// fs := flag.NewFlagSet("myprogram") +// lvl := fs.String("log", "info", "debug, info, warn, error") // // var logger log.Logger // logger = log.NewLogfmtLogger(os.Stderr) diff --git a/level/level.go b/level/level.go index 40f6239..0f442b5 100644 --- a/level/level.go +++ b/level/level.go @@ -75,24 +75,24 @@ func (l *logger) Log(keyvals ...interface{}) error { // Option sets a parameter for the leveled logger. type Option func(*logger) -// Allow allows to configure the log level with a Value. +// Allow the provided log level to pass. func Allow(v Value) Option { - if v != nil { - switch v.String() { - case debugValue.name: - return AllowDebug() + switch v { + case debugValue: + return AllowDebug() - case infoValue.name: - return AllowInfo() + case infoValue: + return AllowInfo() - case warnValue.name: - return AllowWarn() + case warnValue: + return AllowWarn() - case errorValue.name: - return AllowError() - } + case errorValue: + return AllowError() + + default: + return AllowNone() } - return AllowNone() } // AllowAll is an alias for AllowDebug. @@ -129,8 +129,9 @@ func allowed(allowed level) Option { return func(l *logger) { l.allowed = allowed } } -// Parse returns a Value from a level string. Allowed values are "debug", "info", "warn" and "error". -// Levels are normalized via strings.TrimSpace and strings.ToLower +// Parse a string to it's corresponding level value. Valid strings are "debug", +// "info", "warn", and "error". Strings are normalized via strings.TrimSpace and +// strings.ToLower. func Parse(level string) (Value, error) { switch strings.TrimSpace(strings.ToLower(level)) { case debugValue.name: @@ -150,13 +151,12 @@ func Parse(level string) (Value, error) { } } -// ParseDefault returns a Value from a level string or the default if the level is not valid. +// ParseDefault calls Parse and returns the default Value on error. func ParseDefault(level string, def Value) Value { - if v, err := Parse(level); err != nil { - return def - } else { + if v, err := Parse(level); err == nil { return v } + return def } // ErrNotAllowed sets the error to return from Log when it squelches a log diff --git a/level/level_test.go b/level/level_test.go index 94bdb91..b1d3ba1 100644 --- a/level/level_test.go +++ b/level/level_test.go @@ -12,14 +12,13 @@ import ( ) func TestVariousLevels(t *testing.T) { - testCases := []struct { name string allowed level.Option want string }{ { - "Allow - Debug", + "Allow(DebugValue)", level.Allow(level.DebugValue()), strings.Join([]string{ `{"level":"debug","this is":"debug log"}`, @@ -29,7 +28,7 @@ func TestVariousLevels(t *testing.T) { }, "\n"), }, { - "Allow - Info", + "Allow(InfoValue)", level.Allow(level.InfoValue()), strings.Join([]string{ `{"level":"info","this is":"info log"}`, @@ -38,7 +37,7 @@ func TestVariousLevels(t *testing.T) { }, "\n"), }, { - "Allow - Warn", + "Allow(WarnValue)", level.Allow(level.WarnValue()), strings.Join([]string{ `{"level":"warn","this is":"warn log"}`, @@ -46,14 +45,14 @@ func TestVariousLevels(t *testing.T) { }, "\n"), }, { - "Allow - Error", + "Allow(ErrorValue)", level.Allow(level.ErrorValue()), strings.Join([]string{ `{"level":"error","this is":"error log"}`, }, "\n"), }, { - "Allow - nil", + "Allow(nil)", level.Allow(nil), strings.Join([]string{}, "\n"), }, @@ -187,7 +186,7 @@ func TestLevelContext(t *testing.T) { logger = log.With(logger, "caller", log.DefaultCaller) level.Info(logger).Log("foo", "bar") - if want, have := `level=info caller=level_test.go:189 foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `level=info caller=level_test.go:188 foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } @@ -203,7 +202,7 @@ func TestContextLevel(t *testing.T) { logger = level.NewFilter(logger, level.AllowAll()) level.Info(logger).Log("foo", "bar") - if want, have := `caller=level_test.go:205 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `caller=level_test.go:204 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("\nwant '%s'\nhave '%s'", want, have) } } From 3ce8b18c57fa3ce539a25e2d4f86eb79de4c950a Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Sun, 17 Apr 2022 17:33:38 +0200 Subject: [PATCH 5/7] fix: apply some of the suggested changes --- level/level.go | 15 +++------------ level/level_test.go | 18 +++++++++--------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/level/level.go b/level/level.go index 0f442b5..15dfab4 100644 --- a/level/level.go +++ b/level/level.go @@ -7,9 +7,8 @@ import ( "github.com/go-kit/log" ) -var ( - ErrInvalidLevelString = errors.New("invalid level string") -) +// ErrInvalidLevelString is returned whenever an invalid string is passed to Parse +var ErrInvalidLevelString = errors.New("invalid level string") // Error returns a logger that includes a Key/ErrorValue pair. func Error(logger log.Logger) log.Logger { @@ -80,16 +79,12 @@ func Allow(v Value) Option { switch v { case debugValue: return AllowDebug() - case infoValue: return AllowInfo() - case warnValue: return AllowWarn() - case errorValue: return AllowError() - default: return AllowNone() } @@ -129,23 +124,19 @@ func allowed(allowed level) Option { return func(l *logger) { l.allowed = allowed } } -// Parse a string to it's corresponding level value. Valid strings are "debug", +// Parse a string to its corresponding level value. Valid strings are "debug", // "info", "warn", and "error". Strings are normalized via strings.TrimSpace and // strings.ToLower. func Parse(level string) (Value, error) { switch strings.TrimSpace(strings.ToLower(level)) { case debugValue.name: return debugValue, nil - case infoValue.name: return infoValue, nil - case warnValue.name: return warnValue, nil - case errorValue.name: return errorValue, nil - default: return nil, ErrInvalidLevelString } diff --git a/level/level_test.go b/level/level_test.go index b1d3ba1..b42bc19 100644 --- a/level/level_test.go +++ b/level/level_test.go @@ -278,56 +278,56 @@ func TestParse(t *testing.T) { name string level string want level.Value - wantErr bool + wantErr error }{ { name: "Debug", level: "debug", want: level.DebugValue(), - wantErr: false, + wantErr: nil, }, { name: "Info", level: "info", want: level.InfoValue(), - wantErr: false, + wantErr: nil, }, { name: "Warn", level: "warn", want: level.WarnValue(), - wantErr: false, + wantErr: nil, }, { name: "Error", level: "error", want: level.ErrorValue(), - wantErr: false, + wantErr: nil, }, { name: "Case Insensitive", level: "ErRoR", want: level.ErrorValue(), - wantErr: false, + wantErr: nil, }, { name: "Trimmed", level: " Error ", want: level.ErrorValue(), - wantErr: false, + wantErr: nil, }, { name: "Invalid", level: "invalid", want: nil, - wantErr: true, + wantErr: level.ErrInvalidLevelString, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { got, err := level.Parse(tc.level) - if (err != nil) != tc.wantErr { + if err != tc.wantErr { t.Errorf("got unexpected error %#v", err) } From 054363cfdb85434ab0318cbcfb0dfa6d817e99b8 Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Tue, 19 Apr 2022 14:12:24 +0200 Subject: [PATCH 6/7] fix: more idiomatic code --- level/level.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/level/level.go b/level/level.go index 15dfab4..680c9a6 100644 --- a/level/level.go +++ b/level/level.go @@ -144,10 +144,11 @@ func Parse(level string) (Value, error) { // ParseDefault calls Parse and returns the default Value on error. func ParseDefault(level string, def Value) Value { - if v, err := Parse(level); err == nil { - return v + v, err := Parse(level) + if err != nil { + return def } - return def + return v } // ErrNotAllowed sets the error to return from Log when it squelches a log From fbee0cceb1a15ca7720c2bb1ed5793b3711582aa Mon Sep 17 00:00:00 2001 From: Massimo Costa Date: Wed, 20 Apr 2022 19:10:47 +0200 Subject: [PATCH 7/7] Update level/level.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Márk Sági-Kazár --- level/level.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/level/level.go b/level/level.go index 680c9a6..c641d98 100644 --- a/level/level.go +++ b/level/level.go @@ -7,7 +7,7 @@ import ( "github.com/go-kit/log" ) -// ErrInvalidLevelString is returned whenever an invalid string is passed to Parse +// ErrInvalidLevelString is returned whenever an invalid string is passed to Parse. var ErrInvalidLevelString = errors.New("invalid level string") // Error returns a logger that includes a Key/ErrorValue pair.