Skip to content

feat(core): extend MinedRecord with the OTLP-envelope fields (PR-E1) - #43

Merged
jensholdgaard merged 3 commits into
mainfrom
feat/extend-mined-record-otlp-fields
May 21, 2026
Merged

feat(core): extend MinedRecord with the OTLP-envelope fields (PR-E1)#43
jensholdgaard merged 3 commits into
mainfrom
feat/extend-mined-record-otlp-fields

Conversation

@jensholdgaard

Copy link
Copy Markdown
Owner

Summary

Closes the gap between MinedRecord (today's minimal shape) and RFC 0001 §6.1 / RFC 0005 §3.2 (the normative record schema). The miner now copies the full OTLP envelope from OtlpLogRecord through to MinedRecord. Preparatory for the writer (PR-E2), which needs every §3.2 row-level column on the emitted record to faithfully produce the schema.

Until the OTLP receiver (RFC 0003, post-MVP) lands, corpus / bench inputs leave these fields at their OtlpLogRecord::default() values; the writer surfaces them as NULL or empty per RFC 0005's encoding rules.

Fields added to MinedRecord

All populated from OtlpLogRecord in ourios-miner's record_envelope:

  • severity_text: Option<String>
  • scope_version: Option<String>
  • observed_time_unix_nano: Option<u64>
  • attributes: Vec<KeyValue>
  • dropped_attributes_count: u32
  • resource_attributes: Vec<KeyValue>
  • trace_id: Option<[u8; 16]>
  • span_id: Option<[u8; 8]>
  • flags: u32
  • event_name: Option<String>

Why a separate PR

Per CLAUDE.md §6.4 ("escalate to a clean solution before merging") — landing PR-E2's writer against the existing minimal MinedRecord would mean mapping half the §3.2 schema to NULL and reopening it later. Better to extend the record now in its own PR so the contract change is reviewable in one place, then PR-E2 stays focused on Arrow RecordBatch ↔ Parquet plumbing.

Known gap (deliberate, deferred)

MinedRecord.body is Option<String> while RFC 0001 §6.1 / RFC 0005 §3.2 specify raw bytes (potentially non-UTF-8). Unchanged in this PR; PR-E2 papers over it on the write path (UTF-8 in → bytes out is a no-op), and the type swap is a separate concern.

Invariant coverage

No new acceptance criteria — this PR doesn't change behaviour, it widens the record contract. RFC0005.10 (the schema-pin) is unaffected; it pins the on-disk Parquet schema, which is unchanged.

Test plan

  • cargo build --all-features — clean
  • cargo test --all-features — 178 tests pass (no regressions)
  • cargo fmt --all --check — clean
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • CI green

🤖 Generated with Claude Code

jensholdgaard and others added 2 commits May 21, 2026 10:35
Closes the gap between MinedRecord (today's minimal shape) and
RFC 0001 §6.1 / RFC 0005 §3.2 (the normative record schema).
The miner now copies the full OTLP envelope from the input
OtlpLogRecord through to the emitted MinedRecord; until the
receiver (RFC 0003) lands, corpus / bench inputs leave these
fields at their OtlpLogRecord::default() values, which surface
as NULL or empty in the corresponding Parquet columns.

Added MinedRecord fields (all populated from OtlpLogRecord by
ourios-miner's record_envelope):

- severity_text: Option<String>
- scope_version: Option<String>
- observed_time_unix_nano: Option<u64>
- attributes: Vec<KeyValue>
- dropped_attributes_count: u32
- resource_attributes: Vec<KeyValue>
- trace_id: Option<[u8; 16]>
- span_id: Option<[u8; 8]>
- flags: u32
- event_name: Option<String>

Why now: the upcoming Parquet writer (PR-E2) needs every §3.2
row-level column to faithfully emit the schema. Landing the
field set as its own PR keeps PR-E2 focused on the Arrow
RecordBatch ↔ Parquet plumbing, and gives reviewers the
contract change in one place rather than mixed with writer
mechanics. Per CLAUDE.md §6.4: clean solution over papering
over a half-implementation.

Updated callers:
- ourios-miner cluster.rs::record_envelope copies the 10 new
  fields from the incoming OtlpLogRecord.
- Test fixtures in record.rs (sample_clean_record /
  sample_parse_failure_record) and reconstruct.rs's record_envelope
  helper get explicit zero / None / empty values for the new
  fields — keeping construction explicit (no Default impl) so the
  test fixtures remain a readable "expected shape" pin.

Known follow-up: MinedRecord.body is Option<String> while RFC 0001
§6.1 / RFC 0005 §3.2 specify raw bytes (potentially non-UTF-8).
That gap is unchanged here; PR-E2 will paper over it on the
write path (UTF-8 in → bytes out is a no-op) and the
String-vs-Bytes type swap is a separate concern.

Verified:
- cargo build --all-features — clean
- cargo test --all-features — 178 tests pass (no regressions)
- cargo fmt --all --check — clean
- cargo clippy --all-targets --all-features -- -D warnings — clean

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

This PR expands ourios_core::record::MinedRecord to carry the full OTLP-derived “envelope” (per RFC 0001 §6.1 / RFC 0005 §3.2) and updates the miner to copy those fields through from OtlpLogRecord, preparing the contract needed by the upcoming Parquet writer work (PR-E2).

Changes:

  • Extended MinedRecord with additional OTLP envelope fields (severity text, scope version, observed time, attributes/resource attributes, trace/span correlation, flags, event name).
  • Updated ourios-miner to populate the new fields in record_envelope, and adjusted relevant tests/fixtures.
  • Added/updated documentation around structured-body round-tripping expectations.

Reviewed changes

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

File Description
crates/ourios-miner/src/reconstruct.rs Updated test record construction to include new MinedRecord envelope fields.
crates/ourios-miner/src/cluster.rs Miner now copies OTLP envelope fields from OtlpLogRecord into MinedRecord.
crates/ourios-core/src/record.rs Added new OTLP envelope fields to MinedRecord and updated tests/docs accordingly.
crates/ourios-core/src/otlp.rs Added an implementer note about structured body round-trip behavior for future exporting.

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

Comment thread crates/ourios-miner/src/cluster.rs
Comment thread crates/ourios-core/src/otlp.rs Outdated

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

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

@jensholdgaard
jensholdgaard requested a review from Copilot May 21, 2026 09:03
@jensholdgaard
jensholdgaard merged commit 8662a43 into main May 21, 2026
15 checks passed
@jensholdgaard
jensholdgaard deleted the feat/extend-mined-record-otlp-fields branch May 21, 2026 09:06

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment on lines +78 to +79
/// values (zero / `None` / empty `Vec`), which surface as NULL
/// or empty in the corresponding RFC 0005 §3.2 Parquet columns.
jensholdgaard added a commit that referenced this pull request May 21, 2026
#45)

* feat(parquet): reader with §3.9 contract + RFC0005.1/2/3/4/9/11 (PR-F)

Lands the Reader half of RFC 0005 §3.10's crate-shape plan.
Audit stream is still pending (PR-G).

- `crates/ourios-core/src/audit.rs` — adds `ParamType::Unknown(i32)`
  for the RFC 0005 §3.9 catch-all variant. The reader produces it
  for type_tag ordinals 8..; the writer round-trips the carried
  ordinal back to disk via the updated `param_type_ordinal` match
  arm. No existing exhaustive matches on `ParamType` (audited via
  grep), so the new tuple variant doesn't break any callers.

- `crates/ourios-parquet/src/reader.rs` — `Reader::open_partition`
  (production query path, enforces §3.9 row-vs-path validation) +
  `Reader::open_file` (diagnostic, no validation) + `read_all`.
  Implements §3.9's three normative contract clauses end-to-end:
  unknown columns silently ignored, missing OPTIONAL columns
  surface as None, missing baseline REQUIRED columns are a hard
  error naming the column. Row-vs-path validation reuses
  `PartitionKey::derive` from PR-E2 so writer and reader use the
  identical §3.4 fallback algorithm. `MinedRecord` reconstruction
  goes column-by-column with helper functions per Arrow type
  (string / u64 / u32 / u8 / f32 / bool / timestamp / fixed-bytes
  / list / struct-list). Body is UTF-8-lossy-decoded for the
  Option<String> ↔ Bytes column gap noted in PR-E1.

Tests (14 new, all green):
- `tests/round_trip.rs` — RFC0005.1: writes a populated record
  through Writer, reads back through Reader, asserts full struct
  equality. Sub-test pins the body raw-bytes round-trip.
- `tests/reader_compat.rs` — hand-built Parquet files with:
  - RFC0005.2: omitted OPTIONAL column → reader returns None
  - RFC0005.3: extra unknown column → reader silently ignores
  - RFC0005.4: omitted REQUIRED column → reader hard-errors
    naming the column
  - RFC0005.9: type_tag = 99 → reader returns ParamType::Unknown(99)
    (round-trips through Writer → file → Reader)
- `tests/row_vs_path_validation.rs` — RFC0005.11:
  - tenant_id mismatch → PartitionMismatch error
  - hour mismatch → PartitionMismatch error
  - §3.4 fallback (time=0, observed!=0) → validates cleanly
    when supplied partition matches the observed-time bucket
  - open_file mode skips validation entirely

Verified locally:
- cargo build --all-features — clean
- cargo test --all-features — 200+ tests passing
- cargo fmt --all --check — clean
- cargo clippy --all-targets --all-features -- -D warnings — clean

Phase 2 progress (per docs/roadmap.md):
- PR-D ✅ scaffold (#42)
- PR-E1 ✅ MinedRecord extension (#43)
- PR-E2 ✅ Writer (#44)
- PR-F ✅ Reader (this PR)
- PR-G ⏳ AuditWriter / AuditReader

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fixup! feat(parquet): reader with §3.9 contract + RFC0005.1/2/3/4/9/11 (PR-F)

* fixup! feat(parquet): reader with §3.9 contract + RFC0005.1/2/3/4/9/11 (PR-F)

* fixup! feat(parquet): reader with §3.9 contract + RFC0005.1/2/3/4/9/11 (PR-F)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <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