docs(rfc-0001): realign §6.8 telemetry export to OTel SDK + OTLP - #103
Conversation
|
@coderabbitai review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates RFCs and the roadmap to replace Prometheus scrape/registry telemetry language with an OpenTelemetry SDK + OTLP push model, introduces an ourios-telemetry init flow that installs a MeterProvider, converts labels to attributes, updates validation/testing to use an in-memory OTel reader, and clarifies WAL replay metric visibility. ChangesOpenTelemetry Telemetry Architecture Redesign
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
![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 `@docs/rfcs/0001-template-miner.md`:
- Around line 1528-1530: The doc incorrectly lists `service` as a per-metric
attribute; update the RFC text and the metric-attribute table so `service` is
clearly a resource attribute only (use the standard OTel `service.name` resource
attribute set on the provider) and not repeated in per-instrument/metric
attributes; either move `service` into a “Resource attributes” column, remove it
from the “Instrument attributes” column, or add an explicit table note that
`service` == `service.name` is resource-scoped and must not be attached
per-metric to avoid double-attaching; ensure the same clarification is applied
to the repeated section around lines referencing 1538–1547.
🪄 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: f6fbb343-bc8a-4dfd-92d2-605a0c3f2a7f
📒 Files selected for processing (3)
docs/rfcs/0001-template-miner.mddocs/rfcs/0008-wal.mddocs/roadmap.md
There was a problem hiding this comment.
Pull request overview
This PR updates the documentation/specs to realign RFC 0001 §6.8 Telemetry from a Prometheus client + /metrics scrape model to an OpenTelemetry SDK + OTLP push exporter architecture, and propagates the associated terminology updates across referenced docs.
Changes:
- Update
docs/roadmap.mdto reflect OTLP push export (and explicitly de-scope scrape endpoints / direct Prometheus client usage). - Update
docs/rfcs/0008-wal.mdto cite the revised telemetry direction and RFC 0001 §6.8 as the normative architecture reference. - Amend
docs/rfcs/0001-template-miner.md§6.8 telemetry and related scenarios/tests to use OTel terminology (meter provider, attributes, OTLP push) and describe the intended dependency split.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| docs/roadmap.md | Updates the post-MVP telemetry roadmap note to OTLP push export framing. |
| docs/rfcs/0008-wal.md | Updates WAL RFC references/wording to match the new OTel + OTLP export direction. |
| docs/rfcs/0001-template-miner.md | Adds the 2026-06-03 telemetry architecture amendment and updates scenarios/terminology accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Amend RFC 0001 §6.8 to bless the OpenTelemetry SDK + OTLP push exporter as the metrics-export architecture, superseding the Prometheus client/scrape model. Specifies the OTel API/SDK dependency split (library crates depend only on the `opentelemetry` API via `global::meter`; a new `ourios-telemetry` crate owns the SDK/OTLP/transport deps and the MeterProvider lifecycle) and blesses the new crate per CLAUDE.md §7. Fixes the Prometheus-era terminology (registry -> meter provider, scrape -> OTLP push, labels -> attributes) across §§6.8-6.9, the §3.1.2 / RFC0001.8 scenarios, and the §8 metrics test, and updates the roadmap §5 note and RFC 0008's citations to match. Deliberately does NOT rename the metrics: the dotted-semconv redesign (joining semconv/registry/ alongside the compaction set, RFC 0009 §3.6) and the instrument-type / quantile questions are genuine contract changes to the §3.1.2 mandatory set and are deferred to their own reviewed follow-up. RFC0001.8's p50/p01 assertion is preserved, flagged for that redesign. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
05e69f6 to
e0c153f
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
…port to OTel SDK + OTLP
|
@coderabbitai review |
✅ Action performedReview finished.
|
…etry export to OTel SDK + OTLP
|
@coderabbitai review |
✅ Action performedReview finished.
|
…8 telemetry export to OTel SDK + OTLP
|
@coderabbitai review |
✅ Action performedReview finished.
|
…bootstrap) (#104) * feat(telemetry): scaffold ourios-telemetry crate (OTLP MeterProvider bootstrap) First slice of the telemetry code work unblocked by RFC 0001 §6.8 (the OTel SDK + OTLP-push amendment, #103) and RFC 0009 §3.6 (#102). Implements the §6.8 "Export architecture" dependency split: this is the single crate that carries the heavy OTel SDK + OTLP exporter + transport, so instrumented library crates can depend only on the lightweight `opentelemetry` API (`global::meter(...)`). - `init(&TelemetryConfig)` builds the OTLP push MeterProvider (periodic reader, configurable interval), sets `service.name = ourios-<role>` as a Resource attribute (not a data-point attribute, per §6.8), installs it as the process-global provider, and returns a `TelemetryGuard` whose `shutdown()` flushes on exit (Drop flushes best-effort). - `init_in_memory` (behind the `testing` feature) wires an InMemoryMetricExporter for other crates' tests (e.g. the miner's §8 metric-collection test) to collect the exported stream without an OTLP endpoint. No instrumentation yet — that lands in the follow-on slices (weaver name-constant codegen, then compaction instrumentation). Error type is hand-rolled to match the workspace convention (no thiserror dep). Verification: cargo fmt --check, clippy --all-targets --all-features -D warnings, and cargo test --workspace --all-features all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fixup! feat(telemetry): scaffold ourios-telemetry crate (OTLP MeterProvider bootstrap) * fixup! fixup! feat(telemetry): scaffold ourios-telemetry crate (OTLP MeterProvider bootstrap) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Amends RFC 0001 §6.8 (Telemetry) to bless the OpenTelemetry SDK + OTLP push exporter as the metrics-export architecture, superseding the Prometheus client/scrape model (maintainer direction 2026-06-03, already pinned for compaction in RFC 0009 §3.6). This is the prerequisite slice the maintainer chose before standing up the MeterProvider bootstrap and instrumenting compaction (epic #94 audit+telemetry).
Decisions blessed here
ourios-miner,-parquet,-ingester,-querier) depend only on the lightweightopentelemetryAPI and resolve instruments viaglobal::meter("ourios.<subsystem>")— no SDK / OTLP / transport in any library crate.ourios-telemetrycrate ownsopentelemetry_sdk+opentelemetry-otlp+ transport;init()builds the OTLP push MeterProvider (periodic reader, configurable interval), installs it as the process-global provider, returns a shutdown/flush guard.ourios-servercalls it at start-up; benches/tests substitute an in-memory reader.servicedimension is theservice.nameresource attribute.Terminology fixes (maintainer-sanctioned, check preserved)
Registry → meter provider, scrape → OTLP push, labels → attributes — across §§6.8–6.9, the §3.1.2 + RFC0001.8 scenarios, and the §8 metrics-registry test. The §3.1.2 invariant (full mandatory set is exposed) is unchanged — only the mechanism term changes. Roadmap §5 note and RFC 0008's two citations updated to the OTLP-push framing.
Deliberately deferred (NOT in this PR)
Per CLAUDE.md §6.2 I did not silently rewrite the normative mandatory-set contract. The dotted-semconv name redesign (
template_count→ourios.miner.*, joiningsemconv/registry/alongside the compaction set), the instrument-type changes, and the p50/p01 quantile question (RFC0001.8 — in-process gauges vs OTLP-native backend-derived quantiles) are genuine contract changes to the §3.1.2 set and are deferred to their own reviewed follow-up. RFC0001.8's assertion is preserved as-is, explicitly flagged for that redesign.Invariants touched
§3.1 (mandatory metric set): exposure invariant preserved verbatim; only export mechanism + terminology reframed. No metric removed or renamed.
Verification
mdbook build✓ourios-telemetrycrate + bootstrap land in the follow-up code slice once these names/architecture are blessed.🤖 Generated with Claude Code
Summary by CodeRabbit