Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Commit

Permalink
Convert nil pointer log field value to string "nil" (#230)
Browse files Browse the repository at this point in the history
* Set nil pointer to string nil when converting key values.

This causes a panic that will be recovered by the fmt package.
Altought on darwin this makes any project using open tracing impossible to debug.

see cortexproject/cortex#1712

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

* Adds some tests for InterleavedKVToFields.

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
  • Loading branch information
cyriltovena authored and yurishkuro committed Jan 24, 2020
1 parent cdf3382 commit 2876d20
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 1 deletion.
9 changes: 8 additions & 1 deletion log/util.go
@@ -1,6 +1,9 @@
package log

import "fmt"
import (
"fmt"
"reflect"
)

// InterleavedKVToFields converts keyValues a la Span.LogKV() to a Field slice
// a la Span.LogFields().
Expand Down Expand Up @@ -46,6 +49,10 @@ func InterleavedKVToFields(keyValues ...interface{}) ([]Field, error) {
case float64:
fields[i] = Float64(key, typedVal)
default:
if typedVal == nil || (reflect.ValueOf(typedVal).Kind() == reflect.Ptr && reflect.ValueOf(typedVal).IsNil()) {
fields[i] = String(key, "nil")
continue
}
// When in doubt, coerce to a string
fields[i] = String(key, fmt.Sprint(typedVal))
}
Expand Down
86 changes: 86 additions & 0 deletions log/util_test.go
@@ -0,0 +1,86 @@
package log

import (
"errors"
"io"
"testing"

"github.com/stretchr/testify/assert"
)

var nilInterface io.Reader

func TestInterleavedKVToFields(t *testing.T) {

tests := []struct {
name string
keyValues []interface{}
want []Field
wantErr bool
}{
{
"incorrect pair",
[]interface{}{"test"},
nil,
true,
},
{
"non string key",
[]interface{}{struct{}{}, "foo"},
nil,
true,
},
{
"happy path",
[]interface{}{
"bool", true,
"string", "string",
"int", int(1),
"int8", int8(2),
"int16", int16(3),
"int64", int64(4),
"uint", uint(5),
"uint64", uint64(6),
"uint8", uint8(7),
"uint16", uint16(8),
"uint32", uint32(9),
"float32", float32(10),
"float64", float64(11),
"int32", int32(12),
"stringer", errors.New("err"),
"nilInterface", nilInterface,
"nil", nil,
},
[]Field{
Bool("bool", true),
String("string", "string"),
Int("int", int(1)),
Int32("int8", int32(2)),
Int32("int16", int32(3)),
Int64("int64", int64(4)),
Uint64("uint", uint64(5)),
Uint64("uint64", uint64(6)),
Uint32("uint8", uint32(7)),
Uint32("uint16", uint32(8)),
Uint32("uint32", uint32(9)),
Float32("float32", float32(10)),
Float64("float64", float64(11)),
Int32("int32", int32(12)),
String("stringer", errors.New("err").Error()),
String("nilInterface", "nil"),
String("nil", "nil"),
},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := InterleavedKVToFields(tt.keyValues...)
if (err != nil) != tt.wantErr {
t.Errorf("InterleavedKVToFields() error = %v, wantErr %v", err, tt.wantErr)
return
}
assert.Equal(t, tt.want, got)
})
}
}

0 comments on commit 2876d20

Please sign in to comment.