From 1e584f290704a55a918e22c255260eb38fc42991 Mon Sep 17 00:00:00 2001 From: Albert Armea Date: Fri, 2 Jan 2026 19:00:27 -0500 Subject: [PATCH] Add optional child process logging --- config.example.toml | 6 +++ crates/shepherd-config/src/policy.rs | 22 ++++++-- crates/shepherd-config/src/schema.rs | 8 +++ crates/shepherd-host-linux/src/adapter.rs | 2 +- crates/shepherd-host-linux/src/process.rs | 62 +++++++++++++++++++---- crates/shepherdd/src/main.rs | 23 ++++++++- 6 files changed, 108 insertions(+), 15 deletions(-) diff --git a/config.example.toml b/config.example.toml index 3be1ade..5c8d346 100644 --- a/config.example.toml +++ b/config.example.toml @@ -12,6 +12,12 @@ config_version = 1 # log_dir = "~/.local/state/shepherdd" # data_dir = "~/.local/share/shepherdd" +# Capture stdout/stderr from child applications to log files +# Logs are written to child_log_dir (or log_dir/sessions if not set)` to view logs. +# File format: _.log +# capture_child_output = true +# child_log_dir = "~/.local/state/shepherdd/sessions" + # Default max run duration if not specified per entry (1 hour) # Set to 0 for unlimited (no time limit) default_max_run_seconds = 3600 diff --git a/crates/shepherd-config/src/policy.rs b/crates/shepherd-config/src/policy.rs index 4579908..2389e52 100644 --- a/crates/shepherd-config/src/policy.rs +++ b/crates/shepherd-config/src/policy.rs @@ -77,17 +77,28 @@ pub struct ServiceConfig { pub socket_path: PathBuf, pub log_dir: PathBuf, pub data_dir: PathBuf, + /// Whether to capture stdout/stderr from child applications + pub capture_child_output: bool, + /// Directory for child application logs + pub child_log_dir: PathBuf, } impl ServiceConfig { fn from_raw(raw: RawServiceConfig) -> Self { + let log_dir = raw + .log_dir + .clone() + .unwrap_or_else(default_log_dir); + let child_log_dir = raw + .child_log_dir + .unwrap_or_else(|| log_dir.join("sessions")); Self { socket_path: raw .socket_path .unwrap_or_else(socket_path_without_env), - log_dir: raw - .log_dir - .unwrap_or_else(default_log_dir), + log_dir, + capture_child_output: raw.capture_child_output, + child_log_dir, data_dir: raw .data_dir .unwrap_or_else(default_data_dir), @@ -97,10 +108,13 @@ impl ServiceConfig { impl Default for ServiceConfig { fn default() -> Self { + let log_dir = default_log_dir(); Self { socket_path: socket_path_without_env(), - log_dir: default_log_dir(), + child_log_dir: log_dir.join("sessions"), + log_dir, data_dir: default_data_dir(), + capture_child_output: false, } } } diff --git a/crates/shepherd-config/src/schema.rs b/crates/shepherd-config/src/schema.rs index 99b939f..d1b5370 100644 --- a/crates/shepherd-config/src/schema.rs +++ b/crates/shepherd-config/src/schema.rs @@ -31,6 +31,14 @@ pub struct RawServiceConfig { /// Data directory for store (default: $XDG_DATA_HOME/shepherdd) pub data_dir: Option, + /// Capture stdout/stderr from child applications to log files + /// Files are written to child_log_dir (or log_dir/sessions if not set) + #[serde(default)] + pub capture_child_output: bool, + + /// Directory for child application logs (default: log_dir/sessions) + pub child_log_dir: Option, + /// Default warning thresholds (can be overridden per entry) pub default_warnings: Option>, diff --git a/crates/shepherd-host-linux/src/adapter.rs b/crates/shepherd-host-linux/src/adapter.rs index a70aa23..a8bd9dd 100644 --- a/crates/shepherd-host-linux/src/adapter.rs +++ b/crates/shepherd-host-linux/src/adapter.rs @@ -191,7 +191,7 @@ impl HostAdapter for LinuxHost { &argv, &env, cwd.as_ref(), - options.capture_stdout || options.capture_stderr, + options.log_path.clone(), snap_name.clone(), )?; diff --git a/crates/shepherd-host-linux/src/process.rs b/crates/shepherd-host-linux/src/process.rs index 72cd8b5..41bd87a 100644 --- a/crates/shepherd-host-linux/src/process.rs +++ b/crates/shepherd-host-linux/src/process.rs @@ -3,7 +3,9 @@ use nix::sys::signal::{self, Signal}; use nix::unistd::Pid; use std::collections::HashMap; +use std::fs::File; use std::os::unix::process::CommandExt; +use std::path::PathBuf; use std::process::{Child, Command, Stdio}; use tracing::{debug, info, warn}; @@ -126,11 +128,13 @@ impl ManagedProcess { /// /// If `snap_name` is provided, the process is treated as a snap app and will use /// systemd scope-based killing instead of signal-based killing. + /// + /// If `log_path` is provided, stdout and stderr will be redirected to that file. pub fn spawn( argv: &[String], env: &HashMap, cwd: Option<&std::path::PathBuf>, - capture_output: bool, + log_path: Option, snap_name: Option, ) -> HostResult { if argv.is_empty() { @@ -243,11 +247,42 @@ impl ManagedProcess { cmd.current_dir(dir); } - // Configure output capture - // For debugging, inherit stdout/stderr so we can see errors - if capture_output { - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); + // Configure output handling + // If log_path is provided, redirect stdout/stderr to the log file + // Otherwise, inherit from parent so we can see child output for debugging + if let Some(ref path) = log_path { + // Create parent directory if it doesn't exist + if let Some(parent) = path.parent() + && let Err(e) = std::fs::create_dir_all(parent) + { + warn!(path = %parent.display(), error = %e, "Failed to create log directory"); + } + + // Open log file for appending (create if doesn't exist) + match File::create(path) { + Ok(file) => { + // Clone file handle for stderr (both point to same file) + let stderr_file = match file.try_clone() { + Ok(f) => f, + Err(e) => { + warn!(path = %path.display(), error = %e, "Failed to clone log file handle"); + cmd.stdout(Stdio::inherit()); + cmd.stderr(Stdio::inherit()); + cmd.stdin(Stdio::null()); + // Skip to spawn + return Self::spawn_with_cmd(cmd, program, snap_name); + } + }; + cmd.stdout(Stdio::from(file)); + cmd.stderr(Stdio::from(stderr_file)); + info!(path = %path.display(), "Redirecting child output to log file"); + } + Err(e) => { + warn!(path = %path.display(), error = %e, "Failed to open log file, inheriting output"); + cmd.stdout(Stdio::inherit()); + cmd.stderr(Stdio::inherit()); + } + } } else { // Inherit from parent so we can see child output for debugging cmd.stdout(Stdio::inherit()); @@ -256,6 +291,15 @@ impl ManagedProcess { cmd.stdin(Stdio::null()); + Self::spawn_with_cmd(cmd, program, snap_name) + } + + /// Complete the spawn process with the configured command + fn spawn_with_cmd( + mut cmd: Command, + program: &str, + snap_name: Option, + ) -> HostResult { // Store the command name for later use in killing let command_name = program.to_string(); @@ -467,7 +511,7 @@ mod tests { let argv = vec!["true".to_string()]; let env = HashMap::new(); - let mut proc = ManagedProcess::spawn(&argv, &env, None, false, None).unwrap(); + let mut proc = ManagedProcess::spawn(&argv, &env, None, None, None).unwrap(); // Wait for it to complete let status = proc.wait().unwrap(); @@ -479,7 +523,7 @@ mod tests { let argv = vec!["echo".to_string(), "hello".to_string()]; let env = HashMap::new(); - let mut proc = ManagedProcess::spawn(&argv, &env, None, false, None).unwrap(); + let mut proc = ManagedProcess::spawn(&argv, &env, None, None, None).unwrap(); let status = proc.wait().unwrap(); assert!(status.is_success()); } @@ -489,7 +533,7 @@ mod tests { let argv = vec!["sleep".to_string(), "60".to_string()]; let env = HashMap::new(); - let proc = ManagedProcess::spawn(&argv, &env, None, false, None).unwrap(); + let proc = ManagedProcess::spawn(&argv, &env, None, None, None).unwrap(); // Give it a moment to start std::thread::sleep(std::time::Duration::from_millis(50)); diff --git a/crates/shepherdd/src/main.rs b/crates/shepherdd/src/main.rs index 01ceb40..96816ed 100644 --- a/crates/shepherdd/src/main.rs +++ b/crates/shepherdd/src/main.rs @@ -537,6 +537,27 @@ impl Service { .get_entry(&entry_id) .map(|e| e.kind.clone()); + // Build spawn options with log path if capture_child_output is enabled + let spawn_options = if eng.policy().service.capture_child_output { + let log_dir = &eng.policy().service.child_log_dir; + // Create log filename: __.log + let timestamp = now.format("%Y%m%d_%H%M%S").to_string(); + let log_filename = format!( + "{}_{}.log", + entry_id.as_str().replace(['/', '\\', ' '], "_"), + timestamp + ); + let log_path = log_dir.join(log_filename); + shepherd_host_api::SpawnOptions { + capture_stdout: true, + capture_stderr: true, + log_path: Some(log_path), + ..Default::default() + } + } else { + shepherd_host_api::SpawnOptions::default() + }; + drop(eng); // Release lock before spawning if let Some(kind) = entry_kind { @@ -544,7 +565,7 @@ impl Service { .spawn( plan.session_id.clone(), &kind, - shepherd_host_api::SpawnOptions::default(), + spawn_options, ) .await {