Merge pull request #1141 from charithe/fix/logging-ctx

🐛 Handle empty contexts in log.FromContext

pkg/log/log.go

@@ -55,15 +55,15 @@ var Log = NewDelegatingLogger(NullLogger{})
 
 // FromContext returns a logger with predefined values from a context.Context.
 func FromContext(ctx context.Context, keysAndValues ...interface{}) logr.Logger {
-	var log logr.Logger
-	if ctx == nil {
-		log = Log
-	} else {
-		lv := ctx.Value(contextKey)
-		log = lv.(logr.Logger)
+	var log logr.Logger = Log
+
+	if ctx != nil {
+		if lv := ctx.Value(contextKey); lv != nil {
+			log = lv.(logr.Logger)
+		}
 	}
-	log.WithValues(keysAndValues...)
-	return log
+
+	return log.WithValues(keysAndValues...)
 }
 
 // IntoContext takes a context and sets the logger as one of its keys.

pkg/log/log_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package log
 
 import (
+	"context"
+
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -214,4 +216,44 @@ var _ = Describe("logging", func() {
 			))
 		})
 	})
+
+	Describe("logger from context", func() {
+		It("should return default logger when context is empty", func() {
+			gotLog := FromContext(context.Background())
+			Expect(gotLog).To(Not(BeNil()))
+		})
+
+		It("should return existing logger", func() {
+			root := &fakeLoggerRoot{}
+			baseLog := &fakeLogger{root: root}
+
+			wantLog := baseLog.WithName("my-logger")
+			ctx := IntoContext(context.Background(), wantLog)
+
+			gotLog := FromContext(ctx)
+			Expect(gotLog).To(Not(BeNil()))
+
+			gotLog.Info("test message")
+			Expect(root.messages).To(ConsistOf(
+				logInfo{name: []string{"my-logger"}, msg: "test message"},
+			))
+		})
+
+		It("should have added key-values", func() {
+			root := &fakeLoggerRoot{}
+			baseLog := &fakeLogger{root: root}
+
+			wantLog := baseLog.WithName("my-logger")
+			ctx := IntoContext(context.Background(), wantLog)
+
+			gotLog := FromContext(ctx, "tag1", "value1")
+			Expect(gotLog).To(Not(BeNil()))
+
+			gotLog.Info("test message")
+			Expect(root.messages).To(ConsistOf(
+				logInfo{name: []string{"my-logger"}, tags: []interface{}{"tag1", "value1"}, msg: "test message"},
+			))
+		})
+	})
+
 })