From 289adc3cb843772915dfbcfd090b7ebb599fe3b3 Mon Sep 17 00:00:00 2001 From: Thomas Linford Date: Wed, 23 Jun 2021 13:47:18 +0200 Subject: [PATCH] Add Field type with Error function, linting setup, cleanup --- .golangci.yaml | 15 ++++++ yall.go | 35 +++++++++--- {zap_logger => zaplogger}/zap_logger.go | 56 ++++++++++++++------ {zap_logger => zaplogger}/zap_logger_test.go | 32 +++++------ 4 files changed, 100 insertions(+), 38 deletions(-) create mode 100644 .golangci.yaml rename {zap_logger => zaplogger}/zap_logger.go (81%) rename {zap_logger => zaplogger}/zap_logger_test.go (62%) diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..32ecc69 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,15 @@ +linters: + enable: + - deadcode + - errcheck + - gosimple + - govet + - ineffassign + - staticcheck + - structcheck + - typecheck + - unused + - varcheck + - revive + - gofmt + - goimports diff --git a/yall.go b/yall.go index e1c11d0..d57ed5a 100644 --- a/yall.go +++ b/yall.go @@ -14,17 +14,38 @@ const ( ) type Logger interface { + contextLogger + noContextLogger + With(args ...interface{}) Logger +} + +type contextLogger interface { Fatal(ctx context.Context, msg string, keysAndValues ...interface{}) - Fatalnc(msg string, keysAndValues ...interface{}) Panic(ctx context.Context, msg string, keysAndValues ...interface{}) - Panicnc(msg string, keysAndValues ...interface{}) Error(ctx context.Context, msg string, keysAndValues ...interface{}) - Errornc(msg string, keysAndValues ...interface{}) Warn(ctx context.Context, msg string, keysAndValues ...interface{}) - Warnnc(msg string, keysAndValues ...interface{}) Info(ctx context.Context, msg string, keysAndValues ...interface{}) - Infonc(msg string, keysAndValues ...interface{}) Debug(ctx context.Context, msg string, keysAndValues ...interface{}) +} + +type noContextLogger interface { + Fatalnc(msg string, keysAndValues ...interface{}) + Panicnc(msg string, keysAndValues ...interface{}) + Errornc(msg string, keysAndValues ...interface{}) + Warnnc(msg string, keysAndValues ...interface{}) + Infonc(msg string, keysAndValues ...interface{}) Debugnc(msg string, keysAndValues ...interface{}) - With(args ...interface{}) Logger -} \ No newline at end of file +} + +type Field struct { + Name string + Value interface{} +} + +// Error is a convenience method to make logging errors easier. +func Error(err error) *Field { + return &Field{ + Name: "error", + Value: err, + } +} diff --git a/zap_logger/zap_logger.go b/zaplogger/zap_logger.go similarity index 81% rename from zap_logger/zap_logger.go rename to zaplogger/zap_logger.go index 6ab7095..35caf69 100644 --- a/zap_logger/zap_logger.go +++ b/zaplogger/zap_logger.go @@ -1,5 +1,5 @@ // Package zap_logger provides an implementation of the Yall logger interface -package zap_logger +package zaplogger import ( "context" @@ -10,14 +10,7 @@ import ( "go.uber.org/zap/zapcore" ) -type Field = zap.Field - -// Error is a convenience method to make logging errors easier -func Error(err error) Field { - return zap.Error(err) -} - -// Logger is just a thin wrapper around a sugared zap logger with some opinionated defaults +// Logger is just a thin wrapper around a sugared zap logger with some opinionated defaults. type Logger struct { l zap.SugaredLogger conf *loggerConf @@ -55,34 +48,38 @@ func NewLogger(name string, opts ...LoggerOpt) (yall.Logger, error) { type LoggerOpt func(opts *loggerConf) *loggerConf -// Production uses a zap production config, with ISO 8601 timestamps +// Production uses a zap production config, with ISO 8601 timestamps. func Production() LoggerOpt { return func(opts *loggerConf) *loggerConf { opts.production = true + return opts } } -// WithNameKey configures the key to use for the logger's name +// WithNameKey configures the key to use for the logger's name. func WithNameKey(nameKey string) LoggerOpt { return func(opts *loggerConf) *loggerConf { opts.nameKey = nameKey + return opts } } -// WithExecutionIDKey sets the key to use to log the execution id +// WithExecutionIDKey sets the key to use to log the execution id. func WithExecutionIDKey(requestIDKey string) LoggerOpt { return func(opts *loggerConf) *loggerConf { opts.executionIDKey = requestIDKey + return opts } } -// WithExecutionIDContextKey configures the key to use to extract execution id from context +// WithExecutionIDContextKey configures the key to use to extract execution id from context. func WithExecutionIDContextKey(requestIDContextKey interface{}) LoggerOpt { return func(opts *loggerConf) *loggerConf { opts.executionIDContextKey = requestIDContextKey + return opts } } @@ -90,6 +87,7 @@ func WithExecutionIDContextKey(requestIDContextKey interface{}) LoggerOpt { func WithOmitExecutionIDWhenMissing() LoggerOpt { return func(opts *loggerConf) *loggerConf { opts.omitExecutionIDWhenMissing = true + return opts } } @@ -114,55 +112,67 @@ func defaultConf() *loggerConf { func (l *Logger) Fatal(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Fatalw(msg, keysAndValues...) } func (l *Logger) Panic(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Panicw(msg, keysAndValues...) } func (l *Logger) Error(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Errorw(msg, keysAndValues...) } func (l *Logger) Warn(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Warnw(msg, keysAndValues...) } func (l *Logger) Info(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Infow(msg, keysAndValues...) } func (l *Logger) Debug(ctx context.Context, msg string, keysAndValues ...interface{}) { keysAndValues = l.addExecutionIDField(ctx, keysAndValues...) + keysAndValues = extractFields(keysAndValues...) l.l.Debugw(msg, keysAndValues...) } func (l *Logger) Fatalnc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Fatalw(msg, keysAndValues...) } func (l *Logger) Panicnc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Panicw(msg, keysAndValues...) } func (l *Logger) Errornc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Errorw(msg, keysAndValues...) } func (l *Logger) Warnnc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Warnw(msg, keysAndValues...) } func (l *Logger) Infonc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Infow(msg, keysAndValues...) } func (l *Logger) Debugnc(msg string, keysAndValues ...interface{}) { + keysAndValues = extractFields(keysAndValues...) l.l.Debugw(msg, keysAndValues...) } @@ -181,7 +191,7 @@ func (l *Logger) addExecutionIDField(ctx context.Context, keysAndValues ...inter return keysAndValues } -// ExecutionIDFrom retrieves the execution id from the given context, if present +// ExecutionIDFrom retrieves the execution id from the given context, if present. func (l *Logger) ExecutionIDFrom(ctx context.Context) string { if ctx == nil { return yall.MissingExecutionID @@ -192,7 +202,21 @@ func (l *Logger) ExecutionIDFrom(ctx context.Context) string { } if reqID, ok := reqID.(string); ok { return reqID - } else { - return yall.MissingExecutionID } + + return yall.MissingExecutionID +} + +func extractFields(keysAndValues ...interface{}) []interface{} { + var processedKeysAndValues []interface{} + + for _, keyOrValue := range keysAndValues { + if field, ok := keyOrValue.(*yall.Field); ok { + processedKeysAndValues = append(processedKeysAndValues, field.Name, field.Value) + } else { + processedKeysAndValues = append(processedKeysAndValues, keyOrValue) + } + } + + return processedKeysAndValues } diff --git a/zap_logger/zap_logger_test.go b/zaplogger/zap_logger_test.go similarity index 62% rename from zap_logger/zap_logger_test.go rename to zaplogger/zap_logger_test.go index 942c7a8..483e076 100644 --- a/zap_logger/zap_logger_test.go +++ b/zaplogger/zap_logger_test.go @@ -1,52 +1,53 @@ -package zap_logger_test +package zaplogger_test import ( "context" + "fmt" "testing" "github.com/phoops/yall" - "github.com/phoops/yall/zap_logger" + "github.com/phoops/yall/zaplogger" "github.com/stretchr/testify/assert" ) func TestZapLoggerCreate(t *testing.T) { type testCase struct { name string - opts []zap_logger.LoggerOpt + opts []zaplogger.LoggerOpt } testCases := []testCase{ { name: "default", - opts: []zap_logger.LoggerOpt{}, + opts: []zaplogger.LoggerOpt{}, }, { name: "production", - opts: []zap_logger.LoggerOpt{zap_logger.Production()}, + opts: []zaplogger.LoggerOpt{zaplogger.Production()}, }, { name: "name_key", - opts: []zap_logger.LoggerOpt{zap_logger.WithNameKey("test")}, + opts: []zaplogger.LoggerOpt{zaplogger.WithNameKey("test")}, }, { name: "execution_id_key", - opts: []zap_logger.LoggerOpt{zap_logger.WithExecutionIDKey("different_key")}, + opts: []zaplogger.LoggerOpt{zaplogger.WithExecutionIDKey("different_key")}, }, { name: "context_key", - opts: []zap_logger.LoggerOpt{zap_logger.WithExecutionIDContextKey("context_key")}, + opts: []zaplogger.LoggerOpt{zaplogger.WithExecutionIDContextKey("context_key")}, }, { name: "omit_missing_execution_id", - opts: []zap_logger.LoggerOpt{ - zap_logger.WithOmitExecutionIDWhenMissing(), - zap_logger.WithExecutionIDContextKey("context_key"), + opts: []zaplogger.LoggerOpt{ + zaplogger.WithOmitExecutionIDWhenMissing(), + zaplogger.WithExecutionIDContextKey("context_key"), }, }, } for _, tc := range testCases { - logger, err := zap_logger.NewLogger("test", tc.opts...) + logger, err := zaplogger.NewLogger("test", tc.opts...) var _ yall.Logger = logger assert.NoError(t, err) @@ -64,16 +65,17 @@ func TestZapLoggerCreate(t *testing.T) { assert.Panics(t, func() { logger.Panicnc("testing execution_id, panic level") }) - logger.Error(ctx, "testing execution_id, error level", zap_logger.Error(err)) - logger.Errornc("testing execution_id, error level") + logger.Error(ctx, "testing execution_id, error level", yall.Error(fmt.Errorf("this is an error"))) + logger.Errornc("testing execution_id, error level", yall.Error(fmt.Errorf("another error"))) logger.Warn(ctx, "testing execution_id, warn level") logger.Warnnc("testing execution_id, warn level") logger.Info(ctx, "testing execution_id, info level") - logger.Infonc( "testing execution_id, info level") + logger.Infonc("testing execution_id, info level") logger.Debug(ctx, "testing execution_id, debug level") logger.Debugnc("testing execution_id, debug level") // this shouldn't really happen, but better safe than sorry + //nolint:staticcheck logger.Info(nil, "test") } }