Remove fields map from Helper, add Extract method and fix for defaultLogger.Fields (#2274)

logger/default.go

@@ -19,7 +19,7 @@ func init() {
 		lvl = InfoLevel
 	}
 
-	DefaultLogger = NewHelper(NewLogger(WithLevel(lvl)))
+	DefaultLogger = NewLogger(WithLevel(lvl))
 }
 
 type defaultLogger struct {
@@ -41,9 +41,23 @@ func (l *defaultLogger) String() string {
 
 func (l *defaultLogger) Fields(fields map[string]interface{}) Logger {
 	l.Lock()
-	l.opts.Fields = copyFields(fields)
+	nfields := make(map[string]interface{}, len(l.opts.Fields))
+	for k, v := range l.opts.Fields {
+		nfields[k] = v
+	}
 	l.Unlock()
-	return l
+
+	for k, v := range fields {
+		nfields[k] = v
+	}
+
+	return &defaultLogger{opts: Options{
+		Level:           l.opts.Level,
+		Fields:          nfields,
+		Out:             l.opts.Out,
+		CallerSkipCount: l.opts.CallerSkipCount,
+		Context:         l.opts.Context,
+	}}
 }
 
 func copyFields(src map[string]interface{}) map[string]interface{} {

logger/helper.go

@@ -1,93 +1,104 @@
 package logger
 
 import (
+	"context"
 	"os"
 )
 
 type Helper struct {
 	Logger
-	fields map[string]interface{}
 }
 
 func NewHelper(log Logger) *Helper {
 	return &Helper{Logger: log}
 }
 
+// Extract always returns valid Helper with logger from context or with DefaultLogger as fallback.
+// Can be used in pair with function NewContext(ctx context.Context, l Logger) context.Context.
+// (e.g. propagate RequestID to logger in service handler methods).
+func Extract(ctx context.Context) *Helper {
+	if l, ok := FromContext(ctx); ok {
+		return NewHelper(l)
+	}
+
+	return NewHelper(DefaultLogger)
+}
+
 func (h *Helper) Info(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(InfoLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(InfoLevel, args...)
+	h.Log(InfoLevel, args...)
 }
 
 func (h *Helper) Infof(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(InfoLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(InfoLevel, template, args...)
+	h.Logf(InfoLevel, template, args...)
 }
 
 func (h *Helper) Trace(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(TraceLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(TraceLevel, args...)
+	h.Log(TraceLevel, args...)
 }
 
 func (h *Helper) Tracef(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(TraceLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(TraceLevel, template, args...)
+	h.Logf(TraceLevel, template, args...)
 }
 
 func (h *Helper) Debug(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(DebugLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(DebugLevel, args...)
+	h.Log(DebugLevel, args...)
 }
 
 func (h *Helper) Debugf(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(DebugLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(DebugLevel, template, args...)
+	h.Logf(DebugLevel, template, args...)
 }
 
 func (h *Helper) Warn(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(WarnLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(WarnLevel, args...)
+	h.Log(WarnLevel, args...)
 }
 
 func (h *Helper) Warnf(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(WarnLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(WarnLevel, template, args...)
+	h.Logf(WarnLevel, template, args...)
 }
 
 func (h *Helper) Error(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(ErrorLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(ErrorLevel, args...)
+	h.Log(ErrorLevel, args...)
 }
 
 func (h *Helper) Errorf(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(ErrorLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(ErrorLevel, template, args...)
+	h.Logf(ErrorLevel, template, args...)
 }
 
 func (h *Helper) Fatal(args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(FatalLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Log(FatalLevel, args...)
+	h.Log(FatalLevel, args...)
 	os.Exit(1)
 }
 
@@ -95,20 +106,14 @@ func (h *Helper) Fatalf(template string, args ...interface{}) {
 	if !h.Logger.Options().Level.Enabled(FatalLevel) {
 		return
 	}
-	h.Logger.Fields(h.fields).Logf(FatalLevel, template, args...)
+	h.Logf(FatalLevel, template, args...)
 	os.Exit(1)
 }
 
 func (h *Helper) WithError(err error) *Helper {
-	fields := copyFields(h.fields)
+	return &Helper{Logger: h.Logger.Fields(map[string]interface{}{"error": err})}
-	fields["error"] = err
-	return &Helper{Logger: h.Logger, fields: fields}
 }
 
 func (h *Helper) WithFields(fields map[string]interface{}) *Helper {
-	nfields := copyFields(fields)
+	return &Helper{Logger: h.Logger.Fields(fields)}
-	for k, v := range h.fields {
-		nfields[k] = v
-	}
-	return &Helper{Logger: h.Logger, fields: nfields}
 }

logger/logger.go

@@ -3,7 +3,7 @@ package logger
 
 var (
 	// Default logger
-	DefaultLogger Logger = NewHelper(NewLogger())
+	DefaultLogger Logger = NewLogger()
 )
 
 // Logger is a generic logging interface

logger/logger_test.go

@@ -1,18 +1,32 @@
 package logger
 
 import (
+	"context"
 	"testing"
 )
 
 func TestLogger(t *testing.T) {
 	l := NewLogger(WithLevel(TraceLevel))
 	h1 := NewHelper(l).WithFields(map[string]interface{}{"key1": "val1"})
+	h1.Log(TraceLevel, "simple log before trace_msg1")
 	h1.Trace("trace_msg1")
+	h1.Log(TraceLevel, "simple log after trace_msg1")
 	h1.Warn("warn_msg1")
 
 	h2 := NewHelper(l).WithFields(map[string]interface{}{"key2": "val2"})
+	h2.Logf(TraceLevel, "formatted log before trace_msg%s", "2")
 	h2.Trace("trace_msg2")
+	h2.Logf(TraceLevel, "formatted log after trace_msg%s", "2")
 	h2.Warn("warn_msg2")
 
 	l.Fields(map[string]interface{}{"key3": "val4"}).Log(InfoLevel, "test_msg")
 }
+
+func TestExtract(t *testing.T) {
+	l := NewLogger(WithLevel(TraceLevel)).Fields(map[string]interface{}{"requestID": "req-1"})
+
+	ctx := NewContext(context.Background(), l)
+
+	Info("info message without request ID")
+	Extract(ctx).Info("info message with request ID")
+}