fix(logs): isolate non-default data-dir logs to <data-dir>/logs#660
Closed
Dumbris wants to merge 1 commit into
Closed
fix(logs): isolate non-default data-dir logs to <data-dir>/logs#660Dumbris wants to merge 1 commit into
Dumbris wants to merge 1 commit into
Conversation
The phantom "core restarts every ~10s" reports (MCP-2250, split from MCP-2207) were a logging artifact, not a real restart loop. `serve` enables file logging by default and derives the log dir purely from $HOME (internal/logs.GetLogDir), ignoring --data-dir. So every `mcpproxy serve` run — Go integration tests (temp data-dirs), e2e scripts, FE/QA Playwright harnesses, AND the real prod core — appended to the same ~/Library/Logs/mcpproxy/main.log. A reader of that shared file saw dozens of fast, interleaved boots and concluded the core was restarting every ~10s. Forensics on a 72-min window show only 2 real prod-core boots (data_dir ~/.mcpproxy) among ~42, and every shutdown was reason="signal" (no panics/self-restarts). Fix: when the resolved data dir is non-default and no explicit --log-dir was given, co-locate logs under <data-dir>/logs. The default data dir (~/.mcpproxy) is unchanged and still logs to the OS-standard location, so the tray and documented paths keep working. An explicit --log-dir still wins. - Add resolveServeLogDir helper (cmd/mcpproxy/logdir.go) with unit tests covering the full precedence: --log-dir > config log_dir > <data-dir>/logs (non-default) > OS-standard. - Wire it into runServer (cmd/mcpproxy/main.go). - Update TestE2E_MCPProxyWithLogging to assert the binary writes to <data-dir>/logs and does NOT leak into the shared OS-standard dir. - Document the resolution order in docs/logging.md. Related MCP-2250 Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 27498193228 --repo smart-mcp-proxy/mcpproxy-go
|
Member
Author
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
2 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.
Summary
Route a non-default data-dir's logs to
<data-dir>/logsinstead of the shared OS-standard location (~/Library/Logs/mcpproxy/on macOS). This keeps Go test runs, e2e scripts, and FE/QA Playwright harnesses from polluting the productionmain.log.Root cause
Forensics in MCP-2249: the phantom "core restarts every ~10s" was multiple test instances interleaving re-init lines in one shared log file. Every
mcpproxy servewith a custom--data-dirwrote to the same~/Library/Logs/mcpproxy/main.logbecause the log dir was derived purely from$HOME.Design
Precedence (highest to lowest):
--log-dirflag — always winslogging.log_dirin config<data-dir>/logs— only when data-dir is not the default (~/.mcpproxy)Default data dir (
~/.mcpproxy) is unaffected — it still logs to the OS-standard location so the tray and documented paths keep working.Files changed
cmd/mcpproxy/logdir.go— new:resolveServeLogDir(),defaultDataDirPath(),sameDir()cmd/mcpproxy/logdir_test.go— new: 7 table-driven tests covering every branchcmd/mcpproxy/main.go— replaces inlineif cmdLogDir != ""override withresolveServeLogDir()docs/logging.md— documents the new "Custom data directory" log locationinternal/logs/e2e_test.go— updates e2e test to assert co-located path; adds negative assertion that OS-standard path is NOT writtenVerification
go test ./cmd/mcpproxy/... -race -run TestResolveServeLogDir— 7/7 PASSmake build— compiles cleanRelated #2255