Fix WithName, ctx propagation and errorKey

bridges/otellogr/logsink.go

@@ -19,8 +19,10 @@
 //   - Error is always logged as an additional attribute with the key "err" and
 //     with the severity [log.SeverityError].
 //   - Name is logged as an additional attribute with the key "logger".
-//   - Context is not propagated to the OpenTelemetry log record, as logr does
-//     not provide a way to access it.
+//   - The [context.Context] value in KeyAndValues is propagated to OpenTelemetry
+//     log record. All non-nested [context.Context] values are ignored and not
+//     added as attributes. If there are multiple [context.Context] the last one
+//     is used.
 //
 // The Level is transformed by using the [WithLevelSeverity] option. If this is
 // not provided it would default to a function that adds an offset to the logr
@@ -77,8 +79,8 @@ const (
 	bridgeName = "go.opentelemetry.io/contrib/bridges/otellogr"
 	// nameKey is used to log the `WithName` values as an additional attribute.
 	nameKey = "logger"
-	// errKey is used to log the error parameter of Error as an additional attribute.
-	errKey = "err"
+	// errorKey is used to log the error parameter of Error as an additional attribute.
+	errorKey = "error"
 )
 
 type config struct {
@@ -215,7 +217,7 @@ func (l *LogSink) log(err error, msg string, serverity log.Severity, kvList ...a
 
 	if err != nil {
 		record.AddAttributes(log.KeyValue{
-			Key:   errKey,
+			Key:   errorKey,
 			Value: convertValue(err),
 		})
 	}
@@ -224,12 +226,11 @@ func (l *LogSink) log(err error, msg string, serverity log.Severity, kvList ...a
 		record.AddAttributes(l.values...)
 	}
 
-	kv := convertKVs(kvList)
+	ctx, kv := convertKVs(kvList)
 	if len(kv) > 0 {
 		record.AddAttributes(kv...)
 	}
 
-	ctx := context.Background()
 	l.logger.Emit(ctx, record)
 }
 
@@ -263,23 +264,30 @@ func (l *LogSink) Init(info logr.RuntimeInfo) {
 
 // WithName returns a new LogSink with the specified name appended.
 func (l LogSink) WithName(name string) logr.LogSink {
-	if len(l.name) > 0 {
-		l.name += "/"
+	newLogger := l
+
+	if len(newLogger.name) > 0 {
+		newLogger.name += "/"
 	}
-	l.name += name
-	return &l
+	newLogger.name += name
+
+	return &newLogger
 }
 
 // WithValues returns a new LogSink with additional key/value pairs.
 func (l LogSink) WithValues(keysAndValues ...any) logr.LogSink {
-	attrs := convertKVs(keysAndValues)
+	_, attrs := convertKVs(keysAndValues)
 	l.values = append(l.values, attrs...)
 	return &l
 }
 
-func convertKVs(keysAndValues []any) []log.KeyValue {
+// convertKVs converts a list of key-value pairs to a list of [log.KeyValue].
+// The last [context.Context] value is returned as the context.
+func convertKVs(keysAndValues []any) (context.Context, []log.KeyValue) {
+	ctx := context.Background()
+
 	if len(keysAndValues) == 0 {
-		return nil
+		return ctx, nil
 	}
 	if len(keysAndValues)%2 != 0 {
 		// Ensure an odd number of items here does not corrupt the list
@@ -293,12 +301,20 @@ func convertKVs(keysAndValues []any) []log.KeyValue {
 			// Ensure that the key is a string
 			k = fmt.Sprintf("%v", keysAndValues[i])
 		}
+
+		v := keysAndValues[i+1]
+		if vCtx, ok := v.(context.Context); ok {
+			// Special case when a field is of context.Context type.
+			ctx = vCtx
+			continue
+		}
+
 		kv = append(kv, log.KeyValue{
 			Key:   k,
-			Value: convertValue(keysAndValues[i+1]),
+			Value: convertValue(v),
 		})
 	}
-	return kv
+	return ctx, kv
 }
 
 func convertValue(v any) log.Value {

bridges/otellogr/logsink_test.go

@@ -122,7 +122,7 @@ func TestLogSink(t *testing.T) {
 					Body:     log.StringValue("error message"),
 					Severity: log.SeverityError,
 					Attributes: []log.KeyValue{
-						log.String(errKey, "test error"),
+						log.String(errorKey, "test error"),
 					},
 				},
 			},
@@ -146,7 +146,7 @@ func TestLogSink(t *testing.T) {
 					Body:     log.StringValue("msg"),
 					Severity: log.SeverityError,
 					Attributes: []log.KeyValue{
-						log.String(errKey, "error"),
+						log.String(errorKey, "error"),
 						log.String("struct", "{data:1}"),
 						log.Bool("bool", true),
 						log.Int64("duration", 60_000_000_000),
@@ -244,6 +244,15 @@ func TestLogSink(t *testing.T) {
 	}
 }
 
+func TestLogSinkWithName(t *testing.T) {
+	rec := logtest.NewRecorder()
+	ls := NewLogSink(WithLoggerProvider(rec))
+	lsWithName := ls.WithName("test")
+	require.NotEqual(t, ls, lsWithName)
+
+	require.NotEqual(t, lsWithName, ls.WithName("test"))
+}
+
 func TestLogSinkEnabled(t *testing.T) {
 	rec := logtest.NewRecorder(
 		logtest.WithEnabledFunc(func(ctx context.Context, record log.Record) bool {
@@ -267,11 +276,14 @@ func TestLogSinkEnabled(t *testing.T) {
 }
 
 func TestConvertKVs(t *testing.T) {
+	ctx := context.WithValue(context.Background(), "key", "value") // nolint: revive,staticcheck // test context
+
 	for _, tt := range []struct {
 		name string
 
 		kvs         []any
 		expectedKVs []log.KeyValue
+		expectedCtx context.Context
 	}{
 		{
 			name: "empty",
@@ -307,10 +319,26 @@ func TestConvertKVs(t *testing.T) {
 				log.String("42", "value"),
 			},
 		},
+		{
+			name:        "context",
+			kvs:         []any{"ctx", ctx, "key", "value"},
+			expectedKVs: []log.KeyValue{log.String("key", "value")},
+			expectedCtx: ctx,
+		},
+		{
+			name:        "last_context",
+			kvs:         []any{"key", context.Background(), "ctx", ctx},
+			expectedKVs: []log.KeyValue{},
+			expectedCtx: ctx,
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
-			kvs := convertKVs(tt.kvs)
+			ctx, kvs := convertKVs(tt.kvs)
 			assert.Equal(t, tt.expectedKVs, kvs)
+
+			if tt.expectedCtx != nil {
+				assert.Equal(t, tt.expectedCtx, ctx)
+			}
 		})
 	}
 }