feat(bench): RFC 0031 — Ourios-side query extraction (RFC0031.1) - #468
Conversation
Second increment of the equivalence-harness slice: the Ourios half of the RFC0031.1 check. `ourios_query_lines` runs a logs-DSL query against a store in-process (the querier — RFC 0031 §7, no served binary) and lowers the rendered rows to `LineKey`s, so they feed `compare_lines`. - Promote ourios-querier + tokio from dev-deps to regular deps: the comparative harness is a lib gate that produces docs/benchmarks.md §9 numbers (like a1/c1/c2), and producing comparative numbers requires in-process querying — so the querier belongs in the lib, not just the B2 criterion bench. No hidden cost: ourios-bench is a leaf crate (nothing inherits the heavier lib) and there is no dependency cycle (querier deps only core/miner/parquet). - `body_bytes` lowers the `Rendered` (string) body; `Structured`/`Absent` error rather than collapse — RFC 0025's absent-vs-empty distinction must be represented deliberately, which the OTLP-native gates will do by extending `LineKey` with a body-kind discriminator. - Integration test builds a real store from a text corpus and proves extraction returns one LineKey per row with in-span timestamps and self-equivalence through `compare_lines`. Finding this increment surfaced (locally, before any Loki container): reconstructing a cleanly-mined string body needs the audit-derived template registry (RFC 0017), which `build_query_store` does not persist (B1/B2 read only counts). The next increment adds a comparative store-builder that persists the audit stream — like the A1 gate — so bodies render; then the Loki testcontainers side (Dex-style). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 9 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)
📝 WalkthroughWalkthroughThe benchmark crate now runs Ourios queries in-process, converts rendered query rows into ChangesOurios query comparison
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Benchmark as Comparative benchmark
participant Runtime as Tokio runtime
participant Querier as Ourios querier
participant Comparator as LineKey comparator
Benchmark->>Runtime: Parse DSL and run query
Runtime->>Querier: Execute query for tenant and time window
Querier-->>Runtime: Return log rows
Runtime->>Comparator: Convert rows to LineKey values
Comparator-->>Benchmark: Return comparable lines
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 |
`cargo doc -D warnings` rejects a public item's doc linking a private one: `ourios_query_lines` linked `[body_bytes]` (private). Reword to a plain description; no link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements the Ourios-side extraction for the RFC 0031.1 equivalence harness by running logs-DSL queries in-process via ourios-querier, lowering returned rows into LineKeys for compare_lines, and introducing strict body-kind handling to preserve RFC 0025’s absent-vs-empty distinction.
Changes:
- Add
comparative::ourios_query_linesplusbody_bytesto extract(timestamp, body)keys from in-process querier results. - Add tests covering deferred non-string body kinds and a local self-equivalence extraction over a real RFC 0005 store.
- Promote
ourios-querierandtokiofrom dev-dependencies to regular dependencies, and re-exportourios_query_linesfrom the crate root.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/ourios-bench/src/lib.rs | Re-exports ourios_query_lines as part of the public comparative harness surface. |
| crates/ourios-bench/src/comparative.rs | Adds in-process querying + row lowering to LineKey, strict body-kind lowering, and tests. |
| crates/ourios-bench/Cargo.toml | Promotes ourios-querier + tokio to regular dependencies to support the lib-gate harness. |
💡 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.
🧹 Nitpick comments (1)
crates/ourios-bench/src/comparative.rs (1)
369-379: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider testing
LogBody::StructuredalongsideAbsent.The test name
body_bytes_defers_non_string_kinds(plural) covers onlyAbsent. Adding aStructured(AnyValue)case would fully exercise theother =>error arm and match the test's stated scope.🤖 Prompt for 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. In `@crates/ourios-bench/src/comparative.rs` around lines 369 - 379, Extend the body_bytes_defers_non_string_kinds test to also pass a LogBody::Structured value containing an AnyValue and assert that it returns an error. Keep the existing LogBody::Absent assertion, covering both non-string variants handled by the other => error arm.
🤖 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.
Nitpick comments:
In `@crates/ourios-bench/src/comparative.rs`:
- Around line 369-379: Extend the body_bytes_defers_non_string_kinds test to
also pass a LogBody::Structured value containing an AnyValue and assert that it
returns an error. Keep the existing LogBody::Absent assertion, covering both
non-string variants handled by the other => error arm.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f9d77c4b-b55b-4193-8513-990b0cc4b7e7
📒 Files selected for processing (3)
crates/ourios-bench/Cargo.tomlcrates/ourios-bench/src/comparative.rscrates/ourios-bench/src/lib.rs
Copilot review: - ourios_query_lines now enforces the documented limit requirement: it errors unless the rendered row count equals the total match count, so a missing/too-small `| limit` can't silently make the equivalence check compare a truncated (or empty) result. Tested (a limit-less query over matching rows errors). - body_bytes names the body *kind* (structured/absent) in its error instead of Debug-dumping the full LogBody — a structured body can be large or carry sensitive payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Second increment of the RFC 0031 equivalence-harness slice: the Ourios half of the RFC0031.1 check, landing as a locally-verifiable increment (the payoff of the "Adjust" path).
ourios_query_lines— runs a logs-DSL query against a store in-process (the querier; RFC 0031 §7, no served binary) and lowers the rendered rows toLineKeys, so they feedcompare_lines.body_bytes— lowers theRendered(string) body;Structured/Absenterror rather than collapse, because RFC 0025's absent-vs-empty distinction must be represented deliberately (the OTLP-native gates extendLineKeywith a body-kind discriminator).LineKeyper stored row, with in-span timestamps, round-tripping throughcompare_lines(self-equivalence).Dependency promotion (querier + tokio → regular deps)
The comparative harness is a lib gate that produces
docs/benchmarks.md§9 numbers (like a1/c1/c2), and producing comparative numbers requires in-process querying — so the querier belongs in the lib, not just the B2 criterion bench where the only prior in-process query lived.No hidden cost, verified:
ourios-benchis a leaf crate (nothing depends on it, so nothing inherits the heavier lib), there is no dependency cycle (querierdeps only core/miner/parquet), and querier+tokio already compiled for the bench's tests — so build time is unchanged in practice.A real finding this increment surfaced (locally, before any Loki container)
Reconstructing a cleanly-mined string body needs the audit-derived template registry (RFC 0017), which
build_query_storedoes not persist — B1/B2 only ever read row counts, never rendered bodies. So the extraction is correct (right count + timestamps) but bodies render empty against a count-only store. The test therefore asserts extraction shape + comparator integration, and documents that body-content equivalence needs a registry-bearing store — the next increment adds a comparative store-builder that persists the audit stream (like the A1 gate) so bodies render, then the Loki testcontainers side (Dex-style).This is exactly the kind of subtlety the Adjust path was meant to catch with fast local iteration rather than blind CI.
Tests
cargo test -p ourios-bench --lib comparative→ 9 passed. fmt + clippy clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests