search

lucab / libsystemd-rs

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

logging: add function to check for journal connection #80

Closed swsnr closed 3 years ago

swsnr commented 3 years ago

connected_to_journal implements systemd's recommendation for processes to check for a direct connection to the journal.

This allows services to automatically upgrade to the native journald protocol for logging instead of relying on stdout being piped to journald.

swsnr commented 3 years ago

@lucab Would you mind taking a look?

If you need help maintaining this crate I could lend a hand 🙂

lucab commented 3 years ago

@lunaryorn thanks for the PR! I've left some comments in review.

I'm a bit ashamed to admit that I totally missed the initial notification for this. I recently had a look at this logging upgrade protocol and was starting to implement it myself. Thus I was positively surprised when I saw the ping this morning :)

In general yes, I'd be happy to have a co-maintainer here. Feel free to add yourself to Cargo.toml authors list, and I'll figure out the ACLs later on.

swsnr commented 3 years ago

@lucab Please take another look. I've kept the simple top-level function for convenience, but re-implemented it on top of a new JournalStream struct which encapsulates the device/inode number pair. Let me know whether you like it better.

The change in activation.rs was a drive-by fix for a warning from Rustdoc. I hope that's okay :slightly_smiling_face:

I'm happy to help. I'll make a separate MR for the Cargo.toml change which we can use for the whole onboarding process.

swsnr commented 3 years ago

@lucab I removed the excessive error type, but I haven't changed the API yet, because I'd still discuss that.

lucab commented 3 years ago

@lunaryorn LGTM! I did tweak the permission on the repo, you should be able to merge this.

swsnr commented 3 years ago

@lucab Great. I've pressed the button; do I need to update some changelog of sorts?

lucab commented 3 years ago

No need to, I usually gather the list of changes at release time (e.g. https://github.com/lucab/libsystemd-rs/pull/73).

swsnr commented 3 years ago

Cool thanks! Would you make another release with this merge request?

lucab commented 3 years ago

@lunaryorn I'd use this occasion to make sure you have access everywhere. Without any hurry, whenever you are comfortable feel free to grab https://github.com/crate-ci/cargo-release and send a release PR like the one above.

swsnr commented 3 years ago

@lucab I'll do that; will take a bit though, as I'm not quite familiar with GPG, so I'll need to spent some time to get a key and setup the whole commit/tag signing infra, and I'm quite busy over the next two weeks. I'll make a mental note and check this out in November :slightly_smiling_face:

swsnr commented 3 years ago

@lucab I did https://github.com/lucab/libsystemd-rs/pull/86; does it look good?