perf(observability): opt-in DAP trace + proxy logger honors effective log level - #410
Merged
Merged
Conversation
… log level Two always-on observability defaults made every debug session pay for diagnostics almost nobody reads (issue #403): - Per-frame DAP tracing (synchronous appendFileSync of every frame, both directions, uncapped) is now opt-in: DAP_TRACE=1 enables the standard per-session dap-trace-<sessionId>.ndjson, an explicit DAP_TRACE_FILE is honored as-is, default is off. When enabled the trace is capped at 50 MB (one truncation marker, then silence) matching the main logger's maxsize. Sync writes are retained deliberately: tracing is now an opt-in crash- triage tool, and a buffered queue would lose the final frames exactly when the process dies. - The per-session proxy-<sessionId>.log level is no longer hardcoded to 'debug': the effective level (CLI --log-level / DEBUG_MCP_LOG_LEVEL) rides the init payload (ProxyConfig.logLevel -> ProxyInitPayload.logLevel -> ILoggerFactory level param). Legacy parents that send no level keep the historical debug default. Stale session-run log sweeping is deferred to the #399 startup-janitor refactor (session logs live under os.tmpdir()/debug-mcp-server/sessions, out of reach of the flat logger sweeper). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Fixes #403
Two always-on observability defaults in the proxy worker were heavyweight for every session (evidence in the issue: a 658 MB dev
logs/directory):1. Per-frame DAP trace is now opt-in
The default trace-file factory set
DAP_TRACE_FILEfor every session, andMinimalDapClientthenappendFileSync'd every frame in both directions — synchronous I/O on the proxy's event loop in the hot path of every step/variables/stackTrace exchange, into an uncapped ndjson file.Now:
DAP_TRACE=1(ortrue) enables the standard per-sessiondap-trace-<sessionId>.ndjson; an explicitDAP_TRACE_FILE(inherited via the spawn env) is honored as-is, without renaming.maxsize): one truncation marker line, then silence for the client's lifetime. Cap injectable for tests (traceMaxBytes).hooks.createTraceFiletest seam is unchanged.2. Proxy logger honors the effective log level
proxy-<sessionId>.logwas hardcoded to'debug', ignoring--log-levelandDEBUG_MCP_LOG_LEVEL. The effective level now rides the init payload:SessionManager(reads the winston logger's.level, which already resolves CLI flag → env →info) →ProxyConfig.logLevel→ProxyInitPayload.logLevel→ILoggerFactory(sessionId, logDir, level). Legacy parents that send no level keep the historicaldebugdefault, so old payloads behave identically.Sibling noted while here (not fixed):
adapter-registry.ts:397also hardcodeslogLevel: 'debug'when building per-adapter dependencies for dynamically loaded adapters — same bug class, no plumbing available there yet.Deferred to #399's startup-janitor refactor
Sweeping stale per-session run dirs (
proxy-*.log/dap-trace-*.ndjson): session logs live underos.tmpdir()/debug-mcp-server/sessions/<id>/run-<ts>/, out of reach of the flatlogger.tssweeper — a session-dir sweep fits naturally next to the orphan reapers being reworked in #399.Docs
CLAUDE.mddebugging scenarios +docs/development/setup-guide.mdenv table documentDAP_TRACE/DAP_TRACE_FILE.Tests (TDD, all watched fail first)
DAP_TRACE, on withDAP_TRACE=1, explicitDAP_TRACE_FILEhonored unrenamed;logLevelpassed to the logger factory.logLevelincluded in the init command.startCalls[0].logLevel).loggerFactorypasses the level tocreateLogger; legacy debug default without one.npm testgreen (260 files / 3959 tests),npm run lintclean.🤖 Generated with Claude Code