[pkg/ottl] Fix bug where function names were allowed in comparison #33051
Conversation
TylerHelmuth
requested review from
bogdandrutu,
evan-bradley and
a team
as code owners
| @@ -1952,6 +1950,7 @@ func Test_parseCondition(t *testing.T) { | |||
| {`One() == 1`, false}, | |||
| {`test(fail())`, true}, | |||
| {`Test()`, false}, | |||
| {`"test" == Foo`, true}, | |||
There was a problem hiding this comment.
What does the error look like now?
There was a problem hiding this comment.
It looks like
statement has invalid syntax: 1:50: unexpected token "<EOF>" (expected "(" (Argument ("," Argument)*)? ")" Key*)
Which isn't the best, but at least it gives you hints where the issue is. I played around with keeping FunctionName in value and then doing a custom error when it is set, but that caused parsing issues bc value is checked before FunctionName in arguments. It might be possible with some fancy negative lookaheads
There was a problem hiding this comment.
was playing with this today, mainly to understand it better and did some more or less crazy/dumb ideas but didn't managed to get any better error message than this one
There was a problem hiding this comment.
I think overall we should try to avoid printing any grammar parsing errors since I think to most users they're incredibly cryptic. That said, this is still better than an error with "This is a bug in the OpenTelemetry Transformation Language" in it. I'd say let's get this in for now and circle back later.
…pen-telemetry#33051) **Description:** Fix a bug where a condition like `"foo" = Bar` resulted in the error `no value field set. This is a bug in the OpenTelemetry Transformation Language`. **Link to tracking Issue:** <Issue number if applicable> Closes open-telemetry#33048 **Testing:** <Describe what testing was performed and which tests were added.> Updated unit tests
Description:
Fix a bug where a condition like
"foo" = Barresulted in the errorno value field set. This is a bug in the OpenTelemetry Transformation Language.Link to tracking Issue:
Closes #33048
Testing:
Updated unit tests