From 0f4b91816a6c6ffb0e3bbf771c6fe9e787061b70 Mon Sep 17 00:00:00 2001 From: Michael Porter Date: Thu, 3 Aug 2017 22:03:10 -0400 Subject: [PATCH 1/2] Add and use Logger / FancyLogger interface to permit using loggers other than stdlib, eg. golog, logrus, etc. --- logging.go | 71 +++++++++++++++++++---- logging_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 207 insertions(+), 13 deletions(-) diff --git a/logging.go b/logging.go index 3d9aad5..df28bdb 100644 --- a/logging.go +++ b/logging.go @@ -22,64 +22,113 @@ import ( "os" ) +// Logger is a generic logging interface, to allow utilizing +// any stdlib-compatible logger for logging +type Logger interface { + Printf(format string, v ...interface{}) + Println(v ...interface{}) +} + +// FancyLogger represents a leveled logger, with methods representing +// debug, warn, and error levels +type FancyLogger interface { + Logger + Debug(v ...interface{}) + Debugf(format string, v ...interface{}) + Warn(v ...interface{}) + Warnf(format string, v ...interface{}) + Error(v ...interface{}) + Errorf(format string, v ...interface{}) +} + // If true, debug messages will be written to the log var EnableDebugLogging = false -var errLogger = log.New(os.Stderr, "", log.LstdFlags) -var stdLogger = log.New(os.Stderr, "", log.LstdFlags) +var errLogger Logger +var stdLogger Logger func init() { if debugEnvVar := os.Getenv("RIAK_GO_CLIENT_DEBUG"); debugEnvVar != "" { EnableDebugLogging = true } + errLogger = log.New(os.Stderr, "", log.LstdFlags) + stdLogger = log.New(os.Stderr, "", log.LstdFlags) } // SetLogger sets the standard logger used for // WARN and DEBUG (if enabled) -func SetLogger(logger *log.Logger) { +func SetLogger(logger Logger) { stdLogger = logger } // SetErrorLogger sets the logger used for errors -func SetErrorLogger(logger *log.Logger) { +func SetErrorLogger(logger Logger) { errLogger = logger } // logDebug writes formatted string debug messages using Printf only if debug logging is enabled func logDebug(source, format string, v ...interface{}) { if EnableDebugLogging { - stdLogger.Printf(fmt.Sprintf("[DEBUG] %s %s", source, format), v...) + if fancyLogger, ok := stdLogger.(FancyLogger); ok { + fancyLogger.Debugf(fmt.Sprintf("%s %s", source, format), v...) + } else { + stdLogger.Printf(fmt.Sprintf("[DEBUG] %s %s", source, format), v...) + } } } // logDebugln writes string debug messages using Println func logDebugln(source string, v ...interface{}) { if EnableDebugLogging { - stdLogger.Println("[DEBUG]", source, v) + if fancyLogger, ok := stdLogger.(FancyLogger); ok { + fancyLogger.Debug(source, v) + } else { + stdLogger.Println("[DEBUG]", source, v) + } } } // logWarn writes formatted string warning messages using Printf func logWarn(source, format string, v ...interface{}) { - stdLogger.Printf(fmt.Sprintf("[WARNING] %s %s", source, format), v...) + if fancyLogger, ok := stdLogger.(FancyLogger); ok { + fancyLogger.Warnf(fmt.Sprintf("%s %s", source, format), v...) + } else { + stdLogger.Printf(fmt.Sprintf("[WARNING] %s %s", source, format), v...) + } } // logWarnln writes string warning messages using Println func logWarnln(source string, v ...interface{}) { - stdLogger.Println("[WARNING]", source, v) + if fancyLogger, ok := stdLogger.(FancyLogger); ok { + fancyLogger.Warn(source, v) + } else { + stdLogger.Println("[WARNING]", source, v) + } } // logError writes formatted string error messages using Printf func logError(source, format string, v ...interface{}) { - errLogger.Printf(fmt.Sprintf("[ERROR] %s %s", source, format), v...) + if fancyLogger, ok := errLogger.(FancyLogger); ok { + fancyLogger.Errorf(fmt.Sprintf("%s %s", source, format), v...) + } else { + errLogger.Printf(fmt.Sprintf("[ERROR] %s %s", source, format), v...) + } } // logErr writes err.Error() using Println func logErr(source string, err error) { - errLogger.Println("[ERROR]", source, err) + if fancyLogger, ok := errLogger.(FancyLogger); ok { + fancyLogger.Error(source, err) + } else { + errLogger.Println("[ERROR]", source, err) + } } // logErrorln writes an error message using Println func logErrorln(source string, v ...interface{}) { - errLogger.Println("[ERROR]", source, v) + if fancyLogger, ok := errLogger.(FancyLogger); ok { + fancyLogger.Error(source, v) + } else { + errLogger.Println("[ERROR]", source, v) + } } diff --git a/logging_test.go b/logging_test.go index c9b17b8..e873c0d 100644 --- a/logging_test.go +++ b/logging_test.go @@ -3,16 +3,75 @@ package riak import ( "bytes" "fmt" + "io" "log" "strings" "testing" ) +type fakeFancyLogger struct { + buf io.Writer + lastCalled string +} + +// appendNewline adds a newline, if one does not already exist at the +// end of the format string. This mimics log.Print[f|ln]'s behavior. +func appendNewline(s string, w io.Writer) { + // taken from stdlib's log.Printf + if len(s) == 0 || s[len(s)-1] != '\n' { + w.Write([]byte(string('\n'))) + } +} + +func (logger *fakeFancyLogger) Println(v ...interface{}) { + fmt.Fprintln(logger.buf, v...) + logger.lastCalled = "Println" +} + +func (logger *fakeFancyLogger) Printf(format string, v ...interface{}) { + fmt.Fprintf(logger.buf, format, v...) + appendNewline(format, logger.buf) + logger.lastCalled = "Printf" +} + +func (logger *fakeFancyLogger) Debug(v ...interface{}) { + fmt.Fprintln(logger.buf, v...) + logger.lastCalled = "Debug" +} + +func (logger *fakeFancyLogger) Debugf(format string, v ...interface{}) { + fmt.Fprintf(logger.buf, format, v...) + appendNewline(format, logger.buf) + logger.lastCalled = "Debugf" +} + +func (logger *fakeFancyLogger) Warn(v ...interface{}) { + fmt.Fprintln(logger.buf, v...) + logger.lastCalled = "Warn" +} + +func (logger *fakeFancyLogger) Warnf(format string, v ...interface{}) { + fmt.Fprintf(logger.buf, format, v...) + appendNewline(format, logger.buf) + logger.lastCalled = "Warnf" +} + +func (logger *fakeFancyLogger) Error(v ...interface{}) { + fmt.Fprintln(logger.buf, v...) + logger.lastCalled = "Error" +} + +func (logger *fakeFancyLogger) Errorf(format string, v ...interface{}) { + fmt.Fprintf(logger.buf, format, v...) + appendNewline(format, logger.buf) + logger.lastCalled = "Errorf" +} + func TestLog(t *testing.T) { EnableDebugLogging = true tests := []struct { - setLoggerFunc func(*log.Logger) + setLoggerFunc func(Logger) logFunc func(string, string, ...interface{}) prefix string }{ @@ -48,11 +107,54 @@ func TestLog(t *testing.T) { } } +func TestFancyLog(t *testing.T) { + EnableDebugLogging = true + + tests := []struct { + setLoggerFunc func(Logger) + logFunc func(string, string, ...interface{}) + shouldCall string + }{ + { + SetErrorLogger, + logError, + "Errorf", + }, + { + SetLogger, + logWarn, + "Warnf", + }, + { + SetLogger, + logDebug, + "Debugf", + }, + } + + for _, tt := range tests { + buf := &bytes.Buffer{} + fancyLogger := &fakeFancyLogger{buf: buf} + tt.setLoggerFunc(fancyLogger) + tt.logFunc("[test]", "Hello %s!", "World") + + actual := buf.String() + suffix := "[test] Hello World!\n" + + if !strings.HasSuffix(actual, suffix) { + t.Errorf("Expected %s to end with %s", actual, suffix) + } + if fancyLogger.lastCalled != tt.shouldCall { + t.Errorf("Expected call to %s, got %s", tt.shouldCall, fancyLogger.lastCalled) + } + } +} + func TestLogln(t *testing.T) { EnableDebugLogging = true tests := []struct { - setLoggerFunc func(*log.Logger) + setLoggerFunc func(Logger) logFunc func(string, ...interface{}) prefix string }{ @@ -88,6 +190,49 @@ func TestLogln(t *testing.T) { } } +func TestFancyLogln(t *testing.T) { + EnableDebugLogging = true + + tests := []struct { + setLoggerFunc func(Logger) + logFunc func(string, ...interface{}) + shouldCall string + }{ + { + SetErrorLogger, + logErrorln, + "Error", + }, + { + SetLogger, + logWarnln, + "Warn", + }, + { + SetLogger, + logDebugln, + "Debug", + }, + } + + for _, tt := range tests { + buf := &bytes.Buffer{} + fancyLogger := &fakeFancyLogger{buf: buf} + tt.setLoggerFunc(fancyLogger) + tt.logFunc("[test]", "Hello", "World!") + + actual := buf.String() + suffix := "[test] [Hello World!]\n" + + if !strings.HasSuffix(actual, suffix) { + t.Errorf("Expected %s to end with %s", actual, suffix) + } + if fancyLogger.lastCalled != tt.shouldCall { + t.Errorf("Expected call to %s, got %s", tt.shouldCall, fancyLogger.lastCalled) + } + } +} + func TestDebugDisabled(t *testing.T) { EnableDebugLogging = false From 2917996de8be0999b1e408e5a5939018aa98b0fa Mon Sep 17 00:00:00 2001 From: Michael Porter Date: Wed, 4 Apr 2018 09:29:22 -0400 Subject: [PATCH 2/2] Change to using "fancy" logger by default --- logging.go | 70 ++++++++++-------------------------------------------- 1 file changed, 12 insertions(+), 58 deletions(-) diff --git a/logging.go b/logging.go index df28bdb..1042b97 100644 --- a/logging.go +++ b/logging.go @@ -18,21 +18,12 @@ package riak import ( "fmt" - "log" "os" ) -// Logger is a generic logging interface, to allow utilizing -// any stdlib-compatible logger for logging -type Logger interface { - Printf(format string, v ...interface{}) - Println(v ...interface{}) -} - -// FancyLogger represents a leveled logger, with methods representing +// Logger represents a leveled logger, with methods representing // debug, warn, and error levels -type FancyLogger interface { - Logger +type Logger interface { Debug(v ...interface{}) Debugf(format string, v ...interface{}) Warn(v ...interface{}) @@ -43,92 +34,55 @@ type FancyLogger interface { // If true, debug messages will be written to the log var EnableDebugLogging = false - -var errLogger Logger -var stdLogger Logger +var logger Logger func init() { if debugEnvVar := os.Getenv("RIAK_GO_CLIENT_DEBUG"); debugEnvVar != "" { EnableDebugLogging = true } - errLogger = log.New(os.Stderr, "", log.LstdFlags) - stdLogger = log.New(os.Stderr, "", log.LstdFlags) } // SetLogger sets the standard logger used for // WARN and DEBUG (if enabled) -func SetLogger(logger Logger) { - stdLogger = logger -} - -// SetErrorLogger sets the logger used for errors -func SetErrorLogger(logger Logger) { - errLogger = logger +func SetLogger(l Logger) { + logger = l } // logDebug writes formatted string debug messages using Printf only if debug logging is enabled func logDebug(source, format string, v ...interface{}) { if EnableDebugLogging { - if fancyLogger, ok := stdLogger.(FancyLogger); ok { - fancyLogger.Debugf(fmt.Sprintf("%s %s", source, format), v...) - } else { - stdLogger.Printf(fmt.Sprintf("[DEBUG] %s %s", source, format), v...) - } + logger.Debugf(fmt.Sprintf("%s %s", source, format), v...) } } // logDebugln writes string debug messages using Println func logDebugln(source string, v ...interface{}) { if EnableDebugLogging { - if fancyLogger, ok := stdLogger.(FancyLogger); ok { - fancyLogger.Debug(source, v) - } else { - stdLogger.Println("[DEBUG]", source, v) - } + logger.Debug(source, v) } } // logWarn writes formatted string warning messages using Printf func logWarn(source, format string, v ...interface{}) { - if fancyLogger, ok := stdLogger.(FancyLogger); ok { - fancyLogger.Warnf(fmt.Sprintf("%s %s", source, format), v...) - } else { - stdLogger.Printf(fmt.Sprintf("[WARNING] %s %s", source, format), v...) - } + logger.Warnf(fmt.Sprintf("%s %s", source, format), v...) } // logWarnln writes string warning messages using Println func logWarnln(source string, v ...interface{}) { - if fancyLogger, ok := stdLogger.(FancyLogger); ok { - fancyLogger.Warn(source, v) - } else { - stdLogger.Println("[WARNING]", source, v) - } + logger.Warn(source, v) } // logError writes formatted string error messages using Printf func logError(source, format string, v ...interface{}) { - if fancyLogger, ok := errLogger.(FancyLogger); ok { - fancyLogger.Errorf(fmt.Sprintf("%s %s", source, format), v...) - } else { - errLogger.Printf(fmt.Sprintf("[ERROR] %s %s", source, format), v...) - } + logger.Errorf(fmt.Sprintf("%s %s", source, format), v...) } // logErr writes err.Error() using Println func logErr(source string, err error) { - if fancyLogger, ok := errLogger.(FancyLogger); ok { - fancyLogger.Error(source, err) - } else { - errLogger.Println("[ERROR]", source, err) - } + logger.Error(source, err) } // logErrorln writes an error message using Println func logErrorln(source string, v ...interface{}) { - if fancyLogger, ok := errLogger.(FancyLogger); ok { - fancyLogger.Error(source, v) - } else { - errLogger.Println("[ERROR]", source, v) - } + logger.Error(source, v) }