Watcher: Wire up file + Event Log logging on the Windows service path - #68
Merged
Conversation
The service entry point never called the CLI's file-logging setup, so every logger.* call from runtime/uploader/monitor/heartbeat/updater was silently dropped under the SCM — and any crash before SvcDoRun left nothing on disk to triage. Extracts a shared logging_setup module with a rotating file handler and a servicemanager-mirroring Handler, calls both at the top of _run_service_loop, and wraps the dispatcher entry point in a bootstrap try/except that captures pre-dispatcher crashes to service-bootstrap.log. Adds a DATA_HUB_WATCHER_LOG_LEVEL env var so operators can flip a stuck lab PC to DEBUG without redeploying, and documents the LocalSystem log path + win32serviceutil/Event Viewer triage commands in the installing-a-watcher guide. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
wasimxyz
marked this pull request as ready for review
May 13, 2026 21:38
Co-authored-by: Cursor <cursoragent@cursor.com>
…cher Both entry points previously computed DEFAULT_CONFIG_DIR / watcher.log, but `~` resolves against the calling account — so the CLI under the operator wrote to C:\Users\<op>\.data-hub\ while the service under LocalSystem wrote to C:\Windows\System32\config\systemprofile\.data-hub\. The result was two parallel logs that operators had to know about. Introduces WATCHER_LOG_DIR that resolves to C:\ProgramData\DataHubWatcher on Windows (where both accounts have write access by default via the inherited ACL on ProgramData) and to DEFAULT_CONFIG_DIR on non-Windows hosts for dev/test parity. logging_setup, the service-path bootstrap log, and install_service all use the new constant; install_service proactively mkdirs the location under the elevated install shell to avoid a first-writer ACL race. Updates docs across watcher.md, installing-a-watcher.md, and upgrading-the-watcher.md to point at the unified path, and notes that `data-hub-watcher watch` should not run alongside the service since they would race on the same rotating file. Co-authored-by: Cursor <cursoragent@cursor.com>
- Filter SystemExit / KeyboardInterrupt out of the bootstrap log capture so clean upgrade restarts don't append misleading tracebacks to service-bootstrap.log. Extract the dispatcher startup into a testable helper. - Drop the now-dead `sm` parameter from `_repair_upgrade_worker_if_missing` and update its call site + tests. - Tighten `setup_file_logging` idempotency: compare `os.path.abspath`-shaped paths and case-fold only on case-insensitive filesystems (win32, darwin). - Use a trimmed `EVENT_LOG_FORMAT` for the SCM handler since the Windows Event Log already records timestamp + level per entry. - Guard against an empty `%ProgramData%` env var in `_resolve_watcher_log_dir` so the log path never collapses to a relative path. Co-authored-by: Cursor <cursoragent@cursor.com>
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
setup_file_logging()(rotatingwatcher.log, idempotent),attach_servicemanager_handler(sm)(mirrorslogger.*records toLogErrorMsg/LogWarningMsg/LogInfoMsgwith re-entrancy guard, 30 KB truncation under the 32 KB Event Log cap, and never-raise semantics), and aDATA_HUB_WATCHER_LOG_LEVELenv override so operators can flip a stuck lab PC toDEBUGwithout redeploying.WATCHER_LOG_DIRin watcher/src/data_hub_watcher/constants.py that resolves toC:\ProgramData\DataHubWatcher\on Windows (a location both the operator user and LocalSystem can write to by default via the inherited ACL onC:\ProgramData) and to~/.data-hub/on macOS/Linux for dev parity. Both the CLIwatchcommand and the Windows service write to the samewatcher.logandservice-bootstrap.logfiles now, so a singleGet-Content -Waitcovers all entry points._run_service_loop(before the registry read, env load, or API health check) and converts every baresm.LogInfoMsg/LogWarningMsg/LogErrorMsgcall tologger.*. The Handler routes them back to the Event Log AND records them inwatcher.log, eliminating the silent-drop oflogger.*calls fromruntime.py,uploader.py,monitor.py,heartbeat.py, andupdater.pywhen running under the SCM.install_serviceproactivelymkdirsWATCHER_LOG_DIRunder the elevated install shell to avoid a first-writer ACL race.__main__dispatcher block in a try/except that captures phase A/B crashes (broken venv, missingpywin32, bytecode issues — anything beforeSvcDoRunruns) intoC:\ProgramData\DataHubWatcher\service-bootstrap.log. Original exception still propagates to the SCM._setup_file_loggingnow delegates to the shared helper so CLI and service can't drift on log location, rotation policy, or format.C:\ProgramData\DataHubWatcher\path, note the single-writer caveat (don't rundata-hub-watcher watchwhile the service is up), and document theDATA_HUB_WATCHER_LOG_LEVELknob plus the two read-only triage commands (Get-WinEventandpython -m win32serviceutil debug DataHubWatcher).Why
Under the current code, three distinct service-startup failure phases each log to a different place — or nowhere:
python.exe -m data_hub_watcher.serviceStartServiceCtrlDispatcherPython Service, otherwise nothingSvcDoRun->_run_service_loopsm.LogErrorMsgonly. Anylogger.*call from library code was dropped on the floor.The troubleshooting guide already promised operators a
~/.data-hub/watcher.logfile, but_setup_file_loggingwas only ever called from the CLIwatchcommand. And even if it had been called from the service path, the LocalSystem~expanded toC:\Windows\System32\config\systemprofile\.data-hub\— a path most operators don't know to look in. This PR closes both gaps so a service that crashes on start always leaves a usable trail in a discoverable location.Test plan
uv run pytest watcher/tests -q— 386 unit tests pass (332 before, +21 new intest_logging_setup.py, the rest existing — no regressions).make check-all— ruff, pyright, prettier, eslint, tsc all green.data-hub-watcher service reinstalland confirmC:\ProgramData\DataHubWatcher\watcher.logaccumulates records on a successful boot.pywin32site-packages), confirmservice-bootstrap.logcaptures the import-time traceback in the same directory.DATA_HUB_WATCHER_LOG_LEVEL=DEBUGto the service's.envfile, restart the service, and confirm DEBUG records appear in both the file log and Event Viewer.Get-Content C:\ProgramData\DataHubWatcher\watcher.log -Tail 20 -Waitwhile the service is writing.