feat(telemetry): request-scoped spans on ingest, query, mcp, sweep (RFC 0038 slice 2) - #615
Conversation
…ep (RFC 0038) Slice 2 of RFC 0038, on the slice-1 tracer foundation (#614). Opens the request-scope spans pinned in §3.5 at the four entry points: - POST /v1/query (SERVER) on the querier handler - ourios.mcp.{query_logs,list_templates,template_drift} (INTERNAL), children of rmcp's serve_inner span - ourios.ingest.batch (SERVER) per OTLP Export, with an ourios.wal.commit INTERNAL child over the group-commit await - ourios.compaction.sweep (INTERNAL) The batch span is opened once per Export, never per record: the miner, encode-pool, and record-sink inner loops stay span-free (§3.2). The spans open inside the spawned callees (ingest_bound, the sweep body), not at the tokio::spawn / spawn_blocking sites, because span context does not cross those boundaries (§3.3). wal.commit is a child span, not an event, because the group-commit fsync is the one durationful, I/O-bound step. All spans skip_all — no payload on the span. rfc0038_2_ingest_batch_span.rs (scoped WithSubscriber over an InMemorySpanExporter, no process-global tracer) asserts RFC0038.2: a 1-record and a 64-record batch emit the same span count (2 = batch + wal.commit child, correctly parented, no further sub-spans) — O(1) in record count, the ingest arm of RFC0038.1. RFC0038.3 (spawn-boundary correlation) and the query/MCP/sweep arms of RFC0038.1 follow in slice 3; the config-file telemetry.traces.* + sampler precedence (RFC0038.4) in slice 4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds RFC 0038 tracing instrumentation for ingest batches, WAL commits, compaction sweeps, query requests, and MCP tools. Extends ingester test dependencies, configures trace span attributes, and adds integration tests validating ingest span parentage and constant span cardinality. ChangesTelemetry instrumentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant OTLPClient
participant ingest_bound
participant WALCommit
participant SpanExporter
OTLPClient->>ingest_bound: Send OTLP batch
ingest_bound->>WALCommit: Commit payload
WALCommit-->>ingest_bound: Return commit outcome
ingest_bound->>SpanExporter: Export batch and WAL spans
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Implements RFC 0038 slice 2 by adding request-scoped tracing spans at the main service entry points (query HTTP handler, MCP tools, ingest pipeline batch/commit boundary, and compaction sweep), with an integration test enforcing O(1) span count per OTLP export to protect ingest hot-path overhead.
Changes:
- Add
#[tracing::instrument(skip_all, …)]request spans for query (POST /v1/query), MCP tools, ingest batches, and compaction sweeps. - Add a
ourios.wal.commitchild span around the group-commit await inIngestPipeline::ingest_bound. - Add an integration test asserting ingest span structure and constant span count (1 vs 64 records) using an in-memory span exporter and scoped subscriber.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-server/src/querier.rs | Adds a request-scoped server span for the POST /v1/query handler. |
| crates/ourios-server/src/mcp.rs | Adds per-tool internal spans for MCP tool methods to correlate tool calls under rmcp’s request span. |
| crates/ourios-ingester/src/receiver/pipeline.rs | Adds an ourios.ingest.batch span and an ourios.wal.commit child span around the group-commit await. |
| crates/ourios-ingester/src/compactor.rs | Adds a coarse ourios.compaction.sweep span around a sweep invocation. |
| crates/ourios-ingester/tests/it/rfc0038_2_ingest_batch_span.rs | New integration tests asserting span structure and O(1) span count per export batch. |
| crates/ourios-ingester/tests/it/main.rs | Registers the new RFC0038.2 integration test module in the consolidated harness. |
| crates/ourios-ingester/Cargo.toml | Adds dev-deps/features needed for in-memory trace export + scoped subscriber in tests. |
| Cargo.lock | Locks new dev-dependency entries used by the ingest span tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
❌ 2 Tests Failed:
View the top 2 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ourios-ingester/tests/it/rfc0038_2_ingest_batch_span.rs`:
- Around line 61-110: Extend the tests around
rfc0038_2_ingest_batch_emits_one_span_with_a_wal_commit_child and
rfc0038_2_span_count_is_constant_in_record_count to validate each emitted span’s
OTLP kind via span_context.trace_flags and cover the swept tracing spans defined
under the ingester source. Add assertions for expected span names, parent-child
relationships, and constant span counts where applicable, ensuring the new
regression coverage exercises the full span Kind contract rather than only IDs
and names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 188042dc-40d8-44ed-8d07-18998fea9153
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/ourios-ingester/Cargo.tomlcrates/ourios-ingester/src/compactor.rscrates/ourios-ingester/src/receiver/pipeline.rscrates/ourios-ingester/tests/it/main.rscrates/ourios-ingester/tests/it/rfc0038_2_ingest_batch_span.rscrates/ourios-server/src/mcp.rscrates/ourios-server/src/querier.rs
… cleanliness
The RFC 0038 request spans (slice 2) made ourios-server emit spans to the
CI weaver live-check for the first time. tracing-opentelemetry injects
four attributes that fail `weaver registry live-check`: busy_ns/idle_ns
(tracked inactivity) and target carry no semconv namespace, and
with_location emits code.module.name, which collides with the upstream
code namespace. All four are instrumentation-source metadata, not domain
telemetry — our spans carry their identity in the span name plus the
attributes we add deliberately.
Disable tracked_inactivity, location, and target on the layer.
thread.{id,name} are valid semconv (no advice) and stay. Verified with a
local `weaver registry live-check` (v0.23.0, matching CI): the query span
now carries only thread.{id,name}, zero violation-level advisories (was
4), server + weaver both exit 0.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
Putting #[tracing::instrument] directly on the rmcp #[tool] methods dropped the doc-comment tool description the macro derives, breaking RFC0027.7 (output discipline) and RFC0032.5 (description placement) in CI. Keep each #[tool] method a thin delegate — doc comment and #[tool] attribute untouched, so description derivation is restored — and move the span onto a private *_traced helper it calls. The span is still one per tool call, a child of rmcp's serve_inner. Both tests pass again locally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
…t spans Per review: the ingest span tests checked names/parentage/count but not the §3.5 kind contract. Assert ourios.ingest.batch is a SERVER span and its ourios.wal.commit child is INTERNAL — verifying the otel.kind fields survive export as SpanData.span_kind, not just the ids and names. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
RFC 0038 slice 2 — request-scoped spans
Builds on the slice-1 tracer foundation (#614). This PR adds the spans at all four entry points pinned in RFC 0038 §3.5:
POST /v1/queryquerier::handle_queryourios.mcp.{query_logs,list_templates,template_drift}serve_inner)ourios.ingest.batchIngestPipeline::ingest_boundourios.wal.commitourios.compaction.sweeprun_sweep_with_promotedAll are
skip_all— no request/headers/body payload on the span.Hot-path discipline (hazard: tracing overhead)
The batch span is opened once per OTLP
Export, never per record. The miner / encode-pool / record-sink inner loops stay span-free (RFC 0038 §3.2). The two spans a batch produces (ingest.batch+ onewal.commitchild) are O(1) in the record count, which the new test asserts directly: a 1-record batch and a 64-record batch emit the same span count (2).ourios.wal.commitis a child span, not an event, because the group-commit fsync is the one genuinely I/O-bound, durationful step.The spans are opened inside the spawned callees (
ingest_bound, the sweep body), not at thetokio::spawn/spawn_blockingsites — span context does not cross those boundaries (RFC 0038 §3.3).Semconv cleanliness
tracing-opentelemetryinjects synthetic span attributes (busy_ns/idle_ns,target,code.module.name) that failweaver registry live-check. The telemetry layer now disables tracked-inactivity, location, and target;thread.{id,name}(valid semconv) stay. Verified against local weaver v0.23.0 (CI pin): zero violation-level advisories.MCP span placement
#[tracing::instrument]directly on an rmcp#[tool]method drops the doc-comment tool description the macro derives (would break RFC0027.7 / RFC0032.5). Each#[tool]method stays a thin delegate — doc + attribute intact — and calls a private*_tracedhelper that carries the span.Tests
rfc0038_2_ingest_batch_span.rs(consolidated it-harness, scopedWithSubscriberover anInMemorySpanExporter— no process-global tracer):rfc0038_2_ingest_batch_emits_one_span_with_a_wal_commit_child— oneingest.batch(SERVER) span, onewal.commit(INTERNAL) child correctly parented, no further sub-spans, and theotel.kindcontract on both (the ingest arm of RFC0038.1).rfc0038_2_span_count_is_constant_in_record_count— 1 vs 64 records → identical span count (RFC0038.2, O(1)).What lands in later slices
The spans above are all present in this PR. What remains is test coverage and config, not the spans themselves:
telemetry.traces.*+ sampler precedence/validation (RFC0038.4).🤖 Generated with Claude Code