From a43c31e061da5ff7b4e77aa53b6fe9adbd0455d5 Mon Sep 17 00:00:00 2001 From: fchikwekwe Date: Fri, 14 Jul 2023 11:57:51 -0400 Subject: [PATCH 1/7] feat: add boolean behavior for durations --- .chloggen/feat_boolean-duration.yaml | 20 ++++++++++++ pkg/ottl/boolean_value_test.go | 47 ++++++++++++++++++++++++++++ pkg/ottl/compare.go | 14 +++++++++ pkg/ottl/parser_test.go | 16 ++++++++++ 4 files changed, 97 insertions(+) create mode 100755 .chloggen/feat_boolean-duration.yaml diff --git a/.chloggen/feat_boolean-duration.yaml b/.chloggen/feat_boolean-duration.yaml new file mode 100755 index 000000000000..a41b85e2bc86 --- /dev/null +++ b/.chloggen/feat_boolean-duration.yaml @@ -0,0 +1,20 @@ +# Use this changelog template to create an entry for release notes. +# If your change doesn't affect end users, such as a test fix or a tooling change, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. + +# 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: 'Adds support for using boolean expressions with durations' + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [22713] + +# (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: diff --git a/pkg/ottl/boolean_value_test.go b/pkg/ottl/boolean_value_test.go index 81bbbe96d95d..c82dc42b4c27 100644 --- a/pkg/ottl/boolean_value_test.go +++ b/pkg/ottl/boolean_value_test.go @@ -7,6 +7,7 @@ import ( "context" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component/componenttest" @@ -37,6 +38,16 @@ func valueFor(x any) value { case strings.Contains(v, "ENUM"): // if the string contains ENUM construct an EnumSymbol from it. val.Enum = (*EnumSymbol)(ottltest.Strp(v)) + case v == "dur1" || v == "dur2": + val.Literal = &mathExprLiteral{ + Path: &Path{ + Fields: []Field{ + { + Name: v, + }, + }, + }, + } default: val.String = ottltest.Strp(v) } @@ -76,6 +87,30 @@ func Test_newComparisonEvaluator(t *testing.T) { WithEnumParser[any](testParseEnum), ) + JanFirst2023 := time.Date(2023, 1, 1, 0, 0, 0, 0, time.Local) + twelveNanoseconds, err := time.ParseDuration("12ns") + if err != nil { + t.Error() + } + oneMillisecond, err := time.ParseDuration("1ms") + if err != nil { + t.Error() + } + threeSeconds, err := time.ParseDuration("3s") + if err != nil { + t.Error() + } + + twentyTwoMinutes, err := time.ParseDuration("22m") + if err != nil { + t.Error() + } + + oneHundredThirtyFiveHours, err := time.ParseDuration("135h") + if err != nil { + t.Error() + } + var tests = []struct { name string l any @@ -106,6 +141,18 @@ func Test_newComparisonEvaluator(t *testing.T) { {name: "[]byte('a') < []byte('b')", l: []byte("a"), r: []byte("b"), op: "<", want: true}, {name: "nil == nil", op: "==", want: true}, {name: "nil == []byte(nil)", r: []byte(nil), op: "==", want: true}, + {name: "compare equal durations", l: "dur1", r: "dur2", op: "==", want: true, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": oneMillisecond}}, + {name: "compare unequal durations", l: "dur1", r: "dur2", op: "==", want: false, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": threeSeconds}}, + {name: "compare not equal durations", l: "dur1", r: "dur2", op: "!=", want: true, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": threeSeconds}}, + {name: "compare not equal durations", l: "dur1", r: "dur2", op: "!=", want: false, item: map[string]time.Duration{"dur1": threeSeconds, "dur2": threeSeconds}}, + {name: "compare less than durations", l: "dur1", r: "dur2", op: "<", want: true, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": twentyTwoMinutes}}, + {name: "compare not less than durations", l: "dur1", r: "dur2", op: "<", want: false, item: map[string]time.Duration{"dur1": twentyTwoMinutes, "dur2": twentyTwoMinutes}}, + {name: "compare less than equal to durations", l: "dur1", r: "dur2", op: "<=", want: true, item: map[string]time.Duration{"dur1": threeSeconds, "dur2": threeSeconds}}, + {name: "compare not less than equal to durations", l: "dur1", r: "dur2", op: "<=", want: false, item: map[string]time.Duration{"dur1": oneHundredThirtyFiveHours, "dur2": threeSeconds}}, + {name: "compare greater than durations", l: "dur1", r: "dur2", op: ">", want: true, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": twelveNanoseconds}}, + {name: "compare not greater than durations", l: "dur1", r: "dur2", op: ">", want: false, item: map[string]time.Duration{"dur1": twelveNanoseconds, "dur2": twentyTwoMinutes}}, + {name: "compare greater than equal to durations", l: "dur1", r: "dur2", op: ">=", want: true, item: map[string]time.Duration{"dur1": oneHundredThirtyFiveHours, "dur2": threeSeconds}}, + {name: "compare not greater than equal to durations", l: "dur1", r: "dur2", op: ">=", want: false, item: map[string]time.Duration{"dur1": oneMillisecond, "dur2": threeSeconds}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/ottl/compare.go b/pkg/ottl/compare.go index 1b6b589f269c..ad20ca4a163a 100644 --- a/pkg/ottl/compare.go +++ b/pkg/ottl/compare.go @@ -5,6 +5,7 @@ package ottl // import "github.com/open-telemetry/opentelemetry-collector-contri import ( "bytes" + "time" "go.uber.org/zap" "golang.org/x/exp/constraints" @@ -135,6 +136,17 @@ func (p *Parser[K]) compareFloat64(a float64, b any, op compareOp) bool { } } +func (p *Parser[K]) compareDuration(a time.Duration, b any, op compareOp) bool { + switch v := b.(type) { + case time.Duration: + ansecs := a.Nanoseconds() + vnsecs := v.Nanoseconds() + return comparePrimitives(ansecs, vnsecs, op) + default: + return p.invalidComparison("cannot compare invalid duration", op) + } +} + // a and b are the return values from a Getter; we try to compare them // according to the given operator. func (p *Parser[K]) compare(a any, b any, op compareOp) bool { @@ -162,6 +174,8 @@ func (p *Parser[K]) compare(a any, b any, op compareOp) bool { return p.compare(b, nil, op) } return p.compareByte(v, b, op) + case time.Duration: + return p.compareDuration(v, b, op) default: // If we don't know what type it is, we can't do inequalities yet. So we can fall back to the old behavior where we just // use Go's standard equality. diff --git a/pkg/ottl/parser_test.go b/pkg/ottl/parser_test.go index cfafc97df823..9de16f03f9d5 100644 --- a/pkg/ottl/parser_test.go +++ b/pkg/ottl/parser_test.go @@ -9,6 +9,7 @@ import ( "reflect" "regexp" "testing" + "time" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component/componenttest" @@ -874,6 +875,21 @@ func testParsePath(val *Path) (GetSetter[interface{}], error) { }, }, nil } + if val.Fields[0].Name == "dur1" || val.Fields[0].Name == "dur2" { + return &StandardGetSetter[interface{}]{ + Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { + m, ok := tCtx.(map[string]time.Duration) + if !ok { + return nil, fmt.Errorf("unable to convert transform context to map of strings to times") + } + return m[val.Fields[0].Name], nil + }, + Setter: func(ctx context.Context, tCtx interface{}, val interface{}) error { + reflect.DeepEqual(tCtx, val) + return nil + }, + }, nil + } return nil, fmt.Errorf("bad path %v", val) } From 926c775702155d4dbc0902e1358e168abbd900f1 Mon Sep 17 00:00:00 2001 From: fchikwekwe Date: Fri, 14 Jul 2023 14:21:56 -0400 Subject: [PATCH 2/7] fix: allow item to take type any --- pkg/ottl/boolean_value_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/ottl/boolean_value_test.go b/pkg/ottl/boolean_value_test.go index c82dc42b4c27..41b6d5f4ba51 100644 --- a/pkg/ottl/boolean_value_test.go +++ b/pkg/ottl/boolean_value_test.go @@ -87,7 +87,6 @@ func Test_newComparisonEvaluator(t *testing.T) { WithEnumParser[any](testParseEnum), ) - JanFirst2023 := time.Date(2023, 1, 1, 0, 0, 0, 0, time.Local) twelveNanoseconds, err := time.ParseDuration("12ns") if err != nil { t.Error() @@ -116,7 +115,7 @@ func Test_newComparisonEvaluator(t *testing.T) { l any r any op string - item string + item any want bool }{ {name: "literals match", l: "hello", r: "hello", op: "==", want: true}, From e0c39337c2f97e6422923278bcd9a9230b8c7d34 Mon Sep 17 00:00:00 2001 From: fchikwekwe Date: Mon, 17 Jul 2023 11:06:41 -0400 Subject: [PATCH 3/7] test: use require pkg for error checking --- pkg/ottl/boolean_value_test.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/ottl/boolean_value_test.go b/pkg/ottl/boolean_value_test.go index 41b6d5f4ba51..f0d2d6ea4a90 100644 --- a/pkg/ottl/boolean_value_test.go +++ b/pkg/ottl/boolean_value_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component/componenttest" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest" @@ -88,27 +89,19 @@ func Test_newComparisonEvaluator(t *testing.T) { ) twelveNanoseconds, err := time.ParseDuration("12ns") - if err != nil { - t.Error() - } + require.NoError(t, err) + oneMillisecond, err := time.ParseDuration("1ms") - if err != nil { - t.Error() - } + require.NoError(t, err) + threeSeconds, err := time.ParseDuration("3s") - if err != nil { - t.Error() - } + require.NoError(t, err) twentyTwoMinutes, err := time.ParseDuration("22m") - if err != nil { - t.Error() - } + require.NoError(t, err) oneHundredThirtyFiveHours, err := time.ParseDuration("135h") - if err != nil { - t.Error() - } + require.NoError(t, err) var tests = []struct { name string From d80b9a43d58295744550f79637cca6d91ddb4a3f Mon Sep 17 00:00:00 2001 From: fchikwekwe Date: Mon, 17 Jul 2023 11:10:57 -0400 Subject: [PATCH 4/7] fix: add back time var --- pkg/ottl/boolean_value_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/ottl/boolean_value_test.go b/pkg/ottl/boolean_value_test.go index 33d8ef5a8b7f..c82e24272cef 100644 --- a/pkg/ottl/boolean_value_test.go +++ b/pkg/ottl/boolean_value_test.go @@ -113,6 +113,8 @@ func Test_newComparisonEvaluator(t *testing.T) { oneHundredThirtyFiveHours, err := time.ParseDuration("135h") require.NoError(t, err) + JanFirst2023 := time.Date(2023, 1, 1, 0, 0, 0, 0, time.Local) + var tests = []struct { name string l any From c00477bf0acae105f521984814ebf3ac94fb1484 Mon Sep 17 00:00:00 2001 From: fchikwekwe Date: Mon, 17 Jul 2023 13:22:25 -0400 Subject: [PATCH 5/7] doc: add time and duration to comparison doc --- pkg/ottl/README.md | 22 +++++++++++++--------- pkg/ottl/compare.go | 2 +- pkg/ottl/parser_test.go | 4 ++-- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/pkg/ottl/README.md b/pkg/ottl/README.md index 6d556789a3e4..5270b1a466f1 100644 --- a/pkg/ottl/README.md +++ b/pkg/ottl/README.md @@ -236,15 +236,19 @@ For values that are not one of the basic primitive types, the only valid compari A `not equal` notation in the table below means that the "!=" operator returns true, but any other operator returns false. Note that a nil byte array is considered equivalent to nil. - -| base type | bool | int64 | float64 | string | Bytes | nil | -|-----------|-------------|---------------------|---------------------|---------------------------------|--------------------------|------------------------| -| bool | normal, T>F | not equal | not equal | not equal | not equal | not equal | -| int64 | not equal | compared as largest | compared as float64 | not equal | not equal | not equal | -| float64 | not equal | compared as float64 | compared as largest | not equal | not equal | not equal | -| string | not equal | not equal | not equal | normal (compared as Go strings) | not equal | not equal | -| Bytes | not equal | not equal | not equal | not equal | byte-for-byte comparison | []byte(nil) == nil | -| nil | not equal | not equal | not equal | not equal | []byte(nil) == nil | true for equality only | +The `time.Time` and `time.Duration` types are compared using comparison functions from their respective packages. For more details on how those comparisons work, see the golang Time package. + + +| base type | bool | int64 | float64 | string | Bytes | nil | time.Time | time.Duration | +|---------------|-------------|---------------------|---------------------|---------------------------------|--------------------------|------------------------|--------------------------------------------------------------|------------------------------------------------------| +| bool | normal, T>F | not equal | not equal | not equal | not equal | not equal | not equal | not equal | +| int64 | not equal | compared as largest | compared as float64 | not equal | not equal | not equal | not equal | not equal | +| float64 | not equal | compared as float64 | compared as largest | not equal | not equal | not equal | not equal | not equal | +| string | not equal | not equal | not equal | normal (compared as Go strings) | not equal | not equal | not equal | not equal | +| Bytes | not equal | not equal | not equal | not equal | byte-for-byte comparison | []byte(nil) == nil | not equal | not equal | +| nil | not equal | not equal | not equal | not equal | []byte(nil) == nil | true for equality only | not equal | not equal | +| time.Time | not equal | not equal | not equal | not equal | not equal | not equal | uses `time.Equal()`to check equality regardless of time zone | not equal | +| time.Duration | not equal | not equal | not equal | not equal | not equal | not equal | not equal | uses `time.Before()` and `time.After` for comparison | Examples: - `name == "a name"` diff --git a/pkg/ottl/compare.go b/pkg/ottl/compare.go index c3cc7047c020..fc93186c4985 100644 --- a/pkg/ottl/compare.go +++ b/pkg/ottl/compare.go @@ -144,7 +144,7 @@ func (p *Parser[K]) compareDuration(a time.Duration, b any, op compareOp) bool { return comparePrimitives(ansecs, vnsecs, op) default: return p.invalidComparison("cannot compare invalid duration", op) - } + } } func (p *Parser[K]) compareTime(a time.Time, b any, op compareOp) bool { diff --git a/pkg/ottl/parser_test.go b/pkg/ottl/parser_test.go index 7089c82a01bf..55b9e6626dc8 100644 --- a/pkg/ottl/parser_test.go +++ b/pkg/ottl/parser_test.go @@ -880,7 +880,7 @@ func testParsePath(val *Path) (GetSetter[interface{}], error) { return &StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { m, ok := tCtx.(map[string]time.Duration) - if !ok { + if !ok { return nil, fmt.Errorf("unable to convert transform context to map of strings to times") } return m[val.Fields[0].Name], nil @@ -890,7 +890,7 @@ func testParsePath(val *Path) (GetSetter[interface{}], error) { return nil }, }, nil - } + } if val.Fields[0].Name == "time1" || val.Fields[0].Name == "time2" { return &StandardGetSetter[interface{}]{ Getter: func(ctx context.Context, tCtx interface{}) (interface{}, error) { From f1d3cf54347302ac37e547e1297f377af5efa525 Mon Sep 17 00:00:00 2001 From: Faith Chikwekwe Date: Mon, 17 Jul 2023 13:44:43 -0400 Subject: [PATCH 6/7] Update pkg/ottl/README.md Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- pkg/ottl/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/README.md b/pkg/ottl/README.md index 5270b1a466f1..c4f88f8ec6e4 100644 --- a/pkg/ottl/README.md +++ b/pkg/ottl/README.md @@ -236,7 +236,7 @@ For values that are not one of the basic primitive types, the only valid compari A `not equal` notation in the table below means that the "!=" operator returns true, but any other operator returns false. Note that a nil byte array is considered equivalent to nil. -The `time.Time` and `time.Duration` types are compared using comparison functions from their respective packages. For more details on how those comparisons work, see the golang Time package. +The `time.Time` and `time.Duration` types are compared using comparison functions from their respective packages. For more details on how those comparisons work, see the [Golang Time package](https://pkg.go.dev/time). | base type | bool | int64 | float64 | string | Bytes | nil | time.Time | time.Duration | From b56b13e3365c8177f6e94d8e5e269cab57737b43 Mon Sep 17 00:00:00 2001 From: Faith Chikwekwe Date: Mon, 17 Jul 2023 13:45:01 -0400 Subject: [PATCH 7/7] Update pkg/ottl/README.md Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> --- pkg/ottl/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ottl/README.md b/pkg/ottl/README.md index c4f88f8ec6e4..03e7ccdb1f84 100644 --- a/pkg/ottl/README.md +++ b/pkg/ottl/README.md @@ -247,7 +247,7 @@ The `time.Time` and `time.Duration` types are compared using comparison function | string | not equal | not equal | not equal | normal (compared as Go strings) | not equal | not equal | not equal | not equal | | Bytes | not equal | not equal | not equal | not equal | byte-for-byte comparison | []byte(nil) == nil | not equal | not equal | | nil | not equal | not equal | not equal | not equal | []byte(nil) == nil | true for equality only | not equal | not equal | -| time.Time | not equal | not equal | not equal | not equal | not equal | not equal | uses `time.Equal()`to check equality regardless of time zone | not equal | +| time.Time | not equal | not equal | not equal | not equal | not equal | not equal | uses `time.Equal()`to check equality | not equal | | time.Duration | not equal | not equal | not equal | not equal | not equal | not equal | not equal | uses `time.Before()` and `time.After` for comparison | Examples: