Skip to content

Commit cac1480

Browse files
committed
Implement ObjectEncoder and let the fields write themselves
1 parent f7c0b14 commit cac1480

File tree

3 files changed

+178
-95
lines changed

3 files changed

+178
-95
lines changed

pkg/logger/otelzap/encoder.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
package otelzap
2+
3+
import (
4+
"time"
5+
6+
"go.opentelemetry.io/otel/attribute"
7+
"go.uber.org/zap/zapcore"
8+
)
9+
10+
// otelAttrEncoder implements zapcore.ObjectEncoder to encode zap fields into OpenTelemetry attributes
11+
type otelAttrEncoder struct {
12+
attributes []attribute.KeyValue
13+
}
14+
15+
func (e *otelAttrEncoder) AddArray(key string, marshaler zapcore.ArrayMarshaler) error {
16+
// Not implemented
17+
return nil
18+
}
19+
20+
func (e *otelAttrEncoder) AddObject(key string, marshaler zapcore.ObjectMarshaler) error {
21+
// Not implemented
22+
return nil
23+
}
24+
25+
func (e *otelAttrEncoder) AddBinary(key string, value []byte) {
26+
e.attributes = append(e.attributes, attribute.String(key, string(value)))
27+
}
28+
29+
func (e *otelAttrEncoder) AddBool(key string, value bool) {
30+
e.attributes = append(e.attributes, attribute.Bool(key, value))
31+
}
32+
33+
func (e *otelAttrEncoder) AddByteString(key string, value []byte) {
34+
e.attributes = append(e.attributes, attribute.String(key, string(value)))
35+
}
36+
37+
func (e *otelAttrEncoder) AddComplex128(key string, value complex128) {
38+
// Not implemented
39+
}
40+
41+
func (e *otelAttrEncoder) AddComplex64(key string, value complex64) {
42+
// Not implemented
43+
}
44+
45+
func (e *otelAttrEncoder) AddDuration(key string, value time.Duration) {
46+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
47+
}
48+
49+
func (e *otelAttrEncoder) AddFloat64(key string, value float64) {
50+
e.attributes = append(e.attributes, attribute.Float64(key, value))
51+
}
52+
53+
func (e *otelAttrEncoder) AddFloat32(key string, value float32) {
54+
e.attributes = append(e.attributes, attribute.Float64(key, float64(value)))
55+
}
56+
57+
func (e *otelAttrEncoder) AddInt(key string, value int) {
58+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
59+
}
60+
61+
func (e *otelAttrEncoder) AddInt64(key string, value int64) {
62+
e.attributes = append(e.attributes, attribute.Int64(key, value))
63+
}
64+
65+
func (e *otelAttrEncoder) AddInt32(key string, value int32) {
66+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
67+
}
68+
69+
func (e *otelAttrEncoder) AddInt16(key string, value int16) {
70+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
71+
}
72+
73+
func (e *otelAttrEncoder) AddInt8(key string, value int8) {
74+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
75+
}
76+
77+
func (e *otelAttrEncoder) AddString(key, value string) {
78+
e.attributes = append(e.attributes, attribute.String(key, value))
79+
}
80+
81+
func (e *otelAttrEncoder) AddTime(key string, value time.Time) {
82+
e.attributes = append(e.attributes, attribute.String(key, value.Format(time.RFC3339)))
83+
}
84+
85+
func (e *otelAttrEncoder) AddUint(key string, value uint) {
86+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
87+
}
88+
89+
func (e *otelAttrEncoder) AddUint64(key string, value uint64) {
90+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
91+
}
92+
93+
func (e *otelAttrEncoder) AddUint32(key string, value uint32) {
94+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
95+
}
96+
97+
func (e *otelAttrEncoder) AddUint16(key string, value uint16) {
98+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
99+
}
100+
101+
func (e *otelAttrEncoder) AddUint8(key string, value uint8) {
102+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
103+
}
104+
105+
func (e *otelAttrEncoder) AddUintptr(key string, value uintptr) {
106+
e.attributes = append(e.attributes, attribute.Int64(key, int64(value)))
107+
}
108+
109+
func (e *otelAttrEncoder) AddReflected(key string, value any) error {
110+
// Not implemented
111+
return nil
112+
}
113+
114+
func (e *otelAttrEncoder) OpenNamespace(key string) {
115+
// Not implemented
116+
}

pkg/logger/otelzap/otelzap.go

