Skip to content

fix(server): replace production eprintln! with tracing macros (#11) - #52

Merged
rawkode merged 1 commit into
mainfrom
issue-11-tracing-eprintln
May 19, 2026
Merged

rawkode merged 1 commit into
mainfrom
issue-11-tracing-eprintln

Conversation

@rawkode

@rawkode rawkode commented May 19, 2026

Copy link
Copy Markdown
Member

Final acceptance gate on #11 ("Observability"). The graceful-shutdown path was the last production code in `crates/server/src/` using `eprintln!`. Tracing subscriber (PR #28), log rotation (PR #43), and tracing-based ops (PR #28) all landed; this closes the gap so production aggregators see a single JSON stream.

Changes

All in `crates/server/src/main.rs` (graceful-shutdown path):

  • `serve error` → `tracing::error!(%error, ...)`
  • `drain timeout` → `tracing::warn!(timeout_seconds = ..., ...)`
  • `draining complete` → `tracing::info!`
  • `events.jsonl / audit.jsonl fsync failed` → `tracing::error!` with path field
  • SIGINT / SIGTERM received → `tracing::info!`
  • `ctrl_c handler failed` → `tracing::error!`

Remaining test-only matches (permitted by acceptance)

`grep -rn 'eprintln!|println!' crates/server/src/` returns 2 hits — both inside `#[test]` functions printing "SKIP: run bundler first" hints (`wasm_host.rs:2227`, `wasm_registry.rs:649`). #11's acceptance explicitly permits these.

Verification

  • `cargo test -p comtrya-server` — 175/175 green.
  • `cargo clippy -p comtrya-server --bin comtrya-server -- -D warnings` — clean.

Closes #11.

Copilot AI review requested due to automatic review settings May 19, 2026 18:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the server’s graceful-shutdown logging path to use tracing macros instead of writing directly to stderr, aligning shutdown-time observability with the project’s structured logging pipeline for JSON aggregation.

Changes:

  • Replace eprintln! in the graceful shutdown serve/drain branches with tracing::{error,warn,info}!.
  • Emit structured fields for drain timeout and include error context on failure paths.
  • Convert SIGINT/SIGTERM and ctrl_c failure diagnostics to tracing events.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/server/src/main.rs
Comment on lines 226 to +230
if let Err(error) = fsync_jsonl_path(&events_path) {
eprintln!("graceful shutdown: events.jsonl fsync failed: {error}");
tracing::error!(%error, path = ?events_path, "graceful shutdown: events.jsonl fsync failed");
}
if let Err(error) = fsync_jsonl_path(&audit_path) {
eprintln!("graceful shutdown: audit.jsonl fsync failed: {error}");
tracing::error!(%error, path = ?audit_path, "graceful shutdown: audit.jsonl fsync failed");
Comment thread crates/server/src/main.rs
Comment on lines 214 to 216
Ok(Err(error)) => {
eprintln!("graceful shutdown: serve error: {error}");
tracing::error!(%error, "graceful shutdown: serve error");
}
Base automatically changed from v3 to main May 19, 2026 18:25
Final acceptance gate on #11. The graceful-shutdown path was the
last production code in crates/server/src/ using `eprintln!`; the
two other `eprintln!` matches are test-only "SKIP: bundler not
run" hints inside `#[test]` functions (wasm_host.rs, wasm_registry.rs),
which #11's acceptance explicitly permits ("0 or only test-only
matches").

Sites converted (all in `crates/server/src/main.rs`):
- `serve error` → `tracing::error!(%error, ...)`
- `drain timeout` → `tracing::warn!(timeout_seconds = ..., ...)`
- `draining complete` → `tracing::info!`
- `events.jsonl / audit.jsonl fsync failed` → `tracing::error!`
  with path field
- SIGINT / SIGTERM received → `tracing::info!`
- ctrl_c handler failed → `tracing::error!`

Now flow through the tracing subscriber added in PR #28 instead
of bypassing it. Production aggregators see a single JSON stream
including shutdown events.

175/175 server tests green; clippy clean.
@rawkode
rawkode force-pushed the issue-11-tracing-eprintln branch from 7ba52ac to 1c242ba Compare May 19, 2026 18:35
@rawkode
rawkode added this pull request to the merge queue May 19, 2026
Merged via the queue into main with commit 31aa013 May 19, 2026
13 checks passed
@rawkode
rawkode deleted the issue-11-tracing-eprintln branch May 19, 2026 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2] Observability: tracing, structured logs, log rotation

2 participants