diff --git a/CHANGELOG.md b/CHANGELOG.md index 75e6f953d79..a933db4e781 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Transform raw (`slog.KindAny`) attribute values to matching `log.Value` types. + For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) + ### Fixed - Transform nil attribute values to `log.Value` zero value instead of panicking in `go.opentelemetry.io/contrib/bridges/otellogrus`. (#6237) diff --git a/bridges/otellogrus/hook.go b/bridges/otellogrus/hook.go index 16dbc607926..c63d78e5005 100644 --- a/bridges/otellogrus/hook.go +++ b/bridges/otellogrus/hook.go @@ -27,7 +27,7 @@ // - [logrus.PanicLevel] is transformed to [log.SeverityFatal4] // // Field values are transformed based on their type into log attributes, or -// into a string value if there is no matching type. +// into a string value encoded using [fmt.Sprintf] if there is no matching type. // // [OpenTelemetry]: https://opentelemetry.io/docs/concepts/signals/logs/ package otellogrus // import "go.opentelemetry.io/contrib/bridges/otellogrus" diff --git a/bridges/otelslog/convert.go b/bridges/otelslog/convert.go new file mode 100644 index 00000000000..c8ff8bfbe7a --- /dev/null +++ b/bridges/otelslog/convert.go @@ -0,0 +1,123 @@ +// Code created by gotmpl. DO NOT MODIFY. +// source: internal/shared/logutil/convert.go.tmpl + +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otelslog // import "go.opentelemetry.io/contrib/bridges/otelslog" + +import ( + "fmt" + "math" + "reflect" + "strconv" + "time" + + "go.opentelemetry.io/otel/log" +) + +// convertValue converts various types to log.Value. +func convertValue(v any) log.Value { + // Handling the most common types without reflect is a small perf win. + switch val := v.(type) { + case bool: + return log.BoolValue(val) + case string: + return log.StringValue(val) + case int: + return log.Int64Value(int64(val)) + case int8: + return log.Int64Value(int64(val)) + case int16: + return log.Int64Value(int64(val)) + case int32: + return log.Int64Value(int64(val)) + case int64: + return log.Int64Value(val) + case uint: + return convertUintValue(uint64(val)) + case uint8: + return log.Int64Value(int64(val)) + case uint16: + return log.Int64Value(int64(val)) + case uint32: + return log.Int64Value(int64(val)) + case uint64: + return convertUintValue(val) + case uintptr: + return convertUintValue(uint64(val)) + case float32: + return log.Float64Value(float64(val)) + case float64: + return log.Float64Value(val) + case time.Duration: + return log.Int64Value(val.Nanoseconds()) + case complex64: + r := log.Float64("r", real(complex128(val))) + i := log.Float64("i", imag(complex128(val))) + return log.MapValue(r, i) + case complex128: + r := log.Float64("r", real(val)) + i := log.Float64("i", imag(val)) + return log.MapValue(r, i) + case time.Time: + return log.Int64Value(val.UnixNano()) + case []byte: + return log.BytesValue(val) + case error: + return log.StringValue(val.Error()) + } + + t := reflect.TypeOf(v) + if t == nil { + return log.Value{} + } + val := reflect.ValueOf(v) + switch t.Kind() { + case reflect.Struct: + return log.StringValue(fmt.Sprintf("%+v", v)) + case reflect.Slice, reflect.Array: + items := make([]log.Value, 0, val.Len()) + for i := 0; i < val.Len(); i++ { + items = append(items, convertValue(val.Index(i).Interface())) + } + return log.SliceValue(items...) + case reflect.Map: + kvs := make([]log.KeyValue, 0, val.Len()) + for _, k := range val.MapKeys() { + var key string + switch k.Kind() { + case reflect.String: + key = k.String() + default: + key = fmt.Sprintf("%+v", k.Interface()) + } + kvs = append(kvs, log.KeyValue{ + Key: key, + Value: convertValue(val.MapIndex(k).Interface()), + }) + } + return log.MapValue(kvs...) + case reflect.Ptr, reflect.Interface: + if val.IsNil() { + return log.Value{} + } + return convertValue(val.Elem().Interface()) + } + + // Try to handle this as gracefully as possible. + // + // Don't panic here. it is preferable to have user's open issue + // asking why their attributes have a "unhandled: " prefix than + // say that their code is panicking. + return log.StringValue(fmt.Sprintf("unhandled: (%s) %+v", t, v)) +} + +// convertUintValue converts a uint64 to a log.Value. +// If the value is too large to fit in an int64, it is converted to a string. +func convertUintValue(v uint64) log.Value { + if v > math.MaxInt64 { + return log.StringValue(strconv.FormatUint(v, 10)) + } + return log.Int64Value(int64(v)) +} diff --git a/bridges/otelslog/convert_test.go b/bridges/otelslog/convert_test.go new file mode 100644 index 00000000000..eb0d6fff800 --- /dev/null +++ b/bridges/otelslog/convert_test.go @@ -0,0 +1,265 @@ +// Code created by gotmpl. DO NOT MODIFY. +// source: internal/shared/logutil/convert_test.go.tmpl + +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otelslog + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/log" +) + +func TestConvertValue(t *testing.T) { + for _, tt := range []struct { + name string + value any + wantValue log.Value + }{ + { + name: "bool", + value: true, + wantValue: log.BoolValue(true), + }, + { + name: "string", + value: "value", + wantValue: log.StringValue("value"), + }, + { + name: "int", + value: 10, + wantValue: log.Int64Value(10), + }, + { + name: "int8", + value: int8(127), + wantValue: log.Int64Value(127), + }, + { + name: "int16", + value: int16(32767), + wantValue: log.Int64Value(32767), + }, + { + name: "int32", + value: int32(2147483647), + wantValue: log.Int64Value(2147483647), + }, + { + name: "int64", + value: int64(9223372036854775807), + wantValue: log.Int64Value(9223372036854775807), + }, + { + name: "uint", + value: uint(42), + wantValue: log.Int64Value(42), + }, + { + name: "uint8", + value: uint8(255), + wantValue: log.Int64Value(255), + }, + { + name: "uint16", + value: uint16(65535), + wantValue: log.Int64Value(65535), + }, + { + name: "uint32", + value: uint32(4294967295), + wantValue: log.Int64Value(4294967295), + }, + { + name: "uint64", + value: uint64(9223372036854775807), + wantValue: log.Int64Value(9223372036854775807), + }, + { + name: "uint64-max", + value: uint64(18446744073709551615), + wantValue: log.StringValue("18446744073709551615"), + }, + { + name: "uintptr", + value: uintptr(12345), + wantValue: log.Int64Value(12345), + }, + { + name: "float64", + value: float64(3.14159), + wantValue: log.Float64Value(3.14159), + }, + { + name: "time.Duration", + value: time.Second, + wantValue: log.Int64Value(1_000_000_000), + }, + { + name: "complex64", + value: complex64(complex(float32(1), float32(2))), + wantValue: log.MapValue(log.Float64("r", 1), log.Float64("i", 2)), + }, + { + name: "complex128", + value: complex(float64(3), float64(4)), + wantValue: log.MapValue(log.Float64("r", 3), log.Float64("i", 4)), + }, + { + name: "time.Time", + value: time.Unix(1000, 1000), + wantValue: log.Int64Value(time.Unix(1000, 1000).UnixNano()), + }, + { + name: "[]byte", + value: []byte("hello"), + wantValue: log.BytesValue([]byte("hello")), + }, + { + name: "error", + value: errors.New("test error"), + wantValue: log.StringValue("test error"), + }, + { + name: "error", + value: errors.New("test error"), + wantValue: log.StringValue("test error"), + }, + { + name: "error-nested", + value: fmt.Errorf("test error: %w", errors.New("nested error")), + wantValue: log.StringValue("test error: nested error"), + }, + { + name: "nil", + value: nil, + wantValue: log.Value{}, + }, + { + name: "nil_ptr", + value: (*int)(nil), + wantValue: log.Value{}, + }, + { + name: "int_ptr", + value: func() *int { i := 93; return &i }(), + wantValue: log.Int64Value(93), + }, + { + name: "string_ptr", + value: func() *string { s := "hello"; return &s }(), + wantValue: log.StringValue("hello"), + }, + { + name: "bool_ptr", + value: func() *bool { b := true; return &b }(), + wantValue: log.BoolValue(true), + }, + { + name: "int_empty_array", + value: []int{}, + wantValue: log.SliceValue([]log.Value{}...), + }, + { + name: "int_array", + value: []int{1, 2, 3}, + wantValue: log.SliceValue([]log.Value{ + log.Int64Value(1), + log.Int64Value(2), + log.Int64Value(3), + }...), + }, + { + name: "key_value_map", + value: map[string]int{"one": 1}, + wantValue: log.MapValue( + log.Int64("one", 1), + ), + }, + { + name: "int_string_map", + value: map[int]string{1: "one"}, + wantValue: log.MapValue( + log.String("1", "one"), + ), + }, + { + name: "nested_map", + value: map[string]map[string]int{"nested": {"one": 1}}, + wantValue: log.MapValue( + log.Map("nested", + log.Int64("one", 1), + ), + ), + }, + { + name: "struct_key_map", + value: map[struct{ Name string }]int{ + {Name: "John"}: 42, + }, + wantValue: log.MapValue( + log.Int64("{Name:John}", 42), + ), + }, + { + name: "struct", + value: struct { + Name string + Age int + }{ + Name: "John", + Age: 42, + }, + wantValue: log.StringValue("{Name:John Age:42}"), + }, + { + name: "struct_ptr", + value: &struct { + Name string + Age int + }{ + Name: "John", + Age: 42, + }, + wantValue: log.StringValue("{Name:John Age:42}"), + }, + { + name: "nil_struct_ptr", + value: (*struct { + Name string + Age int + })(nil), + wantValue: log.Value{}, + }, + { + name: "ctx", + value: context.Background(), + wantValue: log.StringValue("context.Background"), + }, + { + name: "unhandled type", + value: chan int(nil), + wantValue: log.StringValue("unhandled: (chan int) "), + }, + } { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantValue, convertValue(tt.value)) + }) + } +} + +func TestConvertValueFloat32(t *testing.T) { + value := convertValue(float32(3.14)) + want := log.Float64Value(3.14) + + assert.InDelta(t, value.AsFloat64(), want.AsFloat64(), 0.0001) +} diff --git a/bridges/otelslog/gen.go b/bridges/otelslog/gen.go new file mode 100644 index 00000000000..14dba573a7a --- /dev/null +++ b/bridges/otelslog/gen.go @@ -0,0 +1,8 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package otelslog // import "go.opentelemetry.io/contrib/bridges/otelslog" + +// Generate convert: +//go:generate gotmpl --body=../../internal/shared/logutil/convert_test.go.tmpl "--data={ \"pkg\": \"otelslog\" }" --out=convert_test.go +//go:generate gotmpl --body=../../internal/shared/logutil/convert.go.tmpl "--data={ \"pkg\": \"otelslog\" }" --out=convert.go diff --git a/bridges/otelslog/handler.go b/bridges/otelslog/handler.go index dc3e985a897..1e252c88f70 100644 --- a/bridges/otelslog/handler.go +++ b/bridges/otelslog/handler.go @@ -26,9 +26,8 @@ // // Attribute values are transformed based on their [slog.Kind]: // -// - [slog.KindAny] non-nil values are transformed to [log.StringValue] -// encoded using [fmt.Sprintf]. -// Nil values are transformed to a zero value of [log.Value]. +// - [slog.KindAny] values are transformed based on their type or +// into a string value encoded using [fmt.Sprintf] if there is no matching type. // - [slog.KindBool] are transformed to [log.BoolValue] directly. // - [slog.KindDuration] are transformed to [log.Int64Value] as nanoseconds. // - [slog.KindFloat64] are transformed to [log.Float64Value] directly. @@ -391,7 +390,7 @@ func (b *kvBuffer) AddAttr(attr slog.Attr) bool { for _, a := range attr.Value.Group() { b.data = append(b.data, log.KeyValue{ Key: a.Key, - Value: convertValue(a.Value), + Value: convert(a.Value), }) } return true @@ -404,18 +403,15 @@ func (b *kvBuffer) AddAttr(attr slog.Attr) bool { } b.data = append(b.data, log.KeyValue{ Key: attr.Key, - Value: convertValue(attr.Value), + Value: convert(attr.Value), }) return true } -func convertValue(v slog.Value) log.Value { +func convert(v slog.Value) log.Value { switch v.Kind() { case slog.KindAny: - if v.Any() == nil { - return log.Value{} - } - return log.StringValue(fmt.Sprintf("%+v", v.Any())) + return convertValue(v.Any()) case slog.KindBool: return log.BoolValue(v.Bool()) case slog.KindDuration: @@ -441,7 +437,7 @@ func convertValue(v slog.Value) log.Value { buf.AddAttrs(g) return log.MapValue(buf.data...) case slog.KindLogValuer: - return convertValue(v.Resolve()) + return convert(v.Resolve()) default: // Try to handle this as gracefully as possible. // diff --git a/bridges/otelslog/handler_test.go b/bridges/otelslog/handler_test.go index febc496bcf2..802774ff29f 100644 --- a/bridges/otelslog/handler_test.go +++ b/bridges/otelslog/handler_test.go @@ -120,7 +120,7 @@ func value2Result(v log.Value) any { case log.KindBytes: return v.AsBytes() case log.KindSlice: - return v.AsSlice() + return v case log.KindMap: m := make(map[string]any) for _, val := range v.AsMap() { @@ -241,6 +241,7 @@ func TestSLogHandler(t *testing.T) { "time", now, "uint64", uint64(3), "nil", nil, + "slice", []string{"foo", "bar"}, // KindGroup and KindLogValuer are left for slogtest.TestHandler. ) }, @@ -256,6 +257,7 @@ func TestSLogHandler(t *testing.T) { hasAttr("time", now.UnixNano()), hasAttr("uint64", int64(3)), hasAttr("nil", nil), + hasAttr("slice", log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))), }}, }, { diff --git a/bridges/otelzap/core.go b/bridges/otelzap/core.go index 962ce5bf315..4e222e37a6a 100644 --- a/bridges/otelzap/core.go +++ b/bridges/otelzap/core.go @@ -27,7 +27,8 @@ // - [zapcore.PanicLevel] is transformed to [log.SeverityFatal2] // - [zapcore.FatalLevel] is transformed to [log.SeverityFatal3] // -// Fields are transformed based on their type into log attributes, or into a string value if there is no matching type. +// Fields are transformed based on their type into log attributes, or +// into a string value encoded using [fmt.Sprintf] if there is no matching type. // // [OpenTelemetry]: https://opentelemetry.io/docs/concepts/signals/logs/ package otelzap // import "go.opentelemetry.io/contrib/bridges/otelzap"