Lines changed: 6 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package otelzap
33
import (
44
"context"
55
"fmt"
6-
"math"
76
"time"
87

98
"go.opentelemetry.io/otel/attribute"
@@ -84,7 +83,7 @@ func WithLevelEnabler(levelEnabler zapcore.LevelEnabler) Option {
8483
}
8584

8685
func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error {
87-
var attributes []attribute.KeyValue
86+
encoder := &otelAttrEncoder{}
8887
var spanCtx *oteltrace.SpanContext
8988

9089
// Add core-attached fields
@@ -94,15 +93,18 @@ func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error {
9493
spanCtx = &ctxValue
9594
}
9695
} else {
97-
attributes = append(attributes, mapZapField(f))
96+
f.AddTo(encoder)
9897
}
9998
}
10099

101100
// Add fields passed during log call
102101
for _, f := range fields {
103-
attributes = append(attributes, mapZapField(f))
102+
f.AddTo(encoder)
104103
}
105104

105+
// Start with encoder attributes
106+
attributes := encoder.attributes
107+
106108
// Add exception metadata
107109
if entry.Level > zapcore.InfoLevel {
108110
if entry.Caller.Defined {
@@ -146,61 +148,6 @@ func (o OtelZapCore) Write(entry zapcore.Entry, fields []zapcore.Field) error {
146148
return nil
147149
}
148150

149-
func mapZapField(f zapcore.Field) attribute.KeyValue {
150-
switch f.Type {
151-
case zapcore.StringType:
152-
return attribute.String(f.Key, f.String)
153-
154-
case zapcore.Int64Type, zapcore.Int32Type, zapcore.Int16Type, zapcore.Int8Type:
155-
return attribute.Int64(f.Key, f.Integer)
156-
157-
case zapcore.Uint64Type, zapcore.Uint32Type, zapcore.Uint16Type, zapcore.Uint8Type, zapcore.UintptrType:
158-
return attribute.Int64(f.Key, int64(f.Integer))
159-
160-
case zapcore.BoolType:
161-
return attribute.Bool(f.Key, f.Integer == 1)
162-
163-
case zapcore.Float64Type:
164-
return attribute.Float64(f.Key, math.Float64frombits(uint64(f.Integer)))
165-
166-
case zapcore.ErrorType:
167-
if err, ok := f.Interface.(error); ok {
168-
return attribute.String(f.Key, err.Error())
169-
}
170-
return attribute.String(f.Key, "invalid error field")
171-
172-
case zapcore.StringerType:
173-
return attribute.String(f.Key, f.Interface.(fmt.Stringer).String())
174-
175-
case zapcore.TimeType:
176-
if t, ok := f.Interface.(time.Time); ok {
177-
return attribute.String(f.Key, t.Format(time.RFC3339))
178-
}
179-
return attribute.String(f.Key, fmt.Sprintf("invalid time: %v", f.Interface))
180-
181-
case zapcore.DurationType:
182-
if d, ok := f.Interface.(time.Duration); ok {
183-
return attribute.String(f.Key, d.String())
184-
}
185-
return attribute.String(f.Key, fmt.Sprintf("invalid duration: %v", f.Interface))
186-
187-
case zapcore.BinaryType:
188-
if b, ok := f.Interface.([]byte); ok {
189-
return attribute.String(f.Key, fmt.Sprintf("binary data: %x", b))
190-
}
191-
return attribute.String(f.Key, fmt.Sprintf("invalid binary: %v", f.Interface))
192-
193-
case zapcore.ByteStringType:
194-
if b, ok := f.Interface.([]byte); ok {
195-
return attribute.String(f.Key, fmt.Sprintf("byte string: %x", b))
196-
}
197-
return attribute.String(f.Key, fmt.Sprintf("invalid byte string: %v", f.Interface))
198-
199-
default:
200-
return attribute.String(f.Key, f.String)
201-
}
202-
}
203-
204151
func mapZapSeverity(level zapcore.Level) otellog.Severity {
205152
switch level {
206153
case zapcore.DebugLevel:

pkg/logger/otelzap/otelzap_test.go

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,23 @@ type stringerMock struct{}
2121

2222
func (s stringerMock) String() string { return "stringer-value" }
2323

24-
func Test_mapZapField(t *testing.T) {
24+
type customError struct{}
25+
26+
func (e *customError) Error() string {
27+
return "custom error"
28+
}
29+
30+
// panicError will panic if Error() is called on a nil receiver
31+
type panicError struct {
32+
msg string
33+
}
34+
35+
func (e *panicError) Error() string {
36+
// This will panic if e is nil since we're accessing a field
37+
return e.msg
38+
}
39+
40+
func Test_otelAttrEncoder(t *testing.T) {
2541
now := time.Now()
2642
duration := time.Second * 42
2743

@@ -65,69 +81,73 @@ func Test_mapZapField(t *testing.T) {
6581
field: zapcore.Field{Key: "err", Type: zapcore.ErrorType, Interface: errors.New("fail")},
6682
expected: attribute.String("err", "fail"),
6783
},
68-
{
69-
name: "ErrorType invalid",
70-
field: zapcore.Field{Key: "err", Type: zapcore.ErrorType, Interface: "not-an-error"},
71-
expected: attribute.String("err", "invalid error field"),
72-
},
7384
{
7485
name: "StringerType",
7586
field: zapcore.Field{Key: "stringer", Type: zapcore.StringerType, Interface: stringerMock{}},
7687
expected: attribute.String("stringer", "stringer-value"),
7788
},
7889
{
7990
name: "TimeType valid",
80-
field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Interface: now},
91+
field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Integer: now.UnixNano(), Interface: now.Location()},
8192
expected: attribute.String("time", now.Format(time.RFC3339)),
8293
},
83-
{
84-
name: "TimeType invalid",
85-
field: zapcore.Field{Key: "time", Type: zapcore.TimeType, Interface: "not-a-time"},
86-
expected: attribute.String("time", "invalid time: not-a-time"),
87-
},
8894
{
8995
name: "DurationType valid",
90-
field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Interface: duration},
91-
expected: attribute.String("dur", duration.String()),
92-
},
93-
{
94-
name: "DurationType invalid",
95-
field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Interface: "not-a-duration"},
96-
expected: attribute.String("dur", "invalid duration: not-a-duration"),
96+
field: zapcore.Field{Key: "dur", Type: zapcore.DurationType, Integer: int64(duration)},
97+
expected: attribute.Int64("dur", int64(duration)),
9798
},
9899
{
99100
name: "BinaryType valid",
100101
field: zapcore.Field{Key: "bin", Type: zapcore.BinaryType, Interface: []byte{0x1, 0x2}},
101-
expected: attribute.String("bin", "binary data: 0102"),
102-
},
103-
{
104-
name: "BinaryType invalid",
105-
field: zapcore.Field{Key: "bin", Type: zapcore.BinaryType, Interface: "not-bytes"},
106-
expected: attribute.String("bin", "invalid binary: not-bytes"),
102+
expected: attribute.String("bin", "\x01\x02"),
107103
},
108104
{
109105
name: "ByteStringType valid",
110106
field: zapcore.Field{Key: "bs", Type: zapcore.ByteStringType, Interface: []byte{0x3, 0x4}},
111-
expected: attribute.String("bs", "byte string: 0304"),
107+
expected: attribute.String("bs", "\x03\x04"),
112108
},
109+
}
110+
111+
for _, tt := range tests {
112+
t.Run(tt.name, func(t *testing.T) {
113+
encoder := &otelAttrEncoder{}
114+
tt.field.AddTo(encoder)
115+
116+
require.Len(t, encoder.attributes, 1, "Expected exactly one attribute")
117+
got := encoder.attributes[0]
118+
119+
assert.Equal(t, tt.expected.Key, got.Key)
120+
assert.Equal(t, tt.expected.Value.Type(), got.Value.Type())
121+
assert.Equal(t, tt.expected.Value.AsInterface(), got.Value.AsInterface())
122+
})
123+
}
124+
}
125+
126+
func Test_otelAttrEncoder_nilSafety(t *testing.T) {
127+
tests := []struct {
128+
name string
129+
field zapcore.Field
130+
}{
113131
{
114-
name: "ByteStringType invalid",
115-
field: zapcore.Field{Key: "bs", Type: zapcore.ByteStringType, Interface: "not-bytes"},
116-
expected: attribute.String("bs", "invalid byte string: not-bytes"),
132+
name: "StringerType with nil value - should not panic",
133+
field: zapcore.Field{Key: "nil-stringer", Type: zapcore.StringerType, Interface: (*stringerMock)(nil)},
117134
},
118135
{
119-
name: "Default",
120-
field: zapcore.Field{Key: "def", Type: zapcore.UnknownType, String: "default"},
121-
expected: attribute.String("def", "default"),
136+
name: "ErrorType with nil panic-causing value - should not panic",
137+
field: zapcore.Field{Key: "nil-panic-error", Type: zapcore.ErrorType, Interface: (*panicError)(nil)},
122138
},
123139
}
124140

125141
for _, tt := range tests {
126142
t.Run(tt.name, func(t *testing.T) {
127-
got := mapZapField(tt.field)
128-
assert.Equal(t, tt.expected.Key, got.Key)
129-
assert.Equal(t, tt.expected.Value.Type(), got.Value.Type())
130-
assert.Equal(t, tt.expected.Value.AsInterface(), got.Value.AsInterface())
143+
encoder := &otelAttrEncoder{}
144+
145+
// This should not panic - zap's Field.AddTo handles the safety
146+
assert.NotPanics(t, func() {
147+
tt.field.AddTo(encoder)
148+
})
149+
150+
assert.NotEmpty(t, encoder.attributes)
131151
})
132152
}
133153
}

0 commit comments

Comments
 (0)