feat(telemetry): tracer foundation for self-tracing (RFC 0038 slice 1) - #614
Conversation
Adds the OTel traces signal to the telemetry bootstrap: an OTLP SdkTracerProvider + a tracing-opentelemetry layer, so tracing spans become OTel spans and the appender bridge stamps the active span's trace_id/span_id onto every log record — fixing the reported gap where Ourios's own MCP logs carried no trace context (rmcp's serve_inner span now correlates them, no Ourios-side span needed yet). - TelemetryConfig gains traces_enabled (default on) + trace_sample_ratio (None -> parentbased_always_on; Some(r) -> parentbased_traceidratio). - The loop-guard filter is shared (guarded_env_filter) and now also mutes the trace layer, so the exporter's own tonic/hyper spans can't feed back (RFC0038.5). - TelemetryGuard owns the tracer and flushes it on shutdown + Drop (RFC0038.6). traces_enabled=false installs no tracer (RFC0038.4 disable). Test rfc0038_1_log_within_a_span_carries_trace_context: a log emitted inside a tracing span carries the span's non-zero trace_id/span_id (RFC0038.1 correlation, unit slice). Request-scoped spans on the query / MCP / ingest-batch / sweep paths are the next slice; the hot path stays span-free. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 38 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 (2)
📝 WalkthroughWalkthroughThe telemetry crate enables OpenTelemetry tracing, adds configurable sampling, installs a conditional OTLP trace pipeline, correlates logs with active spans, and extends guard shutdown and tests to cover tracer lifecycle management. ChangesTelemetry tracing support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant tracing_opentelemetry as tracing-opentelemetry layer
participant SdkTracerProvider
participant OTLPExporter as OTLP span exporter
participant OTelLogBridge as OTel log bridge
Application->>tracing_opentelemetry: enter span and emit tracing event
tracing_opentelemetry->>SdkTracerProvider: record span context
tracing_opentelemetry->>OTelLogBridge: forward event with active context
OTelLogBridge->>OTelLogBridge: attach trace_id and span_id
SdkTracerProvider->>OTLPExporter: batch export spans
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
Adds first “foundation slice” of RFC 0038 self-tracing by introducing an OTLP-backed SdkTracerProvider and wiring tracing-opentelemetry so tracing spans become OTel spans and log records are correlated with trace_id/span_id. This extends the existing telemetry bootstrap (metrics + OTLP-exported logs) with a traces signal, shared loop-guard filtering, and tracer shutdown via TelemetryGuard.
Changes:
- Add conditional traces pipeline (
traces_enabled,trace_sample_ratio) and install atracing-opentelemetrylayer alongside the existing OTLP log bridge. - Centralize the telemetry-induced-telemetry loop guard into
guarded_env_filter()and apply it to both the log bridge and traces layer. - Add a unit test asserting that logs emitted inside a
tracingspan carry non-zero trace context, and addtracing-opentelemetrydependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/ourios-telemetry/src/lib.rs | Builds/installs OTLP tracer provider + tracing layer, adds config knobs and loop-guard reuse, and adds correlation test. |
| crates/ourios-telemetry/Cargo.toml | Enables OTel trace features and adds tracing-opentelemetry dependency. |
| Cargo.lock | Locks new dependency graph entries for traces support (e.g., tracing-opentelemetry). |
Comments suppressed due to low confidence (1)
crates/ourios-telemetry/src/lib.rs:355
- To avoid leaving the global tracer provider in an invalid state when
try_init()fails, only install the global tracer provider after the subscriber stack was successfully installed.
let (logger, tracer) = if installed {
(Some(logger), tracer)
} else {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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-telemetry/src/lib.rs`:
- Around line 287-334: Defer global tracer registration until after subscriber
installation succeeds: remove the unconditional global::set_tracer_provider call
from the pre-try_init setup, then register the tracer provider only when
try_init() reports installed == true. Preserve the existing shutdown behavior
for providers that lose the installation race, while keeping the provider alive
when installation fails so later initialization can still use it.
🪄 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: b9412cc3-baca-4921-ac7a-1fe9fd00a64b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
crates/ourios-telemetry/Cargo.tomlcrates/ourios-telemetry/src/lib.rs
Adds tracing-opentelemetry 0.33.0 (MIT) to THIRD-PARTY-LICENSES.md via cargo about generate — the RFC 0038 slice-1 dependency. No new transitive crates; MIT tally 71 -> 72. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
crates/ourios-telemetry/src/lib.rs:326
global::set_tracer_provider(...)is called beforetry_init(). If the subscriber is already installed, this function tears down the tracer provider and returns a guard withtracer: None, but the global tracer provider remains set (now pointing at a shut-down provider). This can leak a broken global tracer into the rest of the process/test harness. Set the global tracer provider only after the subscriber is successfully installed (or avoid setting it at all in this slice).
global::set_meter_provider(provider.clone());
if let Some(tracer_provider) = &tracer {
global::set_tracer_provider(tracer_provider.clone());
}
crates/ourios-telemetry/src/lib.rs:355
- If you do want a process-global tracer provider for future
global::tracer(...)call sites, install it only when the subscriber chain was successfully installed. This avoids leaving a global tracer provider set whentry_init()fails (e.g., under a test harness).
let (logger, tracer) = if installed {
(Some(logger), tracer)
} else {
crates/ourios-telemetry/src/lib.rs:308
TelemetryConfig::trace_sample_ratiois documented as being in[0.0, 1.0], but the value is passed through unvalidated. Clamping (and handling non-finite values) makes the implementation match the documented contract and avoids surprising behavior if a config parser produces values outside the range (or NaN/inf).
let sampler = match config.trace_sample_ratio {
Some(ratio) => Sampler::ParentBased(Box::new(Sampler::TraceIdRatioBased(ratio))),
None => Sampler::ParentBased(Box::new(Sampler::AlwaysOn)),
};
…clamp ratio Two review fixes: - global::set_tracer_provider ran before try_init, so a lost subscriber-install race shut the provider down while the global still pointed at it (a dead tracer for the process). Move the global registration into the try_init-succeeded branch; the metrics global stays set early (it is kept regardless of the subscriber outcome). - trace_sample_ratio is documented [0.0, 1.0] but passed through raw; clamp defensively at the boundary (the RFC 0038 §3.4 config-file layer still rejects out-of-range at startup — this only guards a direct library caller). Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
First implementation slice of RFC 0038 (self-tracing,
specified). Adds the OTel traces signal to the telemetry bootstrap — the foundation the request-scoped spans hang off — and fixes the reported symptom: Ourios's own logs carried notrace_id/span_id.What lands
ourios-telemetry: an OTLPSdkTracerProvider(batch span exporter) + atracing-opentelemetrylayer. Once the layer exists,tracingspans become OTel spans and the existing appender bridge automatically stamps the active span'strace_id/span_idonto every log record. rmcp's ownserve_innerspan now correlates the MCP logs — no Ourios-side span needed for the fix.TelemetryConfig.traces_enabled(default on) +trace_sample_ratio(None→parentbased_always_on;Some(r)→parentbased_traceidratio), per RFC 0038 §3.4.guarded_env_filterapplied to both the appender bridge and the trace layer, so the exporter's own tonic/hyper spans can't feed back.TelemetryGuardowns the tracer and shuts it down onshutdown()+Drop, alongside the logger/meter.traces_enabled: falseinstalls no tracer (the RFC0038.4 disable path).Discipline (unchanged from the RFC)
This slice is the foundation only — it adds no request spans yet. The per-record ingest hot path stays span-free; the query / MCP / per-
Export-batch / sweep spans are the next slice.Verification
cargo test -p ourios-telemetry --features testing→ 3 passed, incl.rfc0038_1_log_within_a_span_carries_trace_context: a log emitted inside atracingspan carries the span's non-zerotrace_id/span_id(RFC0038.1 correlation, unit slice — and proof the appender↔tracing-opentelemetry correlation works with no extra wiring).cargo clippy -p ourios-telemetry -p ourios-server --all-targets --all-features -- -D warnings→ clean (server'sinit()call compiles against the defaulted fields).cargo fmt --all --check→ clean.tracing-opentelemetry 0.33aligns with the pinnedopentelemetry 0.32(the §7 version question — resolved).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes