From 4ebde28ddafc0f225a6d4b19f3c52097e7613e8c Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 2 Jun 2025 10:01:03 +0200 Subject: [PATCH 1/2] otellogr: refactor tests --- bridges/otellogr/logsink_test.go | 275 +++++++++++++++++-------------- 1 file changed, 152 insertions(+), 123 deletions(-) diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index 99bd07f2d33..cd2a2bee224 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -11,7 +11,6 @@ import ( "github.com/go-logr/logr" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/log" @@ -78,13 +77,13 @@ func TestNewLogSink(t *testing.T) { const name = "name" for _, tt := range []struct { - name string - options []Option - wantRecording logtest.Recording + name string + options []Option + want logtest.Recording }{ { name: "with default options", - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{Name: name}: nil, }, }, @@ -95,7 +94,7 @@ func TestNewLogSink(t *testing.T) { WithSchemaURL("https://example.com"), WithAttributes(attribute.String("testattr", "testval")), }, - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{ Name: name, Version: "1.0", @@ -106,19 +105,14 @@ func TestNewLogSink(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - recorder := logtest.NewRecorder() + rec := logtest.NewRecorder() - var l *LogSink - require.NotPanics(t, func() { - l = NewLogSink(name, append( - tt.options, - WithLoggerProvider(recorder), - )...) - }) - require.NotNil(t, l) - require.Len(t, recorder.Result(), 1) + NewLogSink(name, append( + tt.options, + WithLoggerProvider(rec), + )...) - logtest.AssertEqual(t, tt.wantRecording, recorder.Result()) + logtest.AssertEqual(t, tt.want, rec.Result()) }) } } @@ -127,16 +121,16 @@ func TestLogSink(t *testing.T) { const name = "name" for _, tt := range []struct { - name string - f func(*logr.Logger) - levelSeverity func(int) log.Severity - wantRecording logtest.Recording + name string + f func(*logr.Logger) + wantSeverity func(int) log.Severity + want logtest.Recording }{ { name: "no_log", f: func(l *logr.Logger) {}, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{}, + want: logtest.Recording{ + logtest.Scope{Name: name}: nil, }, }, { @@ -144,9 +138,9 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.Info("msg") }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("msg"), log.SeverityInfo, nil), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + {Body: log.StringValue("msg"), Severity: log.SeverityInfo}, }, }, }, @@ -158,12 +152,12 @@ func TestLogSink(t *testing.T) { l.V(2).Info("msg") l.V(3).Info("msg") }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("msg"), log.SeverityInfo, nil), - buildRecord(log.StringValue("msg"), log.SeverityDebug, nil), - buildRecord(log.StringValue("msg"), log.SeverityTrace, nil), - buildRecord(log.StringValue("msg"), log.SeverityTrace, nil), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + {Body: log.StringValue("msg"), Severity: log.SeverityInfo}, + {Body: log.StringValue("msg"), Severity: log.SeverityDebug}, + {Body: log.StringValue("msg"), Severity: log.SeverityTrace}, + {Body: log.StringValue("msg"), Severity: log.SeverityTrace}, }, }, }, @@ -174,7 +168,7 @@ func TestLogSink(t *testing.T) { l.V(1).Info("msg") l.V(2).Info("msg") }, - levelSeverity: func(level int) log.Severity { + wantSeverity: func(level int) log.Severity { switch level { case 1: return log.SeverityError @@ -184,11 +178,11 @@ func TestLogSink(t *testing.T) { return log.SeverityInfo } }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("msg"), log.SeverityInfo, nil), - buildRecord(log.StringValue("msg"), log.SeverityError, nil), - buildRecord(log.StringValue("msg"), log.SeverityWarn, nil), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + {Body: log.StringValue("msg"), Severity: log.SeverityInfo}, + {Body: log.StringValue("msg"), Severity: log.SeverityError}, + {Body: log.StringValue("msg"), Severity: log.SeverityWarn}, }, }, }, @@ -206,18 +200,22 @@ func TestLogSink(t *testing.T) { "uint64", uint64(3), ) }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("msg"), log.SeverityInfo, []log.KeyValue{ - log.String("struct", "{data:1}"), - log.Bool("bool", true), - log.Int64("duration", 60_000_000_000), - log.Float64("float64", 3.14159), - log.Int64("int64", -2), - log.String("string", "str"), - log.Int64("time", time.Unix(1000, 1000).UnixNano()), - log.Int64("uint64", 3), - }), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + { + Body: log.StringValue("msg"), + Severity: log.SeverityInfo, + Attributes: []log.KeyValue{ + log.String("struct", "{data:1}"), + log.Bool("bool", true), + log.Int64("duration", 60_000_000_000), + log.Float64("float64", 3.14159), + log.Int64("int64", -2), + log.String("string", "str"), + log.Int64("time", time.Unix(1000, 1000).UnixNano()), + log.Int64("uint64", 3), + }, + }, }, }, }, @@ -226,10 +224,10 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.WithName("test").Info("info message with name") }, - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{Name: name}: nil, - logtest.Scope{Name: name + "/test"}: []logtest.Record{ - buildRecord(log.StringValue("info message with name"), log.SeverityInfo, nil), + logtest.Scope{Name: name + "/test"}: { + {Body: log.StringValue("info message with name"), Severity: log.SeverityInfo}, }, }, }, @@ -238,11 +236,11 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.WithName("test").WithName("test").Info("info message with name") }, - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{Name: name}: nil, logtest.Scope{Name: name + "/test"}: nil, - logtest.Scope{Name: name + "/test/test"}: []logtest.Record{ - buildRecord(log.StringValue("info message with name"), log.SeverityInfo, nil), + logtest.Scope{Name: name + "/test/test"}: { + {Body: log.StringValue("info message with name"), Severity: log.SeverityInfo}, }, }, }, @@ -251,11 +249,14 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.WithValues("key", "value").Info("info message with attrs") }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("info message with attrs"), log.SeverityInfo, []log.KeyValue{ - log.String("key", "value"), - }), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + { + Body: log.StringValue("info message with attrs"), + Severity: log.SeverityInfo, + Attributes: []log.KeyValue{ + log.String("key", "value"), + }}, }, }, }, @@ -264,12 +265,15 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.WithValues("key1", "value1").Info("info message with attrs", "key2", "value2") }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("info message with attrs"), log.SeverityInfo, []log.KeyValue{ - log.String("key1", "value1"), - log.String("key2", "value2"), - }), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + { + Body: log.StringValue("info message with attrs"), + Severity: log.SeverityInfo, + Attributes: []log.KeyValue{ + log.String("key1", "value1"), + log.String("key2", "value2"), + }}, }, }, }, @@ -279,12 +283,16 @@ func TestLogSink(t *testing.T) { var p *int l.WithValues("key", "value", "nil_pointer", p).Info("info message with attrs") }, - wantRecording: logtest.Recording{ - logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("info message with attrs"), log.SeverityInfo, []log.KeyValue{ - log.String("key", "value"), - log.Empty("nil_pointer"), - }), + want: logtest.Recording{ + logtest.Scope{Name: name}: { + { + Body: log.StringValue("info message with attrs"), + Severity: log.SeverityInfo, + Attributes: []log.KeyValue{ + log.String("key", "value"), + log.Empty("nil_pointer"), + }, + }, }, }, }, @@ -293,11 +301,15 @@ func TestLogSink(t *testing.T) { f: func(l *logr.Logger) { l.Error(errors.New("test"), "error message") }, - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("error message"), log.SeverityError, []log.KeyValue{ - {Key: "exception.message", Value: log.StringValue("test")}, - }), + { + Body: log.StringValue("error message"), + Severity: log.SeverityError, + Attributes: []log.KeyValue{ + log.String("exception.message", "test"), + }, + }, }, }, }, @@ -315,19 +327,23 @@ func TestLogSink(t *testing.T) { "uint64", uint64(3), ) }, - wantRecording: logtest.Recording{ + want: logtest.Recording{ logtest.Scope{Name: name}: []logtest.Record{ - buildRecord(log.StringValue("msg"), log.SeverityError, []log.KeyValue{ - {Key: "exception.message", Value: log.StringValue("test error")}, - log.String("struct", "{data:1}"), - log.Bool("bool", true), - log.Int64("duration", 60_000_000_000), - log.Float64("float64", 3.14159), - log.Int64("int64", -2), - log.String("string", "str"), - log.Int64("time", time.Unix(1000, 1000).UnixNano()), - log.Int64("uint64", 3), - }), + { + Body: log.StringValue("msg"), + Severity: log.SeverityError, + Attributes: []log.KeyValue{ + {Key: "exception.message", Value: log.StringValue("test error")}, + log.String("struct", "{data:1}"), + log.Bool("bool", true), + log.Int64("duration", 60_000_000_000), + log.Float64("float64", 3.14159), + log.Int64("int64", -2), + log.String("string", "str"), + log.Int64("time", time.Unix(1000, 1000).UnixNano()), + log.Int64("uint64", 3), + }, + }, }, }, }, @@ -336,47 +352,69 @@ func TestLogSink(t *testing.T) { rec := logtest.NewRecorder() ls := NewLogSink(name, WithLoggerProvider(rec), - WithLevelSeverity(tt.levelSeverity), + WithLevelSeverity(tt.wantSeverity), ) l := logr.New(ls) tt.f(&l) - logtest.AssertEqual(t, tt.wantRecording, rec.Result()) + logtest.AssertEqual(t, tt.want, rec.Result(), + logtest.Transform(func(r logtest.Record) logtest.Record { + r.Context = nil // Ignore context for comparison. + return r + }), + ) }) } } -// fix https://github.com/open-telemetry/opentelemetry-go-contrib/issues/6509 -func TestLogSinkCtxInInfo(t *testing.T) { - rec := logtest.NewRecorder() - ls := NewLogSink("name", WithLoggerProvider(rec)) - l := logr.New(ls) - ctx := context.WithValue(context.Background(), "key", "value") // nolint:revive,staticcheck +func TestLogSinkContext(t *testing.T) { + name := "name" + ctx := context.WithValue(context.Background(), "key", "value") // nolint:revive,staticcheck // test context tests := []struct { - name string - keyValues []any - wantCtxFunc func(ctx context.Context) + name string + f func(*logr.Logger) + want logtest.Recording }{ - {"default", nil, func(ctx context.Context) { - assert.Equal(t, context.Background(), ctx) - }}, - {"default with context", []any{"ctx", ctx}, func(ctx context.Context) { - assert.Equal(t, "value", ctx.Value("key")) - }}, + { + name: "default", + f: func(l *logr.Logger) { + l.Info("msg") + }, + want: logtest.Recording{ + logtest.Scope{Name: name}: { + {Context: context.Background()}, + }, + }, + }, + { + name: "context in KeyAndValues", + f: func(l *logr.Logger) { + l.WithValues("ctx", ctx).Info("msg") + }, + want: logtest.Recording{ + logtest.Scope{Name: name}: { + {Context: ctx}, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - defer rec.Reset() - - l.Info("msg", tt.keyValues...) - require.Len(t, rec.Result(), 1) + rec := logtest.NewRecorder() + ls := NewLogSink(name, WithLoggerProvider(rec)) + l := logr.New(ls) + tt.f(&l) - got := rec.Result() - records := got[logtest.Scope{Name: "name"}] - require.Len(t, records, 1) - tt.wantCtxFunc(records[0].Context) + logtest.AssertEqual(t, tt.want, rec.Result(), + logtest.Transform(func(r logtest.Record) logtest.Record { + // Only compare the context, ignore the rest. + return logtest.Record{ + Context: r.Context, + } + }), + ) }) } } @@ -404,15 +442,6 @@ func TestLogSinkEnabled(t *testing.T) { assert.False(t, ls.Enabled(1)) } -func buildRecord(body log.Value, severity log.Severity, attrs []log.KeyValue) logtest.Record { - return logtest.Record{ - Context: context.Background(), - Body: body, - Severity: severity, - Attributes: attrs, - } -} - func TestConvertKVs(t *testing.T) { ctx := context.WithValue(context.Background(), "key", "value") // nolint: revive,staticcheck // test context From d0edd5b8bbd6f56bbcf3882fcf653bfe7b7cea8d Mon Sep 17 00:00:00 2001 From: Robert Pajak Date: Mon, 2 Jun 2025 10:41:39 +0200 Subject: [PATCH 2/2] fmt --- bridges/otellogr/logsink_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bridges/otellogr/logsink_test.go b/bridges/otellogr/logsink_test.go index cd2a2bee224..b7746f6fb0c 100644 --- a/bridges/otellogr/logsink_test.go +++ b/bridges/otellogr/logsink_test.go @@ -256,7 +256,8 @@ func TestLogSink(t *testing.T) { Severity: log.SeverityInfo, Attributes: []log.KeyValue{ log.String("key", "value"), - }}, + }, + }, }, }, }, @@ -273,7 +274,8 @@ func TestLogSink(t *testing.T) { Attributes: []log.KeyValue{ log.String("key1", "value1"), log.String("key2", "value2"), - }}, + }, + }, }, }, },