Skip to content

Commit 06c0c90

Browse files
authored
Factor out opaque context conversion for log package (#43131)
### What does this PR do? Factor out opaque context conversion from `interface{}` to `[]slog.Attr` in the log package. ### Motivation Unify the implementation, and ensure they behave exactly the same. In particular the slog one was missing a check on whether the context slice was odd, which made it fail. ### Describe how you validated your changes CI ### Additional Notes Co-authored-by: pierre.gimalac <pierre.gimalac@datadoghq.com>
1 parent 8f18a55 commit 06c0c90

4 files changed

Lines changed: 47 additions & 52 deletions

File tree

pkg/util/log/init_seelog.go

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,18 @@ func parseShortFilePath(_ string) seelog.FormatterFunc {
2121

2222
func createExtraJSONContext(_ string) seelog.FormatterFunc {
2323
return func(_ string, _ seelog.LogLevel, context seelog.LogContextInterface) interface{} {
24-
contextList, ok := context.CustomContext().([]interface{})
25-
if len(contextList) == 0 || !ok || len(contextList)%2 != 0 {
26-
return ""
27-
}
28-
29-
return formatters.ExtraJSONContext(toSlogAttrs(contextList))
24+
return formatters.ExtraJSONContext(toAttrHolder(context.CustomContext()))
3025
}
3126
}
3227

3328
func createExtraTextContext(_ string) seelog.FormatterFunc {
3429
return func(_ string, _ seelog.LogLevel, context seelog.LogContextInterface) interface{} {
35-
contextList, ok := context.CustomContext().([]interface{})
36-
if len(contextList) == 0 || !ok || len(contextList)%2 != 0 {
37-
return ""
38-
}
39-
return formatters.ExtraTextContext(toSlogAttrs(contextList))
30+
return formatters.ExtraTextContext(toAttrHolder(context.CustomContext()))
4031
}
4132
}
4233

43-
func toSlogAttrs(contextList []interface{}) attrHolderImpl {
44-
attrs := make([]slog.Attr, 0, len(contextList)/2)
45-
for i := 0; i < len(contextList); i += 2 {
46-
key, val := contextList[i], contextList[i+1]
47-
// Only add if key is string
48-
if keyStr, ok := key.(string); ok {
49-
attrs = append(attrs, slog.Attr{Key: keyStr, Value: slog.AnyValue(val)})
50-
}
51-
}
52-
return attrHolderImpl(attrs)
34+
func toAttrHolder(context interface{}) formatters.AttrHolder {
35+
return attrHolderImpl(formatters.ToSlogAttrs(context))
5336
}
5437

5538
type attrHolderImpl []slog.Attr

pkg/util/log/init_seelog_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ func BenchmarkLogFormatShortFilePath(b *testing.B) {
3838
}
3939

4040
func TestExtractContextString(t *testing.T) {
41-
assert.Equal(t, `,"foo":"bar"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", "bar"})))
42-
assert.Equal(t, `foo:bar | `, formatters.ExtraTextContext(toSlogAttrs([]interface{}{"foo", "bar"})))
43-
assert.Equal(t, `,"foo":"bar","bar":"buzz"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", "bar", "bar", "buzz"})))
44-
assert.Equal(t, `foo:bar,bar:buzz | `, formatters.ExtraTextContext(toSlogAttrs([]interface{}{"foo", "bar", "bar", "buzz"})))
45-
assert.Equal(t, `,"foo":"b\"a\"r"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", "b\"a\"r"})))
46-
assert.Equal(t, `,"foo":"3"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", 3})))
47-
assert.Equal(t, `,"foo":"4.131313131"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", float64(4.131313131)})))
48-
assert.Equal(t, "", formatters.ExtraJSONContext(toSlogAttrs(nil)))
49-
assert.Equal(t, "", formatters.ExtraJSONContext(toSlogAttrs([]interface{}{2, 3})))
50-
assert.Equal(t, `,"foo":"bar","bar":"buzz"`, formatters.ExtraJSONContext(toSlogAttrs([]interface{}{"foo", "bar", 2, 3, "bar", "buzz"})))
41+
assert.Equal(t, `,"foo":"bar"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", "bar"})))
42+
assert.Equal(t, `foo:bar | `, formatters.ExtraTextContext(toAttrHolder([]interface{}{"foo", "bar"})))
43+
assert.Equal(t, `,"foo":"bar","bar":"buzz"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", "bar", "bar", "buzz"})))
44+
assert.Equal(t, `foo:bar,bar:buzz | `, formatters.ExtraTextContext(toAttrHolder([]interface{}{"foo", "bar", "bar", "buzz"})))
45+
assert.Equal(t, `,"foo":"b\"a\"r"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", "b\"a\"r"})))
46+
assert.Equal(t, `,"foo":"3"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", 3})))
47+
assert.Equal(t, `,"foo":"4.131313131"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", float64(4.131313131)})))
48+
assert.Equal(t, "", formatters.ExtraJSONContext(toAttrHolder(nil)))
49+
assert.Equal(t, "", formatters.ExtraJSONContext(toAttrHolder([]interface{}{2, 3})))
50+
assert.Equal(t, `,"foo":"bar","bar":"buzz"`, formatters.ExtraJSONContext(toAttrHolder([]interface{}{"foo", "bar", 2, 3, "bar", "buzz"})))
5151
}
5252

5353
func benchmarkLogFormatWithContext(logFormat string, b *testing.B) {

pkg/util/log/slog/formatters/context.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,37 @@ type AttrHolder interface {
3636
NumAttrs() int
3737
}
3838

39+
// ToSlogAttrs converts an opaque context to a list of slog.Attr
40+
//
41+
// The context is expected to be a slice of interface{}, containing an even number of elements,
42+
// with keys being strings.
43+
//
44+
// We can lift the restrictions and/or change the API later, but for now we want
45+
// the exact same behavior as previously.
46+
//
47+
// This is exported to allow using it with seelog and slog, once we stop using seelog
48+
// this can be moved to the slog package.
49+
func ToSlogAttrs(context interface{}) []slog.Attr {
50+
if context == nil {
51+
return nil
52+
}
53+
54+
contextList, ok := context.([]interface{})
55+
if !ok || len(contextList) == 0 || len(contextList)%2 != 0 {
56+
return nil
57+
}
58+
59+
attrs := make([]slog.Attr, 0, len(contextList)/2)
60+
for i := 0; i < len(contextList); i += 2 {
61+
key, val := contextList[i], contextList[i+1]
62+
// Only add if key is string
63+
if keyStr, ok := key.(string); ok {
64+
attrs = append(attrs, slog.Attr{Key: keyStr, Value: slog.AnyValue(val)})
65+
}
66+
}
67+
return attrs
68+
}
69+
3970
func extractContextString(format contextFormat, record AttrHolder) string {
4071
if record.NumAttrs() == 0 {
4172
return ""

pkg/util/log/slog/wrapper.go

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"runtime"
1616
"time"
1717

18+
"github.com/DataDog/datadog-agent/pkg/util/log/slog/formatters"
1819
"github.com/DataDog/datadog-agent/pkg/util/log/types"
1920
)
2021

@@ -191,25 +192,5 @@ func (w *Wrapper) SetAdditionalStackDepth(depth int) error {
191192

192193
// SetContext sets context which will be added to every log records
193194
func (w *Wrapper) SetContext(context interface{}) {
194-
if context == nil {
195-
w.attrs = nil
196-
return
197-
}
198-
199-
// See `extractContextString` in pkg/util/log/setup/log.go:
200-
// the context is a slice of interface{}, it contains an even number of elements,
201-
// and keys are strings.
202-
//
203-
// We can lift the restrictions and/or change the API later, but for now we want
204-
// the exact same behavior as we have with seelog
205-
206-
ctx := context.([]interface{})
207-
var attrs []slog.Attr
208-
for i := 0; i < len(ctx); i += 2 {
209-
key, val := ctx[i], ctx[i+1]
210-
if keyStr, ok := key.(string); ok {
211-
attrs = append(attrs, slog.Attr{Key: keyStr, Value: slog.AnyValue(val)})
212-
}
213-
}
214-
w.attrs = attrs
195+
w.attrs = formatters.ToSlogAttrs(context)
215196
}

0 commit comments

Comments
 (0)