enhancement(lua transform): Implement all hooks and timers in version 2

[ghost]

Closes #616, closes #1549, closes #2189.

Currently it implements only process hook, I want to put other hooks to a separate PR because it would involve changing the stream

This is based on top of work from #2095. I'm going to also update the documentation and add some example before merging this.

[Hoverbear]

The inconsistent naming here is unfortunate. :(

[ghost]

I agree, it makes sense to use snake case everywhere.

[Hoverbear]

Can we #[cfg(test)] this?

[ghost]

These are part of the Transform trait, so it is necessary to provide at least some implementation for them.

[Hoverbear]

Only a couple nits :)

[ghost]

Now all three kinds of hooks, as well as timers, are implemented. I spent some time trying to put the runtime into a separate thread, because it is not robust to block Tokio thread. However, the current implementation runs on Tokio thread, so I think it is fine postpone moving the execution into a separate thread. I've created issue #2200 for this.

I'm going to tidy up the documentation tomorrow, but for now it is possible to consult the RFC for the description.

[binarylogic]

Sounds good! I can help with the docs as well. Why don't you make a first pass then I'll review.

[binarylogic]

@a-rodin, I removed the source field documentation since it is not relevant for v2. I don't think we should cater to v1 since we'll be releasing shortly after merging this (assuming it is not blocked by anything major).

Edit: I've re-added this now that I understand it can be used as the single place for all Lua source.

[binarylogic]

Ok, I've improved the configuration examples. As a side effect of doing this I fixed an option sorting bug that resulted in updating a number of other documentation pages.

[lukesteensen]

@lukesteensen is it ok if this transform accepts both logs and metrics? Isn't this currently blocked by #421?

There's no issue with accepting both logs and metrics. The question is really about having output_type return DataType::Any because there aren't many transforms or sinks that accept DataType::Any as their input. That means this transform itself would work fine, but you couldn't use it as input for much of anything.

A quick fix would be to add a config option to specify the output type and filter accordingly.

[binarylogic]

@a-rodin if we can address that final issue then we should be able to merge. If I recall, the proper fix is more involved and will need to be resolved separately (#1998 (comment)). You're also welcome to take a stab at solving this properly.

[ghost]

Actually I feel like solving it properly might be not much harder than adding output_type config option and filtering to only the Lua transform. At least, for downstream transforms the filtering could be added quite easy at the topology building step by replacing input_rx here https://github.com/timberio/vector/blob/7d9ebc3d580eff9d12f31563306d0d8bf3285248/src/topology/builder.rs#L175

by input_rx.filter(|event| ...), where the filtering function can use the input type of the current transform. The signature of transform_stream would need to be changed to take a generic stream instead of just Receiver, but it should be easy.

I'm less familiar with sinks though, so I'll take a look at them.

[binarylogic]

Yeah, depending on what you find I wouldn't mind considering fixing this correctly. That was on Luke's plate but he has more pressing issues he's addressing this week.

[lukesteensen]

Very clever! I should have thought of this myself 😄

Eventually, it would be nice to have this more isolated from topology building, skip filtering when we know it's not needed, etc. But this is very practical and should work quite well! 👏

[lukesteensen]

Did this not require any changes to the tests? I must have missed a case. Could be worth adding a quick one now.

[ghost]

Actually, this was caught by a test. abf4f59 updated it.

Also, with cd5ff29 the filtering is skipped if the component supports any event type.