Skip to content

feat(bench): restore OTLP envelope 1:1 mapping + kvlist fixture (PR-L2) - #63

Merged
jensholdgaard merged 1 commit into
mainfrom
feat/bench-restore-otlp-envelope
May 29, 2026
Merged

feat(bench): restore OTLP envelope 1:1 mapping + kvlist fixture (PR-L2)#63
jensholdgaard merged 1 commit into
mainfrom
feat/bench-restore-otlp-envelope

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented May 29, 2026

Copy link
Copy Markdown
Owner

Summary

Direct follow-up to PR #62 (RFC 0005 §3.3 canonicalisation). PR-K4 had stripped attributes / resource_attributes in the bench loader and removed the kvlist body record from the committed OTLP fixture as workarounds for the deferred §3.3 encoder. Canonicalisation now ships, so the workarounds go:

  • map_log_record maps attributes / resource_attributes 1:1 from the wire per RFC 0003 §6.6 (the resource-attrs per-ResourceLogs hoist is back).
  • crates/ourios-bench/tests/data/otlp/sample.jsonl regains its kvlist body record — back to 3 lines / 4 records.
  • Loader tests revert to asserting the rich envelope (counts of 1 / 2 / 1); the inline-synthetic structured-body test collapses back into a fixture-driven assertion.
  • Module / ingest_otlp_jsonl / map_log_record rustdocs drop the "stripped pending §3.3" caveats.

End-to-end on the restored fixture

corpus crates/ourios-bench/tests/data/otlp — 4 line(s), 1 file(s), 1328 raw byte(s) [dev-laptop]
  A1 compression: ourios 0.001× vs zstd-19 3.500× → delta 0.000× (target ≥ 3.0×) — FAIL (expected on a 1 KB corpus)
  C1 reconstruction: 1.000000 (3/3 non-lossy rows; lossy ratio 0.0000) — PASS
  C2 convergence: ratio n/a (end template count 4, sample cadence 1) — ABSTAIN (corpus < 1 M lines)

C1 is 3/3 (not 4/4) because the kvlist record is correctly excluded from C1's denominator per RFC 0001 §6.4 — that exclusion landed in PR-K4 and remains semantically right (template-based reconstruction doesn't apply to structured bodies).

Test plan

  • cargo fmt --all --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-features292 passed / 19 ignored (same count: −1 inline-synthetic structured-body test, +1 restored fixture assertion).
  • mdbook build clean.
  • Local ourios-bench end-to-end on the restored fixture (above).

What the bench can now do

Triggering Bench with corpus_dir=crates/ourios-bench/tests/data/otlp in CI now exercises every column shape the §3.2 schema admits: scalar primitives, scope, attribute KV lists, resource-attribute KV lists, structured kvlist body. End of the "stripped envelope" caveat for the on-disk corpus path. The production OTLP receiver (RFC 0003 §6.5) is still gated on ourios-wal; that's a separate multi-RFC arc.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • OTLP logs now preserve their complete envelope with full attribute information. Resource attributes are properly retained during log ingestion instead of being stripped. This improves data fidelity and completeness.
  • Tests

    • Updated test fixtures and expectations to reflect restored attribute handling in OTLP log records.

Review Change Stack

PR-K4 stripped `attributes` / `resource_attributes` in the bench
loader and removed the kvlist record from the committed OTLP
fixture, both as workarounds for the deferred RFC 0005 §3.3
canonicalisation. PR-L1 (#62) landed the canonicalisation
encoders/decoders end-to-end, so the workarounds can go:

- `map_log_record` no longer empties `attributes` /
  `resource_attributes`; both fields map verbatim from the
  wire per RFC 0003 §6.6. The resource-attrs per-`ResourceLogs`
  hoist is restored alongside.
- `crates/ourios-bench/tests/data/otlp/sample.jsonl` regains
  its kvlist body record; the fixture is now 3 LogsData lines
  / 4 records (1 + 2 + 1) as PR-K2 originally shipped it.
- Loader test reverts: `loads_otlp_corpus_envelope_one_to_one`
  asserts `first.attributes.len() == 1` and
  `first.resource_attributes.len() == 2` again, and the
  structured-body test reads the fixture's 4th record (the
  inline-synthetic workaround test goes away too).
