Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

journald: add support for specifying syslog facility #2425

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions tracing-journald/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub struct Subscriber {
socket: UnixDatagram,
field_prefix: Option<String>,
syslog_identifier: String,
syslog_facility: Option<libc::c_int>,
}

#[cfg(unix)]
Expand All @@ -109,6 +110,7 @@ impl Subscriber {
.map(|n| n.to_string_lossy().into_owned())
// If we fail to get the name of the current executable fall back to an empty string.
.unwrap_or_else(String::new),
syslog_facility: None,
};
// Check that we can talk to journald, by sending empty payload which journald discards.
// However if the socket didn't exist or if none listened we'd get an error here.
Expand All @@ -131,8 +133,8 @@ impl Subscriber {

/// Sets the syslog identifier for this logger.
///
/// The syslog identifier comes from the classic syslog interface (`openlog()`
/// and `syslog()`) and tags log entries with a given identifier.
/// The syslog identifier comes from the classic syslog interface (`openlog(3)`
/// and `syslog(3)`) and tags log entries with a given identifier.
/// Systemd exposes it in the `SYSLOG_IDENTIFIER` journal field, and allows
/// filtering log messages by syslog identifier with `journalctl -t`.
/// Unlike the unit (`journalctl -u`) this field is not trusted, i.e. applications
Expand All @@ -151,10 +153,39 @@ impl Subscriber {
}

/// Returns the syslog identifier in use.
///
/// A syslog identifier can be set using [`Subscriber::with_syslog_identifier`].
pub fn syslog_identifier(&self) -> &str {
&self.syslog_identifier
}

/// Sets the syslog facility for this logger.
///
/// The syslog facility comes from the classic syslog interface (`openlog(3)`
/// and `syslog(3)`). In syslog, the facility argument is used to specify
/// what type of program is logging the message. This lets the configuration
/// file specify that messages from different facilities will be handled
/// differently. Systemd exposes it in the `SYSLOG_FACILITY` journal field,
/// and allows filtering log messages by syslog facility with `journalctl
/// --facility`.
///
/// See [Journal Fields](https://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html)
/// and [journalctl](https://www.freedesktop.org/software/systemd/man/journalctl.html)
/// for more information.
///
/// If not set, this field is left out.
pub fn with_syslog_facility(mut self, facility: libc::c_int) -> Self {
self.syslog_facility = Some(facility);
Ralith marked this conversation as resolved.
Show resolved Hide resolved
self
}

/// Returns the syslog facility in use.
Ralith marked this conversation as resolved.
Show resolved Hide resolved
///
/// A syslog facility can be set using [`Subscriber::with_syslog_facility`].
pub fn syslog_facility(&self) -> Option<libc::c_int> {
self.syslog_facility
}

#[cfg(not(unix))]
fn send_payload(&self, _opayload: &[u8]) -> io::Result<()> {
Err(io::Error::new(
Expand Down Expand Up @@ -257,6 +288,11 @@ where
put_field_length_encoded(&mut buf, "SYSLOG_IDENTIFIER", |buf| {
write!(buf, "{}", self.syslog_identifier).unwrap()
});
if let Some(facility) = self.syslog_facility {
// libc definitions are bitshifted left by 3, but journald uses
// values without bitshift.
writeln!(buf, "SYSLOG_FACILITY={}", facility >> 3).unwrap();
Comment on lines +292 to +294
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, this seems like a potential footgun. If you Google around for facility definitions and pass one manually rather than via libc, it would be silently mangled, and passing a value different than what you'd use to filter for it is confusing.

Copy link
Author

Choose a reason for hiding this comment

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

@Ralith Do you have any advice how we could go forward?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, I'm wondering if anyone else has ideas before we set this in stone. Both obvious options have significant drawbacks. How do other journald bindings handle it?

Copy link
Author

Choose a reason for hiding this comment

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

No much luck with my Googling of various implementations:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, would it make more sense to punt on abstracting this entirely, and let people supply arbitrary key/value pairs to be appended globally? Particularly as this seems to be a really niche/archaic parameter.

}

event.record(&mut EventVisitor::new(
&mut buf,
Expand Down