From ea01d003d1fe1a95a118913d3798544cce3869ac Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 4 May 2017 19:39:16 +0400 Subject: [PATCH] changes per @ethanfrey comments --- log/logger.go | 50 ++++++++++++++---------------------------- log/nop_logger.go | 11 ++++++++++ log/nop_logger_test.go | 2 +- log/testing_logger.go | 2 +- log/tm_logger.go | 25 +++++++++++++++++++++ log/tm_logger_test.go | 6 ++--- 6 files changed, 57 insertions(+), 39 deletions(-) diff --git a/log/logger.go b/log/logger.go index a895aed6..bf6a3ff6 100644 --- a/log/logger.go +++ b/log/logger.go @@ -1,10 +1,9 @@ package log import ( - "fmt" + "io" kitlog "github.com/go-kit/kit/log" - "github.com/go-kit/kit/log/level" ) // Logger is what any Tendermint library should take. @@ -12,38 +11,21 @@ type Logger interface { Debug(msg string, keyvals ...interface{}) error Info(msg string, keyvals ...interface{}) error Error(msg string, keyvals ...interface{}) error + + With(keyvals ...interface{}) Logger + WithLevel(lvl string) Logger } -// With returns a new contextual logger with keyvals prepended to those passed -// to calls to Info, Debug or Error. -func With(logger Logger, keyvals ...interface{}) Logger { - switch logger.(type) { - case *tmLogger: - return &tmLogger{kitlog.With(logger.(*tmLogger).srcLogger, keyvals...)} - case *nopLogger: - return logger - default: - panic(fmt.Sprintf("Unexpected logger of type %T", logger)) - } -} - -// WithLevel returns a copy of the logger with a level set to lvl. -func WithLevel(logger Logger, lvl string) Logger { - switch logger.(type) { - case *tmLogger: - switch lvl { - case "info": - return &tmLogger{level.NewFilter(logger.(*tmLogger).srcLogger, level.AllowInfo())} - case "debug": - return &tmLogger{level.NewFilter(logger.(*tmLogger).srcLogger, level.AllowDebug())} - case "error": - return &tmLogger{level.NewFilter(logger.(*tmLogger).srcLogger, level.AllowError())} - default: - panic(fmt.Sprintf("Unexpected level %v, expect either \"info\" or \"debug\" or \"error\"", lvl)) - } - case *nopLogger: - return logger - default: - panic(fmt.Sprintf("Unexpected logger of type %T", logger)) - } +// NewSyncWriter returns a new writer that is safe for concurrent use by +// multiple goroutines. Writes to the returned writer are passed on to w. If +// another write is already in progress, the calling goroutine blocks until +// the writer is available. +// +// If w implements the following interface, so does the returned writer. +// +// interface { +// Fd() uintptr +// } +func NewSyncWriter(w io.Writer) io.Writer { + return kitlog.NewSyncWriter(w) } diff --git a/log/nop_logger.go b/log/nop_logger.go index b6e31263..21999817 100644 --- a/log/nop_logger.go +++ b/log/nop_logger.go @@ -2,6 +2,9 @@ package log type nopLogger struct{} +// Interface assertions +var _ Logger = (*nopLogger)(nil) + // NewNopLogger returns a logger that doesn't do anything. func NewNopLogger() Logger { return &nopLogger{} } @@ -16,3 +19,11 @@ func (nopLogger) Debug(string, ...interface{}) error { func (nopLogger) Error(string, ...interface{}) error { return nil } + +func (l *nopLogger) With(...interface{}) Logger { + return l +} + +func (l *nopLogger) WithLevel(lvl string) Logger { + return l +} diff --git a/log/nop_logger_test.go b/log/nop_logger_test.go index 9757d4f1..d2009fdf 100644 --- a/log/nop_logger_test.go +++ b/log/nop_logger_test.go @@ -12,7 +12,7 @@ func TestNopLogger(t *testing.T) { if err := logger.Info("Hello", "abc", 123); err != nil { t.Error(err) } - if err := log.With(logger, "def", "ghi").Debug(""); err != nil { + if err := logger.With("def", "ghi").Debug(""); err != nil { t.Error(err) } } diff --git a/log/testing_logger.go b/log/testing_logger.go index 7ab83e41..31913633 100644 --- a/log/testing_logger.go +++ b/log/testing_logger.go @@ -22,7 +22,7 @@ func TestingLogger() Logger { } if testing.Verbose() { - _testingLogger = NewTMLogger(os.Stdout) + _testingLogger = NewTMLogger(NewSyncWriter(os.Stdout)) } else { _testingLogger = NewNopLogger() } diff --git a/log/tm_logger.go b/log/tm_logger.go index d8550ea6..3a3c9dde 100644 --- a/log/tm_logger.go +++ b/log/tm_logger.go @@ -17,9 +17,14 @@ type tmLogger struct { srcLogger kitlog.Logger } +// Interface assertions +var _ Logger = (*tmLogger)(nil) + // NewTMTermLogger returns a logger that encodes msg and keyvals to the Writer // using go-kit's log as an underlying logger and our custom formatter. Note // that underlying logger could be swapped with something else. +// +// Default logging level is info. You can change it using SetLevel(). func NewTMLogger(w io.Writer) Logger { // Color by level value colorFn := func(keyvals ...interface{}) term.FgBgColor { @@ -58,3 +63,23 @@ func (l *tmLogger) Error(msg string, keyvals ...interface{}) error { lWithLevel := level.Error(l.srcLogger) return kitlog.With(lWithLevel, msgKey, msg).Log(keyvals...) } + +// With returns a new contextual logger with keyvals prepended to those passed +// to calls to Info, Debug or Error. +func (l *tmLogger) With(keyvals ...interface{}) Logger { + return &tmLogger{kitlog.With(l.srcLogger, keyvals...)} +} + +// WithLevel returns a new logger with the level set to lvl. +func (l *tmLogger) WithLevel(lvl string) Logger { + switch lvl { + case "info": + return &tmLogger{level.NewFilter(l.srcLogger, level.AllowInfo())} + case "debug": + return &tmLogger{level.NewFilter(l.srcLogger, level.AllowDebug())} + case "error": + return &tmLogger{level.NewFilter(l.srcLogger, level.AllowError())} + default: + panic(fmt.Sprintf("Unexpected level %v, expect either \"info\" or \"debug\" or \"error\"", lvl)) + } +} diff --git a/log/tm_logger_test.go b/log/tm_logger_test.go index 898316c4..15c940ce 100644 --- a/log/tm_logger_test.go +++ b/log/tm_logger_test.go @@ -13,7 +13,7 @@ func TestTMLogger(t *testing.T) { if err := logger.Info("Hello", "abc", 123); err != nil { t.Error(err) } - if err := log.With(logger, "def", "ghi").Debug(""); err != nil { + if err := logger.With("def", "ghi").Debug(""); err != nil { t.Error(err) } } @@ -27,7 +27,7 @@ func BenchmarkTMLoggerContextual(b *testing.B) { } func benchmarkRunner(b *testing.B, logger log.Logger, f func(log.Logger)) { - lc := log.With(logger, "common_key", "common_value") + lc := logger.With("common_key", "common_value") b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { @@ -37,5 +37,5 @@ func benchmarkRunner(b *testing.B, logger log.Logger, f func(log.Logger)) { var ( baseInfoMessage = func(logger log.Logger) { logger.Info("foo_message", "foo_key", "foo_value") } - withInfoMessage = func(logger log.Logger) { log.With(logger, "a", "b").Info("c", "d", "f") } + withInfoMessage = func(logger log.Logger) { logger.With("a", "b").Info("c", "d", "f") } )