feat(logger): disk-backed log store for bounded CLI memory (PER-7809)#2200
Open
feat(logger): disk-backed log store for bounded CLI memory (PER-7809)#2200
Conversation
pranavz28
added a commit
that referenced
this pull request
Apr 29, 2026
P1 fixes: - Track evicted snapshot keys in `evicted` Set so late entries that land on disk after evictSnapshot don't repopulate the cache (defeats the bounded-memory promise in defer mode). - Per-pid subdir under percy-logs/ instead of clearing the shared percy-logs/ dir at init — concurrent percy processes (CI matrix, parallel workers, npx) no longer clobber each other's JSONLs. - Remove unused `messages` Set-shaped getter; api.js + percy.test.js use query() directly. Also drops the helpers.js force-memory-mode flag so the existing 60-case suite now exercises the disk path end-to-end. P2 fixes: - Drop the write-only snapshotIndex Map (was a per-entry write with no reader). - Consolidate query() and _readAllFromDisk() into one _readDiskFiltered() helper; query() = flush + filter, fallback path = filter all. - Add SIGINT/SIGTERM/uncaughtException hooks (via _installSignalHooks) so Ctrl-C / runner kill don't orphan the JSONL or lose buffered logs. - Bump filename randomness from randomBytes(2) to randomBytes(8). - Preserve meta.snapshot when stringify falls back for circular meta so the entry still routes through snapshotLogs(). Coverage: 100% on logger.js / index.js / utils.js / timing.js. 62/62 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Replaces @percy/logger's unbounded `messages` Set with a JSONL-backed
hybrid store that keeps resident memory bounded across long builds
(10k snapshots, 8-hour deferUploads runs) while preserving byte-for-byte
parity with master's `/logs` upload payload and per-snapshot log resources.
Design
------
- writes go through a 500-entry / 100ms buffer flushed via
fs.appendFileSync to ${tmpdir}/percy-logs/<pid>/<ts>-<rand>.jsonl
- snapshotLogs(meta) reads the disk delta into a bounded `cache` keyed by
snapshot meta; evictSnapshot drops the cache entry once the snapshot's
upload is complete; late entries are allowed to repopulate (retry-safe)
- query(filter) streams the JSONL once per call (chunked 64KB read);
in-memory mode preserves master's identity-mutation contract that
redactSecrets relies on
- disk-init failures, mid-build appendFileSync failures, and the
PERCY_LOGS_IN_MEMORY=1 rollback all flip to an unbounded in-memory Set
(master parity) — including replaying entries already on disk so the
upload still includes them
- circular meta is sanitized via JSON.stringify try/catch, but
meta.snapshot is preserved so the entry still routes via snapshotLogs
- exit cleanup uses a process-scoped Set on `process[Symbol.for(...)]`
shared across module copies; supports multiple live instances and
unlinks every active disk file on `exit` / `beforeExit`
- per-pid subdir prevents concurrent percy processes (CI matrix, parallel
workers, npx invocations) from clobbering each other's files; cleanup
best-effort rmdirs the subdir so long-lived runners don't accumulate
Wiring
------
- packages/core/src/discovery.js — uses snapshotLogs/evictSnapshot for
per-snapshot log resources
- packages/core/src/api.js — /test/logs and the test-mode reset path now
use logger.query() / logger.instance.reset()
- packages/core/test/helpers/index.js — defaults setupTest to memory mode
(master parity) so downstream tests using mockfs don't flake against
the disk path's live volume
Tests
-----
- 68 specs, 100% statements/branches/functions/lines on logger.js
- the existing 37-case logger suite runs under the disk path by default
(no PERCY_LOGS_IN_MEMORY set in helpers.mock); 25+ new specs in
describe('disk-backed storage') cover round-trip, snapshotLogs / evict
retry semantics, fallback after appendFileSync / mkdirSync failures,
the 100ms timer, the 500-entry size cap, per-pid subdir, the
Symbol.for latch, multi-instance cleanup, rmdir best-effort, and
circular-meta snapshot preservation
- cli-exec / cli-snapshot / cli-build / cli-doctor / cli-upload /
cli-command / cli / core / config / client / env / monitoring /
webdriver-utils all green locally on Node 14
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
pranavz28
force-pushed
the
feat/per-7809-disk-logs-minimal
branch
from
9f3539b to
3660a7a
Compare
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
1 participant
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.
Summary
Replace the unbounded
messages = new Set()in@percy/loggerwith a disk-backed JSONL store and a lazily-built in-memory cache for snapshot-tagged entries. CLI memory stays bounded across long debug-heavy builds (8h × 10k snapshots) without changing the/logsupload payload or per-snapshot log resource bytes.Jira: PER-7809
Approach
${TMPDIR}/percy-logs/<pid>-<ts>-<r>.jsonlin batches of 500 entries or every 100 ms — so disk syscalls stay at ~10/sec regardless of log volume.snapshotIndexholds offsets, not entries. ~16 bytes per tagged entry → ~5 MB worst case for an 8-hour 10k-snapshot debug build.cacheis built lazily on firstsnapshotLogs(...)call by reading the disk delta since the previous call. In normal mode the cache holds at most one snapshot's worth (evictSnapshotruns immediately after each snapshot uploads). In defer mode the cache fills at end-of-build and drains as snapshots upload sequentially.query(filter)is sync, flushes the buffer, chunked-reads the disk in 64 KB chunks, JSON-parses linewise, filters. Used twice at end of build (checkForNoSnapshotCommandError,sendBuildLogs).redactSecretslives in@percy/core/src/utils.jsand runs only on ci-logs at upload. Disk content matches master's in-memory shape.ENOSPC, RO fs, unwritabletmpdir, orPERCY_LOGS_IN_MEMORY=1) transparently falls back to master's unbounded in-memorySet. Build continues either way.Parity guarantee
A diff of the
/logsHTTP payload and per-snapshot log resource bytes between a build run on master vs this branch should be empty:{ debug, level, message, meta, timestamp, error }shape.JSON.stringifysemantics (entries round-trip through disk before upload, but the shape is preserved for everything that lands in real Percy log calls).What changed
packages/logger/src/logger.jssnapshotLogs,evictSnapshot,reset. Keepmessagesas a back-compat getter.packages/logger/src/index.jspackages/core/src/discovery.jslogger.snapshotLogs(meta)+logger.evictSnapshot(meta).packages/core/src/api.js/test/api/resetcallslogger.instance.reset()instead of mutating the (now-derived)messagesSet.packages/logger/test/helpers.jsPERCY_LOGS_IN_MEMORY=1) by default; reset properly tears down disk state.packages/logger/test/logger.test.jsmeta, in-memory rollback).packages/core/test/api.test.jsmessages.clear()→reset()change.Test plan
Local
@percy/loggertests: 45/45 passing (37 existing + 8 new disk-mode).yarn lintclean.@percy/coretests: running on CI (local environment lacks built@percy/dombundle for many discovery tests).End-to-end (parity)
Each scenario will be run twice — once against
@percy/[email protected](latest published) and once against this branch — with logs compared via percy-support-cli, and RSS/disk/runtime monitored during the run:PERCY_DEBUG=*(chatty logs / hot path).percy uploaddefer mode (~20 image snapshots) — thedeferUploadsgotcha.PERCY_DEBUG=*— stress combo.PERCY_LOGS_IN_MEMORY=1rollback — confirms master parity.For each: ✅ logs match / ❌ diff lines, RSS-master vs RSS-branch (peak), runtime-master vs runtime-branch, disk file present-during / absent-after.
Known follow-ups (not in this PR)
/logsHTTP upload (would eliminate the end-of-build memory spike).bench-logger.jsinto CI (perf gates).🤖 Generated with Claude Opus 4.7 (1M context) via Claude Code