Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pkg/ottl] Basic math capabilities #15711

Merged
merged 16 commits into from
Nov 10, 2022
Merged

[pkg/ottl] Basic math capabilities #15711

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
8fe8389
Basic math capabilities
TylerHelmuth Oct 26, 2022
51a3889
Add changelog entry
TylerHelmuth Nov 3, 2022
fee15ad
Update docs
TylerHelmuth Nov 3, 2022
30e3ecc
cleanup variables
TylerHelmuth Nov 3, 2022
de58231
Switch to maxlookahead
TylerHelmuth Nov 3, 2022
c776c0d
merge in upstream/main
TylerHelmuth Nov 4, 2022
0f76a46
remove unneeded parentheses
TylerHelmuth Nov 4, 2022
3d5d039
Apply feedback
TylerHelmuth Nov 4, 2022
8f89a6d
Merge remote-tracking branch 'upstream/main' into ottl-add-math-to-gr…
TylerHelmuth Nov 4, 2022
a93e19f
Rename to mathExpression
TylerHelmuth Nov 7, 2022
12d981f
Fix rename being too zealous
TylerHelmuth Nov 9, 2022
37fc294
Update pkg/ottl/README.md
TylerHelmuth Nov 9, 2022
eb28d3c
Update .chloggen/ottl-add-math-to-grammar.yaml
TylerHelmuth Nov 9, 2022
b2f0ad8
Gracefully handle divide by 0
TylerHelmuth Nov 9, 2022
1893be1
Add more test cases
TylerHelmuth Nov 9, 2022
d135410
Apply feedback
TylerHelmuth Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add more test cases
  • Loading branch information
@TylerHelmuth
TylerHelmuth committed Nov 9, 2022
commit 1893be1e39ba27637383b396c7eb6baedf3b2ff5
4 changes: 2 additions & 2 deletions pkg/ottl/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ Note that `*` and `/` take precedence over `+` and `-`.
Operations that share the same level of precedence will be executed in the order that they appear in the Math Expression.
Math Expressions can be grouped with parentheses to override evaluation precedence.
Math Expressions that mix `int64` and `float64` will result in an error.
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
It is up to the function using the Math Expression to determine what to do with that error and the default return value of `0`.
Normal arithmetic operations that would result in a panic, such as division by zero, will still result in a panic.
It is up to the function using the Math Expression to determine what to do with that error and the default return value of `nil`.
Division by zero is gracefully handled with an error, but other arithmetic operations that would result in a panic will still result in a panic.
Division of integers results in an integer and follows Go's rules for division of integers.

Since Math Expressions support `Path` and `Invocation`, they are evaluated during data processing.
Expand Down
41 changes: 41 additions & 0 deletions pkg/ottl/math_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package ottl // import "github.com/open-telemetry/opentelemetry-collector-contri
import (
"context"
"fmt"
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -152,6 +153,46 @@ func Test_evaluateMathExpression(t *testing.T) {
input: "10 / 3",
expected: 3,
},
{
name: "multiply large ints",
input: "9223372036854775807 * 9223372036854775807",
expected: 1,
},
{
name: "division by large ints",
input: "9223372036854775807 / 9223372036854775807",
expected: 1,
},
{
name: "add large ints",
input: "9223372036854775807 + 9223372036854775807",
expected: -2,
},
{
name: "subtraction by large ints",
input: "9223372036854775807 - 9223372036854775807",
expected: 0,
},
{
name: "multiply large floats",
input: "1.79769313486231570814527423731704356798070e+308 * 1.79769313486231570814527423731704356798070e+308",
expected: math.Inf(0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, this opens up a small can of worms. What happens if inf and NaN come through the pipeline? Do we support them as constants in ottl? Should we?

I can definitely imagine somebody wanting something like drop() where attributes["math_field"] == NaN

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya this was an outcome of testing I wasn't really expecting. I think +Inf isn't really a concern, but NaN will be useful once we can get in those situations. I think with the operations we've defined we will always avoid NaN tho.

},
{
name: "division by large floats",
input: "1.79769313486231570814527423731704356798070e+308 / 1.79769313486231570814527423731704356798070e+308",
expected: 1,
},
{
name: "add large numbers",
input: "1.79769313486231570814527423731704356798070e+308 + 1.79769313486231570814527423731704356798070e+308",
expected: math.Inf(0),
},
{
name: "subtraction by large numbers",
input: "1.79769313486231570814527423731704356798070e+308 - 1.79769313486231570814527423731704356798070e+308",
expected: 0,
},
}

functions := map[string]interface{}{
Expand Down