From ae9229128b327d2ef0b2370e0478bdd6144fcb67 Mon Sep 17 00:00:00 2001 From: Victor Vrantchan Date: Thu, 9 Feb 2017 01:51:34 -0500 Subject: [PATCH 1/2] Use variadic function to pass parameters instead of Config struct Using function to pass optional params makes the API more consistent with the rest of go-kit packages. For #427 --- log/experimental_level/benchmark_test.go | 30 ++++------ log/experimental_level/doc.go | 6 +- log/experimental_level/level.go | 76 +++++++++++++----------- log/experimental_level/level_test.go | 37 ++++++------ 4 files changed, 77 insertions(+), 72 deletions(-) diff --git a/log/experimental_level/benchmark_test.go b/log/experimental_level/benchmark_test.go index 176af248d..649e33de4 100644 --- a/log/experimental_level/benchmark_test.go +++ b/log/experimental_level/benchmark_test.go @@ -13,15 +13,13 @@ func BenchmarkNopBaseline(b *testing.B) { } func BenchmarkNopDisallowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewNopLogger(), level.Config{ - Allowed: level.AllowInfoAndAbove(), - })) + benchmarkRunner(b, level.New(log.NewNopLogger(), + level.Allowed(level.AllowInfoAndAbove()))) } func BenchmarkNopAllowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewNopLogger(), level.Config{ - Allowed: level.AllowAll(), - })) + benchmarkRunner(b, level.New(log.NewNopLogger(), + level.Allowed(level.AllowAll()))) } func BenchmarkJSONBaseline(b *testing.B) { @@ -29,15 +27,13 @@ func BenchmarkJSONBaseline(b *testing.B) { } func BenchmarkJSONDisallowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewJSONLogger(ioutil.Discard), level.Config{ - Allowed: level.AllowInfoAndAbove(), - })) + benchmarkRunner(b, level.New(log.NewJSONLogger(ioutil.Discard), + level.Allowed(level.AllowInfoAndAbove()))) } func BenchmarkJSONAllowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewJSONLogger(ioutil.Discard), level.Config{ - Allowed: level.AllowAll(), - })) + benchmarkRunner(b, level.New(log.NewJSONLogger(ioutil.Discard), + level.Allowed(level.AllowAll()))) } func BenchmarkLogfmtBaseline(b *testing.B) { @@ -45,15 +41,13 @@ func BenchmarkLogfmtBaseline(b *testing.B) { } func BenchmarkLogfmtDisallowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewLogfmtLogger(ioutil.Discard), level.Config{ - Allowed: level.AllowInfoAndAbove(), - })) + benchmarkRunner(b, level.New(log.NewLogfmtLogger(ioutil.Discard), + level.Allowed(level.AllowInfoAndAbove()))) } func BenchmarkLogfmtAllowedLevel(b *testing.B) { - benchmarkRunner(b, level.New(log.NewLogfmtLogger(ioutil.Discard), level.Config{ - Allowed: level.AllowAll(), - })) + benchmarkRunner(b, level.New(log.NewLogfmtLogger(ioutil.Discard), + level.Allowed(level.AllowAll()))) } func benchmarkRunner(b *testing.B, logger log.Logger) { diff --git a/log/experimental_level/doc.go b/log/experimental_level/doc.go index 7b81a9756..6c0055973 100644 --- a/log/experimental_level/doc.go +++ b/log/experimental_level/doc.go @@ -6,7 +6,7 @@ // // var logger log.Logger // logger = log.NewLogfmtLogger(os.Stderr) -// logger = level.New(logger, level.Config{Allowed: level.AllowInfoAndAbove}) // <-- +// logger = level.New(logger, level.Allowed(level.AllowInfoAndAbove())) // <-- // logger = log.NewContext(logger).With("ts", log.DefaultTimestampUTC) // // Then, at the callsites, use one of the level.Debug, Info, Warn, or Error @@ -20,8 +20,8 @@ // // The leveled logger allows precise control over what should happen if a log // event is emitted without a level key, or if a squelched level is used. Check -// the Config struct for details. And, you can easily use non-default level +// the Option functions for details. And, you can easily use non-default level // values: create new string constants for whatever you want to change, pass -// them explicitly to the Config struct, and write your own level.Foo-style +// them explicitly to the Allowed Option function, and write your own level.Foo-style // helper methods. package level diff --git a/log/experimental_level/level.go b/log/experimental_level/level.go index 92e780db9..1781eb6b8 100644 --- a/log/experimental_level/level.go +++ b/log/experimental_level/level.go @@ -18,31 +18,31 @@ func AllowAll() []string { } // AllowDebugAndAbove allows all of the four default log levels. -// Its return value may be provided as the Allowed parameter in the Config. +// Its return value may be provided with the Allowed Option. func AllowDebugAndAbove() []string { return []string{errorLevelValue, warnLevelValue, infoLevelValue, debugLevelValue} } // AllowInfoAndAbove allows the default info, warn, and error log levels. -// Its return value may be provided as the Allowed parameter in the Config. +// Its return value may be provided with the Allowed Option. func AllowInfoAndAbove() []string { return []string{errorLevelValue, warnLevelValue, infoLevelValue} } // AllowWarnAndAbove allows the default warn and error log levels. -// Its return value may be provided as the Allowed parameter in the Config. +// Its return value may be provided with the Allowed Option. func AllowWarnAndAbove() []string { return []string{errorLevelValue, warnLevelValue} } // AllowErrorOnly allows only the default error log level. -// Its return value may be provided as the Allowed parameter in the Config. +// Its return value may be provided with the Allowed Option. func AllowErrorOnly() []string { return []string{errorLevelValue} } // AllowNone allows none of the default log levels. -// Its return value may be provided as the Allowed parameter in the Config. +// Its return value may be provided with the Allowed Option. func AllowNone() []string { return []string{} } @@ -67,42 +67,50 @@ func Debug(logger log.Logger) log.Logger { return log.NewContext(logger).WithPrefix(levelKey, debugLevelValue) } -// Config parameterizes the leveled logger. -type Config struct { - // Allowed enumerates the accepted log levels. If a log event is encountered - // with a level key set to a value that isn't explicitly allowed, the event - // will be squelched, and ErrNotAllowed returned. - Allowed []string +// New wraps the logger and implements level checking. See the commentary on the +// Option functions for a detailed description of how to configure levels. +func New(next log.Logger, options ...Option) log.Logger { + l := logger{ + next: next, + } + for _, option := range options { + option(&l) + } + return &l +} - // ErrNotAllowed is returned to the caller when Log is invoked with a level - // key that hasn't been explicitly allowed. By default, ErrNotAllowed is - // nil; in this case, the log event is squelched with no error. - ErrNotAllowed error +// Allowed enumerates the accepted log levels. If a log event is encountered +// with a level key set to a value that isn't explicitly allowed, the event +// will be squelched, and ErrNotAllowed returned. +func Allowed(allowed []string) Option { + return func(l *logger) { l.allowed = makeSet(allowed) } +} - // SquelchNoLevel will squelch log events with no level key, so that they - // don't proceed through to the wrapped logger. If SquelchNoLevel is set to - // true and a log event is squelched in this way, ErrNoLevel is returned to - // the caller. - SquelchNoLevel bool +// ErrNoLevel is returned to the caller when SquelchNoLevel is true, and Log +// is invoked without a level key. By default, ErrNoLevel is nil; in this +// case, the log event is squelched with no error. +func ErrNotAllowed(err error) Option { + return func(l *logger) { l.errNotAllowed = err } +} - // ErrNoLevel is returned to the caller when SquelchNoLevel is true, and Log - // is invoked without a level key. By default, ErrNoLevel is nil; in this - // case, the log event is squelched with no error. - ErrNoLevel error +// SquelchNoLevel will squelch log events with no level key, so that they +// don't proceed through to the wrapped logger. If SquelchNoLevel is set to +// true and a log event is squelched in this way, ErrNoLevel is returned to +// the caller. +func SquelchNoLevel(squelch bool) Option { + return func(l *logger) { l.squelchNoLevel = squelch } } -// New wraps the logger and implements level checking. See the commentary on the -// Config object for a detailed description of how to configure levels. -func New(next log.Logger, config Config) log.Logger { - return &logger{ - next: next, - allowed: makeSet(config.Allowed), - errNotAllowed: config.ErrNotAllowed, - squelchNoLevel: config.SquelchNoLevel, - errNoLevel: config.ErrNoLevel, - } +// ErrNoLevel is returned to the caller when SquelchNoLevel is true, and Log +// is invoked without a level key. By default, ErrNoLevel is nil; in this +// case, the log event is squelched with no error. +func ErrNoLevel(err error) Option { + return func(l *logger) { l.errNoLevel = err } } +// Option sets a parameter for the leveled logger. +type Option func(*logger) + type logger struct { next log.Logger allowed map[string]struct{} diff --git a/log/experimental_level/level_test.go b/log/experimental_level/level_test.go index 09674f23f..f067b43bb 100644 --- a/log/experimental_level/level_test.go +++ b/log/experimental_level/level_test.go @@ -60,7 +60,7 @@ func TestVariousLevels(t *testing.T) { }, } { var buf bytes.Buffer - logger := level.New(log.NewJSONLogger(&buf), level.Config{Allowed: testcase.allowed}) + logger := level.New(log.NewJSONLogger(&buf), level.Allowed(testcase.allowed)) level.Debug(logger).Log("this is", "debug log") level.Info(logger).Log("this is", "info log") @@ -75,10 +75,11 @@ func TestVariousLevels(t *testing.T) { func TestErrNotAllowed(t *testing.T) { myError := errors.New("squelched!") - logger := level.New(log.NewNopLogger(), level.Config{ - Allowed: level.AllowWarnAndAbove(), - ErrNotAllowed: myError, - }) + opts := []level.Option{ + level.Allowed(level.AllowWarnAndAbove()), + level.ErrNotAllowed(myError), + } + logger := level.New(log.NewNopLogger(), opts...) if want, have := myError, level.Info(logger).Log("foo", "bar"); want != have { t.Errorf("want %#+v, have %#+v", want, have) @@ -93,10 +94,11 @@ func TestErrNoLevel(t *testing.T) { myError := errors.New("no level specified") var buf bytes.Buffer - logger := level.New(log.NewJSONLogger(&buf), level.Config{ - SquelchNoLevel: true, - ErrNoLevel: myError, - }) + opts := []level.Option{ + level.SquelchNoLevel(true), + level.ErrNoLevel(myError), + } + logger := level.New(log.NewJSONLogger(&buf), opts...) if want, have := myError, logger.Log("foo", "bar"); want != have { t.Errorf("want %v, have %v", want, have) @@ -108,10 +110,11 @@ func TestErrNoLevel(t *testing.T) { func TestAllowNoLevel(t *testing.T) { var buf bytes.Buffer - logger := level.New(log.NewJSONLogger(&buf), level.Config{ - SquelchNoLevel: false, - ErrNoLevel: errors.New("I should never be returned!"), - }) + opts := []level.Option{ + level.SquelchNoLevel(false), + level.ErrNoLevel(errors.New("I should never be returned!")), + } + logger := level.New(log.NewJSONLogger(&buf), opts...) if want, have := error(nil), logger.Log("foo", "bar"); want != have { t.Errorf("want %v, have %v", want, have) @@ -128,11 +131,11 @@ func TestLevelContext(t *testing.T) { // log.DefaultCaller as per normal. var logger log.Logger logger = log.NewLogfmtLogger(&buf) - logger = level.New(logger, level.Config{Allowed: level.AllowAll()}) + logger = level.New(logger, level.Allowed(level.AllowAll())) logger = log.NewContext(logger).With("caller", log.DefaultCaller) level.Info(logger).Log("foo", "bar") - if want, have := `level=info caller=level_test.go:134 foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `level=info caller=level_test.go:137 foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("want %q, have %q", want, have) } } @@ -145,10 +148,10 @@ func TestContextLevel(t *testing.T) { var logger log.Logger logger = log.NewLogfmtLogger(&buf) logger = log.NewContext(logger).With("caller", log.Caller(5)) - logger = level.New(logger, level.Config{Allowed: level.AllowAll()}) + logger = level.New(logger, level.Allowed(level.AllowAll())) level.Info(logger).Log("foo", "bar") - if want, have := `caller=level_test.go:150 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { + if want, have := `caller=level_test.go:153 level=info foo=bar`, strings.TrimSpace(buf.String()); want != have { t.Errorf("want %q, have %q", want, have) } } From 2fbb1fc92316348e4a26d97323d1ee1228325664 Mon Sep 17 00:00:00 2001 From: Victor Vrantchan Date: Thu, 9 Feb 2017 23:01:15 -0500 Subject: [PATCH 2/2] add message describing default behavior --- log/experimental_level/level.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/log/experimental_level/level.go b/log/experimental_level/level.go index 1781eb6b8..6dfd71adb 100644 --- a/log/experimental_level/level.go +++ b/log/experimental_level/level.go @@ -69,6 +69,8 @@ func Debug(logger log.Logger) log.Logger { // New wraps the logger and implements level checking. See the commentary on the // Option functions for a detailed description of how to configure levels. +// If no options are provided, all leveled log events created with level.Debug, +// Info, Warn or Error helper methods will be squelched. func New(next log.Logger, options ...Option) log.Logger { l := logger{ next: next,