Skip to content

feat(ingester): ingest + sink metrics for perf observability - #247

Merged
jensholdgaard merged 2 commits into
mainfrom
feat/perf-metrics-ingest-sink
Jun 18, 2026
Merged

feat(ingester): ingest + sink metrics for perf observability#247
jensholdgaard merged 2 commits into
mainfrom
feat/perf-metrics-ingest-sink

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Why

The ingest front (OTLP → WAL → miner handoff) and the RFC 0014 record sink emitted no metrics, so a performance run can't see ingest throughput, the WAL-before-ack latency, or flush behaviour. This fills those blind spots ahead of a perf test (CLAUDE.md §6.3 — "every subsystem exposes metrics").

What

8 new instruments + 1 attribute, authored in semconv/registry/ and regenerated into ourios-semconv (the semconv no-diff CI gate stays green; names verified against the OTel metric-naming spec — no units in names, correct pluralization, UpDownCounter not pluralized, usage for buffer-vs-ceiling):

Metric Instrument Unit
ourios.ingest.records counter {record}
ourios.ingest.batches counter {batch}
ourios.wal.append.duration histogram s
ourios.sink.flush.duration histogram s [trigger]
ourios.sink.flush.records counter {record} [trigger]
ourios.sink.flush.errors counter {error}
ourios.sink.derive.errors counter {error}
ourios.sink.buffer.usage updowncounter By

+ ourios.sink.flush.trigger (size | age | rotation | ceiling).

  • IngestMetrics — records throughput + the durable-commit latency (group-commit window + fsync) on each acked batch in IngestPipeline::ingest.
  • SinkMetrics — records flush duration/rows by trigger, flush/derive errors, and buffer occupancy in ParquetRecordSink.
  • Both resolve through the global meter (RFC 0001 §6.8 API/SDK split) — no-op when no provider is installed, no constructor changes.

Design notes

  • System-aggregate (no tenant/service attributes) so a perf run reads total throughput directly; tenant/transport are documented opt-in follow-ons (high cardinality).
  • flush.trigger is the one added dimension: it shows whether flushes are healthy (size right-sizing) or memory-pressured (ceiling). Low cardinality, mirrors the existing compaction.result pattern.
  • Dropped a proposed wal.io bytes counter — the house io convention carries a read/write direction (ourios.io.direction), but WAL append is write-only; byte-rate is derivable, and wal.append.duration is the headline WAL signal.

Tests

  • New tests/perf_metrics.rs — drives the instruments through an in-memory exporter and asserts the registry names, the trigger split (size=5, rotation=3, age=0), the counter/up-down totals, and the histogram's flush count.
  • Idempotent weaver regen (no-diff gate); ingester + server suites green; fmt + clippy clean.

No invariant (§3) is altered — this is pure additive observability per §6.3.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added comprehensive observability metrics for the ingester pipeline, including ingest throughput measurement, WAL append latency tracking, and batch processing visibility.
    • Enhanced sink monitoring with detailed flush operation metrics (duration, record counts, trigger type) and buffer occupancy tracking.
    • Improved error tracking for derive and flush failures.

The ingest front (OTLP → WAL → handoff) and the RFC 0014 record sink
emitted no metrics, so a performance run couldn't see ingest throughput,
the WAL-before-ack latency, or flush behaviour. Fill those blind spots
with 8 new instruments (+1 attribute), authored in the weaver registry
and regenerated into `ourios-semconv` (the no-diff CI gate stays green;
names verified against the OTel metric-naming spec):

  ourios.ingest.records        counter        {record}
  ourios.ingest.batches        counter        {batch}
  ourios.wal.append.duration   histogram      s
  ourios.sink.flush.duration   histogram      s     [trigger]
  ourios.sink.flush.records    counter        {record}  [trigger]
  ourios.sink.flush.errors     counter        {error}
  ourios.sink.derive.errors    counter        {error}
  ourios.sink.buffer.usage     updowncounter  By
  + attribute ourios.sink.flush.trigger (size|age|rotation|ceiling)

`IngestMetrics` records throughput + the durable-commit latency on each
acked batch in `IngestPipeline::ingest`. `SinkMetrics` records flush
duration/rows by trigger, flush/derive errors, and buffer occupancy in
`ParquetRecordSink`. Both resolve through the global meter (RFC 0001 §6.8
API/SDK split) — no-op when no provider is installed. System-aggregate
(no tenant/service attributes) so a perf run reads throughput directly;
`flush.trigger` is the one added dimension — it shows whether flushes are
healthy (size) or memory-pressured (ceiling), low cardinality, mirroring
`compaction.result`.

Verified: weaver registry check; idempotent regen; a new in-memory-export
integration test (`tests/perf_metrics.rs`) asserting the names, the
trigger split, and the recorded values; fmt/clippy clean; ingester +
server suites green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 48 minutes and 59 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c5c8d269-286a-4690-9d07-0e1977034072

📥 Commits

Reviewing files that changed from the base of the PR and between d20a766 and 703b4b0.

📒 Files selected for processing (1)
  • crates/ourios-ingester/tests/perf_metrics.rs
