otelzap: Implement namespace method (#5721)

Part of #5191 Pre-work #5279 The approach is to use linked list to open a namespace and keep track of it. `calculate` method iterates to the most nested namespace and appends data in reverse order. --------- Co-authored-by: Robert Pająk <[email protected]>
open-telemetry · Jun 19, 2024 · 48f7f61 · 48f7f61
1 parent c7fb822
commit 48f7f61
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 23 deletions.
diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go
@@ -183,7 +183,8 @@ func convertField(fields []zapcore.Field) (context.Context, []log.KeyValue) {
 		field.AddTo(enc)
 	}
 
-	return ctx, enc.kv
+	enc.calculate(enc.root)
+	return ctx, enc.root.attrs
 }
 
 func convertLevel(level zapcore.Level) log.Severity {

diff --git a/bridges/otelzap/encoder.go b/bridges/otelzap/encoder.go
@@ -18,47 +18,69 @@ var (
 	_ zapcore.ArrayEncoder  = (*arrayEncoder)(nil)
 )
 
+type namespace struct {
+	name  string
+	attrs []log.KeyValue
+	next  *namespace
+}
+
 // objectEncoder implements zapcore.ObjectEncoder.
 // It encodes given fields to OTel key-values.
 type objectEncoder struct {
-	kv []log.KeyValue
+	// root is a pointer to the default namespace
+	root *namespace
+	// cur is a pointer to the namespace we're currently writing to.
+	cur *namespace
 }
 
-// nolint:unused
 func newObjectEncoder(len int) *objectEncoder {
 	keyval := make([]log.KeyValue, 0, len)
-
+	m := &namespace{
+		attrs: keyval,
+	}
 	return &objectEncoder{
-		kv: keyval,
+		root: m,
+		cur:  m,
 	}
 }
 
+// It iterates to the end of the linked list and appends namespace data.
+// Run this function before accessing complete result.
+func (m *objectEncoder) calculate(o *namespace) {
+	if o.next == nil {
+		return
+	}
+	m.calculate(o.next)
+	o.attrs = append(o.attrs, log.Map(o.next.name, o.next.attrs...))
+}
+
 func (m *objectEncoder) AddArray(key string, v zapcore.ArrayMarshaler) error {
 	// TODO: Use arrayEncoder from a pool.
 	arr := &arrayEncoder{}
 	err := v.MarshalLogArray(arr)
-	m.kv = append(m.kv, log.Slice(key, arr.elems...))
+	m.cur.attrs = append(m.cur.attrs, log.Slice(key, arr.elems...))
 	return err
 }
 
 func (m *objectEncoder) AddObject(k string, v zapcore.ObjectMarshaler) error {
 	// TODO: Use objectEncoder from a pool.
 	newobj := newObjectEncoder(2)
 	err := v.MarshalLogObject(newobj)
-	m.kv = append(m.kv, log.Map(k, newobj.kv...))
+	newobj.calculate(newobj.root)
+	m.cur.attrs = append(m.cur.attrs, log.Map(k, newobj.root.attrs...))
 	return err
 }
 
 func (m *objectEncoder) AddBinary(k string, v []byte) {
-	m.kv = append(m.kv, log.Bytes(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.Bytes(k, v))
 }
 
 func (m *objectEncoder) AddByteString(k string, v []byte) {
-	m.kv = append(m.kv, log.String(k, string(v)))
+	m.cur.attrs = append(m.cur.attrs, log.String(k, string(v)))
 }
 
 func (m *objectEncoder) AddBool(k string, v bool) {
-	m.kv = append(m.kv, log.Bool(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.Bool(k, v))
 }
 
 func (m *objectEncoder) AddDuration(k string, v time.Duration) {
@@ -68,35 +90,35 @@ func (m *objectEncoder) AddDuration(k string, v time.Duration) {
 func (m *objectEncoder) AddComplex128(k string, v complex128) {
 	r := log.Float64("r", real(v))
 	i := log.Float64("i", imag(v))
-	m.kv = append(m.kv, log.Map(k, r, i))
+	m.cur.attrs = append(m.cur.attrs, log.Map(k, r, i))
 }
 
 func (m *objectEncoder) AddFloat64(k string, v float64) {
-	m.kv = append(m.kv, log.Float64(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.Float64(k, v))
 }
 
 func (m *objectEncoder) AddInt64(k string, v int64) {
-	m.kv = append(m.kv, log.Int64(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.Int64(k, v))
 }
 
 func (m *objectEncoder) AddInt(k string, v int) {
-	m.kv = append(m.kv, log.Int(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.Int(k, v))
 }
 
 func (m *objectEncoder) AddString(k string, v string) {
-	m.kv = append(m.kv, log.String(k, v))
+	m.cur.attrs = append(m.cur.attrs, log.String(k, v))
 }
 
 func (m *objectEncoder) AddUint64(k string, v uint64) {
-	m.kv = append(m.kv,
+	m.cur.attrs = append(m.cur.attrs,
 		log.KeyValue{
 			Key:   k,
 			Value: assignUintValue(v),
 		})
 }
 
 func (m *objectEncoder) AddReflected(k string, v interface{}) error {
-	m.kv = append(m.kv,
+	m.cur.attrs = append(m.cur.attrs,
 		log.KeyValue{
 			Key:   k,
 			Value: convertValue(v),
@@ -107,7 +129,13 @@ func (m *objectEncoder) AddReflected(k string, v interface{}) error {
 // OpenNamespace opens an isolated namespace where all subsequent fields will
 // be added.
 func (m *objectEncoder) OpenNamespace(k string) {
-	// TODO
+	keyValue := make([]log.KeyValue, 0, 5)
+	s := &namespace{
+		name:  k,
+		attrs: keyValue,
+	}
+	m.cur.next = s
+	m.cur = s
 }
 
 func (m *objectEncoder) AddComplex64(k string, v complex64) {
@@ -179,7 +207,8 @@ func (a *arrayEncoder) AppendObject(v zapcore.ObjectMarshaler) error {
 	// TODO: Use objectEncoder from a pool.
 	m := newObjectEncoder(2)
 	err := v.MarshalLogObject(m)
-	a.elems = append(a.elems, log.MapValue(m.kv...))
+	m.calculate(m.root)
+	a.elems = append(a.elems, log.MapValue(m.root.attrs...))
 	return err
 }
 

diff --git a/bridges/otelzap/encoder_test.go b/bridges/otelzap/encoder_test.go
@@ -182,14 +182,52 @@ func TestObjectEncoder(t *testing.T) {
 			f:        func(e zapcore.ObjectEncoder) { e.AddComplex64("k", 1+2i) },
 			expected: map[string]interface{}{"i": float64(2), "r": float64(1)},
 		},
+		{
+			desc: "OpenNamespace",
+			f: func(e zapcore.ObjectEncoder) {
+				e.OpenNamespace("k")
+				e.AddInt("foo", 1)
+				e.OpenNamespace("middle")
+				e.AddInt("foo", 2)
+				e.OpenNamespace("inner")
+				e.AddInt("foo", 3)
+			},
+			expected: map[string]interface{}{
+				"foo": int64(1),
+				"middle": map[string]interface{}{
+					"foo": int64(2),
+					"inner": map[string]interface{}{
+						"foo": int64(3),
+					},
+				},
+			},
+		},
+		{
+			desc: "object (with nested namespace) then string",
+			f: func(e zapcore.ObjectEncoder) {
+				e.OpenNamespace("k")
+				assert.NoError(t, e.AddObject("obj", maybeNamespace{true}))
+				e.AddString("not-obj", "should-be-outside-obj")
+			},
+			expected: map[string]interface{}{
+				"obj": map[string]interface{}{
+					"obj-out": "obj-outside-namespace",
+					"obj-namespace": map[string]interface{}{
+						"obj-in": "obj-inside-namespace",
+					},
+				},
+				"not-obj": "should-be-outside-obj",
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
 			enc := newObjectEncoder(1)
 			tt.f(enc)
-			require.Len(t, enc.kv, 1)
-			assert.Equal(t, tt.expected, value2Result((enc.kv[0].Value)), "Unexpected encoder output.")
+			enc.calculate(enc.root)
+			require.Len(t, enc.root.attrs, 1)
+			assert.Equal(t, tt.expected, value2Result((enc.root.attrs[0].Value)), "Unexpected encoder output.")
 		})
 	}
 }
@@ -221,6 +259,43 @@ func TestArrayEncoder(t *testing.T) {
 			},
 			expected: map[string]interface{}{"foo": int64(5)},
 		},
+		{
+			desc: "object (no nested namespace) then string",
+			f: func(e zapcore.ArrayEncoder) {
+				err := e.AppendArray(zapcore.ArrayMarshalerFunc(func(inner zapcore.ArrayEncoder) error {
+					err := inner.AppendObject(maybeNamespace{false})
+					inner.AppendString("should-be-outside-obj")
+					return err
+				}))
+				assert.NoError(t, err)
+			},
+			expected: []interface{}{
+				map[string]interface{}{
+					"obj-out": "obj-outside-namespace",
+				},
+				"should-be-outside-obj",
+			},
+		},
+		{
+			desc: "object (with nested namespace) then string",
+			f: func(e zapcore.ArrayEncoder) {
+				err := e.AppendArray(zapcore.ArrayMarshalerFunc(func(inner zapcore.ArrayEncoder) error {
+					err := inner.AppendObject(maybeNamespace{true})
+					inner.AppendString("should-be-outside-obj")
+					return err
+				}))
+				assert.NoError(t, err)
+			},
+			expected: []interface{}{
+				map[string]interface{}{
+					"obj-out": "obj-outside-namespace",
+					"obj-namespace": map[string]interface{}{
+						"obj-in": "obj-inside-namespace",
+					},
+				},
+				"should-be-outside-obj",
+			},
+		},
 		{"AppendBool", func(e zapcore.ArrayEncoder) { e.AppendBool(true) }, true},
 		{"AppendByteString", func(e zapcore.ArrayEncoder) { e.AppendByteString([]byte("foo")) }, "foo"},
 		{"AppendFloat64", func(e zapcore.ArrayEncoder) { e.AppendFloat64(3.14) }, 3.14},
@@ -252,8 +327,8 @@ func TestArrayEncoder(t *testing.T) {
 				tt.f(arr)
 				return nil
 			})), "Expected AddArray to succeed.")
-
-			assert.Equal(t, []interface{}{tt.expected, tt.expected}, value2Result(enc.kv[0].Value), "Unexpected encoder output.")
+			enc.calculate(enc.root)
+			assert.Equal(t, []interface{}{tt.expected, tt.expected}, value2Result(enc.root.attrs[0].Value), "Unexpected encoder output.")
 		})
 	}
 }
@@ -304,6 +379,18 @@ func (l loggable) MarshalLogArray(enc zapcore.ArrayEncoder) error {
 	return nil
 }
 
+// maybeNamespace is an ObjectMarshaler that sometimes opens a namespace.
+type maybeNamespace struct{ bool }
+
+func (m maybeNamespace) MarshalLogObject(enc zapcore.ObjectEncoder) error {
+	enc.AddString("obj-out", "obj-outside-namespace")
+	if m.bool {
+		enc.OpenNamespace("obj-namespace")
+		enc.AddString("obj-in", "obj-inside-namespace")
+	}
+	return nil
+}
+
 func value2Result(v log.Value) any {
 	switch v.Kind() {
 	case log.KindBool: