[pkg/ottl] Improve OTTL parser error reporting

.chloggen/ottl-improve-parsing-errors.yaml

@@ -0,0 +1,22 @@
+# 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: Improve error reporting for errors during statement parsing
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [23840]
+
+# (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: |
+  - Failures are now printed for all statements within a context, and the statements are printed next to the errors.
+  - Erroneous values found during parsing are now quoted in error logs.

pkg/ottl/functions.go

@@ -20,7 +20,7 @@ type Enum int64
 func (p *Parser[K]) newFunctionCall(ed editor) (Expr[K], error) {
 	f, ok := p.functions[ed.Function]
 	if !ok {
-		return Expr[K]{}, fmt.Errorf("undefined function %v", ed.Function)
+		return Expr[K]{}, fmt.Errorf("undefined function %q", ed.Function)
 	}
 	args := f.CreateDefaultArguments()
 
@@ -30,12 +30,12 @@ func (p *Parser[K]) newFunctionCall(ed editor) (Expr[K], error) {
 		// settability requirements. Non-pointer values are not
 		// modifiable through reflection.
 		if reflect.TypeOf(args).Kind() != reflect.Pointer {
-			return Expr[K]{}, fmt.Errorf("factory for %s must return a pointer to an Arguments value in its CreateDefaultArguments method", ed.Function)
+			return Expr[K]{}, fmt.Errorf("factory for %q must return a pointer to an Arguments value in its CreateDefaultArguments method", ed.Function)
 		}
 
 		err := p.buildArgs(ed, reflect.ValueOf(args).Elem())
 		if err != nil {
-			return Expr[K]{}, fmt.Errorf("error while parsing arguments for call to '%v': %w", ed.Function, err)
+			return Expr[K]{}, fmt.Errorf("error while parsing arguments for call to %q: %w", ed.Function, err)
 		}
 	}
 
@@ -61,17 +61,17 @@ func (p *Parser[K]) buildArgs(ed editor, argsVal reflect.Value) error {
 		fieldTag, ok := argsType.Field(i).Tag.Lookup("ottlarg")
 
 		if !ok {
-			return fmt.Errorf("no `ottlarg` struct tag on Arguments field '%s'", argsType.Field(i).Name)
+			return fmt.Errorf("no `ottlarg` struct tag on Arguments field %q", argsType.Field(i).Name)
 		}
 
 		argNum, err := strconv.Atoi(fieldTag)
 
 		if err != nil {
-			return fmt.Errorf("ottlarg struct tag on field '%s' is not a valid integer: %w", argsType.Field(i).Name, err)
+			return fmt.Errorf("ottlarg struct tag on field %q is not a valid integer: %w", argsType.Field(i).Name, err)
 		}
 
 		if argNum < 0 || argNum >= len(ed.Arguments) {
-			return fmt.Errorf("ottlarg struct tag on field '%s' has value %d, but must be between 0 and %d", argsType.Field(i).Name, argNum, len(ed.Arguments))
+			return fmt.Errorf("ottlarg struct tag on field %q has value %d, but must be between 0 and %d", argsType.Field(i).Name, argNum, len(ed.Arguments))
 		}
 
 		argVal := ed.Arguments[argNum]
@@ -167,7 +167,7 @@ func (p *Parser[K]) buildSliceArg(argVal value, argType reflect.Type) (any, erro
 		}
 		return arg, nil
 	default:
-		return nil, fmt.Errorf("unsupported slice type '%s' for function", argType.Elem().Name())
+		return nil, fmt.Errorf("unsupported slice type %q for function", argType.Elem().Name())
 	}
 }
 

pkg/ottl/go.mod

@@ -13,6 +13,7 @@ require (
 	go.opentelemetry.io/collector/component v0.81.0
 	go.opentelemetry.io/collector/pdata v1.0.0-rcv0013
 	go.opentelemetry.io/otel/trace v1.16.0
+	go.uber.org/multierr v1.11.0
 	go.uber.org/zap v1.24.0
 	golang.org/x/exp v0.0.0-20230510235704-dd950f8aeaea
 )
@@ -35,7 +36,6 @@ require (
 	go.opentelemetry.io/otel v1.16.0 // indirect
 	go.opentelemetry.io/otel/metric v1.16.0 // indirect
 	go.uber.org/atomic v1.10.0 // indirect
-	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/net v0.12.0 // indirect
 	golang.org/x/sys v0.10.0 // indirect
 	golang.org/x/text v0.11.0 // indirect

pkg/ottl/parser.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/alecthomas/participle/v2"
 	"go.opentelemetry.io/collector/component"
+	"go.uber.org/multierr"
 	"go.uber.org/zap"
 )
 
@@ -96,15 +97,27 @@ func WithEnumParser[K any](parser EnumParser) Option[K] {
 	}
 }
 
+// ParseStatements parses string statements into ottl.Statement objects ready for execution.
+// Returns a slice of statements and a nil error on successful parsing.
+// If parsing fails, returns an empty slice  with a multierr error containing
+// an error per failed statement.
 func (p *Parser[K]) ParseStatements(statements []string) ([]*Statement[K], error) {
 	var parsedStatements []*Statement[K]
+	var parseErr error
+
 	for _, statement := range statements {
 		ps, err := p.ParseStatement(statement)
 		if err != nil {
-			return nil, err
+			parseErr = multierr.Append(parseErr, fmt.Errorf("unable to parse OTTL statement %q: %w", statement, err))
+			continue
 		}
 		parsedStatements = append(parsedStatements, ps)
 	}
+
+	if parseErr != nil {
+		return nil, parseErr
+	}
+
 	return parsedStatements, nil
 }
 
