[pkg/ottl] Convert Statement to an interface #14869
Conversation
| // Returns true if the function was run, returns false otherwise. | ||
| // If the statement contains no condition, the function will run and true will be returned. | ||
| // In addition, the functions return value is always returned. | ||
| func (s *Statement[K]) Execute(ctx K) (any, bool) { |
There was a problem hiding this comment.
Would be good (later) to change ParseStatements to return array of pointers.
| // Returns true if the function was run, returns false otherwise. | ||
| // If the statement contains no condition, the function will run and true will be returned. | ||
| // In addition, the functions return value is always returned. | ||
| func (s *Statement[K]) Execute(ctx K) (any, bool) { |
There was a problem hiding this comment.
would be good to also pass context.Context. Also we should return an error.
Do we have use-cases for "returning if func was executed"?
Do we have use-cases for "func returning a value (any)"?
There was a problem hiding this comment.
would be good to also pass context.Context
Would it be better to pass context.Context to execute or pass it to the parser and give every function access to it if they need it?
Also we should return an error.
Yes can add this in the future.
Do we have use-cases for "returning if func was executed"?
Tangentially. The routing processor makes decisions on if the condition returned true (true condition currently guarantees function execution)
Do we have use-cases for "func returning a value (any)"?
Based on my previous attempt to implement drop() I think the return value will be useful. It may also allow interesting situations with factory functions in the future.
There was a problem hiding this comment.
Would it be better to pass context.Context to execute or pass it to the parser and give every function access to it if they need it?
I am thinking about the context associated with the data not with the component that created the Parser.
our ConsumeTraces(ctx context.Context, td ptrace.Traces) so we should pass this context not the one passed in the component.Start(ctx context.Context, ...) which created the parser.
Should the context be passed to all the funcs - yes
Tangentially. The routing processor makes decisions on if the condition returned true (true condition currently guarantees function execution)
Not sure if then worth calling this API. Maybe we provide on this struct just a "CheckCond(...)" and "ExecFunc(...)" and they manually call if they need such granular control (which for example in transformprocessor is not needed)
There was a problem hiding this comment.
Not sure if then worth calling this API.
They want to call the function though. The routing processor allows using expression to match but also update the telemetry when there is a match
processors:
routing:
default_exporters:
- jaeger
table:
- expression: route() where resource.attributes["X-Tenant"] == "acme"
exporters: [jaeger/acme]
- expression: delete_key(resource.attributes, "X-Tenant") where IsMatch(resource.attributes["X-Tenant"], ".*corp") == true
exporters: [jaeger/ecorp]In that example the routing processor will match a rule and send to jaeger/ecorp and delete the X-Tenant key for that data.
So the they need to be able to call a function and also know if it was called.
* Convert Statement to an interface * update documentation * add changelog entry * Add unit tests * adjust routing processor if statements * Reorder return values * Switch from interface to struct
Description:
Converts
Statementto an interface with anExecutefunction. Users can now use this execute function to evaluate statements, instead of needing to handle Condition and Function themselves.Testing:
Ran unit tests