Skip to content

Conversation

@felfert
Copy link
Contributor

@felfert felfert commented Oct 21, 2025

Description

This PR adds support for native systemd-journal logging if running under systemd.

Benefits

  • Eliminates duplicated timestams and useles loglevel texts showing with journalctl
  • Reduces journal log record size
  • As a side effect, enables default loglevel colors in journalctl

Testing

Done manually by running syncserver in a systemd-service and then observing the log with journalctl

@clouserw
Copy link
Collaborator

Hi @felfert

Thanks for the patch.

We're currently focused on a roadmap to provide docker images for releases which should make deployment and maintenance a lot easier for folks. That would mean the logging would typically be taken care of by docker/k8s automatically. I'm expecting that to cover most of the self-hosting crowd and would let us avoid adding these additional dependencies. Do you have a use case that wouldn't be able to use a container and would need this?

Cheers

@felfert
Copy link
Contributor Author

felfert commented Oct 30, 2025

Do you have a use case that wouldn't be able to use a container and would need this?

Well, IMHO it's more like a matter of personal taste. I have packaged syncstorage-rs as rpm for Fedora and RHEL (Ubuntu&Debian in the works) and regarding ease of setup, those are even simpler than dealing with containers. Especially on cheap vhosts with LAMP-stacks where you have mysql/mariadb and apache available by default.

If you worry about the dependencies, I could add a feature. So:
Let me know, if adding a feature for selecting/deselecting would lead to acceptance of this PR.

Regards
-Fritz

@data-sync-user data-sync-user mentioned this pull request Nov 12, 2025
@clouserw
Copy link
Collaborator

Sorry for the delay here. I'm inclined to find a way to land this. We have it on our list to review this sprint.

Comment on lines +11 to +15
.map(|stat| format!("{}:{}", stat.st_dev, stat.st_ino))
.ok()
.and_then(|stderr| {
std::env::var_os("JOURNAL_STREAM").map(|s| s.to_string_lossy() == stderr.as_str())
})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can compare the bytes directly instead of doing to_string_lossy()

use std::os::fd::AsFd;

fn connected_to_journal() -> bool {
rustix::fs::fstat(std::io::stderr().as_fd())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since systemd is linux only you could drop rustix and use MetadataExt to get the information from std::fs::metadata("/dev/stderr"). It's mildly annoying that you can't stat the fd directly, but AFAIK they should be equivalent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 and without rustix, there's really no reason for needing to wrap this under a feature flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants