Geal / rust-syslog

Send syslog messages from Rust
MIT License
109 stars 55 forks source link

Don't set hostname with the UNIX socket backend #80

Closed gkurz closed 5 months ago

gkurz commented 8 months ago

Users of the generic init() function experience a weird rendering when the UNIX socket backend is selected, e.g.

Jan 12 17:03:04 myhostname virtiofsd[70727]: myhostname virtiofsd[70725]: Waiting for vhost-user socket connection...

Notice that the hostname and process[pid] name appear twice. Pids are different because virtiofsd forks at some point ; 70725 is the pid of the virtiofsd process that called init() and 70727 is the pid of the child.

This seems to be the result of init() setting the hostname of the formatter unconditionally, while init_unix() and the documentation suggest to set it to None.

First patch makes sure that None is used in the UNIX case. Second patch is a cosmetic change to make the code more readable.

gkurz commented 8 months ago

Friendly ping @Geal . Any chance you can have a look ?

nborisenkov commented 5 months ago

yes, the logs look strange.

I noticed the same behavior in proxmox-backup.

This is what it looks like in journalctl:

Apr 09 06:07:18 dc01-pbs proxmox-backup-[1464]: dc01-pbs proxmox-backup-proxy[1464]: syncing filesystem

This is what it looks like in syslog file:

Apr  9 06:07:18 dc01-pbs dc01-pbs proxmox-backup-proxy[1464]: syncing filesystem
gkurz commented 5 months ago

yes, the logs look strange.

Hi @nborisenkov ! Cool to see someone else at last :wink:

I noticed the same behavior in proxmox-backup.

This is what it looks like in journalctl:

Apr 09 06:07:18 dc01-pbs proxmox-backup-[1464]: dc01-pbs proxmox-backup-proxy[1464]: syncing filesystem

This is what it looks like in syslog file:

Apr  9 06:07:18 dc01-pbs dc01-pbs proxmox-backup-proxy[1464]: syncing filesystem

Interesting. What's the setup here ? Some other syslog implementation (e.g. syslog-ng) used instead of journald ?

Can you redo your testing with this PR ?

nborisenkov commented 5 months ago

The standard setting in Debian is that first the log goes to syslog, and then to journald (or vice versa, I don’t remember). Therefore, the entries are duplicated. In Debian 12, syslog output is disabled by default.

I wanted to figure out why there were such strange entries in the logs, I looked in the source code (https://git.proxmox.com/?p=proxmox-backup.git;a=blob;f=src/bin/proxmox-backup-api.rs;h=e46557a0f2c5beaa5e101646972543d9d6a4cd6d;hb=HEAD). I found calls to syslog::init, googled examples of use, read the official documentation, tried it on localhost, went to github, went to Issues and saw this pull request. I will create a bug report on https://bugzilla.proxmox.com/ so that the developers know about this problem and maybe someday they will fix it. Or @Geal will come up with something.

I won't be able to test your code. I do not know how to do it.

Fabian-Gruenbichler commented 5 months ago

FWIW (summoned here by @nborisenkov bug report on our end), this change LGTM and fixes the issue the way we use the crate in Proxmox Backup Server.

gkurz commented 5 months ago

Friendly ping @Geal . Any chance you can have a look ?

Ditto or should we consider another approach ?

Geal commented 5 months ago

This looks reasonable yes

Geal commented 5 months ago

Thanks for your help! I'll make a release tomorrow

Geal commented 5 months ago

6.1.1 is released https://crates.io/crates/syslog/6.1.1

c3d commented 5 months ago

6.1.1 is released https://crates.io/crates/syslog/6.1.1

Merci beaucoup! Je te dois une bière 😄