From 07215d123d4e26eba2e2ec2657b1c0efdb6644f4 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Sat, 27 Jul 2024 02:09:20 +0100 Subject: [PATCH 01/21] Add OTTL sort converter --- pkg/ottl/e2e/e2e_test.go | 20 +++ pkg/ottl/ottlfuncs/func_sort.go | 176 +++++++++++++++++++++++++++ pkg/ottl/ottlfuncs/func_sort_test.go | 142 +++++++++++++++++++++ pkg/ottl/ottlfuncs/functions.go | 1 + 4 files changed, 339 insertions(+) create mode 100644 pkg/ottl/ottlfuncs/func_sort.go create mode 100644 pkg/ottl/ottlfuncs/func_sort_test.go diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index f26168acfa22..7f911d57a4f1 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -619,6 +619,24 @@ func Test_e2e_converters(t *testing.T) { tCtx.GetLogRecord().Attributes().PutStr("test", "d74ff0ee8da3b9806b18c877dbf29bbde50b5bd8e4dad7a3a725000feb82e8f1") }, }, + { + statement: `set(attributes["test"], Sort(attributes["slice.any"], "desc"))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetStr("slice") + s.AppendEmpty().SetStr("am") + s.AppendEmpty().SetStr("1") + }, + }, + { + statement: `set(attributes["test"], Sort([Double(11), Double(2.2), Double(-1)]))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetDouble(-1) + s.AppendEmpty().SetDouble(2.2) + s.AppendEmpty().SetDouble(11) + }, + }, { statement: `set(span_id, SpanID(0x0000000000000000))`, want: func(tCtx ottllog.TransformContext) { @@ -921,6 +939,8 @@ func constructLogTransformContext() ottllog.TransformContext { v.SetStr("val") m2 := m.PutEmptyMap("nested") m2.PutStr("test", "pass") + ss := logRecord.Attributes().PutEmptySlice("slice.any") + _ = ss.FromRaw([]any{1, "am", "slice"}) return ottllog.NewTransformContext(logRecord, scope, resource, plog.NewScopeLogs(), plog.NewResourceLogs()) } diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go new file mode 100644 index 000000000000..94e382c37b54 --- /dev/null +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -0,0 +1,176 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs" + +import ( + "cmp" + "context" + "fmt" + "slices" + + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" +) + +const ( + sortAsc = "asc" + sortDesc = "desc" +) + +type SortArguments[K any] struct { + Target ottl.Getter[K] + Order ottl.Optional[string] +} + +func NewSortFactory[K any]() ottl.Factory[K] { + return ottl.NewFactory("Sort", &SortArguments[K]{}, createSortFunction[K]) +} + +func createSortFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ottl.ExprFunc[K], error) { + args, ok := oArgs.(*SortArguments[K]) + + if !ok { + return nil, fmt.Errorf("SortFactory args must be of type *SortArguments[K]") + } + + order := sortAsc + if !args.Order.IsEmpty() { + o := args.Order.Get() + switch o { + case sortAsc, sortDesc: + order = o + default: + return nil, fmt.Errorf("invalid arguments: %s. Order should be either \"%s\" or \"%s\"", o, sortAsc, sortDesc) + } + } + + return Sort(args.Target, order) +} + +func Sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) { + return func(ctx context.Context, tCtx K) (any, error) { + val, err := target.Get(ctx, tCtx) + if err != nil { + return nil, err + } + + if order != sortAsc && order != sortDesc { + return nil, fmt.Errorf("invalid arguments: %s. Order should be either \"%s\" or \"%s\"", order, sortAsc, sortDesc) + } + + switch v := val.(type) { + case pcommon.Slice: + return sortSlice(v, order) + case []any: + // handle Sort([1,2,3]) + slice := pcommon.NewValueSlice().SetEmptySlice() + if err := slice.FromRaw(v); err != nil { + return v, nil + } + return sortSlice(slice, order) + default: + return v, nil + } + }, nil +} + +func sortSlice(slice pcommon.Slice, order string) (any, error) { + length := slice.Len() + if length == 0 { + return slice, nil + } + + commonType, ok := getCommonValueType(slice) + if !ok { + return slice, nil + } + + switch commonType { + case pcommon.ValueTypeInt: + arr := makeTypedCopy(length, func(idx int) int64 { + return slice.At(idx).Int() + }) + return sortTypedSlice(arr, order), nil + case pcommon.ValueTypeDouble: + arr := makeTypedCopy(length, func(idx int) float64 { + return slice.At(idx).Double() + }) + return sortTypedSlice(arr, order), nil + case pcommon.ValueTypeStr: + arr := makeTypedCopy(length, func(idx int) string { + return slice.At(idx).AsString() + }) + return sortTypedSlice(arr, order), nil + default: + return nil, fmt.Errorf("sort with unsupported type: '%T'", commonType) + } +} + +type TargetType interface { + ~int64 | ~float64 | ~string +} + +// getCommonValueType determines the most appropriate common type for all elements in a pcommon.Slice. +// It returns two values: +// - A pcommon.ValueType representing the desired common type for all elements. +// Mixed types, String, Bool, and Empty types return ValueTypeStr as they require string conversion for comparison. +// Numeric types return either ValueTypeInt or ValueTypeDouble. +// - A boolean indicating whether a common type could be determined (true) or not (false). +// returns false for ValueTypeMap, ValueTypeSlice and ValueTypeBytes. They are unsupported types for sort. +func getCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { + length := slice.Len() + if length == 0 { + return pcommon.ValueTypeEmpty, false + } + + wantType := slice.At(0).Type() + wantStr := false + + for i := 0; i < length; i++ { + value := slice.At(i) + currType := value.Type() + + if currType != wantType { + wantStr = true + } + + switch currType { + case pcommon.ValueTypeInt, pcommon.ValueTypeDouble: + case pcommon.ValueTypeStr, pcommon.ValueTypeBool, pcommon.ValueTypeEmpty: + wantStr = true + default: + return pcommon.ValueTypeEmpty, false + } + } + + if wantStr { + wantType = pcommon.ValueTypeStr + } + + return wantType, true +} + +func makeTypedCopy[T TargetType](length int, converter func(idx int) T) []T { + var arr []T + for i := 0; i < length; i++ { + arr = append(arr, converter(i)) + } + return arr +} + +func sortTypedSlice[T TargetType](arr []T, order string) []T { + if len(arr) == 0 { + return arr + } + + slices.SortFunc(arr, func(a, b T) int { + if order == sortDesc { + return cmp.Compare(b, a) + } + return cmp.Compare(a, b) + }) + + return arr +} diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go new file mode 100644 index 000000000000..e457528e53b1 --- /dev/null +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -0,0 +1,142 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package ottlfuncs + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" +) + +func Test_Sort(t *testing.T) { + + pMap := pcommon.NewValueMap().SetEmptyMap() + pMap.PutStr("k", "v") + + tests := []struct { + name string + getter ottl.Getter[any] + order string + expected any + err bool + }{ + { + name: "int slice", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{9, 6, 3}) + return s, nil + }, + }, + order: sortAsc, + expected: []int64{3, 6, 9}, + }, + { + name: "int slice desc", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{3, 6, 9}) + return s, nil + }, + }, + order: sortDesc, + expected: []int64{9, 6, 3}, + }, + { + name: "string slice", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{"i", "am", "awesome", "slice"}) + return s, nil + }, + }, + order: sortAsc, + expected: []string{"am", "awesome", "i", "slice"}, + }, + { + name: "bool slice compares as string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{true, false, true, false}) + return s, nil + }, + }, + order: sortAsc, + expected: []string{"false", "false", "true", "true"}, + }, + { + name: "mixed types slice compares as string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{1, "two", 3.33, false}) + return s, nil + }, + }, + order: sortAsc, + expected: []string{"1", "3.33", "false", "two"}, + }, + { + name: "[]any compares as string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []any{1, "two", 3.33, false}, nil + }, + }, + order: sortAsc, + expected: []string{"1", "3.33", "false", "two"}, + }, + { + name: "unsupported ValueTypeMap remains unchanged", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return pMap, nil + }, + }, + order: sortAsc, + expected: pMap, + }, + { + name: "unsupported bytes remains unchanged", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []byte("still fine"), nil + }, + }, + order: sortAsc, + expected: []byte("still fine"), + }, + { + name: "invalid slice remains unchanged", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []any{map[string]string{"some": "invalid kv"}}, nil + }, + }, + order: sortAsc, + expected: []any{map[string]string{"some": "invalid kv"}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + exprFunc, err := Sort(tt.getter, tt.order) + assert.NoError(t, err) + result, err := exprFunc(nil, nil) + if tt.err { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/ottl/ottlfuncs/functions.go b/pkg/ottl/ottlfuncs/functions.go index 48b3aa330ea4..5320fb7a7fe9 100644 --- a/pkg/ottl/ottlfuncs/functions.go +++ b/pkg/ottl/ottlfuncs/functions.go @@ -70,6 +70,7 @@ func converters[K any]() []ottl.Factory[K] { NewSecondsFactory[K](), NewSHA1Factory[K](), NewSHA256Factory[K](), + NewSortFactory[K](), NewSpanIDFactory[K](), NewSplitFactory[K](), NewFormatFactory[K](), From 04366f53e4c303568c8a3205268274d04c7dbf7a Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Sat, 27 Jul 2024 21:37:28 +0100 Subject: [PATCH 02/21] support mixed of numeric types to sort as double --- pkg/ottl/e2e/e2e_test.go | 2 +- pkg/ottl/ottlfuncs/func_sort.go | 42 ++++++++++++++------ pkg/ottl/ottlfuncs/func_sort_test.go | 57 ++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index 7f911d57a4f1..c5c20ba7d34a 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -629,7 +629,7 @@ func Test_e2e_converters(t *testing.T) { }, }, { - statement: `set(attributes["test"], Sort([Double(11), Double(2.2), Double(-1)]))`, + statement: `set(attributes["test"], Sort([Int(11), Double(2.2), Double(-1)]))`, want: func(tCtx ottllog.TransformContext) { s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") s.AppendEmpty().SetDouble(-1) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 94e382c37b54..07d3b8a1a85b 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -76,13 +76,22 @@ func Sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) }, nil } +// sortSlice sorts a pcommon.Slice based on the specified order. +// It gets the common type for all elements in the slice and converts all elements to this common type, creating a new copy +// Parameters: +// - slice: The pcommon.Slice to be sorted +// - order: The sort order. "asc" for ascending, "desc" for descending +// +// Returns: +// - A sorted slice as []any or the original pcommon.Slice +// - An error if an unsupported type is encountered func sortSlice(slice pcommon.Slice, order string) (any, error) { length := slice.Len() if length == 0 { return slice, nil } - commonType, ok := getCommonValueType(slice) + commonType, ok := findCommonValueType(slice) if !ok { return slice, nil } @@ -95,7 +104,12 @@ func sortSlice(slice pcommon.Slice, order string) (any, error) { return sortTypedSlice(arr, order), nil case pcommon.ValueTypeDouble: arr := makeTypedCopy(length, func(idx int) float64 { - return slice.At(idx).Double() + s := slice.At(idx) + if s.Type() == pcommon.ValueTypeInt { + return float64(s.Int()) + } + + return s.Double() }) return sortTypedSlice(arr, order), nil case pcommon.ValueTypeStr: @@ -112,14 +126,14 @@ type TargetType interface { ~int64 | ~float64 | ~string } -// getCommonValueType determines the most appropriate common type for all elements in a pcommon.Slice. +// findCommonValueType determines the most appropriate common type for all elements in a pcommon.Slice. // It returns two values: // - A pcommon.ValueType representing the desired common type for all elements. -// Mixed types, String, Bool, and Empty types return ValueTypeStr as they require string conversion for comparison. -// Numeric types return either ValueTypeInt or ValueTypeDouble. +// Mixed Numeric types return ValueTypeDouble. Integer type returns ValueTypeInt. Double type returns ValueTypeDouble. +// String, Bool, Empty and mixed of the mentioned types return ValueTypeStr, as they require string conversion for comparison. // - A boolean indicating whether a common type could be determined (true) or not (false). // returns false for ValueTypeMap, ValueTypeSlice and ValueTypeBytes. They are unsupported types for sort. -func getCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { +func findCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { length := slice.Len() if length == 0 { return pcommon.ValueTypeEmpty, false @@ -127,17 +141,21 @@ func getCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { wantType := slice.At(0).Type() wantStr := false + wantDouble := false for i := 0; i < length; i++ { value := slice.At(i) currType := value.Type() - if currType != wantType { - wantStr = true - } - switch currType { - case pcommon.ValueTypeInt, pcommon.ValueTypeDouble: + case pcommon.ValueTypeInt: + if wantType == pcommon.ValueTypeDouble { + wantDouble = true + } + case pcommon.ValueTypeDouble: + if wantType == pcommon.ValueTypeInt { + wantDouble = true + } case pcommon.ValueTypeStr, pcommon.ValueTypeBool, pcommon.ValueTypeEmpty: wantStr = true default: @@ -147,6 +165,8 @@ func getCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { if wantStr { wantType = pcommon.ValueTypeStr + } else if wantDouble { + wantType = pcommon.ValueTypeDouble } return wantType, true diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index e457528e53b1..dfa2253bbed5 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -61,6 +61,18 @@ func Test_Sort(t *testing.T) { order: sortAsc, expected: []string{"am", "awesome", "i", "slice"}, }, + { + name: "double slice desc", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{0.1829374652374, -3.4029435828374, 9.7425639845731}) + return s, nil + }, + }, + order: sortDesc, + expected: []float64{9.7425639845731, 0.1829374652374, -3.4029435828374}, + }, { name: "bool slice compares as string", getter: ottl.StandardGetSetter[any]{ @@ -85,6 +97,30 @@ func Test_Sort(t *testing.T) { order: sortAsc, expected: []string{"1", "3.33", "false", "two"}, }, + { + name: "mixed numeric types slice compares as double", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{0, 2, 3.33, 0}) + return s, nil + }, + }, + order: sortAsc, + expected: []float64{0, 0, 2, 3.33}, + }, + { + name: "mixed numeric types slice compares as double desc", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{3.14, 2, 3.33, 0}) + return s, nil + }, + }, + order: sortDesc, + expected: []float64{3.33, 3.14, 2, 0}, + }, { name: "[]any compares as string", getter: ottl.StandardGetSetter[any]{ @@ -115,6 +151,16 @@ func Test_Sort(t *testing.T) { order: sortAsc, expected: []byte("still fine"), }, + { + name: "unsupported string remains unchanged", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return "no change", nil + }, + }, + order: sortAsc, + expected: "no change", + }, { name: "invalid slice remains unchanged", getter: ottl.StandardGetSetter[any]{ @@ -125,6 +171,17 @@ func Test_Sort(t *testing.T) { order: sortAsc, expected: []any{map[string]string{"some": "invalid kv"}}, }, + { + name: "invalid sort order", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []any{1, 2, 3}, nil + }, + }, + order: "dddd", + expected: nil, + err: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From e2b90e17fc592bab7a3aad13a742ea3a57466b57 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Sun, 28 Jul 2024 22:32:06 +0100 Subject: [PATCH 03/21] change log --- .chloggen/ottl_sort_func.yaml | 27 +++++++++++++++++++++++++++ pkg/ottl/ottlfuncs/README.md | 25 +++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 .chloggen/ottl_sort_func.yaml diff --git a/.chloggen/ottl_sort_func.yaml b/.chloggen/ottl_sort_func.yaml new file mode 100644 index 000000000000..7b9d32749d9c --- /dev/null +++ b/.chloggen/ottl_sort_func.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add `Sort` function to sort array to ascending order or descending order + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34200] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index 7fb232932f2b..0f72e3d64210 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -446,6 +446,7 @@ Available Converters: - [Seconds](#seconds) - [SHA1](#sha1) - [SHA256](#sha256) +- [Sort](#sort) - [SpanID](#spanid) - [Split](#split) - [String](#string) @@ -1208,6 +1209,30 @@ Examples: **Note:** According to the National Institute of Standards and Technology (NIST), SHA256 is no longer a recommended hash function. It should be avoided except when required for compatibility. New uses should prefer FNV whenever possible. +### Sort + +`Sort(target, Optional[order])` + +The `Sort` Converter sorts the `target` array in ascending or descending order. + +`target` is a `pcommon.Slice` type field. `order` is a string that must be one of `asc` or `desc`. The default `order` is `asc`. + +If elements in `target` are + +- Integers: Returns a sorted array of integers. +- Doubles: Returns a sorted array of doubles. +- Integers and doubles: Converts to doubles and returns a sorted array of doubles. +- Strings: Returns a sorted array of strings. +- Booleans: Converts to strings and returns a sorted array of strings. +- Integers, doubles, booleans, and strings: Converts to strings and returns a sorted array of strings. +- Other types: Returns the `target` unchanged. + +Examples: + +- `Sort(attributes["device.tags"])` + +- `Sort(attributes["device.tags"], "desc")` + ### SpanID `SpanID(bytes)` From 73ecb894cafffd94a79c42ced0dcfc30657a4260 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Mon, 29 Jul 2024 11:18:49 +0100 Subject: [PATCH 04/21] add support to array type --- pkg/ottl/e2e/e2e_test.go | 10 +++---- pkg/ottl/ottlfuncs/func_sort.go | 23 ++++++++++++++++ pkg/ottl/ottlfuncs/func_sort_test.go | 40 ++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index c5c20ba7d34a..02b5fb19c6f8 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -620,12 +620,12 @@ func Test_e2e_converters(t *testing.T) { }, }, { - statement: `set(attributes["test"], Sort(attributes["slice.any"], "desc"))`, + statement: `set(attributes["test"], Sort(Split(attributes["flags"], "|"), "desc"))`, want: func(tCtx ottllog.TransformContext) { s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") - s.AppendEmpty().SetStr("slice") - s.AppendEmpty().SetStr("am") - s.AppendEmpty().SetStr("1") + s.AppendEmpty().SetStr("C") + s.AppendEmpty().SetStr("B") + s.AppendEmpty().SetStr("A") }, }, { @@ -939,8 +939,6 @@ func constructLogTransformContext() ottllog.TransformContext { v.SetStr("val") m2 := m.PutEmptyMap("nested") m2.PutStr("test", "pass") - ss := logRecord.Attributes().PutEmptySlice("slice.any") - _ = ss.FromRaw([]any{1, "am", "slice"}) return ottllog.NewTransformContext(logRecord, scope, resource, plog.NewScopeLogs(), plog.NewResourceLogs()) } diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 07d3b8a1a85b..9a698a192517 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "slices" + "strconv" "go.opentelemetry.io/collector/pdata/pcommon" @@ -70,6 +71,22 @@ func Sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) return v, nil } return sortSlice(slice, order) + case []string: + // handle value from Split() + dup := makeCopy(v) + return sortTypedSlice(dup, order), nil + case []int64: + dup := makeCopy(v) + return sortTypedSlice(dup, order), nil + case []float64: + dup := makeCopy(v) + return sortTypedSlice(dup, order), nil + case []bool: + var arr []string + for _, b := range v { + arr = append(arr, strconv.FormatBool(b)) + } + return sortTypedSlice(arr, order), nil default: return v, nil } @@ -180,6 +197,12 @@ func makeTypedCopy[T TargetType](length int, converter func(idx int) T) []T { return arr } +func makeCopy[T TargetType](src []T) []T { + dup := make([]T, len(src)) + copy(dup, src) + return dup +} + func sortTypedSlice[T TargetType](arr []T, order string) []T { if len(arr) == 0 { return arr diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index dfa2253bbed5..8b5f34a4670c 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -131,6 +131,46 @@ func Test_Sort(t *testing.T) { order: sortAsc, expected: []string{"1", "3.33", "false", "two"}, }, + { + name: "[]string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []string{"A", "a", "aa"}, nil + }, + }, + order: sortAsc, + expected: []string{"A", "a", "aa"}, + }, + { + name: "[]bool compares as string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []bool{true, false}, nil + }, + }, + order: sortAsc, + expected: []string{"false", "true"}, + }, + { + name: "[]int64", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []int64{6, 3, 9}, nil + }, + }, + order: sortAsc, + expected: []int64{3, 6, 9}, + }, + { + name: "[]float64", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return []float64{0.1829374652374, -3.4029435828374, 9.7425639845731}, nil + }, + }, + order: sortAsc, + expected: []float64{-3.4029435828374, 0.1829374652374, 9.7425639845731}, + }, { name: "unsupported ValueTypeMap remains unchanged", getter: ottl.StandardGetSetter[any]{ From dab7c864250b7652f7cce8bae215aff71050def4 Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Mon, 29 Jul 2024 23:01:55 +0100 Subject: [PATCH 05/21] Update pkg/ottl/ottlfuncs/func_sort.go Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- pkg/ottl/ottlfuncs/func_sort.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 9a698a192517..4e3b45e0775c 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -50,7 +50,7 @@ func createSortFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ot return Sort(args.Target, order) } -func Sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) { +func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) { return func(ctx context.Context, tCtx K) (any, error) { val, err := target.Get(ctx, tCtx) if err != nil { From 2e36574ae329e9b6b55a4b8b604fdcea85cd6a48 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Mon, 29 Jul 2024 23:48:17 +0100 Subject: [PATCH 06/21] - add support to pcommon.Value - remove redundant order check - return error for unsupported types --- pkg/ottl/ottlfuncs/func_sort.go | 17 ++++++---- pkg/ottl/ottlfuncs/func_sort_test.go | 50 +++++++++++++++++++--------- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 4e3b45e0775c..04864225cc03 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -7,6 +7,7 @@ import ( "cmp" "context" "fmt" + "go.uber.org/multierr" "slices" "strconv" @@ -47,7 +48,7 @@ func createSortFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ot } } - return Sort(args.Target, order) + return sort(args.Target, order) } func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) { @@ -57,18 +58,20 @@ func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) return nil, err } - if order != sortAsc && order != sortDesc { - return nil, fmt.Errorf("invalid arguments: %s. Order should be either \"%s\" or \"%s\"", order, sortAsc, sortDesc) - } - switch v := val.(type) { case pcommon.Slice: return sortSlice(v, order) + case pcommon.Value: + if v.Type() == pcommon.ValueTypeSlice { + return sortSlice(v.Slice(), order) + } + return nil, fmt.Errorf("sort with unsupported type: '%s'. Target is not a list", v.Type().String()) case []any: // handle Sort([1,2,3]) slice := pcommon.NewValueSlice().SetEmptySlice() if err := slice.FromRaw(v); err != nil { - return v, nil + errs := multierr.Append(err, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list of primitive types", v)) + return nil, errs } return sortSlice(slice, order) case []string: @@ -88,7 +91,7 @@ func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) } return sortTypedSlice(arr, order), nil default: - return v, nil + return nil, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list", v) } }, nil } diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index 8b5f34a4670c..75ff72f073af 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -172,60 +172,78 @@ func Test_Sort(t *testing.T) { expected: []float64{-3.4029435828374, 0.1829374652374, 9.7425639845731}, }, { - name: "unsupported ValueTypeMap remains unchanged", + name: "pcommon.Value is a slice", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return pMap, nil + pv := pcommon.NewValueEmpty() + s := pv.SetEmptySlice() + s.FromRaw([]any{"a", "slice", "a"}) + return pv, nil }, }, order: sortAsc, - expected: pMap, + expected: []string{"a", "a", "slice"}, }, { - name: "unsupported bytes remains unchanged", + name: "pcommon.Value is empty", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return []byte("still fine"), nil + pv := pcommon.NewValueEmpty() + return pv, nil }, }, order: sortAsc, - expected: []byte("still fine"), + expected: nil, + err: true, }, { - name: "unsupported string remains unchanged", + name: "unsupported ValueTypeMap", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return "no change", nil + return pMap, nil }, }, order: sortAsc, - expected: "no change", + expected: nil, + err: true, }, { - name: "invalid slice remains unchanged", + name: "unsupported bytes", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return []any{map[string]string{"some": "invalid kv"}}, nil + return []byte("still fine"), nil + }, + }, + order: sortAsc, + expected: nil, + err: true, + }, + { + name: "unsupported string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + return "no change", nil }, }, order: sortAsc, - expected: []any{map[string]string{"some": "invalid kv"}}, + expected: nil, + err: true, }, { - name: "invalid sort order", + name: "[]any with a map", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return []any{1, 2, 3}, nil + return []any{map[string]string{"some": "invalid kv"}}, nil }, }, - order: "dddd", + order: sortAsc, expected: nil, err: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - exprFunc, err := Sort(tt.getter, tt.order) + exprFunc, err := sort(tt.getter, tt.order) assert.NoError(t, err) result, err := exprFunc(nil, nil) if tt.err { From d43e95f67ac69053e7080808577fac736008e816 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Tue, 30 Jul 2024 00:03:48 +0100 Subject: [PATCH 07/21] update doc --- pkg/ottl/ottlfuncs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index 0f72e3d64210..a6019927698f 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -1225,7 +1225,7 @@ If elements in `target` are - Strings: Returns a sorted array of strings. - Booleans: Converts to strings and returns a sorted array of strings. - Integers, doubles, booleans, and strings: Converts to strings and returns a sorted array of strings. -- Other types: Returns the `target` unchanged. +- Other types: Returns error. Examples: From 447df3e4b9c10fdca4e00139c8b163afa6aa03c7 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Tue, 30 Jul 2024 15:01:58 +0100 Subject: [PATCH 08/21] lint --- pkg/ottl/ottlfuncs/func_sort.go | 8 ++++---- pkg/ottl/ottlfuncs/func_sort_test.go | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 04864225cc03..8237dfbfd54e 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -7,11 +7,11 @@ import ( "cmp" "context" "fmt" - "go.uber.org/multierr" "slices" "strconv" "go.opentelemetry.io/collector/pdata/pcommon" + "go.uber.org/multierr" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) @@ -48,10 +48,10 @@ func createSortFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ot } } - return sort(args.Target, order) + return sort(args.Target, order), nil } -func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) { +func sort[K any](target ottl.Getter[K], order string) ottl.ExprFunc[K] { return func(ctx context.Context, tCtx K) (any, error) { val, err := target.Get(ctx, tCtx) if err != nil { @@ -93,7 +93,7 @@ func sort[K any](target ottl.Getter[K], order string) (ottl.ExprFunc[K], error) default: return nil, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list", v) } - }, nil + } } // sortSlice sorts a pcommon.Slice based on the specified order. diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index 75ff72f073af..312b1369db35 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -177,7 +177,7 @@ func Test_Sort(t *testing.T) { Getter: func(_ context.Context, _ any) (any, error) { pv := pcommon.NewValueEmpty() s := pv.SetEmptySlice() - s.FromRaw([]any{"a", "slice", "a"}) + _ = s.FromRaw([]any{"a", "slice", "a"}) return pv, nil }, }, @@ -243,8 +243,7 @@ func Test_Sort(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - exprFunc, err := sort(tt.getter, tt.order) - assert.NoError(t, err) + exprFunc := sort(tt.getter, tt.order) result, err := exprFunc(nil, nil) if tt.err { assert.Error(t, err) From 028744e191f25a0a094eb83796a32a8fa001919e Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Tue, 30 Jul 2024 15:39:39 +0100 Subject: [PATCH 09/21] tidy --- pkg/ottl/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/go.mod b/pkg/ottl/go.mod index 2f894f08a291..9c6600acbc86 100644 --- a/pkg/ottl/go.mod +++ b/pkg/ottl/go.mod @@ -18,6 +18,7 @@ require ( go.opentelemetry.io/otel/sdk v1.28.0 go.opentelemetry.io/otel/trace v1.28.0 go.uber.org/goleak v1.3.0 + go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 golang.org/x/net v0.27.0 @@ -43,7 +44,6 @@ require ( go.opentelemetry.io/otel/exporters/prometheus v0.50.0 // indirect go.opentelemetry.io/otel/metric v1.28.0 // indirect go.opentelemetry.io/otel/sdk/metric v1.28.0 // indirect - go.uber.org/multierr v1.11.0 // indirect golang.org/x/sys v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect From 1eb08f3206b567fd089294094ac22e07fc6dca92 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Tue, 30 Jul 2024 18:49:13 +0100 Subject: [PATCH 10/21] - preserve the data types in the sort result - change visibility of type constraint --- pkg/ottl/ottlfuncs/func_sort.go | 60 +++++++++++++------ pkg/ottl/ottlfuncs/func_sort_test.go | 54 ++++++++++++----- .../internal/metadata/generated_telemetry.go | 5 +- .../metadata/generated_telemetry_test.go | 3 +- 4 files changed, 85 insertions(+), 37 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 8237dfbfd54e..b90e6bb41db2 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -118,12 +118,12 @@ func sortSlice(slice pcommon.Slice, order string) (any, error) { switch commonType { case pcommon.ValueTypeInt: - arr := makeTypedCopy(length, func(idx int) int64 { + arr := makeTypedValCopy(slice, func(idx int) int64 { return slice.At(idx).Int() }) - return sortTypedSlice(arr, order), nil + return sortTypedValSlice(arr, order), nil case pcommon.ValueTypeDouble: - arr := makeTypedCopy(length, func(idx int) float64 { + arr := makeTypedValCopy(slice, func(idx int) float64 { s := slice.At(idx) if s.Type() == pcommon.ValueTypeInt { return float64(s.Int()) @@ -131,18 +131,18 @@ func sortSlice(slice pcommon.Slice, order string) (any, error) { return s.Double() }) - return sortTypedSlice(arr, order), nil + return sortTypedValSlice(arr, order), nil case pcommon.ValueTypeStr: - arr := makeTypedCopy(length, func(idx int) string { + arr := makeTypedValCopy(slice, func(idx int) string { return slice.At(idx).AsString() }) - return sortTypedSlice(arr, order), nil + return sortTypedValSlice(arr, order), nil default: return nil, fmt.Errorf("sort with unsupported type: '%T'", commonType) } } -type TargetType interface { +type targetType interface { ~int64 | ~float64 | ~string } @@ -192,21 +192,13 @@ func findCommonValueType(slice pcommon.Slice) (pcommon.ValueType, bool) { return wantType, true } -func makeTypedCopy[T TargetType](length int, converter func(idx int) T) []T { - var arr []T - for i := 0; i < length; i++ { - arr = append(arr, converter(i)) - } - return arr -} - -func makeCopy[T TargetType](src []T) []T { +func makeCopy[T targetType](src []T) []T { dup := make([]T, len(src)) copy(dup, src) return dup } -func sortTypedSlice[T TargetType](arr []T, order string) []T { +func sortTypedSlice[T targetType](arr []T, order string) []T { if len(arr) == 0 { return arr } @@ -220,3 +212,37 @@ func sortTypedSlice[T TargetType](arr []T, order string) []T { return arr } + +type typedValue[T targetType] struct { + convertedVal T + originalVal any +} + +func makeTypedValCopy[T targetType](slice pcommon.Slice, converter func(idx int) T) []typedValue[T] { + length := slice.Len() + var out []typedValue[T] + for i := 0; i < length; i++ { + val := typedValue[T]{ + convertedVal: converter(i), + originalVal: slice.At(i).AsRaw(), + } + out = append(out, val) + } + return out +} + +func sortTypedValSlice[T targetType](vals []typedValue[T], order string) []any { + slices.SortFunc(vals, func(a, b typedValue[T]) int { + if order == sortDesc { + return cmp.Compare(b.convertedVal, a.convertedVal) + } + return cmp.Compare(a.convertedVal, b.convertedVal) + }) + + var out []any + for _, val := range vals { + out = append(out, val.originalVal) + } + + return out +} diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index 312b1369db35..c9ded1753ba0 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -17,6 +17,7 @@ func Test_Sort(t *testing.T) { pMap := pcommon.NewValueMap().SetEmptyMap() pMap.PutStr("k", "v") + emptySlice := pcommon.NewValueSlice().SetEmptySlice() tests := []struct { name string @@ -35,7 +36,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []int64{3, 6, 9}, + expected: []any{int64(3), int64(6), int64(9)}, }, { name: "int slice desc", @@ -47,7 +48,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortDesc, - expected: []int64{9, 6, 3}, + expected: []any{int64(9), int64(6), int64(3)}, }, { name: "string slice", @@ -59,19 +60,30 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"am", "awesome", "i", "slice"}, + expected: []any{"am", "awesome", "i", "slice"}, }, { - name: "double slice desc", + name: "double slice", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { s := pcommon.NewValueSlice().SetEmptySlice() - _ = s.FromRaw([]any{0.1829374652374, -3.4029435828374, 9.7425639845731}) + _ = s.FromRaw([]any{1.5, 10.2, 2.3, 0.5}) return s, nil }, }, - order: sortDesc, - expected: []float64{9.7425639845731, 0.1829374652374, -3.4029435828374}, + order: sortAsc, + expected: []any{0.5, 1.5, 2.3, 10.2}, + }, + { + name: "empty slice", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + return s, nil + }, + }, + order: sortAsc, + expected: emptySlice, }, { name: "bool slice compares as string", @@ -83,7 +95,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"false", "false", "true", "true"}, + expected: []any{false, false, true, true}, }, { name: "mixed types slice compares as string", @@ -95,7 +107,19 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"1", "3.33", "false", "two"}, + expected: []any{int64(1), 3.33, false, "two"}, + }, + { + name: "double and string slice compares as string", + getter: ottl.StandardGetSetter[any]{ + Getter: func(_ context.Context, _ any) (any, error) { + s := pcommon.NewValueSlice().SetEmptySlice() + _ = s.FromRaw([]any{1.5, "10.2", 2.3, 0.5}) + return s, nil + }, + }, + order: sortAsc, + expected: []any{0.5, 1.5, "10.2", 2.3}, }, { name: "mixed numeric types slice compares as double", @@ -107,7 +131,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []float64{0, 0, 2, 3.33}, + expected: []any{int64(0), int64(0), int64(2), 3.33}, }, { name: "mixed numeric types slice compares as double desc", @@ -119,7 +143,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortDesc, - expected: []float64{3.33, 3.14, 2, 0}, + expected: []any{3.33, 3.14, int64(2), int64(0)}, }, { name: "[]any compares as string", @@ -129,7 +153,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"1", "3.33", "false", "two"}, + expected: []any{int64(1), 3.33, false, "two"}, }, { name: "[]string", @@ -165,11 +189,11 @@ func Test_Sort(t *testing.T) { name: "[]float64", getter: ottl.StandardGetSetter[any]{ Getter: func(_ context.Context, _ any) (any, error) { - return []float64{0.1829374652374, -3.4029435828374, 9.7425639845731}, nil + return []float64{1.5, 10.2, 2.3, 0.5}, nil }, }, order: sortAsc, - expected: []float64{-3.4029435828374, 0.1829374652374, 9.7425639845731}, + expected: []float64{0.5, 1.5, 2.3, 10.2}, }, { name: "pcommon.Value is a slice", @@ -182,7 +206,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"a", "a", "slice"}, + expected: []any{"a", "a", "slice"}, }, { name: "pcommon.Value is empty", diff --git a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go index 94d0af873337..b5890096bc04 100644 --- a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go +++ b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go @@ -5,12 +5,11 @@ package metadata import ( "errors" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" - - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configtelemetry" ) func Meter(settings component.TelemetrySettings) metric.Meter { diff --git a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go index 1517f672ddd7..2cac2886fc99 100644 --- a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go +++ b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go @@ -6,14 +6,13 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/component" "go.opentelemetry.io/otel/metric" embeddedmetric "go.opentelemetry.io/otel/metric/embedded" noopmetric "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" embeddedtrace "go.opentelemetry.io/otel/trace/embedded" nooptrace "go.opentelemetry.io/otel/trace/noop" - - "go.opentelemetry.io/collector/component" ) type mockMeter struct { From a0d9791faecc0fb4abb721decd8b447de1d7d89d Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Tue, 30 Jul 2024 19:06:39 +0100 Subject: [PATCH 11/21] update doc --- pkg/ottl/ottlfuncs/README.md | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index a6019927698f..94fba3748036 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -1213,24 +1213,29 @@ Examples: `Sort(target, Optional[order])` -The `Sort` Converter sorts the `target` array in ascending or descending order. +The `Sort` Converter sorts the `target` array in either ascending or descending order. -`target` is a `pcommon.Slice` type field. `order` is a string that must be one of `asc` or `desc`. The default `order` is `asc`. +`target` is a `pcommon.Slice` type field containing the elements to be sorted. -If elements in `target` are +`order` is a string specifying the sort order. Must be either `asc` or `desc`. The default value is `asc`. + +The Sort Converter preserves the data type of the original elements while sorting. +The behavior varies based on the types of elements in the target slice: + +| Element Types | Sorting Behavior | Return Value | +|---------------|-------------------------------------|--------------| +| Integers | Sorts as integers | Sorted array of integers | +| Doubles | Sorts as doubles | Sorted array of doubles | +| Integers and doubles | Converts all to doubles, then sorts | Sorted array of integers and doubles | +| Strings | Sorts as strings | Sorted array of strings | +| Booleans | Converts all to strings, then sorts | Sorted array of booleans | +| Mix of integers, doubles, booleans, and strings | Converts all to strings, then sorts | Sorted array of mixed types | +| Any other types | N/A | Returns an error | -- Integers: Returns a sorted array of integers. -- Doubles: Returns a sorted array of doubles. -- Integers and doubles: Converts to doubles and returns a sorted array of doubles. -- Strings: Returns a sorted array of strings. -- Booleans: Converts to strings and returns a sorted array of strings. -- Integers, doubles, booleans, and strings: Converts to strings and returns a sorted array of strings. -- Other types: Returns error. Examples: - `Sort(attributes["device.tags"])` - - `Sort(attributes["device.tags"], "desc")` ### SpanID From d676f1a1925eaa07c3f091875b3d4b6382fac26c Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Wed, 31 Jul 2024 11:26:22 +0100 Subject: [PATCH 12/21] fix unit test --- pkg/ottl/e2e/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index 02b5fb19c6f8..5a506d1085b6 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -634,7 +634,7 @@ func Test_e2e_converters(t *testing.T) { s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") s.AppendEmpty().SetDouble(-1) s.AppendEmpty().SetDouble(2.2) - s.AppendEmpty().SetDouble(11) + s.AppendEmpty().SetInt(11) }, }, { From d166048ccac2de4ba3e48a7fe74218f78beecedf Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Wed, 31 Jul 2024 15:34:15 +0100 Subject: [PATCH 13/21] preserve the data type for boolean array --- pkg/ottl/ottlfuncs/func_sort.go | 14 +++++++++++--- pkg/ottl/ottlfuncs/func_sort_test.go | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index b90e6bb41db2..2e6e0f72df35 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -85,11 +85,19 @@ func sort[K any](target ottl.Getter[K], order string) ottl.ExprFunc[K] { dup := makeCopy(v) return sortTypedSlice(dup, order), nil case []bool: - var arr []string + var strings []string for _, b := range v { - arr = append(arr, strconv.FormatBool(b)) + strings = append(strings, strconv.FormatBool(b)) } - return sortTypedSlice(arr, order), nil + + sortTypedSlice(strings, order) + + bools := make([]bool, len(strings)) + for i, s := range strings { + boolValue, _ := strconv.ParseBool(s) + bools[i] = boolValue + } + return bools, nil default: return nil, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list", v) } diff --git a/pkg/ottl/ottlfuncs/func_sort_test.go b/pkg/ottl/ottlfuncs/func_sort_test.go index c9ded1753ba0..48dede0a2fa9 100644 --- a/pkg/ottl/ottlfuncs/func_sort_test.go +++ b/pkg/ottl/ottlfuncs/func_sort_test.go @@ -173,7 +173,7 @@ func Test_Sort(t *testing.T) { }, }, order: sortAsc, - expected: []string{"false", "true"}, + expected: []bool{false, true}, }, { name: "[]int64", From 524e546a77f1ab0c21ddd34b03dd7bc65ca9095e Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Wed, 31 Jul 2024 15:39:07 +0100 Subject: [PATCH 14/21] rename --- pkg/ottl/ottlfuncs/func_sort.go | 42 ++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 2e6e0f72df35..6209e47539dd 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -126,12 +126,12 @@ func sortSlice(slice pcommon.Slice, order string) (any, error) { switch commonType { case pcommon.ValueTypeInt: - arr := makeTypedValCopy(slice, func(idx int) int64 { + arr := makeConvertedCopy(slice, func(idx int) int64 { return slice.At(idx).Int() }) - return sortTypedValSlice(arr, order), nil + return sortConvertedSlice(arr, order), nil case pcommon.ValueTypeDouble: - arr := makeTypedValCopy(slice, func(idx int) float64 { + arr := makeConvertedCopy(slice, func(idx int) float64 { s := slice.At(idx) if s.Type() == pcommon.ValueTypeInt { return float64(s.Int()) @@ -139,12 +139,12 @@ func sortSlice(slice pcommon.Slice, order string) (any, error) { return s.Double() }) - return sortTypedValSlice(arr, order), nil + return sortConvertedSlice(arr, order), nil case pcommon.ValueTypeStr: - arr := makeTypedValCopy(slice, func(idx int) string { + arr := makeConvertedCopy(slice, func(idx int) string { return slice.At(idx).AsString() }) - return sortTypedValSlice(arr, order), nil + return sortConvertedSlice(arr, order), nil default: return nil, fmt.Errorf("sort with unsupported type: '%T'", commonType) } @@ -221,35 +221,35 @@ func sortTypedSlice[T targetType](arr []T, order string) []T { return arr } -type typedValue[T targetType] struct { - convertedVal T - originalVal any +type convertedValue[T targetType] struct { + value T + originalValue any } -func makeTypedValCopy[T targetType](slice pcommon.Slice, converter func(idx int) T) []typedValue[T] { +func makeConvertedCopy[T targetType](slice pcommon.Slice, converter func(idx int) T) []convertedValue[T] { length := slice.Len() - var out []typedValue[T] + var out []convertedValue[T] for i := 0; i < length; i++ { - val := typedValue[T]{ - convertedVal: converter(i), - originalVal: slice.At(i).AsRaw(), + cv := convertedValue[T]{ + value: converter(i), + originalValue: slice.At(i).AsRaw(), } - out = append(out, val) + out = append(out, cv) } return out } -func sortTypedValSlice[T targetType](vals []typedValue[T], order string) []any { - slices.SortFunc(vals, func(a, b typedValue[T]) int { +func sortConvertedSlice[T targetType](cvs []convertedValue[T], order string) []any { + slices.SortFunc(cvs, func(a, b convertedValue[T]) int { if order == sortDesc { - return cmp.Compare(b.convertedVal, a.convertedVal) + return cmp.Compare(b.value, a.value) } - return cmp.Compare(a.convertedVal, b.convertedVal) + return cmp.Compare(a.value, b.value) }) var out []any - for _, val := range vals { - out = append(out, val.originalVal) + for _, cv := range cvs { + out = append(out, cv.originalValue) } return out From a13bdbc9e7a507fe661a8a7923e34c9a0e41497b Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Fri, 2 Aug 2024 00:57:46 +0100 Subject: [PATCH 15/21] fix CI codegen --- .../fileconsumer/internal/metadata/generated_telemetry.go | 5 +++-- .../internal/metadata/generated_telemetry_test.go | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go index b5890096bc04..94d0af873337 100644 --- a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go +++ b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry.go @@ -5,11 +5,12 @@ package metadata import ( "errors" - "go.opentelemetry.io/collector/component" - "go.opentelemetry.io/collector/config/configtelemetry" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/config/configtelemetry" ) func Meter(settings component.TelemetrySettings) metric.Meter { diff --git a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go index 2cac2886fc99..1517f672ddd7 100644 --- a/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go +++ b/pkg/stanza/fileconsumer/internal/metadata/generated_telemetry_test.go @@ -6,13 +6,14 @@ import ( "testing" "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/component" "go.opentelemetry.io/otel/metric" embeddedmetric "go.opentelemetry.io/otel/metric/embedded" noopmetric "go.opentelemetry.io/otel/metric/noop" "go.opentelemetry.io/otel/trace" embeddedtrace "go.opentelemetry.io/otel/trace/embedded" nooptrace "go.opentelemetry.io/otel/trace/noop" + + "go.opentelemetry.io/collector/component" ) type mockMeter struct { From 1bb3d8652377eb881742ce9474f56562359692ba Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:54:16 +0100 Subject: [PATCH 16/21] Update pkg/ottl/ottlfuncs/README.md Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- pkg/ottl/ottlfuncs/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index c0fbeb2b0c32..213d711bd4fa 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -1207,8 +1207,6 @@ Examples: - `SHA256("name")` -**Note:** According to the National Institute of Standards and Technology (NIST), SHA256 is no longer a recommended hash function. It should be avoided except when required for compatibility. New uses should prefer FNV whenever possible. - ### Sort `Sort(target, Optional[order])` From 289498103da037c6dbac670fff742c4a9dc44bfc Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:54:31 +0100 Subject: [PATCH 17/21] Update pkg/ottl/ottlfuncs/func_sort.go Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- pkg/ottl/ottlfuncs/func_sort.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 6209e47539dd..f56c3bc91040 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -75,7 +75,6 @@ func sort[K any](target ottl.Getter[K], order string) ottl.ExprFunc[K] { } return sortSlice(slice, order) case []string: - // handle value from Split() dup := makeCopy(v) return sortTypedSlice(dup, order), nil case []int64: From 9c22e38e7959d21d32824eecddfa586ba3cccaa4 Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:54:36 +0100 Subject: [PATCH 18/21] Update pkg/ottl/ottlfuncs/README.md Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- pkg/ottl/ottlfuncs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/ottlfuncs/README.md b/pkg/ottl/ottlfuncs/README.md index 213d711bd4fa..f3e2b7376310 100644 --- a/pkg/ottl/ottlfuncs/README.md +++ b/pkg/ottl/ottlfuncs/README.md @@ -1213,7 +1213,7 @@ Examples: The `Sort` Converter sorts the `target` array in either ascending or descending order. -`target` is a `pcommon.Slice` type field containing the elements to be sorted. +`target` is an array or `pcommon.Slice` typed field containing the elements to be sorted. `order` is a string specifying the sort order. Must be either `asc` or `desc`. The default value is `asc`. From 660f4fc2b609bdd01ddb10e3ec587d5a249d8cbd Mon Sep 17 00:00:00 2001 From: kaisecheng <69120390+kaisecheng@users.noreply.github.com> Date: Fri, 2 Aug 2024 15:54:44 +0100 Subject: [PATCH 19/21] Update pkg/ottl/ottlfuncs/func_sort.go Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- pkg/ottl/ottlfuncs/func_sort.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index f56c3bc91040..42c6def8d5ed 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -70,8 +70,7 @@ func sort[K any](target ottl.Getter[K], order string) ottl.ExprFunc[K] { // handle Sort([1,2,3]) slice := pcommon.NewValueSlice().SetEmptySlice() if err := slice.FromRaw(v); err != nil { - errs := multierr.Append(err, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list of primitive types", v)) - return nil, errs + return nil, fmt.Errorf("sort with unsupported type: '%T'. Target is not a list of primitive types; %w", v, err) } return sortSlice(slice, order) case []string: From 3325267f8b99eb33d80330a7c685dc91fd610d53 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Fri, 2 Aug 2024 16:02:42 +0100 Subject: [PATCH 20/21] remove multierr dependency --- pkg/ottl/go.mod | 2 +- pkg/ottl/ottlfuncs/func_sort.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/ottl/go.mod b/pkg/ottl/go.mod index 08b9d7da0595..a1a952d8cb85 100644 --- a/pkg/ottl/go.mod +++ b/pkg/ottl/go.mod @@ -18,7 +18,6 @@ require ( go.opentelemetry.io/otel/sdk v1.28.0 go.opentelemetry.io/otel/trace v1.28.0 go.uber.org/goleak v1.3.0 - go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 golang.org/x/net v0.27.0 @@ -44,6 +43,7 @@ require ( go.opentelemetry.io/otel/exporters/prometheus v0.50.0 // indirect go.opentelemetry.io/otel/metric v1.28.0 // indirect go.opentelemetry.io/otel/sdk/metric v1.28.0 // indirect + go.uber.org/multierr v1.11.0 // indirect golang.org/x/sys v0.22.0 // indirect golang.org/x/text v0.16.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect diff --git a/pkg/ottl/ottlfuncs/func_sort.go b/pkg/ottl/ottlfuncs/func_sort.go index 42c6def8d5ed..4c9f56c820ce 100644 --- a/pkg/ottl/ottlfuncs/func_sort.go +++ b/pkg/ottl/ottlfuncs/func_sort.go @@ -11,7 +11,6 @@ import ( "strconv" "go.opentelemetry.io/collector/pdata/pcommon" - "go.uber.org/multierr" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" ) From f2145f33f9718a4848658daede27835a12a1cdb0 Mon Sep 17 00:00:00 2001 From: Kaise Cheng Date: Thu, 8 Aug 2024 17:02:06 +0100 Subject: [PATCH 21/21] add more e2e tests --- pkg/ottl/e2e/e2e_test.go | 44 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index 79b9f85205bf..1c9230c51952 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -625,7 +625,7 @@ func Test_e2e_converters(t *testing.T) { tCtx.GetLogRecord().Attributes().PutStr("test", "5b722b307fce6c944905d132691d5e4a2214b7fe92b738920eb3fce3a90420a19511c3010a0e7712b054daef5b57bad59ecbd93b3280f210578f547f4aed4d25") }, }, - { + { statement: `set(attributes["test"], Sort(Split(attributes["flags"], "|"), "desc"))`, want: func(tCtx ottllog.TransformContext) { s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") @@ -634,6 +634,34 @@ func Test_e2e_converters(t *testing.T) { s.AppendEmpty().SetStr("A") }, }, + { + statement: `set(attributes["test"], Sort([true, false, false]))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetBool(false) + s.AppendEmpty().SetBool(false) + s.AppendEmpty().SetBool(true) + }, + }, + { + statement: `set(attributes["test"], Sort([3, 6, 9], "desc"))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetInt(9) + s.AppendEmpty().SetInt(6) + s.AppendEmpty().SetInt(3) + }, + }, + { + statement: `set(attributes["test"], Sort([Double(1.5), Double(10.2), Double(2.3), Double(0.5)]))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetDouble(0.5) + s.AppendEmpty().SetDouble(1.5) + s.AppendEmpty().SetDouble(2.3) + s.AppendEmpty().SetDouble(10.2) + }, + }, { statement: `set(attributes["test"], Sort([Int(11), Double(2.2), Double(-1)]))`, want: func(tCtx ottllog.TransformContext) { @@ -641,8 +669,18 @@ func Test_e2e_converters(t *testing.T) { s.AppendEmpty().SetDouble(-1) s.AppendEmpty().SetDouble(2.2) s.AppendEmpty().SetInt(11) - }, - }, + }, + }, + { + statement: `set(attributes["test"], Sort([false, Int(11), Double(2.2), "three"]))`, + want: func(tCtx ottllog.TransformContext) { + s := tCtx.GetLogRecord().Attributes().PutEmptySlice("test") + s.AppendEmpty().SetInt(11) + s.AppendEmpty().SetDouble(2.2) + s.AppendEmpty().SetBool(false) + s.AppendEmpty().SetStr("three") + }, + }, { statement: `set(span_id, SpanID(0x0000000000000000))`, want: func(tCtx ottllog.TransformContext) {