feat(tracing): named instrument spans on every service method (PMS-126) #64

Merged

nrupard merged 1 commit from feat/pms-126-tracing-spans into main

2026-06-04 19:33:10 +02:00

nrupard commented

2026-06-04 19:25:21 +02:00

Owner

Summary

Adds #[tracing::instrument(skip_all, fields(tenant_id = %tenant_id))] (or skip_all only when no tenant_id parameter) to every pub async fn across the eighteen src/modules/*/service.rs files and the two workers (notifications/worker.rs, rmm/worker.rs). 249 attribute insertions; no behavior change.
skip_all is the safe default because service args include sqlx pools, Arc<dyn Mailer>, large request DTOs. Capturing tenant_id via Display keeps traces grep-able by tenant without leaking the rest of the parameter surface.
The one tenant_id: Option<Uuid> site (audit::AuditService::list) uses ?tenant_id Debug format because Option is not Display.

Test plan

CI: cargo fmt --all --check
CI: cargo clippy --all-targets -- -D warnings
CI: cargo check --all-targets
CI: cargo test --lib + existing integration tests still pass (attributes are non-behavioral)
Manual: with RUST_LOG=mokosh_server=debug, a GET /api/v1/tickets request shows a TicketService::list_tickets{tenant_id=...} span enclosing the DB call sites.

Closes PMS-126. Cross-cutting issue #4 from dev-docs/codebase-state.md.

## Summary - Adds `#[tracing::instrument(skip_all, fields(tenant_id = %tenant_id))]` (or `skip_all` only when no `tenant_id` parameter) to every `pub async fn` across the eighteen `src/modules/*/service.rs` files and the two workers (`notifications/worker.rs`, `rmm/worker.rs`). 249 attribute insertions; no behavior change. - `skip_all` is the safe default because service args include sqlx pools, `Arc<dyn Mailer>`, large request DTOs. Capturing `tenant_id` via `Display` keeps traces grep-able by tenant without leaking the rest of the parameter surface. - The one `tenant_id: Option<Uuid>` site (`audit::AuditService::list`) uses `?tenant_id` Debug format because `Option` is not `Display`. ## Test plan - [ ] CI: `cargo fmt --all --check` - [ ] CI: `cargo clippy --all-targets -- -D warnings` - [ ] CI: `cargo check --all-targets` - [ ] CI: `cargo test --lib` + existing integration tests still pass (attributes are non-behavioral) - [ ] Manual: with `RUST_LOG=mokosh_server=debug`, a `GET /api/v1/tickets` request shows a `TicketService::list_tickets{tenant_id=...}` span enclosing the DB call sites. Closes PMS-126. Cross-cutting issue #4 from `dev-docs/codebase-state.md`.

nrupard added 1 commit

2026-06-04 19:25:21 +02:00

feat(tracing): add named instrument spans on every service method (PMS-126)

Check / fmt + clippy + compile + tests (pull_request) Successful in 1m8s

Details

Build OCI container / Build and push mokosh-api image (push) Successful in 3m0s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

529e7780db

Closes cross-cutting issue #4 (`dev-docs/codebase-state.md`): the `TraceLayer` already wraps the request, but with no per-method spans the trace flattens to a single HTTP entry and the DB call sites inside a transaction body are invisible. Adds `#[tracing::instrument(skip_all, fields(tenant_id = %tenant_id))]` (or `skip_all` only when the method has no `tenant_id` parameter) to every `pub async fn` across the eighteen `src/modules/*/service.rs` files plus the two background workers (`notifications/worker.rs`, `rmm/worker.rs`). 249 attribute insertions in total; no behavior change.

`skip_all` is the safe default because most service args carry types that do not implement `Debug` cheaply (sqlx pools, `Arc<dyn Mailer>`, large request DTOs). `tenant_id` is captured via its `Display` impl so production traces stay grep-able by tenant without leaking the rest of the parameter surface. The single `tenant_id: Option<Uuid>` site (`audit::AuditService::list`) uses the `?` Debug formatter because `Option` does not implement `Display`.

#PMS-126