feat(bench): OTLP/JSON corpus loader — RFC 0003 §6.5 MVP path (PR-K2) - #58
Merged
Conversation
|
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 ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds multi-format ingestion to ourios-bench: the walker dispatches ChangesOTLP/JSON corpus loader for bench harness
Sequence DiagramsequenceDiagram
participant Walker as CorpusLoad.walk
participant FileExt as ExtensionDispatch
participant TxtLoader as ingest_txt
participant JsonLoader as ingest_otlp_jsonl
participant Mapper as map_log_record
participant Records as OtlpLogRecord output
Walker->>FileExt: path + metadata
alt .txt file
FileExt->>TxtLoader: stream BufReader
TxtLoader->>TxtLoader: strip CRLF, skip blanks, advance ts
TxtLoader->>Records: emit enveloped OtlpLogRecord
else .jsonl/.json file
FileExt->>JsonLoader: read non-blank lines
JsonLoader->>JsonLoader: serde_json::from_slice::<LogsData>()
JsonLoader->>JsonLoader: iterate resource_logs -> scope_logs -> log_records
JsonLoader->>Mapper: per-record wire fields
Mapper->>Mapper: clamp severity, any_value → Body, hoist attrs, validate ids
Mapper->>Records: emit mapped OtlpLogRecord
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ourios-bench/src/corpus.rs (1)
153-160:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the empty-corpus error message for multi-format input.
The message still says "
*.txtlines", but this loader now consumes.txt,.jsonl, and.json. That will mislead operators when OTLP-only corpora fail validation.Suggested patch
- "no non-empty `*.txt` lines under {} (read {} file(s))", + "no non-empty records under {} from supported corpus files (`*.txt`, `*.jsonl`, `*.json`) (read {} file(s))", dir.display(), total_files,🤖 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/corpus.rs` around lines 153 - 160, Update the error text produced in the lines.is_empty() branch where BenchError::Corpus is constructed so it no longer references only "`*.txt` lines"; change the detail string to mention the actual accepted formats (e.g. ".txt, .jsonl, .json" or similar) and the fact there were no non-empty entries under the directory (keep dir.display() and total_files in the message and return value). Locate the block that builds BenchError::Corpus in corpus.rs (the lines.is_empty() check) and replace the format string accordingly.
🤖 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/0006-bench-harness.md`:
- Around line 234-251: Update the RFC's normative definitions for the raw corpus
so they match the amendment: change the description of bytes(raw_corpus) and the
CLI option --corpus to explicitly state that corpus inputs include both
plaintext `*.txt` files and OTLP JSON/JSONL `*.json` / `*.jsonl` (OTLP/JSON
Lines format parsed as LogsData), and clarify that `*.binpb` protobuf-encoded
LogsData remains out of scope; ensure the wording and any examples/validation
rules that previously limited `bytes(raw_corpus)` and `--corpus` to `*.txt` are
broadened to accept the same file extension set as the amendment.
---
Outside diff comments:
In `@crates/ourios-bench/src/corpus.rs`:
- Around line 153-160: Update the error text produced in the lines.is_empty()
branch where BenchError::Corpus is constructed so it no longer references only
"`*.txt` lines"; change the detail string to mention the actual accepted formats
(e.g. ".txt, .jsonl, .json" or similar) and the fact there were no non-empty
entries under the directory (keep dir.display() and total_files in the message
and return value). Locate the block that builds BenchError::Corpus in corpus.rs
(the lines.is_empty() check) and replace the format string accordingly.
🪄 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: 8d285bb6-323f-4551-96f5-b9d148c02ccb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
crates/ourios-bench/Cargo.tomlcrates/ourios-bench/src/corpus.rscrates/ourios-bench/tests/data/otlp/sample.jsonlcrates/ourios-core/Cargo.tomldocs/rfcs/0006-bench-harness.md
…-K2) Extends `ourios-bench`'s corpus loader to consume OTLP/JSON Lines files (`*.jsonl` / `*.json` — the OTel File Exporter format, one `LogsData` per line) alongside the existing plain-text `*.txt` path. This is the route RFC 0003 §6.5 itself names as the MVP bench source for OTLP data: > "the MVP bench reads OTLP from the on-disk corpus, bypassing > this component entirely" (until `ourios-wal` lands; a live > receiver without WAL-before-ack would violate CLAUDE.md §3.4). The walker dispatches on extension; both formats may coexist in the same corpus directory. Wire `LogRecord`s map 1:1 onto the RFC 0003 §6.6 `OtlpLogRecord` shape: - severity_number clamped to OTLP's `0..=24` (FATAL4) - severity_text / scope_name / scope_version / event_name empty-string → `None` collapse - attributes, dropped_attributes_count, resource_attributes preserved (resource attrs copied per record, matching the existing `OtlpLogRecord` doc-comment contract) - trace_id / span_id length-validated to `[u8;16]` / `[u8;8]` (`None` on the wrong length — RFC0003.11 transport-error surface) - body: `stringValue` → `Body::String`, anything else → `Body::Structured(AnyValue)` verbatim per RFC 0003 §6.4 - wire timestamps honoured (file-static = run-reproducible); the §3.3 deterministic baseline still drives the text path. Parsing uses `serde_json::from_str::<LogsData>` against `opentelemetry-proto`'s types — the `with-serde` feature (added on the workspace's existing dep) gives the OTLP/JSON spec mapping for free (camelCase keys, string-encoded `u64`s for `timeUnixNano`, base64 bytes). Pattern follows rotel's OTLP HTTP receiver. Keeps the spec single-sourced in `opentelemetry-proto` rather than a hand-rolled struct that could drift. RFC 0006 §3.1 amended in three places (the "deferred" notes become "landed in PR-K2"); the protobuf (`*.binpb`) `LogsData` decode remains out of scope. Tests (+6, total 284 passing / 19 ignored): committed OTLP fixture round-trips through every envelope field; structured body lands as `Body::Structured`; blank lines skipped; malformed JSON surfaces a `BenchError::Corpus` carrying the 1-based line number; severity above 24 clamps; mixed .txt + .jsonl in one corpus dir both contribute records. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard
force-pushed
the
feat/ourios-bench-otlp-loader
branch
from
May 28, 2026 21:45
c3bea46 to
5bf8356
Compare
|
Actionable comments posted: 0 |
4 tasks
jensholdgaard
added a commit
that referenced
this pull request
May 29, 2026
Follow-up to PR #58. Copilot and CodeRabbit between them flagged that the OTLP/JSON loader landed with a surface that wasn't internally consistent — and one of the issues was a real correctness bug. Bundles the fixes here as a single squash-merge so the seven review threads across PR #58 + PR #59 close together. **Fixes (G — the real bug):** §3.4.1's A1 math invariant — both `bytes(raw_corpus)` and `bytes(zstd_corpus)` must process the same input. PR #58 broadened `bytes(raw_corpus)` to OTLP/JSON via the loader change but left `zstd_level_19_bytes` in `a1.rs` filtering on `*.txt` only. An OTLP-only corpus would therefore have `bytes(raw_corpus) > 0` and `bytes(zstd_corpus) = 0`, producing `zstd_ratio = 0` and an undefined A1 delta. The zstd reference codec now consumes the same extension set (`.txt | .jsonl | .json`) — kept in lockstep with `corpus::walk`'s dispatch via a comment on both sides. RFC §3.4.1 amended to spell out the invariant. **Tightens (consistency):** - `corpus.rs` `CorpusLoad` field docs + the `no non-empty corpus lines` error message now name the actual supported extensions. - The extension dispatch drops the per-entry `to_ascii_lowercase` allocation and uses `eq_ignore_ascii_case` on `&str` (allocation- free, hot loader path). `match`-with-guards per the project's feedback memory. - `Body::from_any_value` (already the single-sourced fork in `ourios-core::otlp`) replaces the local duplicate `any_value_to_body` in the loader. Drift risk → 0. - `main.rs` clap help text on `--corpus` broadened so `ourios-bench --help` matches RFC §3.7. - `ourios-core/Cargo.toml` comment aligned to the actual `serde_json::from_str` call (was `from_slice`). - RFC §3.4.1 reworded so `corpus.directory` no longer claims to disambiguate mixed-format corpora — mixed dirs produce an aggregate number; cleanly comparable runs need one encoding per dir. Per-format byte breakdown noted as a future enhancement. **Test (+1):** `a1::tests::zstd_consumes_every_corpus_extension` — mixed-extension corpus produces > 0 bytes, OTLP-only corpus does too (guards the failure mode the pre-fix `txt`-only filter introduced). Closes the Copilot threads on PR #58 (corpus.rs:38 / :459 / :275 + Cargo.toml:19) and PR #59 (RFC :278 / :289 / :640). 5 files changed (§5.2 phase limit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard
added a commit
that referenced
this pull request
May 29, 2026
* fix(bench): tighten OTLP-loader consistency surface (PR-K2.1) Follow-up to PR #58. Copilot and CodeRabbit between them flagged that the OTLP/JSON loader landed with a surface that wasn't internally consistent — and one of the issues was a real correctness bug. Bundles the fixes here as a single squash-merge so the seven review threads across PR #58 + PR #59 close together. **Fixes (G — the real bug):** §3.4.1's A1 math invariant — both `bytes(raw_corpus)` and `bytes(zstd_corpus)` must process the same input. PR #58 broadened `bytes(raw_corpus)` to OTLP/JSON via the loader change but left `zstd_level_19_bytes` in `a1.rs` filtering on `*.txt` only. An OTLP-only corpus would therefore have `bytes(raw_corpus) > 0` and `bytes(zstd_corpus) = 0`, producing `zstd_ratio = 0` and an undefined A1 delta. The zstd reference codec now consumes the same extension set (`.txt | .jsonl | .json`) — kept in lockstep with `corpus::walk`'s dispatch via a comment on both sides. RFC §3.4.1 amended to spell out the invariant. **Tightens (consistency):** - `corpus.rs` `CorpusLoad` field docs + the `no non-empty corpus lines` error message now name the actual supported extensions. - The extension dispatch drops the per-entry `to_ascii_lowercase` allocation and uses `eq_ignore_ascii_case` on `&str` (allocation- free, hot loader path). `match`-with-guards per the project's feedback memory. - `Body::from_any_value` (already the single-sourced fork in `ourios-core::otlp`) replaces the local duplicate `any_value_to_body` in the loader. Drift risk → 0. - `main.rs` clap help text on `--corpus` broadened so `ourios-bench --help` matches RFC §3.7. - `ourios-core/Cargo.toml` comment aligned to the actual `serde_json::from_str` call (was `from_slice`). - RFC §3.4.1 reworded so `corpus.directory` no longer claims to disambiguate mixed-format corpora — mixed dirs produce an aggregate number; cleanly comparable runs need one encoding per dir. Per-format byte breakdown noted as a future enhancement. **Test (+1):** `a1::tests::zstd_consumes_every_corpus_extension` — mixed-extension corpus produces > 0 bytes, OTLP-only corpus does too (guards the failure mode the pre-fix `txt`-only filter introduced). Closes the Copilot threads on PR #58 (corpus.rs:38 / :459 / :275 + Cargo.toml:19) and PR #59 (RFC :278 / :289 / :640). 5 files changed (§5.2 phase limit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fixup! fix(bench): tighten OTLP-loader consistency surface (PR-K2.1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
ourios-bench's corpus loader to consume OTLP/JSON Lines files (*.jsonl/*.json— the OTel File Exporter format, oneLogsDataper line) alongside the existing plain-text*.txtpath.This is the route RFC 0003 §6.5 itself names as the MVP bench source for OTLP data:
So no architectural new ground — this fills a slot RFC 0003 explicitly carved out.
Mapping (RFC 0003 §6.6 in-memory shape, 1:1)
The walker dispatches on extension; both formats may coexist in the same corpus directory. Each wire
LogRecordmaps to oneOtlpLogRecord:OtlpLogRecordfieldseverityNumber(i32)severity_number(u8)0..=24(FATAL4) — "narrowed from proto's unboundedi32at the receiver boundary"severityText/scope.name/scope.version/eventNameOption<String>Nonecollapseattributes,droppedAttributesCountattributes,dropped_attributes_countresource.attributesresource_attributesOtlpLogRecorddoc-comment contract)traceId/spanId(Vec<u8>)Option<[u8;16]>/Option<[u8;8]>Noneon the wrong length (RFC0003.11 transport-error surface, not a panic)body.stringValueSome(Body::String(_))body.{kvlistValue,arrayValue,intValue,…}Some(Body::Structured(AnyValue))timeUnixNanotime_unix_nanoParsing uses
serde_json::from_str::<LogsData>againstopentelemetry-proto's types. Thewith-serdefeature (added on the workspace's existing dep) gives the OTLP/JSON spec mapping for free — camelCase keys, string-encodedu64s, base64 bytes. Pattern follows rotel's OTLP HTTP receiver. Keeps the spec single-sourced inopentelemetry-protorather than a hand-rolled struct that could drift.RFC 0006 §3.1 amendments
Three places, each using the
**Amendment**block pattern RFC 0003 §6.4 uses (preserves the historical record): the "deferred" notes become "landed in PR-K2 (2026-05-28)." The protobuf (*.binpb)LogsDatadecode remains out of scope.Test plan
cargo fmt --all --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-features— 284 passed / 19 ignored (was 278/19; +6 OTLP tests)mdbook buildThe +6 tests cover, colocated in
corpus.rs:LogsDatalines, 4 records, mixed string + structured + kvlist bodies).Body::Structured(AnyValue).BenchError::Corpuswhose detail carries the 1-based line number (operators paste that intosed -n '<n>p').severityNumberabove 24 clamps to 24..txt+.jsonlin one corpus dir both contribute records.Follow-ups
bench.ymlto a representative OTLP corpus (telemetrygen → collector / fileexporter → JSONL). Out of scope here; PR adds the loader, not the corpus-generation side.*.binpb)LogsDatadecode — a follow-up if the fileexporter output ever lands in protobuf form.ourios-walper §6.5; that's a separate multi-RFC arc.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation