feat(miner): observe structured-body size via a histogram (RFC 0037 §3.2) - #600
Conversation
…3.2) Structured (non-string) log bodies — GenAI/agent event bodies — are retained whole and never capped (truncating would violate §3.3, and the body column has dictionary encoding disabled by design so hazard #2's failure mode does not apply). The guard against oversized payloads is therefore observation, not truncation: a new ourios.miner.structured_body.size histogram (unit By, dimensioned by ourios.service) records the canonical-JSON byte length of every structured body, so an operator can spot a service shipping large payloads and fix it at the emitter. The name is authored in the weaver registry (semconv/registry/metrics.yaml, copying the ourios.miner.confidence group shape) and regenerated into ourios-semconv — never hand-written, so the CI no-diff and live-check gates hold. The instrument reuses the shared service_attrs helper and mirrors record_overflow rather than duplicating the meter plumbing. Acceptance: RFC0037.3 (rfc0037_structured_body.rs) asserts both unbounded fidelity (a large structured body is retained byte-for-byte, lossy_flag=false) and the metric emission (histogram sum == canonical-JSON length, carrying ourios.service). Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughStructured-body ingestion now records canonical JSON payload sizes in a new per-tenant/service histogram. Semantic-convention metadata, metric registration, ingestion wiring, and an RFC0037 integration test were added. ChangesStructured body metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Adds observability for structured (non-string) OTLP log bodies in the miner by recording their canonical-JSON size as a per-tenant / per-service histogram, aligning with RFC 0037 §3.2’s “observe, don’t truncate” approach for structured bodies.
Changes:
- Define a new semantic-convention metric
ourios.miner.structured_body.size(histogram, unitBy) and regenerate theourios-semconvconstant. - Extend
MinerMetricswith aHistogram<u64>and arecord_structured_body_byteshelper that reuses the shared(tenant, service)attribute builder. - Record structured-body byte length during structured ingest, and add an acceptance test asserting both byte-for-byte retention and correct histogram emission (including
ourios.service).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| semconv/registry/metrics.yaml | Adds the metric definition for ourios.miner.structured_body.size with required/recommended attributes. |
| crates/ourios-semconv/src/lib.rs | Regenerates semconv constants to include OURIOS_MINER_STRUCTURED_BODY_SIZE. |
| crates/ourios-miner/src/metrics.rs | Adds the histogram instrument and a record_structured_body_bytes method using existing attribute helpers. |
| crates/ourios-miner/src/cluster.rs | Records the structured-body canonical-JSON size at ingest time before moving bytes into the emitted record. |
| crates/ourios-miner/tests/rfc0037_structured_body.rs | New RFC0037.3 acceptance test covering unbounded structured-body fidelity and metric emission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@crates/ourios-miner/tests/rfc0037_structured_body.rs`:
- Around line 59-62: Update the fidelity assertions in the structured-body test
around expected_bytes to retain and compare the full canonical byte sequence,
not just its length. Reuse the output of canonical::encode_any_value as the
expected bytes, and assert byte-for-byte equality against the retained body;
keep the existing length metric assertion as needed.
- Around line 45-122: The existing RFC0037 integration test needs complementary
colocated unit coverage for structured-body ingestion. Add a focused test module
alongside the implementation in cluster.rs or metrics.rs, exercising the
non-trivial structured-body behavior—whole-body retention without lossy marking
and the corresponding size metric—while keeping the existing end-to-end test
unchanged.
🪄 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: a6aa3a1b-bb21-4c9b-b32c-0dea74eb0527
📒 Files selected for processing (5)
crates/ourios-miner/src/cluster.rscrates/ourios-miner/src/metrics.rscrates/ourios-miner/tests/rfc0037_structured_body.rscrates/ourios-semconv/src/lib.rssemconv/registry/metrics.yaml
…tical assert Review fixes on slice B (CodeRabbit): - Add rfc0037_3_structured_body_retained_byte_for_byte, a unit test colocated with ingest_structured (§6.2 mandate): the structured branch retains the body's canonical JSON byte-for-byte, body_kind = Structured, non-lossy. The integration test keeps the metric-emission coverage, which needs the SDK meter provider. - Strengthen the integration test to assert the full canonical-JSON string (byte-for-byte), not just its length, so a same-length corruption fails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
…metric Review fix (Copilot): the histogram data-point matcher only checked ourios.service; the registry marks ourios.tenant `required`, so the test now asserts both ourios.tenant and ourios.service ride the data point. Dropping tenant would previously have passed silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
The three implementation slices landed (#599 §3.1, #600 §3.2, #601 §3.3). Flip status specified→green and map each §5 scenario to its greppable test: RFC0037.1/.3/.4 are the new slice tests; RFC0037.2 (structured-body reconstruction) is covered by the standing reconstruction property (RFC 0024 generates structured bodies) plus the miner/parquet Structured round-trips and the byte-for-byte retention unit; RFC0037.5 (absent-body parity) by rfc0025_absent_body.rs. `validated` waits on the v9 corpus (§3.4 calibration). Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
* docs(rfc): flip RFC 0037 to green — all five §5 scenarios pass The three implementation slices landed (#599 §3.1, #600 §3.2, #601 §3.3). Flip status specified→green and map each §5 scenario to its greppable test: RFC0037.1/.3/.4 are the new slice tests; RFC0037.2 (structured-body reconstruction) is covered by the standing reconstruction property (RFC 0024 generates structured bodies) plus the miner/parquet Structured round-trips and the byte-for-byte retention unit; RFC0037.5 (absent-body parity) by rfc0025_absent_body.rs. `validated` waits on the v9 corpus (§3.4 calibration). Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): correct RFC0037.2 coverage wording — canonical-JSON equal, not byte-for-byte Review fix (Copilot): the RFC 0024 property tests assert structured bodies round-trip *canonical-JSON equal* (decode_any_value on the rebuilt bytes equals the original AnyValue — decoded value equality), not byte-for-byte. Byte identity of the stored canonical JSON is pinned separately by rfc0037_3_structured_body_retained_byte_for_byte. Reword §6 to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): fix RFC0037.3/.4 §6 wording — attr requirement levels + key order Review fixes (CodeRabbit): ourios.service is recommended (emitted when a service is present), not required — reword RFC0037.3 to "required ourios.tenant plus recommended ourios.service present for this record". And RFC0037.4's result map is keyed in by-list order [model, bucket], not (bucket, model) — correct the §5 scenario and §6 wording to (model, bucket). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
RFC 0037 slice B —
structured_body_bytesobservability (§3.2)Second of three implementation slices (after slice A #599). Implements the hazard-#2 guard for structured bodies.
Design (Option A, maintainer-decided in the spec). A structured (non-string) log body — a GenAI/agent event body — is retained whole and never capped. Truncating would violate §3.3 (bit-identical reconstruction), and the
bodycolumn has dictionary encoding disabled by design (writer.rs§3.6), so hazard #2's dictionary-collapse failure mode is structurally absent for it. The guard against oversized payloads is therefore observation, not truncation: a newourios.miner.structured_body.sizehistogram (unitBy, dimensioned byourios.service) records the canonical-JSON byte length of every structured body, so an operator can spot a service emitting large payloads and fix it at the emitter.Consolidated, not copy-pasted. The instrument is one
Histogram<u64>field added to the existingMinerMetrics, built on theourios.minermeter, and recorded via arecord_structured_body_bytesmethod that reuses the sharedservice_attrshelper and mirrorsrecord_overflow. The record call sits iningest_structured, capturingbytes.len()before the canonical-JSONbytesare moved into the record.Weaver-authored name. The metric name is defined in
semconv/registry/metrics.yaml(copying theourios.miner.confidencegroup shape —instrument: histogram,unit: By,ourios.tenantrequired +ourios.servicerecommended) and regenerated intocrates/ourios-semconv/src/lib.rsviaweaver registry generate. Not hand-written, so the CI no-diff and live-check gates stay green. No new attribute needed —ourios.tenant/ourios.servicealready exist.Invariants (CLAUDE.md §4)
docs/hazards.mdH2 that landed with the spec): structured bodies were previously an unguarded ingress; they now have the metric + per-service-alert guard, with fidelity (§3.3) unconditionally preserved.Verification
rfc0037_structured_body.rsasserts both unbounded fidelity (a large structured body — ~200-element array, >8 KB — is retained byte-for-byte withlossy_flag = false) and the metric emission (histogramsumequals the canonical-JSON byte length, data point carriesourios.service). ✅Refs #546. Slice C (§3.3 group-by-promoted-attribute) follows; RFC flips to
greenonce all §5 pass (RFC0037.2 / .5 verified against existing RFC 0005 / 0025 coverage at that point).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests