Skip to content

feat(core): introduce OtlpLogRecord with AnyValue body - #27

Merged
jensholdgaard merged 2 commits into
mainfrom
feat/core-otlp-log-record
May 14, 2026
Merged

feat(core): introduce OtlpLogRecord with AnyValue body#27
jensholdgaard merged 2 commits into
mainfrom
feat/core-otlp-log-record

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented May 14, 2026

Copy link
Copy Markdown
Owner

Summary

Cashes in the §6.1/§6.2 amendments by giving Ourios its first faithful in-memory OTLP record type. Until this PR no code in the workspace knew what an OTLP record was — the spec said MinerCluster::ingest should take one, but the type didn't exist.

This is PR A of the two-PR plan (discussion in PR #25 / #26 / earlier conversation). PR B (the MinerCluster::ingest signature change + body-fork implementation) follows.

Design

crates/ourios-core/src/otlp.rs:

  • OtlpLogRecord — receiver-flattened wire shape per RFC 0001 §6.1 + RFC 0003 §6.6, field-for-field with the spec. Carries tenant_id as its own field (otel.* is reserved and Resource attributes describe the observed entity, not Ourios's ingest routing — putting tenant_id there would violate spec intent per RFC 0003 §6.3).
  • Body { String(String), Structured(AnyValue) } — the §6.2 step-0 fork. Structured carries the decoded opentelemetry-proto AnyValue directly, not its OTLP-canonical JSON encoding (see RFC 0003 §6.4 amendment below).
  • BodyKind { String, Structured } — derived via OtlpLogRecord::body_kind() -> Option<BodyKind>. Never stored, since the discriminator is the only thing the §6.2 step-0 fork needs.

Why opentelemetry-proto directly, not a hand-rolled re-implementation

This is the spec-faithfulness call. The OTel spec defines non-trivial AnyValue conversion rules (integer ranges, enum handling, non-UTF-8 → bytes_value, …) and any hand-rolled re-implementation carries real drift risk. The usual ACL argument doesn't apply: opentelemetry-proto isn't foreign to Ourios — it IS the ingest contract; insulating from it would be insulating from our own canonical wire format. The wrapping OtlpLogRecord struct exists to attach tenant_id and to flatten ResourceLogs → ScopeLogs → LogRecord nesting, not to duplicate types.

opentelemetry-proto enters with default-features = false, features = ["gen-tonic-messages", "logs"] — minimum for the tonic::logs::v1 and tonic::common::v1 types. RFC 0003 §4.1 already pinned this crate as the wire-stack default, so this isn't a new architectural commitment.

RFC amendments (atomic with this PR so spec ↔ code agree at every commit)

The decision to carry AnyValue directly contradicts RFC 0003's original §6.4 stance that the receiver canonicalises eagerly into Bytes. Both RFCs amended in the same commit:

  • RFC 0003 §6.4 — canonicalisation deferred to the storage layer. Preserves optionality for the future "mine inner field" mode (RFC 0001 §6.1) gated on corpus evidence.
  • RFC 0003 §6.5 step (2) — drops the canonicalises-here claim.
  • RFC 0003 §6.6 — OtlpLogRecord struct sketch updated to match: body: Option<Body> with Body::Structured(AnyValue); body_kind() is a derivation, not a stored column. Authoritative definition is now ourios-core::otlp.
  • RFC 0001 §5 RFC0001.9 — body-column wording updated: canonical JSON is produced at Parquet-write time, not at the receiver.

Invariant / hazard touch

  • [§3.7] (multi-tenancy not bolted on): tenant_id is a top-level field on every record from day one — no code path constructs an OtlpLogRecord without one.
  • §6.1 Template-key composition: the severity_number = 0 = UNSPECIFIED distinct-bucket rule is locked in by the Default test (zero-value default, not "missing").

No invariant in CLAUDE.md §3 is touched destructively; this PR adds a new type and amends two RFC subsections to match it.

Tests

7 unit tests in crates/ourios-core/src/otlp.rs, all passing:

  • Body::from_any_value — String / non-String / empty-value handling locks the §6.2 step-0 fork at the type-construction boundary.
  • OtlpLogRecord::body_kind() — correctly classifies absent / String / Structured bodies (one test per case, strict AAA).
  • OtlpLogRecord::default() — pins the important defaults (severity = 0 = UNSPECIFIED distinct bucket, all OTLP fields zeroed, body absent).

Out of scope

  • MinerCluster::ingest signature unchanged. PR B handles that.
  • None of the H1.4/H1.5/RFC0001.9–11 ignored test stubs are filled — they reference OtlpLogRecord but the miner crate doesn't consume it yet.
  • No receiver, no WAL, no Parquet writer changes.

Test plan

  • cargo fmt --all --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-features — 7 new ourios-core tests pass; workspace 57 passed / 25 ignored / 0 failed
  • mdbook build
  • CI green

🤖 Generated with Claude Code

@jensholdgaard @claude
feat(core): introduce OtlpLogRecord with AnyValue body
7098636 
Cashes in the §6.1/§6.2 amendments by giving Ourios its first
faithful in-memory OTLP record type. Until this PR no code in
the workspace knew what an OTLP record was — the spec said
`MinerCluster::ingest` should take one, but the type didn't exist.

## Type design

`crates/ourios-core/src/otlp.rs` defines:

- `OtlpLogRecord` — the receiver-flattened wire shape per
  RFC 0001 §6.1 + RFC 0003 §6.6. Field-for-field with the spec.
  Carries `tenant_id` as its own field (per RFC 0003 §6.3, the
  `otel.*` namespace is reserved and Resource attributes
  describe the *observed entity*, not Ourios's ingest routing —
  putting `tenant_id` there would violate spec intent).
- `Body { String(String), Structured(AnyValue) }` — the §6.2
  step-0 fork. `Structured` carries the decoded
  `opentelemetry-proto` `AnyValue` directly, not its OTLP-
  canonical JSON encoding (see §6.4 amendment below).
- `BodyKind { String, Structured }` — derived from `body` via
  `OtlpLogRecord::body_kind() -> Option<BodyKind>`. Never
  stored, since the discriminator is the only thing the §6.2
  step-0 fork needs.

The decision to use `opentelemetry-proto`'s `AnyValue` /
`KeyValue` directly (rather than rolling our own minimal types
in `ourios-core`) is the spec-faithfulness call: the OTel
spec defines non-trivial `AnyValue` conversion rules (integer
ranges, enum handling, non-UTF-8 → `bytes_value`, ...) and any
hand-rolled re-implementation carries real drift risk. The
usual ACL argument doesn't apply here — `opentelemetry-proto`
isn't a *foreign* vocabulary to Ourios, it IS the ingest
contract; insulating from it would be insulating from our own
canonical wire format. The wrapping `OtlpLogRecord` struct
exists to attach `tenant_id` and to flatten the
`ResourceLogs → ScopeLogs → LogRecord` nesting, not to
duplicate types.

`opentelemetry-proto` enters the workspace with
`default-features = false, features = ["gen-tonic-messages",
"logs"]` — minimum needed for the `tonic::logs::v1` and
`tonic::common::v1` types. RFC 0003 §4.1 already pins this
crate as the wire-stack default, so this is not a new
architectural commitment.

## RFC amendments

The above design contradicts RFC 0003's original §6.4 stance
that the *receiver* canonicalises structured bodies eagerly
into `Bytes`. Both RFCs amended atomically with this PR so
spec and code agree at every commit:

- RFC 0003 §6.4 — canonicalisation is now deferred to the
  storage layer (Parquet writer, when it lands) so the
  in-memory record carries `Body::Structured(AnyValue)`
  verbatim. Preserves optionality for the future "mine inner
  field" mode (RFC 0001 §6.1) gated on corpus evidence.
- RFC 0003 §6.5 — step (2) reworded to drop the
  canonicalises-here claim.
- RFC 0003 §6.6 — `OtlpLogRecord` struct sketch updated to
  match the as-built type: `body: Option<Body>` with
  `Body::Structured(AnyValue)`, `body_kind()` is a derivation,
  not a stored column. Authoritative definition is now
  `ourios-core::otlp` (not `ourios-ingester`, which doesn't
  exist yet).
- RFC 0001 §5 RFC0001.9 — body-column wording updated to
  reflect that the canonical JSON encoding is produced at
  Parquet-write time, not at the receiver.

## Tests

7 unit tests in `crates/ourios-core/src/otlp.rs`:

- `Body::from_any_value` String / non-String / empty-value
  handling — locks the §6.2 step-0 fork at the type-construction
  boundary.
- `OtlpLogRecord::body_kind()` correctly classifies absent /
  String / Structured bodies.
- `OtlpLogRecord::default()` pins the important defaults
  (severity_number = 0 = UNSPECIFIED — distinct key bucket per
  §6.1; all OTLP fields zeroed; body absent).

## What this *doesn't* do

- `MinerCluster::ingest` signature unchanged (still `&str`).
  That's the next PR (PR B per the discussion). This PR is
  pure type plumbing.
- Doesn't fill any of the new H1.4/H1.5/RFC0001.9–11 stubs;
  they reference `OtlpLogRecord` but the miner crate doesn't
  consume it yet.
- Doesn't touch the receiver or WAL crates (neither exists).

## Verified locally

- `cargo fmt --all --check`
- `cargo clippy --all-targets --all-features -- -D warnings`
- `cargo test --all-features` (7 new ourios-core tests pass;
  full workspace 57 passed / 25 ignored / 0 failed)
- `mdbook build`

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot May 14, 2026 19:09
Copilot AI reviewed May 14, 2026
View reviewed changes

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

Introduces the core in-memory OTLP log record shape that future receiver/miner integration will consume, while updating RFC text to defer structured body canonicalisation to the storage layer.

Changes:

  • Adds ourios-core::otlp with OtlpLogRecord, Body, and BodyKind.
  • Exposes the OTLP module from ourios-core and adds opentelemetry-proto.
  • Updates RFC 0001/0003 wording around structured AnyValue handling.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/ourios-core/src/otlp.rs Adds OTLP record/body types and unit tests.
crates/ourios-core/src/lib.rs Exposes the new otlp module.
crates/ourios-core/Cargo.toml Adds the OTLP proto dependency.
docs/rfcs/0001-template-miner.md Updates structured-body scenario wording.
docs/rfcs/0003-otlp-receiver.md Amends canonicalisation boundary and record-shape documentation.

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

Comment thread crates/ourios-core/src/otlp.rs Outdated
Comment thread docs/rfcs/0003-otlp-receiver.md
Comment thread crates/ourios-core/Cargo.toml
@jensholdgaard @claude
fix: address PR #27 review feedback
a697f6d 
Three substantive review comments, each addressed:

1. **`Body::from_any_value` no longer deep-clones the AnyValue tree.**
   The structured branch now moves the inner `oneof` out and
   reconstructs `AnyValue { value: Some(other) }` rather than
   cloning the whole tree before classification. Arrays /
   kvlists / bytes pay no copy cost on the miner-facing path.
   Aligns with the §6.4 amendment's "no allocation in the
   structured branch" claim. Doc comment updated to state the
   move semantics explicitly.

2. **RFC 0003 now has a single authoritative canonicalisation
   boundary.** The §6.4 amendment was the bottom-line shift, but
   §1 summary, §2.2 third bullet, §5 RFC0003.6 / RFC0003.7,
   §6.1 step (3), §6.2 HTTP/JSON dispatch, §7.5
   (alternative-rejected), and §8 testing-strategy bullet all
   still spoke as if the receiver canonicalised. Each updated
   to reflect the storage-layer-canonicalisation commitment;
   §9's "Where does canonicalisation cost land?" open question
   marked resolved.

3. **`Cargo.lock` committed.** Adding `opentelemetry-proto`
   changed dependency resolution; `.gitignore`:19-20 documents
   the lockfile as part of the reproducible-build contract for
   this binary project. PR A omitted the lockfile change; this
   commit fixes that.

Verified locally: cargo fmt --check, clippy -D warnings, 7
ourios-core tests still pass, full workspace 57 passed / 25
ignored / 0 failed, mdbook builds.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard merged commit af9ab18 into main May 14, 2026
7 checks passed
@jensholdgaard jensholdgaard mentioned this pull request May 14, 2026
Merged
5 tasks
jensholdgaard added a commit that referenced this pull request May 14, 2026
@jensholdgaard @claude
feat(miner): consume OtlpLogRecord with body_kind fork (#28)
d5a7083 
* feat(miner): consume OtlpLogRecord with body_kind fork

PR B in the OtlpLogRecord rollout (PR #27 was PR A). With the
type now defined, this PR makes the miner actually consume it.

## API change

`MinerCluster::ingest`:

  before: pub fn ingest(&mut self, tenant_id: &TenantId, raw: &str) -> u64
  after:  pub fn ingest(&mut self, record: &OtlpLogRecord) -> u64

The `tenant_id` now lives on the record per RFC 0001 §6.1 (and
per RFC 0003 §6.3, the receiver derives it once per
`ResourceLogs.resource`). The body comes from `record.body`.

## Body fork (RFC 0001 §6.2 step 0)

- `Body::String(s)` — existing tokenize → mask → descend
  pipeline. The leaf-list lookup now also compares the leaf's
  `(severity_number, scope_name)` against the record's, so two
  records with the same masked tokens but different severity or
  scope get distinct `template_id`s. Locks the implementation
  side of H1.4 / H1.5.
- `Body::Structured(_)` — short-circuit. The `AnyValue` tree is
  not walked; a per-tenant
  `(severity_number, scope_name) -> template_id` map keyed on
  the §6.1 *Template-key composition* tuple is the entire
  lookup. Same tuple reuses the same id (RFC0001.9-shape
  behaviour, modulo the not-yet-emitted Parquet columns).
- `None` body — returns `NO_TEMPLATE` (no allocation). §6.1
  doesn't pin a behaviour for the absent case; the conservative
  sentinel-return never silently coalesces into the structured
  bucket. Documented inline as a forward-pointer for a future
  amendment if the case turns out to need its own template.

## Type / impl details

- `Leaf` gains `severity_number: u8` and `scope_name:
  Option<String>` fields (the §6.1 template-key tuple's
  discriminator). `matches_exactly` extended to compare them.
- `TenantState` gains a `structured_templates:
  HashMap<(u8, Option<String>), u64>` for the §6.2 step-0
  short-circuit. The cache invariant on `template_count` now
  covers both stores: every fresh allocation (tree leaf or
  structured-map insert) increments the cache by exactly one.
- `ourios-core::otlp` now re-exports `AnyValue`, `KeyValue`,
  and `any_value` from `opentelemetry-proto`. Downstream crates
  (the miner today, the receiver and Parquet writer later) get
  a single import path through `ourios_core::otlp::*` and
  don't take a direct `opentelemetry-proto` dep.

## Test discipline

Existing tests adapted, not weakened, per CLAUDE.md §6.2
"Tests are specifications, not friction":

- The 7 cluster.rs unit tests (repeat-shape, distinct-shape,
  unseen-tenant, lazy-allocation, near-match-separate-leaves,
  empty-input sentinel, cache-invariant) all carry the same
  assertions. Only their call sites changed: `cluster.ingest(&t,
  "text")` becomes `cluster.ingest(&string_record(&t, "text"))`
  via a small in-test helper. With default severity (UNSPECIFIED
  / 0) and scope (None) on every record, the new template-key
  discriminator is constant, so the previously-asserted
  behaviour is preserved.
- The two crate-level integration tests in `tests/invariants.rs`
  (§3.7.1 cross-tenant pollination, §3.7.2 distinct ids per
  tenant) get the same migration. Same defaults, same
  assertions.
- 7 new cluster.rs unit tests cover the new code paths:
  * `ingest_returns_no_template_for_absent_body`
  * `structured_body_short_circuit_allocates_one_template_per_severity_scope_tuple`
  * `structured_body_distinguishes_severity_within_one_scope`
  * `structured_body_distinguishes_scope_within_one_severity`
  * `structured_body_with_scope_none_is_its_own_bucket`
  * `structured_and_string_share_no_template_ids_at_same_severity_scope`
  * `string_body_distinguishes_severity_within_one_scope`
- 7 tree.rs Leaf-construction sites updated for the new
  fields (severity_number=0, scope_name=None — what those
  tests assert is independent of the template-key
  discriminator).

The H1.4/H1.5/§3.7.3/RFC0001.9-11 ignored test stubs
(crates/ourios-miner/tests/{hazards,invariants,rfc_internal}.rs)
remain ignored. Their assertions are about the *emitted record*
(`confidence`, `lossy_flag`, Parquet columns), which need the
Parquet writer crate to land before they become fillable.
This PR's behavioural changes are pinned by the new cluster
unit tests above; the §5 acceptance-criteria stubs get
implemented as the dependent crates land.

## Doc tweak (team feedback follow-up)

`ourios-core::otlp` crate-level docstring extended with one
paragraph explaining `tenant_id` is deliberately a sibling
field rather than synthesised into `resource_attributes` (the
OTel `Resource` spec restricts what belongs there, and
`otel.*` is reserved). Captures the rationale already in
PR #27's commit message but in the place a future reader is
most likely to look.

## Verified locally

- `cargo fmt --all --check`
- `cargo clippy --all-targets --all-features -- -D warnings`
- `cargo test --all-features` — workspace 64 passed / 25
  ignored / 0 failed (49 miner unit, 7 ourios-core, 5
  invariants integration, 3 rfc_internal integration,
  hazards.rs all-ignored).
- `mdbook build`

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: address PR #28 review feedback

Three substantive review comments, each addressed:

1. **Structured-store cross-tenant isolation regression test.**
   The §3.7.2 scenario in `tests/invariants.rs` exercises the
   String branch only (its line is "user 42 logged in from
   10.0.0.1"). The §3.7 invariant — *"every template tree is
   scoped per tenant"* — applies equally to the new
   structured-templates store, but PR #28 had no test pinning
   that. Added `structured_body_isolates_template_ids_across_tenants`
   in `cluster.rs` covering the structured branch: identical
   structured records from two tenants produce distinct
   template_ids, and each tenant's `template_count` is exactly 1
   (no cross-pollination).

2. **`MinerCluster::ingest` doc no longer claims the Parquet
   writer "reads `confidence`/`lossy_flag` off the record".**
   `OtlpLogRecord` has no such fields; those values are derived
   *by the miner* (sentinel-constants for the structured
   branch). Doc reworded to make the derivation explicit and to
   note that the values aren't on the cluster's return surface
   yet because the Parquet writer crate consuming them doesn't
   exist.

3. **`ourios-core::otlp` docstring no longer over-specifies
   tenant derivation.** Removed the "(or auth context)"
   parenthetical from the `Resource`-attribute derivation
   description — auth-context-driven binding is currently a
   RFC 0003 §9 open question, not part of the accepted
   contract. The docstring now flags it as a possible future
   extension that joins the same derivation rule rather than
   displacing it.

Verified locally: cargo fmt --check, clippy -D warnings, full
workspace 65 passed / 25 ignored / 0 failed (50 miner unit
tests now, +1 from the new structured-tenant-isolation case),
mdbook builds.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jensholdgaard