journald: Set syslog identifier #1822
Conversation
|
@Ralith , what do you think? |
Ralith
left a comment
Ralith
left a comment
There was a problem hiding this comment.
Good catch! I didn't realize this wasn't populated by journald via socket magic.
|
@hawkw I'm not sure whether I've caused these build failures here… it looks as if there are some missing doc links elsewhere? Can I do something about those? |
I don't think those are your fault, I believe they were caused by a recent RustDoc release detecting missing docs links that it didn't previously detect. I'll fix the docs on |
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
This looks good to me overall! I had a couple very small suggestions.
Upstream does this as well, and it's quite handy with journalctl -t.
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
the docs changes look good to me! i'm on board with not making the identifier an Option; it would be nice to hear from @Ralith on whether he thinks saving a few bytes for programs that only run as systemd units is worth making the interface less simple?
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
|
Personally I don't think it is. For reference, the upstream C library which C programs normally use for logging to journald always sets the syslog identifier if it's missing in the log inputs. Callers can only set an empty identifier, by passing an empty string, but they cannot suppress the field entirely. And what's good enough for upstream systems is good enough for tracing as well I think 😊 |
|
I don't have a strong opinion, the upstream prior art is compelling, and if we ever decide it matters in the future we can special-case the empty identifier or add a separate method or something. |
|
Cool, just wanted to make sure everyone agreed before clicking "merge"! Thanks for working on this @lunaryorn! |
|
@hawkw Thanks for reviewing and merging 🙏 Would you mind to make a new release with this change? Not super important, but it'd be nice to have (I just can't get |
Set the `SYSLOG_IDENTIFIER` field on all events. I noticed that the subscriber didn't do this so far. ## Motivation The identifier is used with `journalctl -t`, and while it's normally better to filter by unit with `journalctl -u` the identifier is still nice for processes that are not started through systemd units. Upstream does this as well, see [here]:  `program_invocation_short_name` is a glibc variable which holds the file name of the current executable; I tried to replicate this behaviour in Rust. ## Solution Add a syslog identifier field to the subscriber, defaulting to the filename of the current executable, and write this value as `SYSLOG_IDENTIFIER` with every event. It's not written for spans, because it's a global value and inevitably the same for each span. [here]: https://github.com/systemd/systemd/blob/81218ac1e14b4b50b4337938bcf55cacc76f0728/src/libsystemd/sd-journal/journal-send.c#L270
Set the `SYSLOG_IDENTIFIER` field on all events. I noticed that the subscriber didn't do this so far. ## Motivation The identifier is used with `journalctl -t`, and while it's normally better to filter by unit with `journalctl -u` the identifier is still nice for processes that are not started through systemd units. Upstream does this as well, see [here]:  `program_invocation_short_name` is a glibc variable which holds the file name of the current executable; I tried to replicate this behaviour in Rust. ## Solution Add a syslog identifier field to the subscriber, defaulting to the filename of the current executable, and write this value as `SYSLOG_IDENTIFIER` with every event. It's not written for spans, because it's a global value and inevitably the same for each span. [here]: https://github.com/systemd/systemd/blob/81218ac1e14b4b50b4337938bcf55cacc76f0728/src/libsystemd/sd-journal/journal-send.c#L270
# 0.2.2 (January 14, 2022) ### Added - Include a syslog identifier in log messages ([#1822]) - Added `Layer::with_syslog_identifier` method to override the syslog identifier ([#1822]) Thanks to @lunaryorn for contributing to this release! [#1822]: #1822
# 0.2.2 (January 14, 2022) ### Added - Include a syslog identifier in log messages ([#1822]) - Added `Layer::with_syslog_identifier` method to override the syslog identifier ([#1822]) Thanks to @lunaryorn for contributing to this release! [#1822]: #1822
# 0.2.2 (January 14, 2022) ### Added - Include a syslog identifier in log messages ([tokio-rs#1822]) - Added `Layer::with_syslog_identifier` method to override the syslog identifier ([tokio-rs#1822]) Thanks to @lunaryorn for contributing to this release! [tokio-rs#1822]: tokio-rs#1822
3 participants
Set the
SYSLOG_IDENTIFIERfield on all events. I noticed that the subscriber didn't do this so far.Motivation
The identifier is used with
journalctl -t, and while it's normally better to filter by unit withjournalctl -uthe identifier is still nice for processes that are not started through systemd units.Upstream does this as well, see [https://github.com/systemd/systemd/blob/81218ac1e14b4b50b4337938bcf55cacc76f0728/src/libsystemd/sd-journal/journal-send.c#L270]:
program_invocation_short_nameis a glibc variable which holds the file name of the current executable; I tried to replicate this behaviour in Rust.Solution
Add a syslog identifier field to the subscriber, defaulting to the filename of the current executable, and write this value as SYSLOG_IDENTIFIER with every event. It's not written for spans, because it's a global value and inevitably the same for each span.