Skip to content

broker: preserve log levels in system instance #6918

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

Merged
merged 1 commit into from
Jul 17, 2025

Conversation

garlick
Copy link
Member

@garlick garlick commented Jul 17, 2025

Problem: all Flux logs go to the systemd journal (via stderr) at LOG_INFO level, which means they cannot be usefully filtered using the journalctl(1) -p option.

When log-stderr-mode=local is selected, use a prefix that can be interpreted by systemd instead of embedding the level name in the log message. For example:

$ flux logger --severity=err help

before:

logger.err[0]: help

after:

<3>logger[0]: help

Edit: as a bonus, journalctl output for flux is now properly colorized by severity.

Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo
Copy link
Contributor

grondo commented Jul 17, 2025

Is there a reason to replace the log level string with the prefix, instead of just adding the prefix for systemd's benefit?
AFAIK, the <N> prefix is dropped when log messages go from the systemd journal to /var/log/messages or other log message destinations. When debugging, I have found it useful to grep these files for things like broker.err to find error messages when I don't know what I'm looking for. Unless I've missed something, that will no longer be possible after this change?

@garlick
Copy link
Member Author

garlick commented Jul 17, 2025

Oh, you are right, for some reason I was assuming the syslog severity levels landed in our log files but they do not. (I guess I also assumed that if systemd knows the log level, it would preserve it when forwarding to syslog, although I didn't check that).

Sure, I will add that back in.

Problem: all Flux logs for system instance brokers go to the
systemd journal (via stderr) at LOG_INFO level, which means
they cannot be usefully filtered using the journalctl -p option.

When log-stderr-mode=local is selected, add a <level> prefix
that can be interpreted by systemd.  For example:

$ flux logger --severity=err help

before:
  logger.err[0]: help

after:
  <3>logger.err[0]: help
@garlick
Copy link
Member Author

garlick commented Jul 17, 2025

Thanks, I'll set MWP

@mergify mergify bot merged commit ddc51a5 into flux-framework:master Jul 17, 2025
59 of 60 checks passed
Copy link

codecov bot commented Jul 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.91%. Comparing base (d4929cf) to head (1824d14).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6918      +/-   ##
==========================================
+ Coverage   83.90%   83.91%   +0.01%     
==========================================
  Files         540      540              
  Lines       90525    90526       +1     
==========================================
+ Hits        75957    75967      +10     
+ Misses      14568    14559       -9     
Files with missing lines Coverage Δ
src/broker/log.c 76.02% <100.00%> (+0.07%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@garlick garlick deleted the log_levels branch July 17, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants