docs(rfc): specify RFC 0037 — GenAI / structured-event log handling - #598
Conversation
📝 WalkthroughWalkthroughAdds RFC 0037 for structured GenAI log events, documenting event-aware templates, uncapped structured bodies, promoted-attribute grouping, calibration, acceptance criteria, and related hazard guidance. ChangesGenAI structured log events
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
Adds RFC 0037 to specify how Ourios should handle OpenTelemetry GenAI-style structured log events (event_name + structured AnyValue bodies), and updates the hazards doc to explicitly cover structured bodies under the parameter-cardinality hazard.
Changes:
- Add RFC 0037 defining three deltas: event_name-keyed structured templates, structured-body hazard mitigation via observability, and
count … bysupport for promoted attribute columns. - Link RFC 0037 into the mdBook navigation.
- Amend
docs/hazards.mdH2 to cover structured bodies and their (proposed) mitigation approach.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/SUMMARY.md | Adds RFC 0037 to the RFC list for mdBook navigation. |
| docs/rfcs/0037-genai-structured-log-events.md | New RFC specifying intended GenAI/structured-event log handling behavior and acceptance criteria. |
| docs/hazards.md | Extends hazard H2 to explicitly address structured (non-string) bodies and the intended mitigation strategy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
17aa0bd to
21af0be
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/rfcs/0037-genai-structured-log-events.md (1)
273-298: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository’s acceptance-scenario blockquote format.
Based on learnings, each §5 scenario should be a separate
> **Scenario ...**blockquote with blank lines between scenarios rather than a bullet list.🤖 Prompt for 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. In `@docs/rfcs/0037-genai-structured-log-events.md` around lines 273 - 298, Rewrite the acceptance criteria in §5 from bullet items into separate blockquotes using the repository’s `> **Scenario ...**` format, with a blank line between each scenario. Preserve the existing RFC0037.1–RFC0037.5 identifiers and all scenario details unchanged.Source: Learnings
🤖 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 `@docs/rfcs/0037-genai-structured-log-events.md`:
- Around line 14-19: Correct the status sentence in the RFC so its
acceptance-criteria date is not later than the document’s July 22, 2026 date, or
explicitly state that the document is intentionally postdated; preserve the
existing status and section references.
- Around line 156-167: The promoted-attribute check in field_group_expr must
also require an explicit groupability policy, rather than treating every
promoted column as groupable. Add groupability metadata for promoted fields,
mark gen_ai.conversation.id and gen_ai.response.id as filtering-only, reject
them with the promotion/grouping guidance error, and extend acceptance coverage
for these grouping attempts while preserving grouping for promoted groupable
attributes.
- Around line 124-147: Define an explicit admission/backpressure procedure for
oversized structured bodies in the structured-body policy at
docs/rfcs/0037-genai-structured-log-events.md:124-147, including the
availability behavior when the soft threshold is exceeded; do not rely on
structured_body_bytes alerting or RFC 0036 storage sizing as the resource bound.
Update the corresponding hazard entry at docs/hazards.md:101-114 to describe the
same admission/backpressure control and avoid presenting observation alone as
sufficient protection.
---
Nitpick comments:
In `@docs/rfcs/0037-genai-structured-log-events.md`:
- Around line 273-298: Rewrite the acceptance criteria in §5 from bullet items
into separate blockquotes using the repository’s `> **Scenario ...**` format,
with a blank line between each scenario. Preserve the existing
RFC0037.1–RFC0037.5 identifiers and all scenario details 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: 65e13240-d710-4452-ba00-f4e9ff03bef0
📒 Files selected for processing (3)
docs/SUMMARY.mddocs/hazards.mddocs/rfcs/0037-genai-structured-log-events.md
OpenTelemetry models LLM/agent activity as log events (event_name + structured AnyValue body such as gen_ai.input.messages). Ourios already ingests, stores, and reconstructs these correctly (body_kind=Structured, event_name column, RFC 0025 absent-body); this RFC scopes three deltas on top: fold event_name into the structured-template key (§3.1); guard structured-body size by observation not truncation — Option A, full fidelity + structured_body_bytes metric (§3.2); and extend `count … by` to group on a promoted attribute column (§3.3). §3.5 records why a true GenAI vertical slice is rejected (promotion already yields the typed columns) and the SIG-grounded recommended promotion set. §5 acceptance criteria frozen; docs/hazards.md H2 amended for structured bodies. Corpus: synthetic fixture now, Claude Code's own OTLP export as a real available-now AI-agent source, otel-demo v9 for gen_ai.* validation. Status specified — implementation follows in slices. 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>
21af0be to
c4b915d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
docs/rfcs/0037-genai-structured-log-events.md:135
- This contains an unresolved wiki-style reference
[[telemetry-otel-meters-not-prometheus-client]]that doesn’t appear elsewhere in the docs, so it’s likely to render as literal text and not as a link. Suggest replacing it with an explicit reference to the existing “OTel meters, not the Prometheus client” convention (e.g. RFC 0016 §3.6).
- **`structured_body_bytes` metric.** A histogram (OTel meter, per
[[telemetry-otel-meters-not-prometheus-client]]) observing the
canonical-JSON byte length of every structured body, dimensioned by
service so an operator can see which service emits large bodies.
docs/hazards.md:109
- This paragraph explains the structured-body path as avoiding Parquet dictionary encoding because bodies are “unique per record”. In the current implementation, the stronger/clearer reason is that the
bodycolumn is explicitly configured with dictionary encoding disabled (e.g.crates/ourios-parquet/src/writer.rs:993-1005). It would be more robust to document the actual encoding policy rather than relying on uniqueness assumptions.
column and is **not** capped: truncating it would violate the
bit-identical-reconstruction invariant (`CLAUDE.md` §3.3), and unlike
a runaway `params` slot it never populated dictionary encoding to
begin with (structured bodies are unique per record). The blowup risk
here is raw storage size, not dictionary collapse, so it is guarded by
Review fixes on the RFC 0037 spec (Copilot + CodeRabbit): - §5 reformatted to the repo's canonical scenario format (blockquoted `Scenario <id> — <title>` + Given/When/Then/And bullets, greppable ids). - §3.2 / hazards H2: corrected the no-cap rationale — the real reason hazard #2 doesn't apply to structured bodies is that the writer disables dictionary encoding on the `body` column by design (writer.rs §3.6), not that bodies are "unique per record". hazards H2 reworded to read as RFC 0037's proposed (not-yet-implemented) mitigation, and structured_body_bytes as a meter. - Dropped the inconsistent "(gap N)" heading labels on §3.1–§3.3. - Removed a leaked `[[...]]` memory-link placeholder from §3.2. 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>
RFC 0037 — GenAI / structured-event log handling (
specified)OpenTelemetry models LLM/agent activity as log events (
event_name+ a structuredAnyValuebody such asgen_ai.input.messages). Ourios already handles the base case correctly — structured bodies stored whole as canonical JSON (body_kind = Structured),event_namea first-class column, RFC 0025 covering the event-shaped absent-body record. This RFC scopes three deltas on top:event_nameinto the structured-template key(severity, scope, event_name)so distinct event types get distincttemplate_ids.param_byte_limitguard) without violating §3.3 fidelity. Direction resolved (maintainer): Option A — never truncate; guard by astructured_body_bytesmetric + per-service alert, not a cap. (Former Option B rejected, §4.)count … byto group on a promoted attribute column, socount by attr.gen_ai.request.modelis expressible.Plus §3.5: why a true GenAI vertical slice is rejected (promotion already yields the typed columns), and the SIG-grounded recommended promotion set. §3.4 corpus: a synthetic fixture now, Claude Code's own OTLP export normalized to
gen_ai.*via a Collector+OTTL step (the architecturally-correct placement) as a real available-now source, and otel-demo v9 for nativegen_ai.*validation.Status
specified— the §3.2 fork is decided, §5 acceptance criteria are frozen (canonical scenario format), anddocs/hazards.mdH2 is amended. This PR is the frozen contract; implementation follows in three verified slices (§3.1 → §3.2 → §3.3), each red→green, reachingvalidatedon the v9 corpus.Invariants/hazards touched (CLAUDE.md §4)
Hazard #1 (miner — template population, re-bases RFC 0024 calibration), #2 (structured-body guard), #6 (DSL group-by surface). Invariant §3.3 (fidelity) preserved unconditionally; §3.5 (schema) additive only.
🤖 Generated with Claude Code