feat(logging): rotate embedded core + shell logs to ~/.openhuman/logs#1278
Open
senamakel wants to merge 1 commit intotinyhumansai:mainfrom
Open
feat(logging): rotate embedded core + shell logs to ~/.openhuman/logs#1278senamakel wants to merge 1 commit intotinyhumansai:mainfrom
senamakel wants to merge 1 commit intotinyhumansai:mainfrom
Conversation
The Tauri shell previously initialized `env_logger` to stderr and the
embedded core never installed a tracing subscriber at all (only the CLI
path called `init_for_cli_run`). Packaged GUI builds therefore had no
visible diagnostics when users hit issues like "stuck on Initializing
OpenHuman..." — there was nothing for support to ask them to share.
Add `core::logging::init_for_embedded(data_dir, verbose)` which:
* installs the existing `CleanCliFormat` against both stderr (ANSI when
attached to a TTY) and a non-blocking, daily-rotated file appender at
`<data_dir>/logs/openhuman-YYYY-MM-DD.log` (7-day retention),
* keeps the Sentry tracing layer + `EnvFilter` honoring `RUST_LOG`,
* bridges the shell's `log::*` calls via `tracing_log::LogTracer` so
every shell + core event lands in one file.
Wire it from `app/src-tauri/src/file_logging.rs` (called early in
`run()`, before CEF preflight) and surface the folder in Settings →
Developer Options via two new commands `logs_folder_path` /
`reveal_logs_folder` so users can grab today's log and send it.
Refactor `init_for_cli_run` to share `seed_rust_log`,
`build_env_filter`, and `sentry_tracing_layer` with the embedded path —
single source of truth for the formatter + filter.
Contributor
📝 WalkthroughWalkthroughThis PR introduces a file-based logging system that captures daily-rotated logs to disk in both CLI and embedded (Tauri) contexts. It adds a tracing-appender dependency, refactors the core logging initialization to support file output with constraint-based filtering, wires a new Tauri module to expose log directory access, and adds UI in the developer options panel to view and reveal the logs folder. ChangesFile-Based Logging with Tauri Integration
Sequence DiagramsequenceDiagram
participant Frontend as Developer Panel
participant Tauri as Tauri Shell
participant Core as Embedded Core
participant FileSystem as File System
rect rgba(100, 200, 100, 0.5)
note over Frontend,FileSystem: Fetch Logs Folder Path
Frontend->>Tauri: invoke("logs_folder_path")
Tauri->>Core: query log directory
Core->>FileSystem: resolve logs path
FileSystem-->>Core: path
Core-->>Tauri: logs path
Tauri-->>Frontend: path string
Frontend->>Frontend: display path
end
rect rgba(100, 150, 200, 0.5)
note over Frontend,FileSystem: Open Logs Folder
Frontend->>Tauri: invoke("reveal_logs_folder")
Tauri->>FileSystem: spawn platform cmd<br/>(open/explorer/xdg-open)
FileSystem-->>Tauri: folder open
Tauri-->>Frontend: success
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/logging.rs (1)
241-241: 💤 Low valueMinor: duplicate constraint parsing.
parse_log_file_constraints()is called twice (lines 241 and 264), each creating a separateVec<String>. Since this runs once at startup, the overhead is negligible. If you want to tighten it, parse once and clone for the second filter closure.♻️ Optional: parse constraints once
appender.map(|appender| { let (writer, guard) = tracing_appender::non_blocking(appender); let _ = FILE_GUARD.set(guard); let _ = LOG_DIR.set(logs_dir.clone()); - let constraints = parse_log_file_constraints(); + let file_constraints = constraints.clone(); tracing_subscriber::fmt::layer() .with_ansi(false) .event_format(CleanCliFormat) .with_writer(writer) .with_filter(tracing_subscriber::filter::filter_fn(move |meta| { - event_matches_file_constraints(meta, &constraints) + event_matches_file_constraints(meta, &file_constraints) })) }) } // ... earlier in the function, before the match: + let constraints = parse_log_file_constraints();Also applies to: 264-264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/logging.rs` at line 241, parse_log_file_constraints() is being called twice producing two Vec<String>; call it once, store the result in a local variable (e.g. let constraints = parse_log_file_constraints()), then reuse it by cloning when needed for the second filter/closure (e.g. let constraints_for_other = constraints.clone()) so both closures use the same parsed data; update the two places that currently call parse_log_file_constraints() to use the stored variable and its clone instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/src/file_logging.rs`:
- Around line 39-41: The fallback to PathBuf::from(".openhuman") in the call to
default_root_openhuman_dir() can put logs in a relative, unpredictable location;
change the fallback to a robust absolute location (for example use
std::env::temp_dir().join("openhuman") or another absolute directory) and emit a
warning when default_root_openhuman_dir() fails so the user/maintainer is aware;
update the call site that uses default_root_openhuman_dir() and the surrounding
error handling to construct an absolute fallback path and call the logger (or
e.g., warn!()) with context including that the home-dir lookup failed and which
fallback was chosen.
---
Nitpick comments:
In `@src/core/logging.rs`:
- Line 241: parse_log_file_constraints() is being called twice producing two
Vec<String>; call it once, store the result in a local variable (e.g. let
constraints = parse_log_file_constraints()), then reuse it by cloning when
needed for the second filter/closure (e.g. let constraints_for_other =
constraints.clone()) so both closures use the same parsed data; update the two
places that currently call parse_log_file_constraints() to use the stored
variable and its clone instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f71d657a-3409-4e6a-b44c-77d3c5ee323e
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
Cargo.tomlapp/src-tauri/permissions/allow-core-process.tomlapp/src-tauri/src/file_logging.rsapp/src-tauri/src/lib.rsapp/src/components/settings/panels/DeveloperOptionsPanel.tsxsrc/core/logging.rs
Comment on lines
+39
to
+41
| openhuman_core::openhuman::config::default_root_openhuman_dir() | ||
| .unwrap_or_else(|_| PathBuf::from(".openhuman")) | ||
| } |
Contributor
There was a problem hiding this comment.
Fallback to relative path ".openhuman" may resolve unexpectedly.
If default_root_openhuman_dir() fails (unlikely but possible if dirs::home_dir() returns None), the fallback PathBuf::from(".openhuman") resolves relative to the current working directory — which varies depending on how the app is launched. Logs would land in an unexpected location.
Consider logging a warning or using a more robust fallback (e.g., temp dir):
🛡️ Suggested fix
openhuman_core::openhuman::config::default_root_openhuman_dir()
- .unwrap_or_else(|_| PathBuf::from(".openhuman"))
+ .unwrap_or_else(|err| {
+ eprintln!("[file_logging] default_root_openhuman_dir failed: {err}; using temp dir");
+ std::env::temp_dir().join("openhuman")
+ })
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src-tauri/src/file_logging.rs` around lines 39 - 41, The fallback to
PathBuf::from(".openhuman") in the call to default_root_openhuman_dir() can put
logs in a relative, unpredictable location; change the fallback to a robust
absolute location (for example use std::env::temp_dir().join("openhuman") or
another absolute directory) and emit a warning when default_root_openhuman_dir()
fails so the user/maintainer is aware; update the call site that uses
default_root_openhuman_dir() and the surrounding error handling to construct an
absolute fallback path and call the logger (or e.g., warn!()) with context
including that the home-dir lookup failed and which fallback was chosen.
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
<data_dir>/logs/openhuman-YYYY-MM-DD.log(7-day retention) viatracing-appender, with the sameCleanCliFormatused byopenhuman run.log::*calls are bridged into the same subscriber viatracing_log::LogTracer— replaces the prior stderr-onlyenv_logger. Sentry tracing layer +RUST_LOGenv filter preserved.logs_folder_path/reveal_logs_folderTauri commands and a "Open logs folder" row in Settings → Developer Options so users can grab today's log file when reporting issues like "stuck on Initializing OpenHuman...".src/core/logging.rssoinit_for_cli_runand the newinit_for_embeddedshareseed_rust_log/build_env_filter/sentry_tracing_layer— single source of truth for formatter + filter.Problem
When the Tauri shell hits a startup issue (RPC bootstrap failure, CEF cache lock, sidecar crash) we have no log file to point users at. The shell's
env_loggerwrites to stderr — invisible in a packaged.app/.exe. The embedded core'stracingsubscriber (init_for_cli_runinsrc/core/logging.rs) was only invoked from CLI paths, so everytracing::info!in the core was a no-op when running inside the Tauri shell. Result: support has nothing concrete to ask the user for.Solution
src/core/logging.rs— extractseed_rust_log/build_env_filter/sentry_tracing_layerand addinit_for_embedded(data_dir, verbose). The new entry point installs a non-blocking, daily-rotated file appender (tracing-appender) at<data_dir>/logs/openhuman-YYYY-MM-DD.logplus the existing stderr layer (still useful undertauri dev). Both shareCleanCliFormat.Once-guarded; first caller wins. The non-blocking writer'sWorkerGuardand the resolved log dir are stashed inOnceLocks so they live for the entire process and the UI can resolve the path on demand.app/src-tauri/src/file_logging.rs— small shell-side wrapper that resolves the data dir the same way the core does (OPENHUMAN_WORKSPACEoverride, thendefault_root_openhuman_dir) and callsinit_for_embedded. Exposes two Tauri commands:logs_folder_path(returns the absolute path for display) andreveal_logs_folder(open/explorer/xdg-openper platform).app/src-tauri/src/lib.rs— callsfile_logging::init()early inrun()(before CEF preflight, before the Sentry smoke trigger), drops theenv_logger::Builder::try_initpath, and registers the new commands ininvoke_handler. Sentry init still runs first since that just builds a client guard.app/src/components/settings/panels/DeveloperOptionsPanel.tsx— adds a "App logs" row that shows the resolved path (logs_folder_path) and a button that callsreveal_logs_folder. Hidden in non-Tauri builds viaisTauri().app/src-tauri/permissions/allow-core-process.toml— capability allowlist entries for the two new commands.Tradeoffs
<data_dir>/logs/(i.e.~/.openhuman/logs/by default, or underOPENHUMAN_WORKSPACE) keeps logs next toactive_user.toml, the per-userusers/tree, and the CEF caches a support engineer would also need. OS-conventional paths (~/Library/Logs/OpenHuman,%LOCALAPPDATA%\OpenHuman\Logs) are whatConsole.app/ Event Viewer expect but split logs per user and need per-platform code; theOPENHUMAN_WORKSPACEtest override would also stop working.info(ordebugifOPENHUMAN_VERBOSE=1/RUST_LOG=debug). Bumpable later if a debug session needs deeper history.init_for_embeddedruns aftersentry::initbut before everything else (CEF preflight included). Anything that fails beforerun()enters our init still hits stderr only — that's currentlymain.rsargv parsing andtauri::cef_entry_point, neither of which has user-actionable failures today.Submission Checklist
N/A: this PR is a logging plumbing change. The new file appender is exercised by the existinginit_for_cli_runpath tests and by manual smoke (open Settings → Developer Options → Open logs folder; verify a daily file appears). Adding a test that asserts file contents would require sequencing aroundtracing-appender's background flushing thread and is brittle.N/A: same reason. Will follow up with a focused test if the coverage gate flags it.N/A: behaviour-only change(no new product feature).## Related—N/A.tracing-appenderis local-only).N/A(no tracking issue).Impact
tracing_appender::non_blocking, so the UI thread never blocks on disk I/O. Background thread is owned by a process-lifetimeOnceLock<WorkerGuard>.info, daily logs are typically <1 MB. 7-day cap keeps total footprint bounded.~/.openhuman/data dir, same TCC/permission scope as today's storage.logs/subdir.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— ran viapnpm formatpnpm typecheckcargo check --manifest-path Cargo.toml --libcargo check --manifest-path app/src-tauri/Cargo.tomlValidation Blocked
Behavior Changes
<data_dir>/logs/instead of stderr only.Parity Contract
init_for_cli_run(used byopenhuman run/ CLI subcommands) keeps the same stderr output andEnvFilterdefaults.Once-guarded subscriber init; non-blocking writer guard pinned for the process lifetime;reveal_logs_folderreturns a typed error if logging hasn't been initialized.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Chores