[pkg/ottl] Refactor grammar validation to use the AST visitor and combining errors

[edmocosta]

Description

This PR is a follow up of https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/35174#discussion_r1777283192, and replaces the grammar's checkForCustomError mechanism by a new ottl.grammarVisitor implementation grammarCustomErrorsVisitor.

Differently of the current checkForCustomError, it joins and displays all errors at once instead of one by one, IMO, it's specially useful for statements with more than one error, for example: set(name, int(2), float(1))..

The parsing tests were also modified to ensure the error captured is in fact the one expected by the failing statement/condition.

Testing

Unit tests

[edmocosta]

@edmocosta can you share what an user would see now if they pass in a statement that fails multiple errors?

Users would see errors separated by line breaks, for example:

set(name, int(2), float(1))

2024/10/14 12:02:38 collector server run finished with error: invalid configuration: processors::transform: unable to parse OTTL statement "set(name, int(2), float(1))": converter names must start with an uppercase letter but got 'int'
converter names must start with an uppercase letter but got 'float'

set(name, int(2), float(1))["foo"]

2024/10/14 12:02:38 collector server run finished with error: invalid configuration: processors::transform: unable to parse OTTL statement "set(name, int(2), float(1))[\"foo\"]": only paths and converters may be indexed, not editors, but got set[foo]
converter names must start with an uppercase letter but got 'int'
converter names must start with an uppercase letter but got 'float'

Another option would be wrapping it using the multierr.Combine instead of errors.Join, in that case, the previous message would be printed in a single line separated by ;:

2024/10/14 12:02:38 collector server run finished with error: invalid configuration: processors::transform: unable to parse OTTL statement "set(name, int(2), float(1))[\"foo\"]": only paths and converters may be indexed, not editors, but got set[foo]; converter names must start with an uppercase letter but got 'int'; converter names must start with an uppercase letter but got 'float'

[evan-bradley]

Another option would be wrapping it using the multierr.Combine instead of errors.Join, in that case, the previous message would be printed in a single line separated by ;

Thanks for investigating the alternative. We're trying to move to errors.Join in the Collector codebase since it's in the standard library rather than a third-party package. I'm fine going with multi-line errors for now, if we decide that we want to change the formatting later, we can manually modify the error slice before printing the error.

[TylerHelmuth]

Is there any way to do single line with errors.Join? Including \n in log lines make them so much more annoying to scrape.

[evan-bradley]

Enabling JSON encoding should escape the new lines and put them inside the log body, right? I don't mind it if they're encoded, but if they split a JSON log then I agree it's an issue.

[edmocosta]

Is there any way to do single line with errors.Join? Including \n in log lines make them so much more annoying to scrape.

I might be wrong but I think it's not possible to do that using only the errors.Join. I think we could either have a custom error wrapper that prints it similarly to the multierr.Combine, or instead of errors.Join them, we could errors.Unwrap and build a single errors.New with all messages combined.

[TylerHelmuth]

Any solution that puts them in the same log line works for me - I really really don't want to write separate lines - I can't think of another place in the collector that writes multiple-line logs.

[edmocosta]

@TylerHelmuth, @evan-bradley, I've addressed the message format suggestion at https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/35728/commits/49b63e39c25fd5a20964f9ce2a7b82aba2b24b26, it now prints the messages separated by semicolons instead of line breakers:

2024/10/22 14:27:46 collector server run finished with error: invalid configuration: processors::transform: unable to parse OTTL statement "set(name, int(2), float(1))[\"foo\"]": only paths and converters may be indexed, not editors, but got set[foo]; converter names must start with an uppercase letter but got 'int'; converter names must start with an uppercase letter but got 'float'

Thank you!