Skip to content

feat(parquet): RFC 0005 §3.6 — bloom filters on trace_id and span_id - #489

Merged
jensholdgaard merged 3 commits into
mainfrom
rfc0005-trace-bloom
Jul 12, 2026
Merged

feat(parquet): RFC 0005 §3.6 — bloom filters on trace_id and span_id#489
jensholdgaard merged 3 commits into
mainfrom
rfc0005-trace-bloom

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 12, 2026

Copy link
Copy Markdown
Owner

What

Bloom filters on trace_id and span_id (RFC 0005 §3.6 pattern, identical to the existing template_id and promoted-column blooms). One writer-properties change + a metadata-pinning test.

Measured result (pre-merge validation, run #14 on this branch — the measure-before-merge workflow)

The RFC 0031 L3 must-win flips to PASS on both metrics — the first L-class must-win pass in the comparative program:

L3: 9-row trace in 4.9M records before (run #12, main) after (run #14, this branch)
Ourios total bytes 72,935,984 4,812,668 (15.2× collapse)
vs Loki storage-side (105,353,837) 1.41× — FAIL at margin 10 21.9× — PASS
vs Loki processed (2,476,749,585) 33.2× 514.6× — PASS

The other three pairs are byte-identical on the Ourios side (deterministic store build), confirming the bloom perturbs nothing else. Equivalence held on all four pairs. Run #14 also validates #490's ingester-visibility fix: the L3 pair measured deterministically.

Why

Run #12 measured the no-bloom L3 baseline at 72.4 MB — the trace_id column scanned corpus-wide, because random ids defeat min/max statistics. Loki's side (105 MB fetched, 2.48 GB decompressed for 9 rows) is structural: trace ids cannot be labels in its model. DataFusion consults blooms by default (bloom_filter_on_read = true).

Invariants / hazards

§3.5: no schema change — blooms are optional column metadata; old readers and old files are unaffected in both directions. Writer-side only. The RFC 0005 §3.6 amendment text follows with these measured numbers (evidence-first sequencing per maintainer).

Checks run

cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings (workspace-wide), cargo nextest run -p ourios-parquet -p ourios-bench -p ourios-querier (447 passed). Pre-merge comparative bench: run #14 (29190408893), exit 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

@jensholdgaard
jensholdgaard requested a review from Copilot July 12, 2026 10:24
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Parquet writer properties now enable bloom filters for trace_id and span_id. A new integration test encodes records and verifies bloom-filter metadata for those columns and template_id.

Changes

Trace-context bloom filters

Layer / File(s) Summary
Bloom-filter writer configuration
crates/ourios-parquet/src/writer.rs
Enables per-column bloom filters for trace_id and span_id.
Integration validation
crates/ourios-parquet/tests/it/main.rs, crates/ourios-parquet/tests/it/trace_bloom.rs
Registers a test that inspects Parquet metadata and verifies bloom-filter offsets for trace_id, span_id, and template_id.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding bloom filters for trace_id and span_id.
Description check ✅ Passed The description covers the summary, related RFC context, and validation results, but it does not follow the template headings exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0005-trace-bloom

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Adds Parquet bloom filters for the trace-context identifier columns to enable row-group pruning for exact trace/span lookups (per RFC 0005 §3.6), matching the existing bloom strategy used for template_id and promoted attribute columns.

Changes:

  • Enable bloom filters for trace_id and span_id in Parquet writer properties.
  • Add an integration test that pins bloom filter presence for trace_id, span_id, and template_id.
  • Register the new test module in the integration test harness.

Reviewed changes

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

File Description
crates/ourios-parquet/src/writer.rs Enables bloom filters on trace_id/span_id via writer properties to improve exact-id read pruning.
crates/ourios-parquet/tests/it/trace_bloom.rs Adds a metadata-level assertion that bloom filters are written for the trace-context id columns.
crates/ourios-parquet/tests/it/main.rs Wires the new trace_bloom integration test module into the test suite.

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

Comment thread crates/ourios-parquet/tests/it/trace_bloom.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 3 out of 3 changed files in this pull request and generated no new comments.

jensholdgaard and others added 3 commits July 12, 2026 13:04
Random 16/8-byte ids defeat min/max statistics, so an exact-id lookup
(RFC 0031 L3) degenerates to a whole-column scan: measured 72.4 MB for
a 9-row trace on otel-demo-v8 (comparative run #12). Same §3.6 pattern
as the template_id and promoted-column blooms; readers are unaffected
(blooms are optional metadata) and DataFusion consults them by default.

Validation: pre-merge comparative dispatch on this branch (run #13)
per the maintainer's measure-before-merge workflow; the RFC 0005 §3.6
amendment text follows with the measured numbers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

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 3 out of 3 changed files in this pull request and generated no new comments.

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