Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/skills/memory/memories/healthcheck-redirect-fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Fixed a critical bug where newly generated projects failed to start because the

1. **Redirect policy** (`crates/core/src/dev/process.rs`): Added `.redirect(reqwest::redirect::Policy::none())` to `HEALTH_CLIENT`. A 302 is a valid HTTP response proving the server is listening -- no need to follow it.

2. **Log path canonicalization** (`crates/core/src/dev/process.rs`): `ProcessManager::new()` now canonicalizes `app_dir` so that `forward_log_to_flux()` uses the same path as `StartupLogStreamer`. On macOS, `/tmp/foo` canonicalizes to `/private/tmp/foo`, and the mismatch caused startup logs to appear empty.
2. **Log path canonicalization** (`crates/core/src/dev/process.rs`): `ProcessManager::new()` now canonicalizes `app_dir` so that `forward_log_to_collector()` uses the same path as `StartupLogStreamer`. On macOS, `/tmp/foo` canonicalizes to `/private/tmp/foo`, and the mismatch caused startup logs to appear empty.

3. **Module extraction**: Moved `wait_for_healthy_with_logs()` from `ops/dev.rs` to new `ops/healthcheck.rs` for better separation of concerns.

Expand Down
18 changes: 9 additions & 9 deletions .claude/skills/memory/memories/logging-refactor.md
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
---
name: logging-refactor
created: 2026-02-21
tags: [logging, refactor, apx_common, format, severity, flux, agent]
tags: [logging, refactor, apx_common, format, severity, collector, agent]
---

# Logging System Refactoring

## Summary

Centralized all log formatting, timestamp handling, severity parsing, and skip-filtering into `apx_common::format`. Fixed stderr severity mapping (uvicorn INFO was stored as ERROR), added agent version checking in flux daemon, and removed dead code.
Centralized all log formatting, timestamp handling, severity parsing, and skip-filtering into `apx_common::format`. Fixed stderr severity mapping (uvicorn INFO was stored as ERROR), added agent version checking in collector daemon, and removed dead code.

## Context

The APX logging system had grown organically across 6+ crates with inconsistent patterns:
- 5 different timestamp formats (UTC vs Local, with/without date)
- All uvicorn stderr was tagged as ERROR (severity 17) — even normal `INFO: Uvicorn running...`
- Duplicate `should_skip_log()` implementations with different filter sets
- No agent version check — old flux daemon kept running after apx upgrade
- No agent version check — old collector daemon kept running after apx upgrade
- Dead code: `APX_COLLECT_LOGS` env var, `installed_version()` function

### Key decisions

1. **All user-facing timestamps use Local timezone** — `format_timestamp()` always converts to local. The old `format_log_line` in process.rs used UTC; now uses Local.
2. **Single `format_startup_log` uses full timestamp** (`YYYY-MM-DD HH:MM:SS.mmm`), not short (`HH:MM:SS.mmm`). User explicitly requested this.
3. **Stderr severity parsing** via `parse_python_severity()` — matches patterns like `INFO /`, `WARNING:`, `ERROR /` etc. Defaults to `"INFO"` since most uvicorn stderr is informational.
4. **Version check via lock file** — `FluxLock` now has `version: Option<String>` (with `#[serde(default)]` for backward compat). `ensure_running()` compares lock version to `apx_common::VERSION`.
4. **Version check via lock file** — `CollectorLock` now has `version: Option<String>` (with `#[serde(default)]` for backward compat). `ensure_running()` compares lock version to `apx_common::VERSION`.
5. **Unified skip filtering** — `should_skip_log_message(message: &str)` is the raw-string entry point; `should_skip_log(&LogRecord)` wraps it with severity-based _core filtering. Password patterns (`WITH PASSWORD`, `PASSWORD '`) now filtered in both paths.

## Diagram
Expand Down Expand Up @@ -65,16 +65,16 @@ graph TD

- `crates/common/src/format.rs` — **NEW** centralized formatting module
- `crates/common/src/storage.rs` — unified `should_skip_log` + `should_skip_log_message`
- `crates/common/src/lib.rs` — `VERSION` const, `FluxLock.version` field, `pub mod format`
- `crates/common/src/lib.rs` — `VERSION` const, `CollectorLock.version` field, `pub mod format`
- `crates/core/src/dev/process.rs` — uses `parse_python_severity` for stderr, `format_process_log_line` for timestamps
- `crates/core/src/dev/otel.rs` — removed duplicate `severity_to_number` and `should_skip_log`
- `crates/core/src/ops/logs.rs` — removed local format fns, imports from `apx_common::format`
- `crates/core/src/ops/startup_logs.rs` — removed local format fns, imports from `apx_common::format`
- `crates/core/src/flux/mod.rs` — `ensure_running()` checks version, restarts on mismatch
- `crates/core/src/collector/mod.rs` — `ensure_running()` checks version, restarts on mismatch
- `crates/cli/src/dev/logs.rs` — updated imports to `apx_common::format`
- `crates/agent/src/main.rs` — tracing uses `APX_LOG` env, stderr writer, file/line info
- `crates/agent/src/server.rs` — replaced `eprintln!` with `info!()`
- `crates/core/src/agent.rs` — removed dead `installed_version()`
- `crates/tracing/src/main.rs` — tracing uses `APX_LOG` env, stderr writer, file/line info
- `crates/tracing/src/server.rs` — replaced `eprintln!` with `info!()`
- `crates/core/src/tracing_binary.rs` — removed dead `installed_version()`
- `crates/core/src/ops/dev.rs` — removed dead `APX_COLLECT_LOGS` env var

## Notes
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ jobs:
uses: actions/cache@v5
with:
path: .bins/agent
key: apx-agent-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('crates/agent/**', 'crates/common/**', 'Cargo.lock') }}
key: apx-agent-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('crates/tracing/**', 'crates/common/**', 'Cargo.lock') }}

- name: Build apx-agent
if: steps.cache-agent.outputs.cache-hit != 'true'
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
uses: actions/cache@v5
with:
path: .bins/agent
key: apx-agent-${{ runner.os }}-${{ matrix.target }}-${{ hashFiles('crates/agent/**', 'crates/common/**', 'Cargo.lock') }}
key: apx-agent-${{ runner.os }}-${{ matrix.target }}-${{ hashFiles('crates/tracing/**', 'crates/common/**', 'Cargo.lock') }}

# Map matrix target to Rust target for agent build
- name: Build apx-agent (Linux x86_64)
Expand Down Expand Up @@ -223,7 +223,7 @@ jobs:
uses: actions/cache@v5
with:
path: .bins/agent
key: apx-agent-${{ runner.os }}-${{ matrix.rust_target }}-${{ hashFiles('crates/agent/**', 'crates/common/**', 'Cargo.lock') }}
key: apx-agent-${{ runner.os }}-${{ matrix.rust_target }}-${{ hashFiles('crates/tracing/**', 'crates/common/**', 'Cargo.lock') }}

- name: Build apx-agent (native)
if: steps.cache-agent.outputs.cache-hit != 'true' && !matrix.cross
Expand Down
Loading