search

lucab / libsystemd-rs

A pure-Rust client library to work with systemd
https://crates.io/crates/libsystemd
Other
106 stars 19 forks source link

logging: only check stderr for protocol upgrading #121

Closed lucab closed 1 year ago

lucab commented 1 year ago

This changes connected_to_journal() behavior to only check stderr against $JOURNAL_STREAM for protocol upgrading.

Protocol upgrading assumes that all the logging happens on stderr. For that purpose, it is not relevant whether stdout is connected to journal.

lucab commented 1 year ago

/cc @lunaryorn

This is the result of a recent conversation I had with systemd folks. There is a summary of that in https://github.com/coreos/go-systemd/pull/410#issuecomment-1301871688.

In short, my understanding of "native protocol upgrading" was wrong: in reality the systemd approach assumes that all logging happens on stderr, so it shouldn't be concerned with stdout. Instead, the stdout/stderr checks are two separate lower level primitives which can be used independently.

lucab commented 1 year ago

Yes, I don't think there is going to be much usage for the stdout helper, it is mostly there for symmetry with stderr. If you prefer we can drop them both and consider adding them later if anybody has a need for those.

JournalStream is already public: https://docs.rs/libsystemd/0.5.0/libsystemd/logging/struct.JournalStream.html So in theory the same stdour/stderr helpers can be already implemented directly in consumers.

Overall, it sounds like I can just only change connected_to_journal() in this PR, agreed?

Speaking of public interfaces, do you have a need for public JournalStream::parse() and JournalStream::from_env()? It looks like those can be turned into private methods, only leaving from_fd() and from_env_default() as public methods, right?

swsnr commented 1 year ago

@lucab Yes, I'd just change connected_to_journal() 🙂 If someone needs to access the plumbing here they can open a ticket (or even just copy the code, it's pretty simple after all 🙂 )