📝 Walkthrough

Walkthrough

Adds OpenTelemetry metrics instrumentation to the ingester write path. New IngestMetrics and SinkMetrics structs wrap OTel global-meter instruments. IngestPipeline records WAL-before-ack latency and batch throughput; ParquetRecordSink records flush duration, trigger-labeled record counts, error counts, and buffer occupancy. Semantic convention definitions and generated Rust constants are added alongside an end-to-end in-memory exporter test.

Changes

Ingest and Sink OTel Metrics

Layer / File(s) Summary
Semantic convention definitions and generated constants
semconv/registry/attributes.yaml, semconv/registry/metrics.yaml, crates/ourios-semconv/src/lib.rs
Defines ourios.sink.flush.trigger attribute enum (size, age, rotation, ceiling) and all new ingest/sink metric instrument definitions in YAML; adds the corresponding generated Rust string constants.
IngestMetrics and SinkMetrics structs
crates/ourios-ingester/src/metrics.rs
Implements IngestMetrics (records, batches, WAL append duration) and SinkMetrics (flush duration/records/errors, derive errors, buffer usage UpDownCounter) with OTel global-meter instrument construction, zero-seeding, and public recording methods.
IngestPipeline instrumentation
crates/ourios-ingester/src/receiver/pipeline.rs
Adds metrics: IngestMetrics field to IngestPipeline, starts an Instant timer before CommitCoordinator::commit, and calls record_batch on the successful ack path only.
ParquetRecordSink instrumentation
crates/ourios-ingester/src/record_sink.rs
Adds metrics: SinkMetrics to ParquetRecordSink, threads trigger: &'static str through flush_partition, records flush metrics and buffer occupancy deltas on success, and records flush/derive errors on failure paths.
End-to-end perf_metrics test
crates/ourios-ingester/tests/perf_metrics.rs
Sets up an in-memory OTel exporter, exercises both metrics structs, forces a flush, and asserts metric names, counter sums, trigger-dimension splits (size/rotation), buffer usage netting, and histogram datapoint counts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • jensholdgaard/ourios#105: Introduces the semconv weaver-forge generation workflow that produces the ourios-semconv constants being extended in this PR.
  • jensholdgaard/ourios#134: Introduced IngestPipeline with WAL-before-ack commit logic; this PR adds IngestMetrics instrumentation to that same pipeline path.
  • jensholdgaard/ourios#243: Added flush-policy triggers to ParquetRecordSink; this PR adds SinkMetrics instrumentation and trigger-label tracking for those same flush operations.

Poem

🐇 Hoppity-hop through the write path I go,
Counting each batch with a histogram's glow,
Flush triggers labeled — size, age, rotation, ceil —
UpDownCounter tracks every buffered byte's feel.
The WAL before ack? I time it with care,
Zero-seeded at start, so no gaps are there!
My metrics are tidy, my gauges aligned 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding metrics for performance observability to the ingest and sink pipelines.
Description check ✅ Passed The description is comprehensive and well-structured, covering why (perf observability gaps), what (8 instruments + 1 attribute), design notes, and tests. All required sections from the template are present and filled out appropriately.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/perf-metrics-ingest-sink

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end performance observability for the ingester ingest front (OTLP → WAL durable commit → miner handoff) and the RFC 0014 Parquet record sink by defining new semantic-convention metrics/attributes and wiring them into the ingest and sink hot paths, with an in-memory-exporter integration test to validate the exported stream.

Changes:

  • Define 8 new metrics and 1 new attribute (ourios.sink.flush.trigger) in semconv/registry/* and regenerate ourios-semconv constants.
  • Instrument IngestPipeline::ingest with throughput counters and a WAL-before-ack latency histogram via IngestMetrics.
  • Instrument ParquetRecordSink flush behavior (duration/records by trigger), error counters, and buffer usage via SinkMetrics, plus add a dedicated integration test asserting exported names and basic aggregates.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
semconv/registry/metrics.yaml Adds ingest/WAL and sink metric definitions (names, instruments, units, and required trigger attribute where applicable).
semconv/registry/attributes.yaml Adds ourios.sink.flush.trigger enum attribute with `size
crates/ourios-semconv/src/lib.rs Regenerates semconv constants for new metric names and the new attribute key.
crates/ourios-ingester/src/metrics.rs Introduces IngestMetrics and SinkMetrics using the global meter, with seeding for attribute-free instruments.
crates/ourios-ingester/src/receiver/pipeline.rs Records per-acked-batch throughput and durable-commit latency in the ingest pipeline.
crates/ourios-ingester/src/record_sink.rs Records sink flush metrics (including trigger), error counters, and buffer usage deltas during emit/flush paths.
crates/ourios-ingester/tests/perf_metrics.rs New integration test using in-memory metrics exporter to validate names, trigger split, and basic totals/counts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`cargo clippy --all-targets` flagged `|dp| dp.value()` / `|dp| dp.count()`
in tests/perf_metrics.rs (redundant_closure_for_method_calls). Use the
method references, matching the same pattern already used in the file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants