feat: use systemd-journal-logger when running as systemd service - #437
feat: use systemd-journal-logger when running as systemd service#437erictapen wants to merge 2 commits into
Conversation
This produces nicer logging output with colors for log levels and no duplicate time stamps.
|
Thanks for the PR, this will be an improvement in our logging stack. |
pando85
left a comment
There was a problem hiding this comment.
Thanks for this — I like the direction and the native journal integration is a real improvement. I’d like to get this merged; there are just a few small compatibility details I’d like us to keep from the current logging behavior:
-
Preserve
PASSLESS_LOG_LEVELwhen journald is active. Right now the journal branch bypasses theenv_loggerfilter entirely, so values such aswarnor module-specific filters stop working under systemd. We can keep the nativeJournalLogand use anenv_logger::Loggerbuilt withBuilder::build()purely as the filtering layer in front of it (it implementslog::Logand can be nested), so no extra dependency should be necessary. -
Fall back instead of panicking if the native journal socket is unavailable.
connected_to_journal()only checksJOURNAL_STREAMagainst stderr;JournalLog::new()separately connects to the native journal socket and can fail, for example in some container/root-image setups. IfJournalLog::new()fails, falling back to the existing stderrenv_loggerpath would keep Passless usable. A shorteprintln!explaining the fallback would be useful too. -
Keep the agent journal identifier.
contrib/systemd/passless-agent.serviceintentionally usesSyslogIdentifier=passless-agent, while the new native logger always writesSYSLOG_IDENTIFIER=passless. Because native journal fields bypass systemd’s stdout/stderrSyslogIdentifier=handling, agent logs would becomepassless. I suggest an env override such asPASSLESS_SYSLOG_IDENTIFIER(defaulting topassless) and settingEnvironment=PASSLESS_SYSLOG_IDENTIFIER=passless-agentin the agent unit. The normal service can keep the default.
PASSLESS_LOG_STYLE can remain relevant only to the stderr fallback; native journald already carries the priority structurally, so that part does not need to be emulated.
There is also an unrelated getrandom lockfile bump through tempfile; nice to avoid if Cargo lets us, but I would not block the PR on that.
Thanks again for the contribution — the core approach is good; these are mostly Passless-specific integration details rather than a change in direction.
Feel free to squash this when merging. |
This produces nicer logging output with colors for log levels and no duplicate time stamps.
Currently it looks like this: