feat(ingester): materialize LogRecord into OtlpLogRecord (RFC0003.7–.10) - #132
Conversation
Third green slice of the OTLP receiver (RFC 0003 §6.1 steps 2–3). Adds `receiver::materialize::materialize_record`: the per-record mapping from a decoded `LogRecord` to the flat `OtlpLogRecord` the miner consumes, and flips RFC0003.7–.10 live. The mapping: - body fork via `ourios_core::otlp::Body::from_any_value` — `string_value` → `Body::String` (unwrapped `L_raw`, RFC0003.8), every other variant → `Body::Structured` verbatim (RFC0003.7); absent body → `None`; - narrows proto's empty-value sentinels into `Option`/`None` at the wire boundary (RFC0003.9): wire `observed_time_unix_nano`/empty scope-name/version → `None`, while `severity_number = 0` (UNSPECIFIED) and `time_unix_nano = 0` (unknown) are explicit values kept as-is; - reflects `dropped_attributes_count` verbatim, never recomputed (RFC0003.10); - inherits the `Resource` attributes and `InstrumentationScope` name/version onto each record so downstream never walks the hierarchy. Consumes the `LogRecord` so the body `AnyValue` + per-record attributes move (no deep clone of structured trees, per the §6.4 amendment). API grown incrementally: this slice adds only the per-record unit the four scenarios exercise. Tenant derivation is taken as a parameter — fan-out + grouping over `ResourceLogs` (RFC0003.3/.4) build on this next. No new dependencies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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 selected for processing (7)
📝 WalkthroughWalkthroughThis PR implements the ChangesReceiver Materialization
Sequence DiagramsequenceDiagram
participant Input as LogRecord
participant Materialize as materialize_record
participant Helpers as sentinels
participant Output as OtlpLogRecord
Input->>Materialize: LogRecord with metadata
Materialize->>Helpers: observed_time_unix_nano
Helpers-->>Materialize: None if zero
Materialize->>Helpers: severity_number
Helpers-->>Materialize: u8 clamped at boundaries
Materialize->>Helpers: scope name and version
Helpers-->>Materialize: None if empty string
Materialize->>Helpers: body AnyValue
Helpers-->>Materialize: Body variant
Materialize->>Helpers: trace and span ids
Helpers-->>Materialize: validated fixed length
Materialize->>Output: fully coerced record
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Implements the “record materialisation” slice of the OTLP receiver by mapping decoded OTLP LogRecord values into the flattened ourios_core::otlp::OtlpLogRecord shape consumed by the miner, and turns RFC0003.7–.10 acceptance tests from ignored stubs into live tests.
Changes:
- Added
receiver::materialize::materialize_recordto materialize a decodedLogRecordintoOtlpLogRecord(body fork, sentinel narrowing, passthrough fields). - Updated receiver module exports and crate docs to reflect the new materialization layer.
- Replaced RFC0003.7–.10 red-gate test stubs with active assertions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/ourios-ingester/src/receiver/materialize.rs | New materialization mapping from decoded OTLP LogRecord to flat OtlpLogRecord. |
| crates/ourios-ingester/src/receiver.rs | Exposes the new materialize module and re-exports materialize_record. |
| crates/ourios-ingester/src/lib.rs | Updates crate-level docs to list materialization as landed functionality. |
| crates/ourios-ingester/tests/rfc0003_7_body_structured_verbatim.rs | Makes RFC0003.7 assertions live for non-string AnyValue → Body::Structured. |
| crates/ourios-ingester/tests/rfc0003_8_body_string_lraw.rs | Makes RFC0003.8 assertions live for string body unwrapping + absent body. |
| crates/ourios-ingester/tests/rfc0003_9_edge_otlp_fields.rs | Makes RFC0003.9 assertions live for edge field narrowing/passthrough. |
| crates/ourios-ingester/tests/rfc0003_10_dropped_attributes_count.rs | Makes RFC0003.10 assertion live for dropped_attributes_count passthrough. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `UNSPECIFIED` (`0`) is an explicit OTLP value, kept as `0` | ||
| // (RFC0003.9). Proto's `i32` is narrowed to the schema's `u8`; | ||
| // an out-of-range/invalid value narrows to `0`/UNSPECIFIED. | ||
| severity_number: u8::try_from(record.severity_number).unwrap_or(0), | ||
| severity_text: nonempty(record.severity_text), |
There was a problem hiding this comment.
Good catch — fixed. The narrowing now enforces the OtlpLogRecord 0..=24 contract: severity_to_u8 maps anything outside 0..=24 (the u8-representable 25..=255, negatives, and >255) to 0/UNSPECIFIED, while valid values incl. 0 and 24 are preserved. The misleading "out-of-range/invalid narrows to 0" comment is corrected, and a new test pins 0/24/25/1000/-5. This keeps the downstream template-key + Parquet assumptions valid, and is consistent with RFC0003.9 (it normalises invalid input rather than coalescing valid values).
The prior narrowing (`u8::try_from(i32).unwrap_or(0)`) only mapped values that don't fit a `u8` to 0; invalid-but-representable `25..=255` passed through, violating the documented `OtlpLogRecord` severity contract (`0..=24`) that the miner's template key and the Parquet schema rely on. Add `severity_to_u8`, which narrows anything outside `0..=24` — `25..=255`, negative, or `> 255` — to `0`/UNSPECIFIED. Valid values (incl. `0` UNSPECIFIED and `24` FATAL4) are preserved. New test covers `0/24/25/1000/-5`. Surfaced in review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| scope_name: scope.and_then(|s| nonempty(s.name.clone())), | ||
| scope_version: scope.and_then(|s| nonempty(s.version.clone())), |
There was a problem hiding this comment.
Fixed in fa1f7b9 — scope_name/scope_version now clone only on the non-empty path ((!s.name.is_empty()).then(|| s.name.clone())), so an empty scope name no longer allocates a string just to discard it.
| /// Proto scalar `0` → `None`, else `Some` — the §6.9-style narrowing of | ||
| /// a "0 = unset" wire sentinel. |
There was a problem hiding this comment.
Fixed in fa1f7b9 — the nonzero doc now references RFC0003.9 instead of the stray "§6.9".
… ref Two review nits, non-behavioral: - `scope_name`/`scope_version` checked `is_empty` on a clone then discarded it when empty; now clone only on the non-empty path. - `nonzero`'s doc referenced "§6.9-style" narrowing; the relevant reference is RFC0003.9. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Third green slice of the OTLP receiver (RFC 0003 §6.1 steps 2–3): record materialisation, flipping RFC0003.7–.10 live. Builds on the decode layer (#129, #131).
receiver::materialize::materialize_recordmaps a decodedLogRecord→ the flatOtlpLogRecordthe miner consumes, inheriting theResourceattributes andInstrumentationScopename/version so downstream never walks the OTLP hierarchy. It consumes the record so the bodyAnyValue+ per-record attributes move (no deep clone of structured trees, per the §6.4 amendment).Scenarios (now live)
string_valueAnyValue→Body::Structuredverbatim (table over bool/int/double/bytes/array/kvlist), viaBody::from_any_value.string_value→Body::String(s)unwrapped byte-for-byte; absent body →None.severity_number = 0(UNSPECIFIED) kept as0,time_unix_nano = 0(unknown) kept as theu640; wireobserved_time_unix_nano = 0and emptyscope_name/scope_versionnarrow toNone; nothing coalesced/substituted; inherited resource attributes pass through verbatim.dropped_attributes_countreflected verbatim, never recomputed.Scope
API grown incrementally per the maintainer decision: only the per-record unit the four scenarios exercise. Tenant derivation is a parameter — fan-out + grouping over
ResourceLogs(RFC0003.3/.4) build on this next. No new dependencies.The narrowing helpers (
nonzero,nonempty,fixed_len) collapse proto's "empty value = absence" into a singleOption/Noneat the boundary; out-of-rangeseverity_number(invalid OTLP) narrows to0/UNSPECIFIED.Verification
cargo test -p ourios-ingester✓ — RFC0003.5–.10 live; 9 scenarios remain ignored (.1/.2/.3/.4/.11–.15).cargo fmt --all --check✓ ·cargo clippy --all-targets --all-features -- -D warnings✓ (workspace)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation