Append ErrMissingValue to odd length keyvals rather than panic.

json_logger.go

@@ -17,12 +17,15 @@ func NewJSONLogger(w io.Writer) Logger {
 }
 
 func (l *jsonLogger) Log(keyvals ...interface{}) error {
-	if len(keyvals)%2 == 1 {
-		panic("odd number of keyvals")
-	}
-	m := make(map[string]interface{}, len(keyvals)/2)
+	n := (len(keyvals) + 1) / 2 // +1 to handle case when len is odd
+	m := make(map[string]interface{}, n)
 	for i := 0; i < len(keyvals); i += 2 {
-		merge(m, keyvals[i], keyvals[i+1])
+		k := keyvals[i]
+		v := ErrMissingValue
+		if i+1 < len(keyvals) {
+			v = keyvals[i+1]
+		}
+		merge(m, k, v)
 	}
 	return json.NewEncoder(l.Writer).Encode(m)
 }

json_logger_test.go

@@ -10,13 +10,26 @@ import (
 )
 
 func TestJSONLogger(t *testing.T) {
+	t.Parallel()
 	buf := &bytes.Buffer{}
 	logger := log.NewJSONLogger(buf)
 	if err := logger.Log("err", errors.New("err"), "m", map[string]int{"0": 0}, "a", []int{1, 2, 3}); err != nil {
 		t.Fatal(err)
 	}
 	if want, have := `{"a":[1,2,3],"err":"err","m":{"0":0}}`+"\n", buf.String(); want != have {
-		t.Errorf("want %#v, have %#v", want, have)
+		t.Errorf("\nwant %#v\nhave %#v", want, have)
+	}
+}
+
+func TestJSONLoggerMissingValue(t *testing.T) {
+	t.Parallel()
+	buf := &bytes.Buffer{}
+	logger := log.NewJSONLogger(buf)
+	if err := logger.Log("k"); err != nil {
+		t.Fatal(err)
+	}
+	if want, have := `{"k":"(MISSING)"}`+"\n", buf.String(); want != have {
+		t.Errorf("\nwant %#v\nhave %#v", want, have)
 	}
 }
 

log.go

@@ -4,7 +4,10 @@
 // key/value data.
 package log
 
-import "sync/atomic"
+import (
+	"errors"
+	"sync/atomic"
+)
 
 // Logger is the fundamental interface for all log operations. Log creates a
 // log event from keyvals, a variadic sequence of alternating keys and values.
@@ -15,6 +18,10 @@ type Logger interface {
 	Log(keyvals ...interface{}) error
 }
 
+// ErrMissingValue may be appended to keyval slices with an odd length to
+// populate the missing value.
+var ErrMissingValue interface{} = errors.New("(MISSING)")
+
 // NewContext returns a new Context that logs to logger.
 func NewContext(logger Logger) Context {
 	if c, ok := logger.(Context); ok {
@@ -37,10 +44,10 @@ type Context struct {
 // stored context with their generated value, appends keyvals, and passes the
 // result to the wrapped Logger.
 func (l Context) Log(keyvals ...interface{}) error {
-	if len(keyvals)%2 != 0 {
-		panic("bad keyvals")
-	}
 	kvs := append(l.keyvals, keyvals...)
+	if len(kvs)%2 != 0 {
+		kvs = append(kvs, ErrMissingValue)
+	}
 	if l.hasValuer {
 		// If no keyvals were appended above then we must copy l.keyvals so
 		// that future log events will reevaluate the stored Valuers.
@@ -57,17 +64,17 @@ func (l Context) With(keyvals ...interface{}) Context {
 	if len(keyvals) == 0 {
 		return l
 	}
-	if len(keyvals)%2 != 0 {
-		panic("bad keyvals")
+	kvs := append(l.keyvals, keyvals...)
+	if len(kvs)%2 != 0 {
+		kvs = append(kvs, ErrMissingValue)
 	}
-	// Limiting the capacity of the stored keyvals ensures that a new
-	// backing array is created if the slice must grow in Log or With.
-	// Using the extra capacity without copying risks a data race that
-	// would violate the Logger interface contract.
-	n := len(l.keyvals) + len(keyvals)
 	return Context{
-		logger:    l.logger,
-		keyvals:   append(l.keyvals, keyvals...)[:n:n],
+		logger: l.logger,
+		// Limiting the capacity of the stored keyvals ensures that a new
+		// backing array is created if the slice must grow in Log or With.
+		// Using the extra capacity without copying risks a data race that
+		// would violate the Logger interface contract.
+		keyvals:   kvs[:len(kvs):len(kvs)],
 		hasValuer: l.hasValuer || containsValuer(keyvals),
 	}
 }
@@ -78,16 +85,19 @@ func (l Context) WithPrefix(keyvals ...interface{}) Context {
 	if len(keyvals) == 0 {
 		return l
 	}
-	if len(keyvals)%2 != 0 {
-		panic("bad keyvals")
-	}
 	// Limiting the capacity of the stored keyvals ensures that a new
 	// backing array is created if the slice must grow in Log or With.
 	// Using the extra capacity without copying risks a data race that
 	// would violate the Logger interface contract.
 	n := len(l.keyvals) + len(keyvals)
+	if len(keyvals)%2 != 0 {
+		n++
+	}
 	kvs := make([]interface{}, 0, n)
 	kvs = append(kvs, keyvals...)
+	if len(kvs)%2 != 0 {
+		kvs = append(kvs, ErrMissingValue)
+	}
 	kvs = append(kvs, l.keyvals...)
 	return Context{
 		logger:    l.logger,

log_test.go

@@ -11,6 +11,7 @@ import (
 var discard = log.Logger(log.LoggerFunc(func(...interface{}) error { return nil }))
 
 func TestContext(t *testing.T) {
+	t.Parallel()
 	buf := &bytes.Buffer{}
 	logger := log.NewLogfmtLogger(buf)
 
@@ -36,6 +37,35 @@ func TestContext(t *testing.T) {
 	}
 }
 
+func TestContextMissingValue(t *testing.T) {
+	t.Parallel()
+	var output []interface{}
+	logger := log.Logger(log.LoggerFunc(func(keyvals ...interface{}) error {
+		output = keyvals
+		return nil
+	}))
+
+	lc := log.NewContext(logger)
+
+	lc.Log("k")
+	if want, have := 2, len(output); want != have {
+		t.Errorf("want len(output) == %v, have %v", want, have)
+	}
+	if want, have := log.ErrMissingValue, output[1]; want != have {
+		t.Errorf("want %#v, have %#v", want, have)
+	}
+
+	lc.With("k1").WithPrefix("k0").Log("k2")
+	if want, have := 6, len(output); want != have {
+		t.Errorf("want len(output) == %v, have %v", want, have)
+	}
+	for i := 1; i < 6; i += 2 {
+		if want, have := log.ErrMissingValue, output[i]; want != have {
+			t.Errorf("want output[%d] == %#v, have %#v", i, want, have)
+		}
+	}
+}
+
 // Test that With returns a Logger safe for concurrent use. This test
 // validates that the stored logging context does not get corrupted when
 // multiple clients concurrently log additional keyvals.