From 749e0e0d9cf2566f01dab76f4bf235d604e7edb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Sat, 23 May 2026 16:06:38 +1200 Subject: [PATCH 1/4] plan: tamanu logs multi-name + caddy unification --- docs/plans/tamanu-logs-multiname.md | 64 +++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 docs/plans/tamanu-logs-multiname.md diff --git a/docs/plans/tamanu-logs-multiname.md b/docs/plans/tamanu-logs-multiname.md new file mode 100644 index 00000000..9077fdc6 --- /dev/null +++ b/docs/plans/tamanu-logs-multiname.md @@ -0,0 +1,64 @@ +# `tamanu logs` multi-name + caddy unification + +Follow-up to the lifecycle PR (`tamanu-lifecycle.md`). Reshapes +`tamanu logs` to share the matcher surface introduced there, and folds +caddy log tailing into the same code path as tamanu service logs. + +Linear: TAM-6782 (same ticket as lifecycle). + +## Why a separate PR + +The lifecycle PR introduces `lifecycle::match_names` and the +`Criticality` field. It's already a large change. Reshaping `logs.rs` +on top of those primitives is mechanically straightforward but is its +own concern (operator UX rather than service lifecycle), and the diff +touches a different file (`logs.rs`) than the lifecycle work. Splitting +keeps each PR reviewable on its own. + +## Changes + +- `LogsArgs.name: String` → `LogsArgs.names: Vec`. +- Empty `names` = every expected-Up tamanu service plus caddy. +- Each name in `names` is a substring match via + `lifecycle::match_names`. The literal pseudo-service `caddy` is + recognised as a named match in addition to tamanu expectations. +- `--grep`, `-n`, `-f` flags unchanged in shape. + +## Behaviour + +On Linux the union collapses to a single `journalctl` call: + +``` +journalctl -u -u ... [-u caddy.service] -n N [-f] [-g REGEX] --output=cat +``` + +The existing JSON highlighter in `format_log_line` is content-based +(already opportunistic per line), so caddy lines get highlighted and +tamanu lines don't, in the same stream. + +On Windows the existing `tail_files` already takes multiple +`TailSource`s. Extend the source collection so when caddy is in the +matched set, `caddy_log_files_windows()` contributes its `.log` files +alongside the pm2 sources from `pm2::log_sources()`. + +## Error UX + +- Any name that matches zero expectations and is not the literal + `caddy`: bail with the available names (same UX as the lifecycle + commands). +- The literal `caddy` always matches; it's not subject to discovery. + +## Testing + +- Unit tests on the args parse: zero names, one name, multiple names, + `caddy` alongside tamanu names. +- Integration testing is manual (real journalctl / pm2 logs streams). + +## PR + +Title: `feat(tamanu/logs): TAM-6782: multi-name + unified caddy tailing` + +## Stacking + +Stack on top of the lifecycle PR. Once that merges, this one +auto-rebases to main. From a71f7168a8372a5de8fdae491619231166440886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Sat, 23 May 2026 16:36:58 +1200 Subject: [PATCH 2/4] refactor(tamanu): move match_names from lifecycle to services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The matcher is the only pure piece of lifecycle.rs — it operates on Expectation and has no IO. Moving it to services.rs alongside the Expectation type means logs.rs (which doesn't pull in the tamanu-lifecycle feature) can share the same primitive without a cross-feature dependency. --- .../bestool/src/actions/tamanu/lifecycle.rs | 97 ------------------- crates/bestool/src/actions/tamanu/restart.rs | 4 +- crates/bestool/src/actions/tamanu/services.rs | 90 +++++++++++++++++ crates/bestool/src/actions/tamanu/start.rs | 4 +- crates/bestool/src/actions/tamanu/status.rs | 4 +- crates/bestool/src/actions/tamanu/stop.rs | 4 +- 6 files changed, 98 insertions(+), 105 deletions(-) diff --git a/crates/bestool/src/actions/tamanu/lifecycle.rs b/crates/bestool/src/actions/tamanu/lifecycle.rs index d454899b..67161474 100644 --- a/crates/bestool/src/actions/tamanu/lifecycle.rs +++ b/crates/bestool/src/actions/tamanu/lifecycle.rs @@ -428,43 +428,6 @@ fn is_running(supervisor: Supervisor, target: &str) -> bool { } } -/// Filter an expectation set by zero or more substring patterns. -/// -/// - Empty `names`: returns every expectation unchanged. -/// - Otherwise: an expectation matches if **any** name in `names` is a -/// substring of the expectation's name. -/// -/// Returns an error if any name in `names` matched zero expectations -/// (typo safety in multi-name invocations). -pub fn match_names<'a>( - expectations: &'a [Expectation], - names: &[&str], -) -> Result> { - if names.is_empty() { - return Ok(expectations.iter().collect()); - } - - let unmatched: Vec<&str> = names - .iter() - .copied() - .filter(|name| !expectations.iter().any(|e| e.name.contains(name))) - .collect(); - if !unmatched.is_empty() { - let available: Vec<&str> = expectations.iter().map(|e| e.name).collect(); - bail!( - "no service matches: {}; available names are: {}", - unmatched.join(", "), - available.join(", "), - ); - } - - let matched: Vec<&Expectation> = expectations - .iter() - .filter(|e| names.iter().any(|name| e.name.contains(name))) - .collect(); - Ok(matched) -} - #[cfg(test)] mod tests { use super::*; @@ -479,66 +442,6 @@ mod tests { } } - #[test] - fn empty_names_returns_everything() { - let es = [exp("tamanu-api"), exp("tamanu-tasks"), exp("tamanu-sync")]; - let m = match_names(&es, &[]).unwrap(); - assert_eq!(m.len(), 3); - } - - #[test] - fn single_name_substring_matches() { - let es = [exp("tamanu-central-api"), exp("tamanu-central-tasks")]; - let m = match_names(&es, &["api"]).unwrap(); - assert_eq!(m.len(), 1); - assert_eq!(m[0].name, "tamanu-central-api"); - } - - #[test] - fn multi_name_union() { - let es = [ - exp("tamanu-central-api"), - exp("tamanu-central-tasks"), - exp("tamanu-central-fhir-resolve"), - ]; - let m = match_names(&es, &["api", "fhir"]).unwrap(); - assert_eq!(m.len(), 2); - assert_eq!( - m.iter().map(|e| e.name).collect::>(), - vec!["tamanu-central-api", "tamanu-central-fhir-resolve"], - ); - } - - #[test] - fn zero_match_name_bails() { - let es = [exp("tamanu-api"), exp("tamanu-tasks")]; - let err = match_names(&es, &["nope"]).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("nope"), "error should name the bad pattern: {msg}"); - assert!(msg.contains("tamanu-api"), "error should list available: {msg}"); - } - - #[test] - fn mixed_match_and_no_match_still_bails() { - // One typo in a multi-name invocation should bail rather than silently - // drop the bad pattern and process the rest. - let es = [exp("tamanu-api"), exp("tamanu-tasks")]; - let err = match_names(&es, &["api", "nope"]).unwrap_err(); - let msg = format!("{err}"); - assert!(msg.contains("nope"), "error should name the bad pattern: {msg}"); - } - - #[test] - fn name_substring_can_match_multiple() { - let es = [ - exp("tamanu-central-fhir-resolve"), - exp("tamanu-central-fhir-refresh"), - exp("tamanu-api"), - ]; - let m = match_names(&es, &["fhir"]).unwrap(); - assert_eq!(m.len(), 2); - } - fn templated_exp(name: &'static str) -> Expectation { Expectation { name, diff --git a/crates/bestool/src/actions/tamanu/restart.rs b/crates/bestool/src/actions/tamanu/restart.rs index 27e43c91..14a5d569 100644 --- a/crates/bestool/src/actions/tamanu/restart.rs +++ b/crates/bestool/src/actions/tamanu/restart.rs @@ -11,7 +11,7 @@ use crate::actions::{ tamanu::{ TamanuArgs, lifecycle::{self, Instance}, - services::{Criticality, ExpectedState, Expectation, Supervisor}, + services::{self, Criticality, ExpectedState, Expectation, Supervisor}, }, }; @@ -58,7 +58,7 @@ pub async fn run(args: RestartArgs, ctx: Context) -> Result<()> { let (supervisor, expectations) = lifecycle::config_and_expectations(tamanu)?; let names: Vec<&str> = args.names.iter().map(String::as_str).collect(); - let matched = lifecycle::match_names(&expectations, &names)?; + let matched = services::match_names(&expectations, &names)?; let discovered = lifecycle::discover(supervisor)?; let groups = lifecycle::group_by_expectation(&matched, &discovered); diff --git a/crates/bestool/src/actions/tamanu/services.rs b/crates/bestool/src/actions/tamanu/services.rs index 374e8a28..bc9a1b45 100644 --- a/crates/bestool/src/actions/tamanu/services.rs +++ b/crates/bestool/src/actions/tamanu/services.rs @@ -204,6 +204,45 @@ pub fn expected( out } +/// Filter an expectation set by zero or more substring patterns. +/// +/// - Empty `names`: returns every expectation unchanged. +/// - Otherwise: an expectation matches if **any** name in `names` is a +/// substring of the expectation's name. +/// +/// Returns an error if any name in `names` matched zero expectations +/// (typo safety in multi-name invocations). +pub fn match_names<'a>( + expectations: &'a [Expectation], + names: &[&str], +) -> miette::Result> { + use miette::bail; + + if names.is_empty() { + return Ok(expectations.iter().collect()); + } + + let unmatched: Vec<&str> = names + .iter() + .copied() + .filter(|name| !expectations.iter().any(|e| e.name.contains(name))) + .collect(); + if !unmatched.is_empty() { + let available: Vec<&str> = expectations.iter().map(|e| e.name).collect(); + bail!( + "no service matches: {}; available names are: {}", + unmatched.join(", "), + available.join(", "), + ); + } + + let matched: Vec<&Expectation> = expectations + .iter() + .filter(|e| names.iter().any(|name| e.name.contains(name))) + .collect(); + Ok(matched) +} + /// Parse a systemd unit name (`tamanu-foo@1.service`, `tamanu-foo.service`, /// `tamanu-foo`) into its base name and optional instance. /// @@ -391,6 +430,57 @@ mod tests { ); } + fn exp(name: &'static str) -> Expectation { + Expectation { + name, + instances: Instances::Single, + state: ExpectedState::Up, + criticality: Criticality::Background, + } + } + + #[test] + fn match_names_empty_returns_everything() { + let es = [exp("tamanu-api"), exp("tamanu-tasks"), exp("tamanu-sync")]; + let m = match_names(&es, &[]).unwrap(); + assert_eq!(m.len(), 3); + } + + #[test] + fn match_names_substring() { + let es = [exp("tamanu-central-api"), exp("tamanu-central-tasks")]; + let m = match_names(&es, &["api"]).unwrap(); + assert_eq!(m.len(), 1); + assert_eq!(m[0].name, "tamanu-central-api"); + } + + #[test] + fn match_names_union() { + let es = [ + exp("tamanu-central-api"), + exp("tamanu-central-tasks"), + exp("tamanu-central-fhir-resolve"), + ]; + let m = match_names(&es, &["api", "fhir"]).unwrap(); + assert_eq!(m.len(), 2); + } + + #[test] + fn match_names_zero_match_bails() { + let es = [exp("tamanu-api"), exp("tamanu-tasks")]; + let err = match_names(&es, &["nope"]).unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("nope")); + assert!(msg.contains("tamanu-api")); + } + + #[test] + fn match_names_partial_typo_still_bails() { + let es = [exp("tamanu-api"), exp("tamanu-tasks")]; + let err = match_names(&es, &["api", "nope"]).unwrap_err(); + assert!(format!("{err}").contains("nope")); + } + #[test] fn parse_systemd_unit_works() { assert_eq!( diff --git a/crates/bestool/src/actions/tamanu/start.rs b/crates/bestool/src/actions/tamanu/start.rs index 2dce1557..ffa95873 100644 --- a/crates/bestool/src/actions/tamanu/start.rs +++ b/crates/bestool/src/actions/tamanu/start.rs @@ -8,7 +8,7 @@ use crate::actions::{ tamanu::{ TamanuArgs, lifecycle::{self, Instance}, - services::{ExpectedState, Expectation, Supervisor}, + services::{self, ExpectedState, Expectation, Supervisor}, }, }; @@ -29,7 +29,7 @@ pub async fn run(args: StartArgs, ctx: Context) -> Result<()> { let (supervisor, expectations) = lifecycle::config_and_expectations(tamanu)?; let names: Vec<&str> = args.names.iter().map(String::as_str).collect(); - let matched = lifecycle::match_names(&expectations, &names)?; + let matched = services::match_names(&expectations, &names)?; let discovered = lifecycle::discover(supervisor)?; let groups = lifecycle::group_by_expectation(&matched, &discovered); diff --git a/crates/bestool/src/actions/tamanu/status.rs b/crates/bestool/src/actions/tamanu/status.rs index af1a2cce..ccb4bdd9 100644 --- a/crates/bestool/src/actions/tamanu/status.rs +++ b/crates/bestool/src/actions/tamanu/status.rs @@ -8,7 +8,7 @@ use crate::actions::{ tamanu::{ TamanuArgs, lifecycle::{self, Instance}, - services::{Criticality, ExpectedState, Expectation}, + services::{self, Criticality, ExpectedState, Expectation}, }, }; @@ -35,7 +35,7 @@ pub async fn run(args: StatusArgs, ctx: Context) -> Result<()> { let (supervisor, expectations) = lifecycle::config_and_expectations(tamanu)?; let names: Vec<&str> = args.names.iter().map(String::as_str).collect(); - let matched = lifecycle::match_names(&expectations, &names)?; + let matched = services::match_names(&expectations, &names)?; let discovered = lifecycle::discover(supervisor)?; let groups = lifecycle::group_by_expectation(&matched, &discovered); diff --git a/crates/bestool/src/actions/tamanu/stop.rs b/crates/bestool/src/actions/tamanu/stop.rs index ce6ffd7b..9aa69216 100644 --- a/crates/bestool/src/actions/tamanu/stop.rs +++ b/crates/bestool/src/actions/tamanu/stop.rs @@ -6,7 +6,7 @@ use crate::actions::{ tamanu::{ TamanuArgs, lifecycle::{self, Instance}, - services::{ExpectedState, Expectation, Supervisor}, + services::{self, ExpectedState, Expectation, Supervisor}, }, }; @@ -28,7 +28,7 @@ pub async fn run(args: StopArgs, ctx: Context) -> Result<()> { let (supervisor, expectations) = lifecycle::config_and_expectations(tamanu)?; let names: Vec<&str> = args.names.iter().map(String::as_str).collect(); - let matched = lifecycle::match_names(&expectations, &names)?; + let matched = services::match_names(&expectations, &names)?; let discovered = lifecycle::discover(supervisor)?; let groups = lifecycle::group_by_expectation(&matched, &discovered); From beb216e63c352cdcd75b91891541ed50ab4c2209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Sat, 23 May 2026 16:40:40 +1200 Subject: [PATCH 3/4] feat(tamanu/logs): TAM-6782: multi-name positional + caddy unified into the main tail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LogsArgs.name (single String) becomes LogsArgs.names (Vec). Each name is a substring against the expected service list (matched via services::match_names). The literal pseudo-service `caddy` is recognised alongside tamanu names, so `tamanu logs caddy api` is a single combined tail. With no names, every expected-Up tamanu service plus caddy is tailed — `tamanu logs` alone is now the all-stack tail operators want. On Linux this collapses to one journalctl call with `-u ...` plus `-u caddy.service` when caddy is matched, all piped through the existing JSON highlighter. On Windows, the pm2 log sources are merged with caddy's .log files into a single `tail_files` call. --- crates/bestool/USAGE.md | 28 +- crates/bestool/src/actions/tamanu/logs.rs | 354 +++++++++++----------- 2 files changed, 189 insertions(+), 193 deletions(-) diff --git a/crates/bestool/USAGE.md b/crates/bestool/USAGE.md index b8237baa..9a8c7afc 100644 --- a/crates/bestool/USAGE.md +++ b/crates/bestool/USAGE.md @@ -1087,7 +1087,7 @@ Alias: t * `download` — Download Tamanu artifacts * `find` — Find Tamanu installations * `greenmask-config` — Generate a Greenmask config file -* `logs` — Tail logs for a Tamanu service, in a supervisor-agnostic way. +* `logs` — Tail logs for tamanu services and (optionally) caddy. * `meta-ticket` — Generate a meta-ticket for this Tamanu server * `psql` — Connect to Tamanu's database * `restart` — Rolling-restart all running tamanu services. @@ -1688,23 +1688,25 @@ Generate a Greenmask config file ## `bestool tamanu logs` -Tail logs for a Tamanu service, in a supervisor-agnostic way. +Tail logs for tamanu services and (optionally) caddy. -On Linux this drives `journalctl -u`; on Windows it reads pm2's log files -directly. The service name is matched as a substring against the expected -service list, so `tamanu logs api` picks up -`tamanu-{central,facility}-api@*` on systemd and `tamanu-api` on pm2. +Each NAME is matched as a substring against the expected-Up service +list, so `tamanu logs api` picks up `tamanu-{central,facility}-api@*` +on systemd and `tamanu-api` on pm2. Multiple names combine: `tamanu +logs api fhir` tails both. With no names at all, every expected-Up +tamanu service is tailed alongside caddy. -The special name `caddy` tails the caddy service: from `journalctl -u -caddy.service` on Linux, and from `.log` files under `C:\Caddy\logs` (or -`C:\Caddy`) on Windows. Caddy emits JSON-per-line logs; bestool detects -these and applies opportunistic syntax highlighting. +The literal name `caddy` is recognised as a pseudo-service that +tails caddy: from `journalctl -u caddy.service` on Linux, and from +`.log` files under `C:\Caddy\logs` (or `C:\Caddy`) on Windows. Caddy +emits JSON-per-line logs; bestool detects these and applies +opportunistic syntax highlighting per line. -**Usage:** `bestool tamanu logs [OPTIONS] ` +**Usage:** `bestool tamanu logs [OPTIONS] [NAMES]...` ###### **Arguments:** -* `` — Service name. Matched as a substring against the expected service list +* `` — Service names. Each is matched as a substring against the expected service list. `caddy` is a recognised pseudo-service. With no names, tails everything (every expected-Up tamanu service plus caddy) ###### **Options:** @@ -1712,7 +1714,7 @@ these and applies opportunistic syntax highlighting. Default value: `10` * `-f`, `--follow` — Follow: keep printing new lines as they arrive. Equivalent to `tail -f` -* `-g`, `--grep ` — Only print lines matching this regex. On Linux this is passed to `journalctl -g`; on Windows it's applied client-side after reading from the pm2 log files +* `-g`, `--grep ` — Only print lines matching this regex. On Linux this is passed to `journalctl -g`; on Windows it's applied client-side after reading from the log files diff --git a/crates/bestool/src/actions/tamanu/logs.rs b/crates/bestool/src/actions/tamanu/logs.rs index 25cf0a6b..9fd0df9d 100644 --- a/crates/bestool/src/actions/tamanu/logs.rs +++ b/crates/bestool/src/actions/tamanu/logs.rs @@ -25,6 +25,10 @@ use crate::actions::{ }, }; +/// The literal pseudo-service name that triggers caddy log tailing +/// alongside whatever tamanu services are matched. +const CADDY: &str = "caddy"; + /// Per-source line formatter used by `tail_one`. Takes the raw line and the /// caller's "use colours" setting, returns the (possibly transformed) line. type LineFormatter = fn(&str, bool) -> String; @@ -40,22 +44,27 @@ struct TailSource { formatter: LineFormatter, } -/// Tail logs for a Tamanu service, in a supervisor-agnostic way. +/// Tail logs for tamanu services and (optionally) caddy. /// -/// On Linux this drives `journalctl -u`; on Windows it reads pm2's log files -/// directly. The service name is matched as a substring against the expected -/// service list, so `tamanu logs api` picks up -/// `tamanu-{central,facility}-api@*` on systemd and `tamanu-api` on pm2. +/// Each NAME is matched as a substring against the expected-Up service +/// list, so `tamanu logs api` picks up `tamanu-{central,facility}-api@*` +/// on systemd and `tamanu-api` on pm2. Multiple names combine: `tamanu +/// logs api fhir` tails both. With no names at all, every expected-Up +/// tamanu service is tailed alongside caddy. /// -/// The special name `caddy` tails the caddy service: from `journalctl -u -/// caddy.service` on Linux, and from `.log` files under `C:\Caddy\logs` (or -/// `C:\Caddy`) on Windows. Caddy emits JSON-per-line logs; bestool detects -/// these and applies opportunistic syntax highlighting. +/// The literal name `caddy` is recognised as a pseudo-service that +/// tails caddy: from `journalctl -u caddy.service` on Linux, and from +/// `.log` files under `C:\Caddy\logs` (or `C:\Caddy`) on Windows. Caddy +/// emits JSON-per-line logs; bestool detects these and applies +/// opportunistic syntax highlighting per line. #[derive(Debug, Clone, Parser)] #[clap(verbatim_doc_comment)] pub struct LogsArgs { - /// Service name. Matched as a substring against the expected service list. - pub name: String, + /// Service names. Each is matched as a substring against the + /// expected service list. `caddy` is a recognised pseudo-service. + /// With no names, tails everything (every expected-Up tamanu + /// service plus caddy). + pub names: Vec, /// Number of trailing lines to print before tailing. #[arg(short = 'n', long = "lines", default_value = "10")] @@ -66,23 +75,44 @@ pub struct LogsArgs { pub follow: bool, /// Only print lines matching this regex. On Linux this is passed to - /// `journalctl -g`; on Windows it's applied client-side after reading from - /// the pm2 log files. + /// `journalctl -g`; on Windows it's applied client-side after reading + /// from the log files. #[arg(short = 'g', long = "grep", value_name = "REGEX")] pub grep: Option, } -pub async fn run(args: LogsArgs, ctx: Context) -> Result<()> { - let tamanu = ctx.require::(); +/// Result of partitioning the NAMES argument into tamanu service patterns +/// and the caddy pseudo-service flag. +struct Selection { + tamanu_names: Vec, + include_caddy: bool, +} - if args.name == "caddy" { - return run_caddy_logs( - args.lines, - args.follow, - args.grep.as_ref(), - tamanu.use_colours, - ); +fn select(names: &[String]) -> Selection { + if names.is_empty() { + return Selection { + tamanu_names: Vec::new(), + include_caddy: true, + }; + } + let mut tamanu_names = Vec::new(); + let mut include_caddy = false; + for n in names { + if n == CADDY { + include_caddy = true; + } else { + tamanu_names.push(n.clone()); + } } + Selection { + tamanu_names, + include_caddy, + } +} + +pub async fn run(args: LogsArgs, ctx: Context) -> Result<()> { + let tamanu = ctx.require::(); + let selection = select(&args.names); let (_, root) = find_tamanu(tamanu)?; let config = load_config(&root, None)?; @@ -100,125 +130,71 @@ pub async fn run(args: LogsArgs, ctx: Context) -> Result<()> { bail!("tamanu logs is only supported on Linux (systemd) and Windows (pm2)"); }; - let expectations = services::expected(supervisor, kind, &config); - let matches: Vec<&Expectation> = match_name(&expectations, &args.name).collect(); - - if matches.is_empty() { - let candidates: Vec<&str> = up_names(&expectations).collect(); - bail!( - "no service matches {:?}; expected services are: {}", - args.name, - candidates.join(", ") - ); - } - - debug!( - ?matches, - "matched expectations for `tamanu logs {}`", - args.name - ); - - match supervisor { - Supervisor::Systemd => run_journalctl(&matches, args.lines, args.follow, args.grep.as_ref()), - Supervisor::Pm2 => run_pm2_logs(&matches, args.lines, args.follow, args.grep), - } -} - -fn up_names(expectations: &[Expectation]) -> impl Iterator { - expectations + let all_expectations = services::expected(supervisor, kind, &config); + let up_expectations: Vec<&Expectation> = all_expectations .iter() .filter(|e| e.state == ExpectedState::Up) - .map(|e| e.name) -} + .collect(); -fn match_name<'a>( - expectations: &'a [Expectation], - needle: &'a str, -) -> impl Iterator { - expectations + // We use the same matcher as the lifecycle commands but only consider + // expected-Up services as candidates for the NAMES filter. + let tamanu_name_refs: Vec<&str> = selection + .tamanu_names .iter() - .filter(|e| e.state == ExpectedState::Up) - .filter(move |e| e.name.contains(needle)) -} - -fn run_caddy_logs( - lines: usize, - follow: bool, - grep: Option<&Regex>, - use_colours: bool, -) -> Result<()> { - if cfg!(target_os = "linux") { - run_caddy_logs_journalctl(lines, follow, grep, use_colours) - } else if cfg!(target_os = "windows") { - run_caddy_logs_files(lines, follow, grep.cloned(), use_colours) - } else { - bail!("caddy logs are only supported on Linux (systemd) and Windows (C:\\Caddy)"); - } -} - -fn run_caddy_logs_journalctl( - lines: usize, - follow: bool, - grep: Option<&Regex>, - use_colours: bool, -) -> Result<()> { - // --output=cat strips journalctl's prefix (timestamp, host, unit) so we - // see the raw log lines caddy emits. Caddy puts its own ts into each line. - let mut cmd = Command::new("journalctl"); - cmd.arg("-u").arg("caddy.service"); - cmd.arg("-n").arg(lines.to_string()); - if follow { - cmd.arg("-f"); - } - if let Some(re) = grep { - cmd.arg("-g").arg(re.as_str()); - } - cmd.arg("--output=cat"); - cmd.stdout(Stdio::piped()); + .map(String::as_str) + .collect(); + let matches: Vec<&Expectation> = { + let up_owned: Vec = up_expectations.iter().map(|e| (*e).clone()).collect(); + let m = services::match_names(&up_owned, &tamanu_name_refs)?; + m.iter().map(|e| { + up_expectations + .iter() + .copied() + .find(|x| x.name == e.name) + .expect("match came from up_expectations") + }).collect() + }; - let mut child = cmd.spawn().into_diagnostic()?; - let stdout = child.stdout.take().ok_or_else(|| miette!("no stdout pipe"))?; - let reader = BufReader::new(stdout); + debug!( + matched = ?matches.iter().map(|m| m.name).collect::>(), + caddy = selection.include_caddy, + "logs selection" + ); - let out_handle = std::io::stdout(); - let mut out = out_handle.lock(); - for line in reader.lines() { - let line = match line { - Ok(l) => l, - Err(_) => break, - }; - let formatted = format_log_line(&line, use_colours); - if writeln!(out, "{formatted}").is_err() { - break; - } - } - let status = child.wait().into_diagnostic()?; - if !status.success() { - bail!("journalctl exited with {status}"); + match supervisor { + Supervisor::Systemd => run_journalctl( + &matches, + selection.include_caddy, + args.lines, + args.follow, + args.grep.as_ref(), + tamanu.use_colours, + ), + Supervisor::Pm2 => run_pm2_logs( + &matches, + selection.include_caddy, + args.lines, + args.follow, + args.grep, + tamanu.use_colours, + ), } - Ok(()) } /// Windows: caddy runs out of `C:\Caddy`, with log files typically in /// `C:\Caddy\logs\*.log`. We probe that directory first, then the install /// root, and tail every `.log` we find. -fn run_caddy_logs_files( - lines: usize, - follow: bool, - grep: Option, - use_colours: bool, -) -> Result<()> { +fn caddy_tail_sources_windows() -> Result> { let files = caddy_log_files_windows()?; - debug!(count = files.len(), "tailing caddy log files"); - let sources: Vec = files + debug!(count = files.len(), "found caddy log files"); + Ok(files .into_iter() .map(|path| TailSource { prefix: caddy_prefix(&path), path, formatter: format_log_line, }) - .collect(); - tail_files(sources, lines, follow, grep, use_colours) + .collect()) } fn caddy_log_files_windows() -> Result> { @@ -313,16 +289,31 @@ fn write_colored_json(v: &Value, out: &mut String) { } } +/// Single journalctl call covering every matched tamanu unit plus, +/// optionally, caddy. Caddy emits JSON-per-line logs; on systemd we run +/// with `--output=cat` so journalctl's own prefix doesn't double up +/// with caddy's timestamps, and pipe stdout through the JSON +/// highlighter (which is opportunistic — non-JSON lines pass through +/// unchanged). fn run_journalctl( matches: &[&Expectation], + include_caddy: bool, lines: usize, follow: bool, grep: Option<&Regex>, + use_colours: bool, ) -> Result<()> { + if matches.is_empty() && !include_caddy { + bail!("nothing to tail: no matched services and caddy not included"); + } + let mut cmd = Command::new("journalctl"); for m in matches { cmd.arg("-u").arg(journalctl_pattern(m)); } + if include_caddy { + cmd.arg("-u").arg("caddy.service"); + } cmd.arg("-n").arg(lines.to_string()); if follow { cmd.arg("-f"); @@ -330,7 +321,26 @@ fn run_journalctl( if let Some(re) = grep { cmd.arg("-g").arg(re.as_str()); } - let status = cmd.status().into_diagnostic()?; + cmd.arg("--output=cat"); + cmd.stdout(Stdio::piped()); + + let mut child = cmd.spawn().into_diagnostic()?; + let stdout = child.stdout.take().ok_or_else(|| miette!("no stdout pipe"))?; + let reader = BufReader::new(stdout); + + let out_handle = std::io::stdout(); + let mut out = out_handle.lock(); + for line in reader.lines() { + let line = match line { + Ok(l) => l, + Err(_) => break, + }; + let formatted = format_log_line(&line, use_colours); + if writeln!(out, "{formatted}").is_err() { + break; + } + } + let status = child.wait().into_diagnostic()?; if !status.success() { bail!("journalctl exited with {status}"); } @@ -348,21 +358,36 @@ fn journalctl_pattern(expectation: &Expectation) -> String { fn run_pm2_logs( matches: &[&Expectation], + include_caddy: bool, lines: usize, follow: bool, grep: Option, + use_colours: bool, ) -> Result<()> { - let names: Vec<&str> = matches.iter().map(|m| m.name).collect(); - let sources = pm2::log_sources(&names) - .map_err(|e| miette!("could not locate pm2 log files: {e}"))?; - if sources.is_empty() { - bail!( - "no pm2 log files found for {}; was the deployment saved with `pm2 save`?", - names.join(", ") - ); + let mut tail_sources: Vec = Vec::new(); + + if !matches.is_empty() { + let names: Vec<&str> = matches.iter().map(|m| m.name).collect(); + let sources = pm2::log_sources(&names) + .map_err(|e| miette!("could not locate pm2 log files: {e}"))?; + if sources.is_empty() { + bail!( + "no pm2 log files found for {}; was the deployment saved with `pm2 save`?", + names.join(", ") + ); + } + tail_sources.extend(sources.into_iter().map(pm2_log_to_tail)); + } + + if include_caddy { + tail_sources.extend(caddy_tail_sources_windows()?); } - let tail_sources: Vec = sources.into_iter().map(pm2_log_to_tail).collect(); - tail_files(tail_sources, lines, follow, grep, false) + + if tail_sources.is_empty() { + bail!("nothing to tail: no matched services and caddy not included"); + } + + tail_files(tail_sources, lines, follow, grep, use_colours) } fn pm2_log_to_tail(source: LogSource) -> TailSource { @@ -517,63 +542,32 @@ mod tests { serde_json::from_value(json).unwrap() } - fn names(matched: Vec<&Expectation>) -> Vec<&str> { - matched.iter().map(|e| e.name).collect() - } - #[test] - fn substring_matches_facility_api_on_systemd() { - let exps = services::expected(Supervisor::Systemd, ApiServerKind::Facility, &cfg(true, false)); - let m: Vec<&Expectation> = match_name(&exps, "api").collect(); - assert_eq!(names(m), vec!["tamanu-facility-api"]); + fn select_empty_includes_caddy_and_no_tamanu_filter() { + let s = select(&[]); + assert!(s.include_caddy); + assert!(s.tamanu_names.is_empty()); } #[test] - fn substring_matches_central_api_on_systemd() { - let exps = services::expected(Supervisor::Systemd, ApiServerKind::Central, &cfg(false, false)); - let m: Vec<&Expectation> = match_name(&exps, "api").collect(); - assert_eq!(names(m), vec!["tamanu-central-api"]); + fn select_caddy_alone() { + let s = select(&["caddy".to_string()]); + assert!(s.include_caddy); + assert!(s.tamanu_names.is_empty()); } #[test] - fn substring_matches_pm2_api() { - let exps = services::expected(Supervisor::Pm2, ApiServerKind::Facility, &cfg(true, false)); - let m: Vec<&Expectation> = match_name(&exps, "api").collect(); - assert_eq!(names(m), vec!["tamanu-api"]); + fn select_caddy_with_others() { + let s = select(&["caddy".to_string(), "api".to_string()]); + assert!(s.include_caddy); + assert_eq!(s.tamanu_names, vec!["api".to_string()]); } #[test] - fn fhir_matches_both_workers_on_central() { - let exps = services::expected(Supervisor::Systemd, ApiServerKind::Central, &cfg(false, true)); - let m: Vec<&Expectation> = match_name(&exps, "fhir").collect(); - assert_eq!( - names(m), - vec!["tamanu-central-fhir-resolve", "tamanu-central-fhir-refresh"] - ); - } - - #[test] - fn does_not_match_forbidden_facility_singleton() { - // On systemd, expectations include the forbidden `tamanu-facility` (Down). - // Logs should skip forbidden services entirely. - let exps = services::expected(Supervisor::Systemd, ApiServerKind::Facility, &cfg(true, false)); - let m: Vec<&Expectation> = match_name(&exps, "tamanu-facility").collect(); - // The literal `tamanu-facility` (Down) is excluded; matches are - // substring hits on the kind-prefixed names. - assert!( - m.iter().all(|e| e.state == ExpectedState::Up), - "matched a Down expectation: {m:?}" - ); - // Ensure at least `tamanu-facility-api` is in there to prove the - // substring matcher does include legitimate `tamanu-facility-*` units. - assert!(m.iter().any(|e| e.name == "tamanu-facility-api")); - } - - #[test] - fn no_match_returns_empty() { - let exps = services::expected(Supervisor::Systemd, ApiServerKind::Facility, &cfg(true, false)); - let m: Vec<&Expectation> = match_name(&exps, "nope-no-such-thing").collect(); - assert!(m.is_empty()); + fn select_without_caddy_excludes_it() { + let s = select(&["api".to_string(), "tasks".to_string()]); + assert!(!s.include_caddy); + assert_eq!(s.tamanu_names, vec!["api".to_string(), "tasks".to_string()]); } #[test] From 6c1057c8888dc460896e2c95606adb9f69e5ae9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Sat, 23 May 2026 17:02:00 +1200 Subject: [PATCH 4/4] unplan: tamanu logs multi-name + caddy unification The reshape is shipped: LogsArgs takes variadic NAMES, the matcher is shared via services::match_names, caddy is a recognised pseudo-service in the unified tail, and the empty-NAMES default tails the whole stack. --- docs/plans/tamanu-logs-multiname.md | 64 ----------------------------- 1 file changed, 64 deletions(-) delete mode 100644 docs/plans/tamanu-logs-multiname.md diff --git a/docs/plans/tamanu-logs-multiname.md b/docs/plans/tamanu-logs-multiname.md deleted file mode 100644 index 9077fdc6..00000000 --- a/docs/plans/tamanu-logs-multiname.md +++ /dev/null @@ -1,64 +0,0 @@ -# `tamanu logs` multi-name + caddy unification - -Follow-up to the lifecycle PR (`tamanu-lifecycle.md`). Reshapes -`tamanu logs` to share the matcher surface introduced there, and folds -caddy log tailing into the same code path as tamanu service logs. - -Linear: TAM-6782 (same ticket as lifecycle). - -## Why a separate PR - -The lifecycle PR introduces `lifecycle::match_names` and the -`Criticality` field. It's already a large change. Reshaping `logs.rs` -on top of those primitives is mechanically straightforward but is its -own concern (operator UX rather than service lifecycle), and the diff -touches a different file (`logs.rs`) than the lifecycle work. Splitting -keeps each PR reviewable on its own. - -## Changes - -- `LogsArgs.name: String` → `LogsArgs.names: Vec`. -- Empty `names` = every expected-Up tamanu service plus caddy. -- Each name in `names` is a substring match via - `lifecycle::match_names`. The literal pseudo-service `caddy` is - recognised as a named match in addition to tamanu expectations. -- `--grep`, `-n`, `-f` flags unchanged in shape. - -## Behaviour - -On Linux the union collapses to a single `journalctl` call: - -``` -journalctl -u -u ... [-u caddy.service] -n N [-f] [-g REGEX] --output=cat -``` - -The existing JSON highlighter in `format_log_line` is content-based -(already opportunistic per line), so caddy lines get highlighted and -tamanu lines don't, in the same stream. - -On Windows the existing `tail_files` already takes multiple -`TailSource`s. Extend the source collection so when caddy is in the -matched set, `caddy_log_files_windows()` contributes its `.log` files -alongside the pm2 sources from `pm2::log_sources()`. - -## Error UX - -- Any name that matches zero expectations and is not the literal - `caddy`: bail with the available names (same UX as the lifecycle - commands). -- The literal `caddy` always matches; it's not subject to discovery. - -## Testing - -- Unit tests on the args parse: zero names, one name, multiple names, - `caddy` alongside tamanu names. -- Integration testing is manual (real journalctl / pm2 logs streams). - -## PR - -Title: `feat(tamanu/logs): TAM-6782: multi-name + unified caddy tailing` - -## Stacking - -Stack on top of the lifecycle PR. Once that merges, this one -auto-rebases to main.