Migrate the DAO-layer context threading to tracing #141

New Issue

2026-09-29T14:57:11Z

cameron commented

2026-09-29 14:57:11 +00:00

Part of #135

Work

Convert the DAO-layer manual OTel context threading to tracing and drop the params. The naming is inconsistent today:

ctx: &opentelemetry::Context - faces.rs
context: &opentelemetry::Context - tags.rs
database/mod.rs - threads none
Handlers currently use Context::current_with_span(span) and pass it down.
Replace DAO spans with #[instrument] (or explicit info_span!) so DB spans still nest under the request span.
Drop the context params from DAO trait methods and call sites.
Confirm DB spans still appear nested in the baseline comparison.

Resolution

Confirmation DAO spans nest as before with no context params.

Part of #135 ## Work Convert the DAO-layer manual OTel context threading to `tracing` and drop the params. The naming is inconsistent today: - `ctx: &opentelemetry::Context` - `faces.rs` - `context: &opentelemetry::Context` - `tags.rs` - `database/mod.rs` - threads none - Handlers currently use `Context::current_with_span(span)` and pass it down. - Replace DAO spans with `#[instrument]` (or explicit `info_span!`) so DB spans still nest under the request span. - Drop the context params from DAO trait methods and call sites. - Confirm DB spans still appear nested in the baseline comparison. ## Resolution Confirmation DAO spans nest as before with no context params.

cameron added a new dependency 2026-09-29 14:57:55 +00:00

#138 Wire the tracing subscriber (unify dev console + release OTLP)

cameron added a new dependency 2026-09-29 14:57:57 +00:00

#143 Verify parity against the baseline

cameron referenced this issue

2026-09-29 17:14:06 +00:00

Add the per-request root span (middleware) and drop per-handler boilerplate #139

cameron referenced this issue

2026-09-29 17:16:40 +00:00

Wayfinder: migrate spans from raw OTel SDK to tracing + tracing-opentelemetry #135

cameron referenced this issue

2026-09-29 18:36:03 +00:00

Migrate the LLM client + streaming spans to tracing #140

cameron self-assigned this 2026-09-29 18:45:07 +00:00

cameron commented

2026-09-29 19:09:01 +00:00

Resolved on feature/tracing-migration (commit a59894a).

What was done

DAO spans without context. otel::trace_db_call(query_type, operation, f) opens a tracing span on the caller's ambient span, entered for the synchronous DAO body. The exported name is still db.<type>.<op> (via otel.name), with the same db.query_type / db.operation attrs and Ok/Error status. The closure gets &tracing::Span, so its set_attribute calls are unchanged.
All context params are gone. cx / ctx / context (and span_context / otel_context) params are removed from every DAO trait method, impl and call site. That covers all 195 trace_db_call sites, the helpers/drains that only threaded the param (backfill, watcher, knowledge, reels, face_watch…), the bins and the tests (their fn ctx() helpers are deleted). The ticket said database/mod.rs "threads none", but it had ~90 sites; all converted. The naming drift is moot, since no param remains to rename.
Handler spans. The remaining request-handler spans (files, tags, faces, image, video, favorites, duplicates, memories) are now #[tracing::instrument(name = ..., skip_all)]. The Context::current_with_span / with_remote_span_context plumbing is gone, and so is extract_context_from_request: the tracing-actix-web root span replaces it. otel::set_span_attributes replaces BoxedSpan::set_attributes.
Parenting answer: the ambient span is enough. Trait methods don't need #[instrument], because the single trace_db_call funnel creates the span inside whatever span is current.
CLAUDE.md now documents the conventions: no context threading, #[instrument], .instrument across tokio::spawn, and no enter() held across an await.

Verification

New tests/otel_db_spans.rs checks, for spans created inside an instrumented tokio::spawn on a multi-thread runtime: the name db.query.get_tags, the db.* attrs plus closure-recorded attrs, Ok/Error status, and parent = ambient span. otel_traceparent now uses an #[instrument] handler.
cargo clippy --all-targets: 0 warnings. Full suite green: 690 lib + 775 integration + 4 OTel regression tests + doc-tests.

Notes for parity (#143)

DB spans used to land in their own single-span traces, or under whatever Context::new() gave them. They now nest under the request / feature span, so traces get deeper.
DB spans now come from the image-server tracer scope instead of db.
The zero-length file write span in upload_image (started and immediately dropped, no children) was removed.
Left for #142: the raw background spans in thumbnails.rs, video/actors.rs, video/mod.rs, and the RefreshThumbnailsMessage span in files.rs.

Resolved on `feature/tracing-migration` (commit a59894a). **What was done** - **DAO spans without context.** `otel::trace_db_call(query_type, operation, f)` opens a `tracing` span on the caller's ambient span, entered for the synchronous DAO body. The exported name is still `db.<type>.<op>` (via `otel.name`), with the same `db.query_type` / `db.operation` attrs and Ok/Error status. The closure gets `&tracing::Span`, so its `set_attribute` calls are unchanged. - **All context params are gone.** `cx` / `ctx` / `context` (and `span_context` / `otel_context`) params are removed from every DAO trait method, impl and call site. That covers all 195 `trace_db_call` sites, the helpers/drains that only threaded the param (backfill, watcher, knowledge, reels, face_watch…), the bins and the tests (their `fn ctx()` helpers are deleted). The ticket said `database/mod.rs` "threads none", but it had ~90 sites; all converted. The naming drift is moot, since no param remains to rename. - **Handler spans.** The remaining request-handler spans (files, tags, faces, image, video, favorites, duplicates, memories) are now `#[tracing::instrument(name = ..., skip_all)]`. The `Context::current_with_span` / `with_remote_span_context` plumbing is gone, and so is `extract_context_from_request`: the tracing-actix-web root span replaces it. `otel::set_span_attributes` replaces `BoxedSpan::set_attributes`. - **Parenting answer:** the ambient span is enough. Trait methods don't need `#[instrument]`, because the single `trace_db_call` funnel creates the span inside whatever span is current. - **CLAUDE.md** now documents the conventions: no context threading, `#[instrument]`, `.instrument` across `tokio::spawn`, and no `enter()` held across an await. **Verification** - New `tests/otel_db_spans.rs` checks, for spans created inside an instrumented `tokio::spawn` on a multi-thread runtime: the name `db.query.get_tags`, the `db.*` attrs plus closure-recorded attrs, Ok/Error status, and parent = ambient span. `otel_traceparent` now uses an `#[instrument]` handler. - `cargo clippy --all-targets`: 0 warnings. Full suite green: 690 lib + 775 integration + 4 OTel regression tests + doc-tests. **Notes for parity (#143)** - DB spans used to land in their own single-span traces, or under whatever `Context::new()` gave them. They now nest under the request / feature span, so traces get deeper. - DB spans now come from the `image-server` tracer scope instead of `db`. - The zero-length `file write` span in `upload_image` (started and immediately dropped, no children) was removed. - Left for #142: the raw background spans in `thumbnails.rs`, `video/actors.rs`, `video/mod.rs`, and the `RefreshThumbnailsMessage` span in `files.rs`.

cameron closed this issue

2026-09-29 19:09:02 +00:00

cameron referenced this issue from a commit

2026-09-29 21:41:49 +00:00

Tracing: per-request root span via tracing-actix-web middleware (#139)

cameron referenced this issue from a commit

2026-09-29 21:41:49 +00:00

Tracing: LLM clients + streaming spans on tracing, drop cx threading (#140)

cameron referenced this issue from a commit

2026-09-29 21:41:49 +00:00

Tracing: DAO layer + handler spans on tracing, drop all Context params (#141)

cameron referenced this issue