Add Field type with Error function, linting setup, cleanup

.golangci.yaml

@@ -0,0 +1,15 @@
+linters:
+  enable:
+    - deadcode
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unused
+    - varcheck
+    - revive
+    - gofmt
+    - goimports

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
-}
+}
+
+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,
+	}
+}

zap_logger/zap_logger.go → 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,41 +48,46 @@ 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
 	}
 }
 
 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
 }

zap_logger/zap_logger_test.go → 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")
 	}
 }