Skip to content

fix: bound the log file and stop the Volume Control tab flooding it - #1722

Open
laurentiu021 wants to merge 1 commit into
mainfrom
fix/bound-the-log-file
Open

fix: bound the log file and stop the Volume Control tab flooding it#1722
laurentiu021 wants to merge 1 commit into
mainfrom
fix/bound-the-log-file

Conversation

@laurentiu021

Copy link
Copy Markdown
Owner

Closes #1645.

The problem, measured

The sink passed no fileSizeLimitBytes and no rollOnFileSizeLimit, so it took Serilog's defaults — 1 GB per file with rolling off — and the only bound on the folder was the 14-file count.

I drove the real sink rather than reasoning from the docs. 4000 lines of ~4 KB each:

files largest
before 1 15 MB and still growing
after 2 exactly 10 MB

That matters because the documented support path is "attach the log" (SUPPORT.md, the bug-report template) — an unattachable file breaks the evidence trail exactly when it's needed. And Debug is a real volume tier here: 290 Log.Debug call sites, some in loops.

The fix

fileSizeLimitBytes: MaxLogFileBytes + rollOnFileSizeLimit: true, keeping the 14-file retention. 10 MB × 14 is a predictable ~140 MB worst case that also respects the low-end laptop this app targets.

One correction to the issue: it placed the fix on a plain WriteTo.File call, but it's actually inside the custom UserPathScrubbingSink's inner logger — that's where it went. MaxLogFileBytes and RetainedFileCount are now named constants so Init and the sink can't drift apart, and the tests assert the same values the app uses.

The flood, stopped at source

AudioMixerService.GetPeak logged every COMException at Debug, and it's driven by the Volume Control tab's 50 ms peak-meter timer — one bad audio session wrote ~20 identical lines a second for as long as the tab stayed open. That both fills the file and pushes out everything worth reading.

Now logged once per session-handle generation, mirroring the existing _routingProbed one-shot pattern, and reset in ReleaseGroups so a genuinely new failure after a device change is still reported.

Tests

Three added to the existing sink suite, which already drives the real private sink type by reflection into a temp directory:

  • TheSizeLimit_IsSmallEnoughToAttachToABugReport — pins the intent against GitHub's 25 MB attachment ceiling, not just the number.
  • TheWholeLogFolder_IsBounded — per-file ceiling × retained count, the predictable worst case the file-count-alone version never gave.
  • TheSink_KeepsEveryFileUnderTheLimit — drives the sink hard and asserts no produced file exceeds the limit. Asserting the constants alone would have passed even with the arguments still missing, which was the actual defect. Uses RollingInterval.Infinite so only the size limit can create a second file — with a daily interval a date change could, and the test would pass for the wrong reason.

Not covered by a test: the GetPeak guard needs real COM audio sessions failing, which a unit test can't produce. Saying so rather than implying coverage.

All four projects rebuild --no-incremental with 0 errors, 0 warnings.

Closes #1645.

The sink passed no fileSizeLimitBytes and no rollOnFileSizeLimit, so it took
Serilog's defaults — 1 GB per file with rolling OFF — and the only bound on the
folder was the 14-FILE count. Measured against the real sink rather than reasoning
from the docs: 4000 lines of ~4 KB produced ONE 15 MB file with no roll. With the
limit and rollOnFileSizeLimit it produces two files, the largest exactly 10 MB.

That matters because the documented support path is "attach the log" (SUPPORT.md and
the bug-report template), so an unattachable file breaks the evidence trail exactly
when it is needed — and Debug is a real volume tier here, 290 Log.Debug call sites,
some in loops. 10 MB x 14 files is a predictable ~140 MB worst case that also
respects the low-end laptop this app targets.

The issue placed the fix on a plain WriteTo.File call; it is actually inside the
custom UserPathScrubbingSink's inner logger, which is where it went. MaxLogFileBytes
and RetainedFileCount are now named constants, so Init and the sink cannot drift
apart and the tests assert the same values the app uses.

Also stopped the flood at source. AudioMixerService.GetPeak logged every COMException
at Debug, and it is driven by the Volume Control tab's 50 ms peak-meter timer — one
bad audio session wrote ~20 identical lines a second for as long as the tab stayed
open, which both fills the file and pushes out everything worth reading. Now logged
once per session-handle generation, mirroring the existing _routingProbed one-shot
pattern, and reset in ReleaseGroups so a genuinely new failure after a device change
is still reported.

Tests: three added to the existing sink suite, which already drives the real private
sink type by reflection into a temp file. Two pin the intent (small enough to attach
to an issue at GitHub's 25 MB ceiling; whole folder bounded) and one drives the sink
hard and asserts no produced file exceeds the limit — asserting the constants alone
would have passed even with the arguments still missing, which was the actual defect.

Not covered by a test: the GetPeak guard needs real COM audio sessions failing, which
a unit test cannot produce. Stated rather than implied.

All four projects rebuild with 0 warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement]: Cross-app - Log files have no size cap and Debug-level logging, so the log the user must attach can grow past uplo...

1 participant