[pkg/ottl] OTTL contexts paths are ignoring failed setters executions

[edmocosta]

Component(s)

pkg/ottl

Describe the issue you're reporting

Currently, many OTTL contexts path setters are ignoring the value type assertion failures, which I'm not sure if it was a design decision, or some bad pattern that was copied from one context to another.

For example, the following statements would NOT fail the execution, and IMO, it should, as severity_number must be an int64, and not an string:

log_statements:
 - context: log
   statements:
    - set(severity_number, "bar")

It happens because those paths setters are ignoring type assertion failures, and are not returning any error, making the set operation silently fail:

func accessSeverityNumber[K Context]() ottl.StandardGetSetter[K] {
	return ottl.StandardGetSetter[K]{
                // ...
		Setter: func(_ context.Context, tCtx K, val any) error {
			if i, ok := val.(int64); ok {
				tCtx.GetLogRecord().SetSeverityNumber(plog.SeverityNumber(i))
			}
			return nil
		},
	}
}

I understand that changing this behavior would be a kind of breaking change (noop statements doing that would starting failing). So I'm wondering, Do we have any reason for that work like this? If we don't, I really think we should start fixing them and propagating some error when those set operation fails to execute.

[github-actions]

Pinging code owners:

• pkg/ottl: @TylerHelmuth @evan-bradley @edmocosta

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[TylerHelmuth]

@edmocosta ya this came from all the setters being created before we had the error_mode stuff and the defined pattern to return errors whenever possible. I support the setters/getters error when they receive something unexpected.

[rockdaboot]

I am all in for changing the behavior towards reporting errors (better sooner than later).
While there is some risk of breaking some rare things, I'd argue that things only break because it hasn't been realized that the corresponding code has an issue. So possibly there are even situations where wrong/unexpected data is generated unseen and being propagated through levels of processing.
I hardly can imagine that anyone consciously relies on these (undocumented!?) suppression of errors.
Better break things than silently ignoring errors, tbh.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• pkg/ottl: @TylerHelmuth @evan-bradley @edmocosta

See Adding Labels via Comments if you do not have permissions to add labels yourself.