feat(bench): RFC 0031 — indicative comparative run + dispatch workflow - #474
Conversation
The §7 calibration input: the first Ourios-vs-Loki bytes-read comparison on a real corpus (the frozen corpus/otel-demo-v* releases; v8 = 48 h / 4.9 M records / 2.96 GB). - pick_error_pair scans the corpus and picks the first query pair dynamically: the service with the FEWEST severity>=17 rows in 1..=4000 (under Loki's 5000-line query cap, so the complete result fits one page) whose error rows are text-consistent (num>=17 ⟺ text=="ERROR"), so DSL `severity >= 17` and LogQL `severity_text="ERROR"` express the same question. Locally unit-tested on the shared fixture. - rfc0031_indicative_comparative_run (dispatch-only, #[ignore]d): builds the registry-bearing store, replays the WHOLE corpus into a Loki container over batched OTLP (~500 LogsData per ~2 MB push, 429/5xx retry), runs the pair on both systems. Equivalence is ASSERTED; the bytes gate is REPORTED under the provisional §7 margins — this run is what the margins get frozen against. - Loki runs the stock image config plus two explicit ingest-side flag deviations, both in LOKI'S favour (anti-strawman direction): reject-old-samples off (frozen captures carry original, weeks-old timestamps — grounding caught this before it burned CI runs) and raised ingest-rate limits so a 3 GB replay isn't throttled by dev-scale defaults. Query side stays stock. - start_loki/push_otlp extracted and shared with the RFC0031.1 test; query limit raised to Loki's stock 5000 cap. - comparative-bench.yml: workflow_dispatch mirroring bench.yml (release build, frozen-release corpus fetch, report block into the job summary). Folding a number into docs/benchmarks.md §9 stays a maintainer step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 49 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 ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an RFC 0031 comparative benchmark workflow and expands the Rust test harness to replay a corpus into Loki, compare Loki and Ourios results, and report provisional bytes-processed gating data. ChangesRFC 0031 comparative benchmark
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant CorpusRelease
participant ComparativeTest
participant Ourios
participant Loki
GitHubActions->>CorpusRelease: download validated corpus release asset
GitHubActions->>ComparativeTest: run release-mode comparative test
ComparativeTest->>Ourios: build store and query selected service
ComparativeTest->>Loki: start container and push corpus batches
ComparativeTest->>Loki: query lines and bytes_processed
ComparativeTest->>GitHubActions: write comparative report
GitHubActions->>GitHubActions: append report to job summary
Possibly related PRs
🚥 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
This PR adds the RFC 0031 “indicative comparative run” harness (Ourios vs real Loki on a frozen corpus/otel-demo-v* release) and a workflow_dispatch GitHub Actions workflow to run it on demand, producing an equivalence-gated report plus Loki bytes-processed stats for §7 calibration.
Changes:
- Refactors Loki testcontainer startup and OTLP push logic (readiness wait, longer HTTP timeout, 429/5xx retry) and raises Loki query
limitto 5000 to avoid truncation. - Introduces
pick_error_pair+ a dispatch-only ignored test (rfc0031_indicative_comparative_run) that replays a full corpus into Loki, asserts equivalence, and reports the bytes gate. - Adds
.github/workflows/comparative-bench.ymlto fetch a frozen corpus release asset and run the indicative comparative test in release mode, writing a job summary report block.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/ourios-bench/tests/rfc0031_comparative.rs | Adds the indicative comparative run logic, corpus pair picker, Loki ingest/query helpers, and improves Loki round-trip robustness. |
| .github/workflows/comparative-bench.yml | New dispatch-only workflow to fetch a frozen corpus release and run/report the indicative comparative run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- pick_error_pair: no-service (empty-key) records are counted for the diagnostic but excluded from candidacy — an empty service can't form a valid DSL/LogQL pair. - One HashMap entry lookup per ResourceLogs group (moving the service string) instead of a clone per record — the scan walks multi-million- record corpora. - Post-scan assertion: a corpus with no non-zero time_unix_nano fails immediately instead of producing a u64::MAX query window. - comparative-bench.yml: validate the dispatcher-controlled corpus tag (same guard as query-bench.yml) before it reaches the gh CLI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/comparative-bench.yml:
- Around line 52-58: Update the corpus_dir output in the workflow’s download
step to emit the absolute path of CORPUS_DIR rather than the relative
comparative-corpus value. Preserve the existing download and extraction flow so
rfc0031_indicative_comparative_run resolves the downloaded corpus from the
workflow root.
In `@crates/ourios-bench/tests/rfc0031_comparative.rs`:
- Around line 198-219: Update push_otlp to handle errors from send().await
within the existing deadline loop instead of panicking via expect. Retry
transport errors with the same deadline enforcement and backoff used for
retryable HTTP responses, while preserving successful responses and existing
429/5xx handling.
🪄 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: f9093e14-1f72-4565-9d89-02e372619853
📒 Files selected for processing (2)
.github/workflows/comparative-bench.ymlcrates/ourios-bench/tests/rfc0031_comparative.rs
- Workflow emits an ABSOLUTE corpus path: the test binary's CWD is the package root, not the workspace root — the relative dir would have silently pointed at the wrong place (real bug, caught in review). - push_otlp: Bytes payload (refcount bump per attempt, not a ~2 MB copy per push across thousands) and transport errors (reset/timeout) retry within the same deadline instead of aborting a long replay on a blip. - Doc names the actual Loki flag (-validation.reject-old-samples=false). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Steps 3+4 to the first benchmark number: the indicative comparative run — Ourios vs a real Loki on a frozen
corpus/otel-demo-v*release (v8 = 48 h, 4.9 M records, 2.96 GB) — plus thecomparative-benchdispatch workflow that runs it.pick_error_pair— scans the corpus and picks the first query pair dynamically: the service with the fewestseverity ≥ 17rows in1..=4000(under Loki's 5000-line query cap so the complete result fits one page), text-consistent (num ≥ 17⟺text == "ERROR") so DSLseverity >= 17and LogQLseverity_text="ERROR"express the same question. Locally unit-tested on the shared fixture.rfc0031_indicative_comparative_run(dispatch-only,#[ignore]d) — registry-bearing store + in-process query on the Ourios side; full-corpus batched OTLP replay (~500LogsDataper ~2 MB push, 429/5xx retry) into the Loki container; the pair run on both. Equivalence asserted; the bytes gate reported under the provisional §7 margins — this run is exactly what the margins get frozen against (RFC 0031 §7).reject-old-samples=false(the frozen captures carry their original, weeks-old timestamps — grounding caught this before it could burn CI runs) and raised ingest-rate limits so a 3 GB replay isn't throttled by dev-scale defaults. The query side stays stock.comparative-bench.yml—workflow_dispatchmirroringbench.yml: release build, frozen-release fetch, the report block into the job summary. Folding a number intodocs/benchmarks.md§9 stays a maintainer step.Validation
workflow_dispatchrequires the workflow file on the default branch, so the validation run happens immediately post-merge: dispatchingcomparative-bench.ymlonmainproduces the first indicative Ourios-vs-Loki bytes-read ratio in its job summary — will link the run on this PR when it completes. (Regular CI validates compile + the picker unit test; the default test binary stays green —1 passed; 12 ignored.)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes