Skip to content

Allow skipping of appenders when demoting log level via middleware#398

Closed
DerGuteMoritz wants to merge 1 commit intotaoensso:masterfrom
bevuta:allow-skipping-appenders-on-level-change-via-middleware
Closed

Allow skipping of appenders when demoting log level via middleware#398
DerGuteMoritz wants to merge 1 commit intotaoensso:masterfrom
bevuta:allow-skipping-appenders-on-level-change-via-middleware

Conversation

@DerGuteMoritz
Copy link
Contributor

@DerGuteMoritz DerGuteMoritz commented Aug 12, 2025

When a middleware demotes a log's :level below an appender's :min-level, that appender will now be skipped. This behavior is consistent with e.g. :msg_ which was already exposed to the changed log level.

Review notes

  • The patch also affects the calculation of :hash_. I'm not 100% sure whether this makes sense.
  • The test assertion violates the formatting symmetry relative to the other assertions. Given how much longer it is, it felt a bit off to re-indent all others to match but if you prefer it, I'm happy to adjust.
  • Not sure whether the levels test is the best place for the assertion but it seemed like a good fit to me.

When a middleware demotes a log's `:level` below an appender's `:min-level`, that appender will now
be skipped. This behavior is consistent with e.g. `:msg_` which was already exposed to the changed
log level.
@ptaoussanis
Copy link
Member

@DerGuteMoritz Nice work, thank you for this Moritz! 🙏🎸 Merging manually now.

I plan to include this in a new release later today or tomorrow 👍

ptaoussanis pushed a commit that referenced this pull request Aug 21, 2025
…Moritz)

Before this commit:

  If middleware changed a log call's level (e.g. from `:info` to `:debug`),
  appenders would still filter based on the initial level (`:info`).

After this commit:

  If middleware changes a log call's level (e.g. from `:info` to `:debug`),
  appenders now filter based on the updated level (`:debug`).

  This behavior is consistent with e.g. `:msg_` which was already exposed to
  the updated level.
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.

2 participants