@@ -134,7 +147,7 @@ func parseStatement(raw string) (*parsedStatement, error) {
 	parsed, err := parser.ParseString("", raw)
 
 	if err != nil {
-		return nil, fmt.Errorf("unable to parse OTTL statement: %w", err)
+		return nil, fmt.Errorf("statement has invalid syntax: %w", err)
 	}
 	err = parsed.checkForCustomError()
 	if err != nil {

pkg/ottl/parser_test.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.uber.org/multierr"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottltest"
 )
@@ -1365,6 +1366,33 @@ func testParseEnum(val *EnumSymbol) (*Enum, error) {
 	return nil, fmt.Errorf("enum symbol not provided")
 }
 
+func Test_ParseStatements_Error(t *testing.T) {
+	statements := []string{
+		`set(`,
+		`set("foo)`,
+		`set(name.)`,
+	}
+
+	p, _ := NewParser(
+		CreateFactoryMap[any](),
+		testParsePath,
+		componenttest.NewNopTelemetrySettings(),
+		WithEnumParser[any](testParseEnum),
+	)
+
+	_, err := p.ParseStatements(statements)
+
+	assert.Error(t, err)
+
+	multiErrs := multierr.Errors(err)
+
+	assert.Len(t, multiErrs, len(statements), "ParseStatements didn't return an error per statement")
+
+	for i, statementErr := range multiErrs {
+		assert.ErrorContains(t, statementErr, fmt.Sprintf("unable to parse OTTL statement %q", statements[i]))
+	}
+}
+
 // This test doesn't validate parser results, simply checks whether the parse succeeds or not.
 // It's a fast way to check a large range of possible syntaxes.
 func Test_parseStatement(t *testing.T) {

processor/filterprocessor/config_test.go

@@ -893,24 +893,19 @@ func TestLoadingConfigOTTL(t *testing.T) {
 			errorMessage: "cannot use ottl conditions and include/exclude for logs at the same time",
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_span"),
-			errorMessage: "unable to parse OTTL statement: 1:25: unexpected token \"test\" (expected (<string> | <int>) \"]\")",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_span"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_spanevent"),
-			errorMessage: "unable to parse OTTL statement: 1:25: unexpected token \"test\" (expected (<string> | <int>) \"]\")",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_spanevent"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_metric"),
-			errorMessage: "unable to parse OTTL statement: 1:34: unexpected token \"test\" (expected (<string> | <int>) \"]\")",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_metric"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_datapoint"),
-			errorMessage: "unable to parse OTTL statement: 1:25: unexpected token \"test\" (expected (<string> | <int>) \"]\")",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_datapoint"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_log"),
-			errorMessage: "unable to parse OTTL statement: 1:25: unexpected token \"test\" (expected (<string> | <int>) \"]\")",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_log"),
 		},
 	}
 
@@ -924,7 +919,11 @@ func TestLoadingConfigOTTL(t *testing.T) {
 			require.NoError(t, component.UnmarshalConfig(sub, cfg))
 
 			if tt.expected == nil {
-				assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage)
+				if tt.errorMessage != "" {
+					assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage)
+				} else {
+					assert.Error(t, component.ValidateConfig(cfg))
+				}
 			} else {
 				assert.NoError(t, component.ValidateConfig(cfg))
 				assert.Equal(t, tt.expected, cfg)

processor/transformprocessor/config_test.go

@@ -20,9 +20,8 @@ func TestLoadConfig(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		id           component.ID
-		expected     component.Config
-		errorMessage string
+		id       component.ID
+		expected component.Config
 	}{
 		{
 			id: component.NewIDWithName(metadata.Type, ""),
@@ -92,28 +91,22 @@ func TestLoadConfig(t *testing.T) {
 			},
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_trace"),
-			errorMessage: "unable to parse OTTL statement: 1:18: unexpected token \"where\" (expected \")\" Key*)",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_trace"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "unknown_function_trace"),
-			errorMessage: "undefined function not_a_function",
+			id: component.NewIDWithName(metadata.Type, "unknown_function_trace"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_metric"),
-			errorMessage: "unable to parse OTTL statement: 1:18: unexpected token \"where\" (expected \")\" Key*)",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_metric"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "unknown_function_metric"),
-			errorMessage: "undefined function not_a_function",
+			id: component.NewIDWithName(metadata.Type, "unknown_function_metric"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "bad_syntax_log"),
-			errorMessage: "unable to parse OTTL statement: 1:18: unexpected token \"where\" (expected \")\" Key*)",
+			id: component.NewIDWithName(metadata.Type, "bad_syntax_log"),
 		},
 		{
-			id:           component.NewIDWithName(metadata.Type, "unknown_function_log"),
-			errorMessage: "undefined function not_a_function",
+			id: component.NewIDWithName(metadata.Type, "unknown_function_log"),
 		},
 	}
 	for _, tt := range tests {
@@ -129,7 +122,7 @@ func TestLoadConfig(t *testing.T) {
 			assert.NoError(t, component.UnmarshalConfig(sub, cfg))
 
 			if tt.expected == nil {
-				assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage)
+				assert.Error(t, component.ValidateConfig(cfg))
 				return
 			}
 			assert.NoError(t, component.ValidateConfig(cfg))