fix(logger): writer level filtering - #1761
Open
ajw221 wants to merge 4 commits into
Open
Conversation
ajw221
requested review from
InKryption and
dnut
and removed request for
InKryption
July 24, 2026 20:08
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
InKryption
suggested changes
Jul 28, 2026
InKryption
left a comment
InKryption
left a comment
Contributor
There was a problem hiding this comment.
Just a couple small details, otherwise lgtm.
jbuckmccready
force-pushed
the
jbuckmccready/logging-improvements
branch
2 times, most recently
from
July 30, 2026 15:21
841ddd5 to
bddebca
Compare
- Compute the most verbose level across all log filters at init and store on Region.Info.max_log_level - Loggers carry max_level (preserved through withScope/from) and bail early in Entry.print when a message exceeds it - Avoids double-rendering (length computation + write) and swap buffer writes for messages that streamLogs would drop anyway - Add log.maxLevelEncoded helper; returns .trace on empty/malformed input so the telemetry service still surfaces the diagnostic
- Verify logf drops entries more verbose than max_level and writes matching entries whole - Confirm max_level propagates through withScope/from and that acquireLogger picks up the region's filter maximum - Exercise maxLevelEncoded on empty, default, multi-filter, and truncated inputs, plus a bound check against findClosestFilter for every named and fall-through (service, scope) pair
- Add Filter.Iterator wrapping the fixed-reader/takeStruct/ getFilterFromFixedReader pattern with a single InvalidFilter error - Use it in maxLevelEncoded, its tests, and services/telemetry.zig filter decoding to eliminate duplication
ajw221
force-pushed
the
adamw/fix-logger-max-level-bug
branch
from
July 30, 2026 16:04
0c39c7f to
f82806c
Compare
…yout inputs - Drop default `.trace` on Region.Info.max_log_level and Logger.max_level so callers must set them, preventing silent regressions where a logger fails to consult the region's filters - Extract Region.Info.Lengths struct capturing the only fields that affect region layout, and make regionSize take Lengths rather than a full Info - Update noop logger and TestLogStore.logger to set max_level explicitly - Reword the acquireLogger test comment now that the default is gone
ajw221
force-pushed
the
adamw/fix-logger-max-level-bug
branch
from
July 30, 2026 16:14
f82806c to
107a8c9
Compare
InKryption
approved these changes
Jul 30, 2026
jbuckmccready
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
streamLogsis filtering the max log level, which has the side effect of potentially making the swap buffer running out of space if a log was being written, but should have been filtered out for the actual max log level provided.Implementation
Filterwhile loop logic into new Iterator typeRamifications
onDiscoveredValue, which was not outputted at all during the entire run.Tests