Knotx / knotx-fragments

Fragments map-reduce processing using Graph flows, supplier and assembler.
https://knotx.io
Apache License 2.0
3 stars 5 forks source link

Always provide action alias in behaviour's invocation log. #215

Open marcinus opened 3 years ago

marcinus commented 3 years ago

Is your feature request related to a problem? Please describe. As recognized when investigating #214, when an action does not expose logs in the returned FragmentResult, the invocation logs of the wrapping behaviour contains an invocation that does not describe event action's alias.

This case may occur for custom actions, but is also present in the library at the time of writing for copy-payload-key action.

Describe the solution you'd like Preferably, the metada should be exposed by the behaviour. This is, however, not achieveable, because ActionFactory's interface has create function that does not take doActions alias as an argument. Therefore, ActionFactory for a behaviour (be it CacheActionFactory, CircuitBreakerActionFactory or any other) does not contain any metadata related to the invoked action.

I think we should change the pattern of behaviour's composition. Preferably, a central register of actions should be used, and action's metadata should be queried from there.

In general, I suggest creating a different interface for a behaviour. A behaviour could have a reference to an entry point, like action registry, that would be responsible for invoking the actual action (the problem with this approach might be concurrency - I will elaborate on that below).

We do not have an established approach to execution graph composition. Some things happen online, but others happen offline. Graph creation for a task is done sequentially, before the processing begins. Some actions might be cached. This is a good approach, as creation of some actions might be costly. The problem is - again - concurrency. Cacheable action must not store any mutable state, or the stored state must be thread-safe (preferably, we would resign from the latter possibility as well).

Behaviour -> Registry -> wrapped Action

So behaviour calls registry for invoking an action with the given FragmentContext. Registry is responsible for basic logging and invoking an action, as well as handling the errors.

What are the downsides of this solution? Well, it creates a common point of failure and potentially requires synchronization when accessing an action, that could be costly.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.