File Sink Client Typing

[michaeldjeffrey]

Typing a FileSInkClient

FileSinkClient is widely used in this codebase, it will currently write anything to a file that is Vec<u8>.
Some parts of Oracles take more than a few handles to FileSinkClient, and the only way to know what is being written is by the variable name, and following it's usage to where an actual .write() happens.

Written filenames are determined by a provided FileType, but there is no guarantee that the message being written matches the FileType or the variable name. An example where this could go wrong is during a refactor of a component that takes multiple FileSinkClients, and the argument order of a constructor is changed. Being hidden in a larger diff block, it would be a very difficult thing to find. Or adding a file sink to a component, and the argument provided is not in the same position as the argument added.

With a FileSinkClient<T>, there's less to find. Even if the variable is named incorrectly the compiler would shout at you if the wrong message type was being written.

FileSinkWriteExt trait

An example of this trait

impl_file_sink!(
    packet_verifier::InvalidPacket,    // proto type to be written
    FileType::InvalidPacket.to_str(),  // filename is `&'static str`, does not include file ext.
    "invalid_packets"                  // metric suffix
);

and usage

//  (FileSinkClient<InvalidPacket>, FileSink<InvalidPacket>)
let (invalid_packets, invalid_packets_server) = InvalidPacket::file_sink(
            store_base_path,
            file_upload.clone(),
            Some(DEFAULT_ROLL_TIME),
            env!("CARGO_PKG_NAME"), // metric prefix
        )
        .await?;

FileSinks and prost::Message

impl<T: prost::Message> FileSinkWriteExt for T {} seemed like a good idea until it was made abundantly clear that prost::Message has an impl for basically every type there is. Which makes things like having a FileSinkClient<String> or FileSinkClient<Vec<u8>> very difficult, as they would use prost::Message and the values you get out of the file would be tainted with proto encoding.

MsgBytes

The FileSinkWriteExt trait sits on top of MsgBytes, anything that implements MsgBytes can be used with a FileSink. Implementations for String, Vec<u8>, and bytes::Bytes are provided already in case someone wants to use a FileSink without a proto message.

Assumptions

From what I understand...
auto_commit saves a file for every write.
roll_time attempts to save a file every so often if there is something to save.

It doesn't seem to make sense to me to see if there's something to save and save after every write. The helper combines these two flags behind an Option.

[jeffgrunewald]

🙌