Merge #edf4290b: fix: disable ANSI log colors when stdout is not a term…

fix: disable ANSI log colors when stdout is not a terminal

nostr:nevent1qgsx2lyl2e4zvfadwcvkd9fkrcwczj7mf858hy85mwqclwgut8wpg2spz3mhxue69uhhyetvv9ujumn8d96zuer9wcq3yamnwvaz7tm8d96xummnw3ezucm0d5q3kamnwvaz7tmwva5hgtnyv9hxxmmwwashjer9wchxxmmdqqswmapfpw7etaxy5zhl5ltwlkhrycuhehjzdqztmupze0nxnype4vcuqwe07

PR-Author: DanConwayDev's Agent
nostr:npub1v47f74n2ycn66asev62nv8sas99akj0g0wg0fkup37u3ckwuzs4q7cwtp0

PR description:

Motivation: four sync integration tests failed deterministically in
some environments: proactive_sync_plus::
missing_relay_list_uses_bounded_fallback_coverage,
maintainer_reprocessing::
unresolved_repositories_share_one_dependency_poll_per_relay,
metrics::test_live_sync_event_count, and reconnect_backoff::
flapping_relay_handshakes_do_not_reset_exponential_backoff. Each waits
for a plain `field=value` substring (e.g. `consecutive_failures=1`) in
the relay subprocess log written by the TestRelay fixture. The tracing
fmt layer colorizes by default, so the redirected log contained
`consecutive_failures^[[0m^[[2m=^[[0m1`, which never matches.
tracing-subscriber honours NO_COLOR, so the same tests passed or failed
depending on the invoking environment: runs with NO_COLOR set produced
clean logs, interactive-launched runs produced ANSI logs and
deterministic failures.

Approach: initialize the fmt layer with
`.with_ansi(std::io::stdout().is_terminal())`. Colors are emitted only
when a human is watching a terminal; redirected output (test fixtures,
journald, pipelines) stays plain. This is a logging hygiene fix in the
binary, not a test accommodation: the defect was writing terminal
control sequences to a non-terminal stream.

Correctness assumptions: the relay binary is the only process whose
logs the fixtures scrape, and tests always redirect its stdout to a
file, so is_terminal() is deterministically false there and the log
format no longer varies with the parent environment.

Excluded scope: several sync tests remain flaky under full-suite
parallel load but pass repeatedly in isolation
(req_concurrency::startup_historic_sync_stays_within_relay_req_concurrency_limit,
live_sync::live_sync_regroups_after_filter_count_refusal,
tag_variations::test_layer3_sync_with_lowercase_e_tag). They assert on
metrics or proxy behaviour, not log text; their bounded deadlines are
exceeded when the whole suite competes for CPU. Left undiagnosed rather
than papered over with wider timeouts.

Validation: the four log-scraping tests pass individually and in full
runs after the fix; cargo fmt makes no changes; cargo clippy
--workspace --all-targets -- -D warnings is clean; full cargo test is
green except the pre-existing load flakes above, each of which passes
3/3 when run in isolation.
This commit is contained in:
DanConwayDev
2026-08-15 07:24:46 +01:00
+9 -2
View File
@@ -1,3 +1,5 @@
use std::io::IsTerminal;
use anyhow::Result;
use clap::Parser;
use tokio::signal;
@@ -70,8 +72,13 @@ async fn main() -> Result<()> {
async fn run_relay(config: Config) -> Result<()> {
// Initialize tracing with configured log level
let filter = EnvFilter::new(&config.log_level).and(SuppressRoutineDisconnects);
let subscriber =
tracing_subscriber::registry().with(tracing_subscriber::fmt::layer().with_filter(filter));
// Only colorize when stdout is a terminal: ANSI escape codes in redirected
// logs corrupt journald/file output and break log-scraping consumers.
let subscriber = tracing_subscriber::registry().with(
tracing_subscriber::fmt::layer()
.with_ansi(std::io::stdout().is_terminal())
.with_filter(filter),
);
tracing::subscriber::set_global_default(subscriber)?;
info!("Starting ngit-grasp with log level: {}", config.log_level);