diff --git a/pkg/logger/otelzap/encoder.go b/pkg/logger/otelzap/encoder.go new file mode 100644 index 000000000..5de627b57 --- /dev/null +++ b/pkg/logger/otelzap/encoder.go @@ -0,0 +1,217 @@ +package otelzap + +import ( + "fmt" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.uber.org/zap/zapcore" +) + +// otelAttrEncoder implements zapcore.ObjectEncoder to encode zap fields into OpenTelemetry attributes +type otelAttrEncoder struct { + attributes []attribute.KeyValue + namespace string +} + +// otelArrayEncoder implements zapcore.ArrayEncoder to collect array elements as strings +type otelArrayEncoder struct { + elements []string +} + +func (a *otelArrayEncoder) AppendString(v string) { a.elements = append(a.elements, v) } +func (a *otelArrayEncoder) AppendInt64(v int64) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendInt(v int) { a.elements = append(a.elements, fmt.Sprintf("%d", v)) } +func (a *otelArrayEncoder) AppendInt32(v int32) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendInt16(v int16) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendInt8(v int8) { a.elements = append(a.elements, fmt.Sprintf("%d", v)) } +func (a *otelArrayEncoder) AppendUint64(v uint64) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendUint32(v uint32) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendUint16(v uint16) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendUint8(v uint8) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendUint(v uint) { a.elements = append(a.elements, fmt.Sprintf("%d", v)) } +func (a *otelArrayEncoder) AppendUintptr(v uintptr) { + a.elements = append(a.elements, fmt.Sprintf("%d", v)) +} +func (a *otelArrayEncoder) AppendFloat64(v float64) { + a.elements = append(a.elements, fmt.Sprintf("%g", v)) +} +func (a *otelArrayEncoder) AppendFloat32(v float32) { + a.elements = append(a.elements, fmt.Sprintf("%g", v)) +} +func (a *otelArrayEncoder) AppendBool(v bool) { a.elements = append(a.elements, fmt.Sprintf("%t", v)) } +func (a *otelArrayEncoder) AppendArray(zapcore.ArrayMarshaler) error { + a.elements = append(a.elements, "[nested array]") + return nil +} +func (a *otelArrayEncoder) AppendObject(zapcore.ObjectMarshaler) error { + a.elements = append(a.elements, "[object]") + return nil +} +func (a *otelArrayEncoder) AppendReflected(v any) error { + a.elements = append(a.elements, fmt.Sprintf("%+v", v)) + return nil +} +func (a *otelArrayEncoder) AppendByteString(v []byte) { a.elements = append(a.elements, string(v)) } +func (a *otelArrayEncoder) AppendComplex128(v complex128) { + a.elements = append(a.elements, fmt.Sprintf("%v", v)) +} +func (a *otelArrayEncoder) AppendComplex64(v complex64) { + a.elements = append(a.elements, fmt.Sprintf("%v", v)) +} +func (a *otelArrayEncoder) AppendDuration(v time.Duration) { + a.elements = append(a.elements, v.String()) +} +func (a *otelArrayEncoder) AppendTime(v time.Time) { + a.elements = append(a.elements, v.Format(time.RFC3339)) +} + +func (e *otelAttrEncoder) AddArray(key string, marshaler zapcore.ArrayMarshaler) error { + // Create a simple array encoder that converts everything to strings + encoder := &otelArrayEncoder{} + err := marshaler.MarshalLogArray(encoder) + if err != nil { + return err + } + + e.attributes = append(e.attributes, attribute.StringSlice(e.prefixKey(key), encoder.elements)) + return nil +} + +func (e *otelAttrEncoder) AddObject(key string, marshaler zapcore.ObjectMarshaler) error { + // Create a nested encoder for the object + objectEncoder := &otelAttrEncoder{} + err := marshaler.MarshalLogObject(objectEncoder) + if err != nil { + return err + } + + // Add all attributes from the object with the key as prefix + for _, attr := range objectEncoder.attributes { + prefixedKey := key + "." + string(attr.Key) + e.attributes = append(e.attributes, attribute.KeyValue{ + Key: attribute.Key(prefixedKey), + Value: attr.Value, + }) + } + return nil +} + +func (e *otelAttrEncoder) AddBinary(key string, value []byte) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), string(value))) +} + +func (e *otelAttrEncoder) AddBool(key string, value bool) { + e.attributes = append(e.attributes, attribute.Bool(e.prefixKey(key), value)) +} + +func (e *otelAttrEncoder) AddByteString(key string, value []byte) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), string(value))) +} + +func (e *otelAttrEncoder) AddComplex128(key string, value complex128) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), fmt.Sprintf("%v", value))) +} + +func (e *otelAttrEncoder) AddComplex64(key string, value complex64) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), fmt.Sprintf("%v", value))) +} + +func (e *otelAttrEncoder) AddDuration(key string, value time.Duration) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddFloat64(key string, value float64) { + e.attributes = append(e.attributes, attribute.Float64(e.prefixKey(key), value)) +} + +func (e *otelAttrEncoder) AddFloat32(key string, value float32) { + e.attributes = append(e.attributes, attribute.Float64(e.prefixKey(key), float64(value))) +} + +func (e *otelAttrEncoder) AddInt(key string, value int) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddInt64(key string, value int64) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), value)) +} + +func (e *otelAttrEncoder) AddInt32(key string, value int32) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddInt16(key string, value int16) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddInt8(key string, value int8) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddString(key string, value string) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), value)) +} + +func (e *otelAttrEncoder) AddTime(key string, value time.Time) { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), value.Format(time.RFC3339))) +} + +func (e *otelAttrEncoder) AddUint(key string, value uint) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddUint64(key string, value uint64) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddUint32(key string, value uint32) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddUint16(key string, value uint16) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddUint8(key string, value uint8) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddUintptr(key string, value uintptr) { + e.attributes = append(e.attributes, attribute.Int64(e.prefixKey(key), int64(value))) +} + +func (e *otelAttrEncoder) AddReflected(key string, value any) error { + e.attributes = append(e.attributes, attribute.String(e.prefixKey(key), fmt.Sprintf("%+v", value))) + return nil +} + +func (e *otelAttrEncoder) OpenNamespace(key string) { + if e.namespace == "" { + e.namespace = key + } else { + e.namespace = e.namespace + "." + key + } +} + +// helper method to apply namespace prefix to keys +func (e *otelAttrEncoder) prefixKey(key string) string { + if e.namespace == "" { + return key + } + return e.namespace + "." + key +} diff --git a/pkg/logger/otelzap/otelzap.go b/pkg/logger/otelzap/otelzap.go index ca5288faa..89b3f33f2 100644 --- a/pkg/logger/otelzap/otelzap.go +++ b/pkg/logger/otelzap/otelzap.go @@ -3,7 +3,6 @@ package otelzap import ( "context" "fmt" - "math" "time" "go.opentelemetry.io/otel/attribute" @@ -84,7 +83,7 @@ func WithLevelEnabler(levelEnabler zapcore.LevelEnabler) Option { } func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error { - var attributes []attribute.KeyValue + encoder := &otelAttrEncoder{} var spanCtx *oteltrace.SpanContext // Add core-attached fields @@ -94,15 +93,18 @@ func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error { spanCtx = &ctxValue } } else { - attributes = append(attributes, mapZapField(f)) + f.AddTo(encoder) } } // Add fields passed during log call for _, f := range fields { - attributes = append(attributes, mapZapField(f)) + f.AddTo(encoder) } + // Start with encoder attributes + attributes := encoder.attributes + // Add exception metadata if entry.Level > zapcore.InfoLevel { if entry.Caller.Defined { @@ -146,61 +148,6 @@ func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error { return nil } -func mapZapField(f zapcore.Field) attribute.KeyValue { - switch f.Type { - case zapcore.StringType: - return attribute.String(f.Key, f.String) - - case zapcore.Int64Type, zapcore.Int32Type, zapcore.Int16Type, zapcore.Int8Type: - return attribute.Int64(f.Key, f.Integer) - - case zapcore.Uint64Type, zapcore.Uint32Type, zapcore.Uint16Type, zapcore.Uint8Type, zapcore.UintptrType: - return attribute.Int64(f.Key, int64(f.Integer)) - - case zapcore.BoolType: - return attribute.Bool(f.Key, f.Integer == 1) - - case zapcore.Float64Type: - return attribute.Float64(f.Key, math.Float64frombits(uint64(f.Integer))) - - case zapcore.ErrorType: - if err, ok := f.Interface.(error); ok { - return attribute.String(f.Key, err.Error()) - } - return attribute.String(f.Key, "invalid error field") - - case zapcore.StringerType: - return attribute.String(f.Key, f.Interface.(fmt.Stringer).String()) - - case zapcore.TimeType: - if t, ok := f.Interface.(time.Time); ok { - return attribute.String(f.Key, t.Format(time.RFC3339)) - } - return attribute.String(f.Key, fmt.Sprintf("invalid time: %v", f.Interface)) - - case zapcore.DurationType: - if d, ok := f.Interface.(time.Duration); ok { - return attribute.String(f.Key, d.String()) - } - return attribute.String(f.Key, fmt.Sprintf("invalid duration: %v", f.Interface)) - - case zapcore.BinaryType: - if b, ok := f.Interface.([]byte); ok { - return attribute.String(f.Key, fmt.Sprintf("binary data: %x", b)) - } - return attribute.String(f.Key, fmt.Sprintf("invalid binary: %v", f.Interface)) - - case zapcore.ByteStringType: - if b, ok := f.Interface.([]byte); ok { - return attribute.String(f.Key, fmt.Sprintf("byte string: %x", b)) - } - return attribute.String(f.Key, fmt.Sprintf("invalid byte string: %v", f.Interface)) - - default: - return attribute.String(f.Key, f.String) - } -} - func mapZapSeverity(level zapcore.Level) otellog.Severity { switch level { case zapcore.DebugLevel: diff --git a/pkg/logger/otelzap/otelzap_test.go b/pkg/logger/otelzap/otelzap_test.go index 312d87465..7825fbb24 100644 --- a/pkg/logger/otelzap/otelzap_test.go +++ b/pkg/logger/otelzap/otelzap_test.go @@ -21,7 +21,23 @@ type stringerMock struct{} func (s stringerMock) String() string { return "stringer-value" } -func Test_mapZapField(t *testing.T) { +type customError struct{} + +func (e *customError) Error() string { + return "custom error" +} + +// panicError will panic if Error() is called on a nil receiver +type panicError struct { + msg string +} + +func (e *panicError) Error() string { + // This will panic if e is nil since we're accessing a field + return e.msg +} + +func Test_otelAttrEncoder(t *testing.T) { now := time.Now() duration := time.Second * 42 @@ -65,11 +81,6 @@ func Test_mapZapField(t *testing.T) { field: zapcore.Field{Key: "err", Type: zapcore.ErrorType, Interface: errors.New("fail")}, expected: attribute.String("err", "fail"), }, - { - name: "ErrorType invalid", - field: zapcore.Field{Key: "err", Type: zapcore.ErrorType, Interface: "not-an-error"}, - expected: attribute.String("err", "invalid error field"), - }, { name: "StringerType", field: zapcore.Field{Key: "stringer", Type: zapcore.StringerType, Interface: stringerMock{}}, @@ -77,54 +88,54 @@ func Test_mapZapField(t *testing.T) { }, { name: "TimeType valid", - field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Interface: now}, + field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Integer: now.UnixNano(), Interface: now.Location()}, expected: attribute.String("time", now.Format(time.RFC3339)), }, - { - name: "TimeType invalid", - field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Interface: "not-a-time"}, - expected: attribute.String("time", "invalid time: not-a-time"), - }, { name: "DurationType valid", - field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Interface: duration}, - expected: attribute.String("dur", duration.String()), - }, - { - name: "DurationType invalid", - field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Interface: "not-a-duration"}, - expected: attribute.String("dur", "invalid duration: not-a-duration"), + field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Integer: int64(duration)}, + expected: attribute.Int64("dur", int64(duration)), }, { name: "BinaryType valid", field: zapcore.Field{Key: "bin", Type: zapcore.BinaryType, Interface: []byte{0x1, 0x2}}, - expected: attribute.String("bin", "binary data: 0102"), - }, - { - name: "BinaryType invalid", - field: zapcore.Field{Key: "bin", Type: zapcore.BinaryType, Interface: "not-bytes"}, - expected: attribute.String("bin", "invalid binary: not-bytes"), + expected: attribute.String("bin", "\x01\x02"), }, { name: "ByteStringType valid", field: zapcore.Field{Key: "bs", Type: zapcore.ByteStringType, Interface: []byte{0x3, 0x4}}, - expected: attribute.String("bs", "byte string: 0304"), + expected: attribute.String("bs", "\x03\x04"), }, { - name: "ByteStringType invalid", - field: zapcore.Field{Key: "bs", Type: zapcore.ByteStringType, Interface: "not-bytes"}, - expected: attribute.String("bs", "invalid byte string: not-bytes"), + name: "Complex128Type", + field: zapcore.Field{Key: "complex128", Type: zapcore.Complex128Type, Interface: complex(3.14, 2.71)}, + expected: attribute.String("complex128", "(3.14+2.71i)"), }, { - name: "Default", - field: zapcore.Field{Key: "def", Type: zapcore.UnknownType, String: "default"}, - expected: attribute.String("def", "default"), + name: "Complex64Type", + field: zapcore.Field{Key: "complex64", Type: zapcore.Complex64Type, Interface: complex64(1.1 + 2.2i)}, + expected: attribute.String("complex64", "(1.1+2.2i)"), + }, + { + name: "ReflectType with struct", + field: zapcore.Field{Key: "reflect", Type: zapcore.ReflectType, Interface: struct{ Name string }{Name: "test"}}, + expected: attribute.String("reflect", "{Name:test}"), + }, + { + name: "ReflectType with map", + field: zapcore.Field{Key: "reflect_map", Type: zapcore.ReflectType, Interface: map[string]int{"key": 42}}, + expected: attribute.String("reflect_map", "map[key:42]"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := mapZapField(tt.field) + encoder := &otelAttrEncoder{} + tt.field.AddTo(encoder) + + require.Len(t, encoder.attributes, 1, "Expected exactly one attribute") + got := encoder.attributes[0] + assert.Equal(t, tt.expected.Key, got.Key) assert.Equal(t, tt.expected.Value.Type(), got.Value.Type()) assert.Equal(t, tt.expected.Value.AsInterface(), got.Value.AsInterface()) @@ -132,6 +143,35 @@ func Test_mapZapField(t *testing.T) { } } +func Test_otelAttrEncoder_nilSafety(t *testing.T) { + tests := []struct { + name string + field zapcore.Field + }{ + { + name: "StringerType with nil value - should not panic", + field: zapcore.Field{Key: "nil-stringer", Type: zapcore.StringerType, Interface: (*stringerMock)(nil)}, + }, + { + name: "ErrorType with nil panic-causing value - should not panic", + field: zapcore.Field{Key: "nil-panic-error", Type: zapcore.ErrorType, Interface: (*panicError)(nil)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + encoder := &otelAttrEncoder{} + + // This should not panic - zap's Field.AddTo handles the safety + assert.NotPanics(t, func() { + tt.field.AddTo(encoder) + }) + + assert.NotEmpty(t, encoder.attributes) + }) + } +} + func TestOtelZapCore_Write(t *testing.T) { var buf bytes.Buffer @@ -259,3 +299,194 @@ func TestOtelZapCore_Write(t *testing.T) { }) } } + +func Test_otelAttrEncoder_AddObject(t *testing.T) { + tests := []struct { + name string + field zapcore.Field + expected []attribute.KeyValue + }{ + { + name: "Object with nested fields", + field: zapcore.Field{ + Key: "user", + Type: zapcore.ObjectMarshalerType, + Interface: zapcore.ObjectMarshalerFunc(func(enc zapcore.ObjectEncoder) error { + enc.AddString("name", "john") + enc.AddInt64("age", 30) + enc.AddBool("active", true) + return nil + }), + }, + expected: []attribute.KeyValue{ + attribute.String("user.name", "john"), + attribute.Int64("user.age", 30), + attribute.Bool("user.active", true), + }, + }, + { + name: "Empty object", + field: zapcore.Field{ + Key: "empty", + Type: zapcore.ObjectMarshalerType, + Interface: zapcore.ObjectMarshalerFunc(func(enc zapcore.ObjectEncoder) error { + return nil + }), + }, + expected: []attribute.KeyValue{}, + }, + { + name: "Object with complex fields", + field: zapcore.Field{ + Key: "config", + Type: zapcore.ObjectMarshalerType, + Interface: zapcore.ObjectMarshalerFunc(func(enc zapcore.ObjectEncoder) error { + enc.AddString("host", "localhost") + enc.AddInt("port", 8080) + enc.AddDuration("timeout", time.Second*30) + enc.AddFloat64("ratio", 0.75) + return nil + }), + }, + expected: []attribute.KeyValue{ + attribute.String("config.host", "localhost"), + attribute.Int64("config.port", 8080), + attribute.Int64("config.timeout", int64(30*time.Second)), + attribute.Float64("config.ratio", 0.75), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + encoder := &otelAttrEncoder{} + tt.field.AddTo(encoder) + + require.Len(t, encoder.attributes, len(tt.expected), "Expected %d attributes", len(tt.expected)) + + for i, expected := range tt.expected { + got := encoder.attributes[i] + assert.Equal(t, expected.Key, got.Key, "Key mismatch at index %d", i) + assert.Equal(t, expected.Value.Type(), got.Value.Type(), "Value type mismatch at index %d", i) + assert.Equal(t, expected.Value.AsInterface(), got.Value.AsInterface(), "Value mismatch at index %d", i) + } + }) + } +} + +func Test_otelAttrEncoder_AddArray(t *testing.T) { + tests := []struct { + name string + key string + array zapcore.ArrayMarshaler + expected attribute.KeyValue + }{ + { + name: "string array", + key: "strings", + array: &testStringArray{data: []string{"hello", "world"}}, + expected: attribute.StringSlice("strings", []string{"hello", "world"}), + }, + { + name: "int array", + key: "ints", + array: &testIntArray{data: []int{1, 2, 3}}, + expected: attribute.StringSlice("ints", []string{"1", "2", "3"}), + }, + { + name: "mixed array", + key: "mixed", + array: &testMixedArray{data: []interface{}{"hello", 42, true}}, + expected: attribute.StringSlice("mixed", []string{"hello", "42", "true"}), + }, + { + name: "float array", + key: "floats", + array: &testFloatArray{data: []float64{1.5, 2.7, 3.14}}, + expected: attribute.StringSlice("floats", []string{"1.5", "2.7", "3.14"}), + }, + { + name: "bool array", + key: "bools", + array: &testBoolArray{data: []bool{true, false, true}}, + expected: attribute.StringSlice("bools", []string{"true", "false", "true"}), + }, + { + name: "empty array", + key: "empty", + array: &testStringArray{data: []string{}}, + expected: attribute.StringSlice("empty", []string{}), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + encoder := &otelAttrEncoder{} + err := encoder.AddArray(tt.key, tt.array) + + assert.NoError(t, err) + assert.Len(t, encoder.attributes, 1) + + got := encoder.attributes[0] + assert.Equal(t, tt.expected.Key, got.Key) + assert.Equal(t, tt.expected.Value.Type(), got.Value.Type()) + assert.Equal(t, tt.expected.Value.AsInterface(), got.Value.AsInterface()) + }) + } +} + +// Test helper types that implement ArrayMarshaler +type testStringArray struct { + data []string +} + +func (t *testStringArray) MarshalLogArray(enc zapcore.ArrayEncoder) error { + for _, v := range t.data { + enc.AppendString(v) + } + return nil +} + +type testIntArray struct { + data []int +} + +func (t *testIntArray) MarshalLogArray(enc zapcore.ArrayEncoder) error { + for _, v := range t.data { + enc.AppendInt(v) + } + return nil +} + +type testFloatArray struct { + data []float64 +} + +func (t *testFloatArray) MarshalLogArray(enc zapcore.ArrayEncoder) error { + for _, v := range t.data { + enc.AppendFloat64(v) + } + return nil +} + +type testBoolArray struct { + data []bool +} + +func (t *testBoolArray) MarshalLogArray(enc zapcore.ArrayEncoder) error { + for _, v := range t.data { + enc.AppendBool(v) + } + return nil +} + +type testMixedArray struct { + data []interface{} +} + +func (t *testMixedArray) MarshalLogArray(enc zapcore.ArrayEncoder) error { + for _, v := range t.data { + enc.AppendReflected(v) + } + return nil +}