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

Add new --level-prefix option #646

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Conversation

smcv
Copy link
Collaborator

@smcv smcv commented Jul 24, 2024

  • test-run: Assert that repeating --chdir logs a warning

    This is the case since commit 0d369cd "main: Warn when
    non-repeatable options are repeated".

  • utils: Put nearly all diagnostic output through a common log function

    This takes a syslog-style severity level, allowing us to act based on
    the severity.

    Take the opportunity to make the __debug__ macro (which normally expands
    to nothing, but can be enabled by changing a #if 0 to #if 1) less
    weird and easier to use, by taking it out of the reserved-for-the-compiler
    namespace, adding a newline automatically, and not requiring nested
    parentheses.

  • Add new --level-prefix option

    This prepends a syslog-style priority level such as <3> to each line of
    diagnostic output, so that the diagnostic output can be parsed by
    tools like systemd-cat --level-prefix=1. A future version of Steam's
    pressure-vessel is likely to use this to make warnings and fatal errors
    from bubblewrap more visible.

cc @refi64 @RyuzakiKK

@smcv smcv marked this pull request as ready for review July 24, 2024 17:55
utils.h Outdated Show resolved Hide resolved
utils.h Show resolved Hide resolved
@smcv smcv force-pushed the wip/smcv/level-prefix branch from db40f0b to a58b8b6 Compare July 25, 2024 17:39
@smcv smcv requested a review from refi64 July 29, 2024 10:11
@smcv smcv force-pushed the wip/smcv/level-prefix branch 2 times, most recently from f71b028 to ff6baae Compare August 1, 2024 12:37
@smcv
Copy link
Collaborator Author

smcv commented Aug 1, 2024

Updated: while reviewing a similar change in libcapsule, @refi64 pointed out that I'd described the prefixes as "syslog-style" here, but not in libcapsule. In fact saying "syslog-style" is misleading, because https://datatracker.ietf.org/doc/html/rfc5424#section-6.2 describes a prefix that encodes both the severity and the facility, whereas here we're using only the severity, in order to be compatible with systemd-cat(1). Consistently say "severity" in preference to "priority" to reflect this.

(Documentation updates only; the only code change is to rename a level function parameter to severity.)

@smcv smcv requested a review from refi64 August 1, 2024 18:13
@smcv
Copy link
Collaborator Author

smcv commented Aug 5, 2024

Any thoughts on this from other maintainers? My colleague @refi64 already provided some useful feedback, which I've addressed.

@RyuzakiKK
Copy link
Contributor

I'm not a maintainer, but FWIW this looks good to me as well.

smcv added 3 commits August 15, 2024 15:00
This is the case since commit 0d369cd "main: Warn when
non-repeatable options are repeated".

Signed-off-by: Simon McVittie <[email protected]>
This takes a syslog-style severity level, allowing a larger program
that runs bwrap and reads a pipe from its stderr to filter or highlight
messages  based on the severity.

Take the opportunity to make the __debug__ macro (which normally expands
to nothing, but can be enabled by changing a `#if 0` to `#if 1`) less
weird and easier to use, by taking it out of the reserved-for-the-compiler
namespace, adding a newline automatically, and not requiring nested
parentheses.

Signed-off-by: Simon McVittie <[email protected]>
This prepends a severity level such as <3> to each line of diagnostic
output, with numeric severity levels taken from matching syslog(3)
(such as LOG_ERR = 3), so that the diagnostic output can be parsed by
tools like `logger --prio-prefix` and `systemd-cat --level-prefix=1`
that support that encoding.

The facility (LOG_USER, etc.) is not included, since it makes little
sense to vary on a per-message basis. logger(1) supports prefixes
with or without a facility, and systemd-cat(1) only supports prefixes
without a facility, so this is compatible with both.

A future version of Steam's pressure-vessel is likely to use this to
make warnings and fatal errors from bubblewrap more visible.

Signed-off-by: Simon McVittie <[email protected]>
@smcv smcv force-pushed the wip/smcv/level-prefix branch from ff6baae to 89ae6b1 Compare August 15, 2024 14:03
@smcv smcv merged commit 2cca54f into containers:main Sep 30, 2024
5 checks passed
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