diff --git a/CHANGELOG.md b/CHANGELOG.md index d13497c6ae0..79760b035bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `sdktrace.TraceProvider.Shutdown` and `sdktrace.TraceProvider.ForceFlush` to not return error when no processor register. (#3268) +### Fixed + +- Slice attributes of `attribute` package are now comparable based on their value, not instance. (#3108 #3252) + ## [1.11.0/0.32.3] 2022-10-12 ### Added diff --git a/attribute/value.go b/attribute/value.go index 57899f682e7..80a37bd6ff3 100644 --- a/attribute/value.go +++ b/attribute/value.go @@ -17,9 +17,11 @@ package attribute // import "go.opentelemetry.io/otel/attribute" import ( "encoding/json" "fmt" + "reflect" "strconv" "go.opentelemetry.io/otel/internal" + "go.opentelemetry.io/otel/internal/attribute" ) //go:generate stringer -type=Type @@ -66,12 +68,7 @@ func BoolValue(v bool) Value { // BoolSliceValue creates a BOOLSLICE Value. func BoolSliceValue(v []bool) Value { - cp := make([]bool, len(v)) - copy(cp, v) - return Value{ - vtype: BOOLSLICE, - slice: &cp, - } + return Value{vtype: BOOLSLICE, slice: attribute.SliceValue(v)} } // IntValue creates an INT64 Value. @@ -81,13 +78,14 @@ func IntValue(v int) Value { // IntSliceValue creates an INTSLICE Value. func IntSliceValue(v []int) Value { - cp := make([]int64, 0, len(v)) - for _, i := range v { - cp = append(cp, int64(i)) + var int64Val int64 + cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeOf(int64Val))) + for i, val := range v { + cp.Elem().Index(i).SetInt(int64(val)) } return Value{ vtype: INT64SLICE, - slice: &cp, + slice: cp.Elem().Interface(), } } @@ -101,12 +99,7 @@ func Int64Value(v int64) Value { // Int64SliceValue creates an INT64SLICE Value. func Int64SliceValue(v []int64) Value { - cp := make([]int64, len(v)) - copy(cp, v) - return Value{ - vtype: INT64SLICE, - slice: &cp, - } + return Value{vtype: INT64SLICE, slice: attribute.SliceValue(v)} } // Float64Value creates a FLOAT64 Value. @@ -119,12 +112,7 @@ func Float64Value(v float64) Value { // Float64SliceValue creates a FLOAT64SLICE Value. func Float64SliceValue(v []float64) Value { - cp := make([]float64, len(v)) - copy(cp, v) - return Value{ - vtype: FLOAT64SLICE, - slice: &cp, - } + return Value{vtype: FLOAT64SLICE, slice: attribute.SliceValue(v)} } // StringValue creates a STRING Value. @@ -137,12 +125,7 @@ func StringValue(v string) Value { // StringSliceValue creates a STRINGSLICE Value. func StringSliceValue(v []string) Value { - cp := make([]string, len(v)) - copy(cp, v) - return Value{ - vtype: STRINGSLICE, - slice: &cp, - } + return Value{vtype: STRINGSLICE, slice: attribute.SliceValue(v)} } // Type returns a type of the Value. @@ -159,10 +142,7 @@ func (v Value) AsBool() bool { // AsBoolSlice returns the []bool value. Make sure that the Value's type is // BOOLSLICE. func (v Value) AsBoolSlice() []bool { - if s, ok := v.slice.(*[]bool); ok { - return *s - } - return nil + return attribute.AsSlice[bool](v.slice) } // AsInt64 returns the int64 value. Make sure that the Value's type is @@ -174,10 +154,7 @@ func (v Value) AsInt64() int64 { // AsInt64Slice returns the []int64 value. Make sure that the Value's type is // INT64SLICE. func (v Value) AsInt64Slice() []int64 { - if s, ok := v.slice.(*[]int64); ok { - return *s - } - return nil + return attribute.AsSlice[int64](v.slice) } // AsFloat64 returns the float64 value. Make sure that the Value's @@ -189,10 +166,7 @@ func (v Value) AsFloat64() float64 { // AsFloat64Slice returns the []float64 value. Make sure that the Value's type is // FLOAT64SLICE. func (v Value) AsFloat64Slice() []float64 { - if s, ok := v.slice.(*[]float64); ok { - return *s - } - return nil + return attribute.AsSlice[float64](v.slice) } // AsString returns the string value. Make sure that the Value's type @@ -204,10 +178,7 @@ func (v Value) AsString() string { // AsStringSlice returns the []string value. Make sure that the Value's type is // STRINGSLICE. func (v Value) AsStringSlice() []string { - if s, ok := v.slice.(*[]string); ok { - return *s - } - return nil + return attribute.AsSlice[string](v.slice) } type unknownValueType struct{} @@ -239,19 +210,19 @@ func (v Value) AsInterface() interface{} { func (v Value) Emit() string { switch v.Type() { case BOOLSLICE: - return fmt.Sprint(*(v.slice.(*[]bool))) + return fmt.Sprint(v.AsBoolSlice()) case BOOL: return strconv.FormatBool(v.AsBool()) case INT64SLICE: - return fmt.Sprint(*(v.slice.(*[]int64))) + return fmt.Sprint(v.AsInt64Slice()) case INT64: return strconv.FormatInt(v.AsInt64(), 10) case FLOAT64SLICE: - return fmt.Sprint(*(v.slice.(*[]float64))) + return fmt.Sprint(v.AsFloat64Slice()) case FLOAT64: return fmt.Sprint(v.AsFloat64()) case STRINGSLICE: - return fmt.Sprint(*(v.slice.(*[]string))) + return fmt.Sprint(v.AsStringSlice()) case STRING: return v.stringly default: diff --git a/attribute/value_test.go b/attribute/value_test.go index c9370880b54..ec79dc62a27 100644 --- a/attribute/value_test.go +++ b/attribute/value_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/attribute" ) @@ -104,3 +105,82 @@ func TestValue(t *testing.T) { } } } + +func TestSetComparability(t *testing.T) { + pairs := [][2]attribute.KeyValue{ + { + attribute.Bool("Bool", true), + attribute.Bool("Bool", true), + }, + { + attribute.BoolSlice("BoolSlice", []bool{true, false, true}), + attribute.BoolSlice("BoolSlice", []bool{true, false, true}), + }, + { + attribute.Int("Int", 34), + attribute.Int("Int", 34), + }, + { + attribute.IntSlice("IntSlice", []int{312, 1, -2}), + attribute.IntSlice("IntSlice", []int{312, 1, -2}), + }, + { + attribute.Int64("Int64", 98), + attribute.Int64("Int64", 98), + }, + { + attribute.Int64Slice("Int64Slice", []int64{12, 1298, -219, 2}), + attribute.Int64Slice("Int64Slice", []int64{12, 1298, -219, 2}), + }, + { + attribute.Float64("Float64", 19.09), + attribute.Float64("Float64", 19.09), + }, + { + attribute.Float64Slice("Float64Slice", []float64{12398.1, -37.1713873737, 3}), + attribute.Float64Slice("Float64Slice", []float64{12398.1, -37.1713873737, 3}), + }, + { + attribute.String("String", "string value"), + attribute.String("String", "string value"), + }, + { + attribute.StringSlice("StringSlice", []string{"one", "two", "three"}), + attribute.StringSlice("StringSlice", []string{"one", "two", "three"}), + }, + } + + for _, p := range pairs { + s0, s1 := attribute.NewSet(p[0]), attribute.NewSet(p[1]) + m := map[attribute.Set]struct{}{s0: {}} + _, ok := m[s1] + assert.Truef(t, ok, "%s not comparable", p[0].Value.Type()) + } +} + +func TestAsSlice(t *testing.T) { + bs1 := []bool{true, false, true} + kv := attribute.BoolSlice("BoolSlice", bs1) + bs2 := kv.Value.AsBoolSlice() + assert.Equal(t, bs1, bs2) + + i64s1 := []int64{12, 1298, -219, 2} + kv = attribute.Int64Slice("Int64Slice", i64s1) + i64s2 := kv.Value.AsInt64Slice() + assert.Equal(t, i64s1, i64s2) + + is1 := []int{12, 1298, -219, 2} + kv = attribute.IntSlice("IntSlice", is1) + i64s2 = kv.Value.AsInt64Slice() + assert.Equal(t, i64s1, i64s2) + + fs1 := []float64{12398.1, -37.1713873737, 3} + kv = attribute.Float64Slice("Float64Slice", fs1) + fs2 := kv.Value.AsFloat64Slice() + assert.Equal(t, fs1, fs2) + + ss1 := []string{"one", "two", "three"} + kv = attribute.StringSlice("StringSlice", ss1) + ss2 := kv.Value.AsStringSlice() + assert.Equal(t, ss1, ss2) +} diff --git a/internal/attribute/attribute.go b/internal/attribute/attribute.go new file mode 100644 index 00000000000..22034894473 --- /dev/null +++ b/internal/attribute/attribute.go @@ -0,0 +1,45 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/* +Package attribute provide several helper functions for some commonly used +logic of processing attributes. +*/ +package attribute // import "go.opentelemetry.io/otel/internal/attribute" + +import ( + "reflect" +) + +// SliceValue convert a slice into an array with same elements as slice. +func SliceValue[T bool | int64 | float64 | string](v []T) any { + var zero T + cp := reflect.New(reflect.ArrayOf(len(v), reflect.TypeOf(zero))) + copy(cp.Elem().Slice(0, len(v)).Interface().([]T), v) + return cp.Elem().Interface() +} + +// AsSlice convert an array into a slice into with same elements as array. +func AsSlice[T bool | int64 | float64 | string](v any) []T { + rv := reflect.ValueOf(v) + if rv.Type().Kind() != reflect.Array { + return nil + } + var zero T + correctLen := rv.Len() + correctType := reflect.ArrayOf(correctLen, reflect.TypeOf(zero)) + cpy := reflect.New(correctType) + _ = reflect.Copy(cpy.Elem(), rv) + return cpy.Elem().Slice(0, correctLen).Interface().([]T) +} diff --git a/sdk/trace/span.go b/sdk/trace/span.go index c7cf8e94ec9..b5d6f544176 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -313,26 +313,13 @@ func truncateAttr(limit int, attr attribute.KeyValue) attribute.KeyValue { return attr.Key.String(safeTruncate(v, limit)) } case attribute.STRINGSLICE: - // Do no mutate the original, make a copy. - trucated := attr.Key.StringSlice(attr.Value.AsStringSlice()) - // Do not do this. - // - // v := trucated.Value.AsStringSlice() - // cp := make([]string, len(v)) - // /* Copy and truncate values to cp ... */ - // trucated.Value = attribute.StringSliceValue(cp) - // - // Copying the []string and then assigning it back as a new value with - // attribute.StringSliceValue will copy the data twice. Instead, we - // already made a copy above that only this function owns, update the - // underlying slice data of our copy. - v := trucated.Value.AsStringSlice() + v := attr.Value.AsStringSlice() for i := range v { if len(v[i]) > limit { v[i] = safeTruncate(v[i], limit) } } - return trucated + return attr.Key.StringSlice(v) } return attr }