Request: Ability to log to Logger

[sapieneptus]

HEY MIKKEL!!!

Request

I think the ability to supply a standard logger object to a CommandRunner invocation would allow for a useful abstraction. By that, I mean an object which responds to :info and :error. By default you could push everything to logger.info and we provide the split_output option, then stdout could go to logger.info and stderr can go to logger.error.

Motivation

We are currently using CommandRunner to do something secret and while the ability to log to a file is great (and I'm sure we could pipe the info around to get it into our logger), 100% of our logging needs to go through our logger object which ultimately sends the data to a backend service for aggregation. I realize there are workarounds but I think it'd be very convenient to be able to just set a logger when making an invocation.

[kamstrup]

I am not entirely sold on the idea, to be honest. stdout an stderr can be spewing binary data for all we know. Calling info() or error() with binary data does not seem the right way to go. Also I am not sure I want to guarantee any structured logging format from the internals of the runner. That could lead to some subtle bugs down the road.

The correct way to go about this would be to pass a closure as debug_logger (we could make it so that you don't need to responds_to?(:puts), but any old callable would do), or provide the writing end of a pipe, and make sure you actively empty the reading end of that pipe while the process runs (to ensure the runner doesn't block once the pipe buffer fills up).

But I am not sure I totally understand your problem :-) We can have a detailed chat on Slack if you have time...

[kamstrup]

Here's a quick spec for what we came up with:

A 2 new keyword params: on_stdout and on_stderr which will be closures called when there is data available on either stream. If on_stderr is provided it implies split_stderr: true.

The final return value of either closure is set in the result[:out] and result[:err]. This can be nil, the full buffer, or anything.

If a closure raise an error, the child process is killed (SIGKILL), and the error is raised to the caller of run().

If the need arises in the future, we can provide a special exception type the closures can use to provide precise control of what happens to the child process as a result of some output data.