- Module / `ingest_otlp_jsonl` / `map_log_record` rustdocs lose
  the "stripped pending §3.3" caveats now that the writer
  carries the envelope through.

Local end-to-end on the restored fixture: 4 records ingest +
write + read cleanly. C1 = 1.000000 (3/3 non-lossy strings;
the kvlist record is correctly excluded from C1's denominator
per RFC 0001 §6.4 — that exclusion landed in PR-K4 and stays).

Test plan: cargo fmt / clippy / test --all-features (292
passed / 19 ignored, same count: -1 inline-synthetic test, +1
fixture-restored assertion). `mdbook build` clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot May 29, 2026 13:00
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a7fe630-2d29-4a22-ae05-296548406c3f

📥 Commits

Reviewing files that changed from the base of the PR and between 618d8c2 and bc7fdde.

📒 Files selected for processing (2)
  • crates/ourios-bench/src/corpus.rs
  • crates/ourios-bench/tests/data/otlp/sample.jsonl

📝 Walkthrough

Walkthrough

The PR modifies the OTLP/JSON Lines corpus loader to preserve LogRecord attributes and resource attributes by hoisting resource attributes once per resourceLogs group and passing them into the mapping function. Documentation, implementation, and test expectations are updated consistently; a new sample record is added to validate the restored behavior.

Changes

OTLP Attribute Preservation

Layer / File(s) Summary
Documentation and import contract
crates/ourios-bench/src/corpus.rs
Format documentation is updated to indicate RFC 0003 §6.6 shape is now preserved (no workaround stripping), and KeyValue is imported for resource attribute handling.
Resource attribute extraction and mapping
crates/ourios-bench/src/corpus.rs
OTLP ingest now extracts resource.attributes once per resourceLogs group and passes them to map_log_record, which assigns LogRecord attributes and resource_attributes instead of returning empty vectors.
Test expectations and sample data
crates/ourios-bench/src/corpus.rs, crates/ourios-bench/tests/data/otlp/sample.jsonl
Test assertions are updated to expect non-empty attributes and resource_attributes counts, structured-body expectations are aligned with preserved behavior, and a new kvlistValue record with startup event is added to the OTLP sample fixture.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • jensholdgaard/ourios#61: Both PRs modify the OTLP JSONL ingest path in crates/ourios-bench/src/corpus.rs and adjust test expectations, but in opposite directions regarding attribute preservation.
  • jensholdgaard/ourios#58: Introduced the OTLP JSONL sample data and map_log_record function that this PR now modifies to preserve full envelope attributes.

Poem

A corpus reborn with wisdom old,
Attributes now shine, no longer cold.
Resources hoisted, mapped with care,
Envelopes full—complete and fair. 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: restoring OTLP envelope 1:1 mapping and adding back the kvlist fixture, directly addressing the core intent of reverting workarounds.
Description check ✅ Passed The description comprehensively covers the summary, related context (PR #62), test results, and end-to-end validation, matching all key sections of the template.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/bench-restore-otlp-envelope

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

Reverts the PR-K4 workarounds now that RFC 0005 §3.3 canonical-JSON encoding (PR #62) has landed: the bench OTLP loader restores 1:1 envelope mapping including attributes / resource_attributes, and the committed fixture regains its kvlist body record.

Changes:

  • map_log_record maps attributes / resource_attributes from the wire and hoists Resource.attributes per ResourceLogs group.
  • Sample fixture gains the kvlist record (3 lines / 4 records).
  • Loader tests assert the rich envelope; the inline-synthetic structured-body test reverts to a fixture-driven assertion. Docs drop the "stripped pending §3.3" caveats.

Reviewed changes

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

File Description
crates/ourios-bench/src/corpus.rs Restore resource-attr hoist + 1:1 attribute mapping; update doc comments and tests.
crates/ourios-bench/tests/data/otlp/sample.jsonl Add third LogsData line containing a kvlistValue body record.

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

@jensholdgaard
jensholdgaard merged commit 3dabe02 into main May 29, 2026
10 checks passed
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