Skip to content

test: land RFC0001.10 (ts preserved) + §3.7.3 (per-ResourceLogs tenant), relocated - #168

Merged
jensholdgaard merged 4 commits into
mainfrom
test/rfc0001-10-tenant-3-7-3
Jun 9, 2026
Merged

test: land RFC0001.10 (ts preserved) + §3.7.3 (per-ResourceLogs tenant), relocated#168
jensholdgaard merged 4 commits into
mainfrom
test/rfc0001-10-tenant-3-7-3

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Lands two RFC 0001 §5 acceptance criteria as integration tests and relocates their misplaced miner-crate stubs. Both behaviours already exist — this is tests + stub relocation only; no production code changes. Same pattern as the already-relocated RFC0001.5/.6 (query semantics → ourios-querier) and H5.3 (drift → ourios-querier).

Criterion 1 — RFC0001.10 (time_unix_nano preserved verbatim) → ourios-querier

time_unix_nano is a pass-through field (MinedRecord.time_unix_nano), so the §5 scenario is an end-to-end ingest → Parquet → query check that the miner crate cannot run (it can run neither the Parquet write nor a query).

The test (crates/ourios-querier/tests/rfc0001_time_preserved.rs):

  • writes a row at the RFC §5 instant 1_715_700_000_000_000_000 (plus an hour-earlier control) via the shared tests/common fixtures (simple / write_all, which lay down real Parquet through ourios-parquet);
  • runs a DSL range(...) query whose window (2024-05-13T11:33:20Z .. 2024-05-15T19:06:40Z, the RFC §5 bounds in RFC3339 — range takes RFC3339/duration/now, not bare nanos) straddles the instant, via Querier::run_query, and asserts exactly the in-window row is returned (control excluded). Gates benchmarks B1.
  • reads the row back through the production ourios_parquet::Reader and asserts the stored time_unix_nano equals the wire input byte-for-byte.

Criterion 2 — §3.7.3 (tenant derivation runs per ResourceLogs) → ourios-ingester

CLAUDE.md §3.7 / RFC 0003 §6.3: the receiver derives tenant_id once per ResourceLogs, not per export batch. This is a receiver/pipeline behaviour the miner crate cannot drive — the miner-side stub's own comment already anticipated the move ("lives with RFC 0003 once the receiver crate exists").

The test (crates/ourios-ingester/tests/invariant_3_7_3_tenant_per_resource_logs.rs):

  • builds one ExportLogsServiceRequest with two ResourceLogs whose service.name resource attrs resolve (default TenantRule::service_name()) to tenants a and b, two records each;
  • drives it through an IngestPipeline (real WAL + a MinerCluster wired with a SharedRecordSink) via ingest;
  • asserts each ResourceLogs group's records are mined under their own tenant (drained sink, partitioned by tenant_id), that no record is cross-tagged (each carries its Resource's service.name), and that each per-tenant tree holds exactly its own two templates (template_count).

The receiver-side fan-out is also covered by RFC0003.3; this scenario carries it the rest of the way, through the miner, to prove no record lands in the wrong tenant's tree.

Stub relocation

  • crates/ourios-miner/tests/rfc_internal.rs::rfc0001_10_… (#[ignore] + todo!()) → one-line pointer comment.
  • crates/ourios-miner/tests/invariants.rs::invariant_3_7_3_… (#[ignore] + todo!()) → one-line pointer comment.

Invariants touched

§3.7 (multi-tenancy is not bolted on): the §3.7.3 test is a direct positive assertion of per-ResourceLogs tenant scoping with a no-cross-contamination guard — it strengthens, not weakens, the invariant. No production code changed.

Verification

cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings, and cargo test --all-features all pass locally: 587 passed, 0 failed, 26 ignored (the two relocated stubs no longer among the ignored). No benchmarks touched.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Tests
    • Added comprehensive test coverage validating tenant isolation to ensure data from different tenants is properly segregated and not inadvertently shared.
    • Added test validation confirming timestamp values are preserved accurately in storage without rounding or truncation.
    • Reorganized test suites for better architectural alignment.

…t), relocated

Both behaviours already exist; this lands their RFC 0001 §5 acceptance
criteria as integration tests in the crates that can exercise them and
removes the misplaced miner-crate stubs (same pattern as RFC0001.5/.6
and H5.3).

RFC0001.10 (`time_unix_nano` preserved verbatim) → ourios-querier:
an end-to-end ingest → Parquet → query scenario. A row written at the
§5 instant is returned by a `range(...)` query straddling it, and the
value read back via `ourios_parquet::Reader` equals the wire input
byte-for-byte (a pass-through field). Lives in
crates/ourios-querier/tests/rfc0001_time_preserved.rs; the miner crate
can run neither the Parquet write nor a query.

§3.7.3 (tenant derivation runs per `ResourceLogs`) → ourios-ingester:
one ExportLogsServiceRequest with two ResourceLogs resolving to distinct
tenants a/b is driven through the receiver pipeline; each group's records
are mined under their own tenant with no cross-contamination. Lives in
crates/ourios-ingester/tests/invariant_3_7_3_tenant_per_resource_logs.rs
— a receiver/RFC 0003 §6.3 behaviour the miner crate cannot drive (its
own stub comment anticipated this relocation).

The two miner stubs (`#[ignore]`/`todo!()`) become one-line pointer
comments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot June 9, 2026 21:39
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 14 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f8087b6c-c1df-4d4e-b58b-1a4278ac4fd9

📥 Commits

Reviewing files that changed from the base of the PR and between c1efe46 and 0973a39.

📒 Files selected for processing (1)
  • crates/ourios-querier/tests/rfc0001_time_preserved.rs
📝 Walkthrough

Walkthrough

This PR relocates two integration test scenarios from placeholder stubs in the miner crate to complete end-to-end implementations: tenant derivation validation moves to the ingester layer, and RFC0001 time preservation validation moves to the querier layer. Miner stubs are removed and replaced with comments explaining the relocation rationale.

Changes

Tenant Derivation Per ResourceLogs

Layer / File(s) Summary
Ingester tenant derivation test
crates/ourios-ingester/tests/invariant_3_7_3_tenant_per_resource_logs.rs
Implements Scenario §3.7.3 integration test with WAL-backed pipeline, two-tenant ResourceLogs ingestion, assertions on correct tenant tagging, cross-tenant isolation, and per-tenant template distribution. Includes service_name helper to extract and validate resource attributes.
Miner test stub removal
crates/ourios-miner/tests/invariants.rs
Removes ignored test stub and replaces with comment explaining tenant derivation is receiver-layer behavior per RFC 0003 §6.3, with pointer to ingester test location.

RFC0001 Time Preservation Across Pipeline

Layer / File(s) Summary
Querier RFC0001 time preservation test
crates/ourios-querier/tests/rfc0001_time_preserved.rs
Implements RFC0001.10 end-to-end test: constructs two records with controlled time_unix_nano, queries using explicit range window, asserts only in-window row is returned, then reads Parquet and verifies time_unix_nano matches wire value exactly. Adds sole_parquet helper to discover single Parquet file in directory.
Miner test stub removal
crates/ourios-miner/tests/rfc_internal.rs
Removes ignored RFC0001.10 test stub and replaces with comment explaining relocation to querier test file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • jensholdgaard/ourios#134: Implements WAL-before-ack ingest pipeline behavior that fan_out per ResourceLogs and derives tenants, which this PR directly validates through integration test.

Poem

🐰 Tests find their rightful homes today,
From miner stubs to querier's way.
Tenants sorted, timestamps true,
Each layer tested, brave and new! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: landing two relocated tests (RFC0001.10 and §3.7.3) with clear references to what each test validates and that stubs are being relocated.
Description check ✅ Passed The description comprehensively covers all required template sections: detailed explanations of both criterion implementations, stub relocation details, invariant impact, verification results (587 passed, 0 failed, 26 ignored), and explicit note that no production code changed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/rfc0001-10-tenant-3-7-3

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 and usage tips.

@jensholdgaard

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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 integration coverage for two already-implemented behaviors from RFC 0001 / multi-tenancy invariants by landing new end-to-end tests in the appropriate crates, and replaces misplaced miner-crate red-gate stubs with pointer comments.

Changes:

  • Add ourios-querier integration test asserting time_unix_nano is preserved through Parquet and that a DSL range(...) query returns the expected row (RFC0001.10).
  • Add ourios-ingester integration test asserting tenant derivation is performed per ResourceLogs group throughout the ingest pipeline (Invariant §3.7.3 / RFC 0003 §6.3).
  • Relocate miner-crate ignored stubs to one-line pointers to the new test locations.

Reviewed changes

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

File Description
crates/ourios-querier/tests/rfc0001_time_preserved.rs New querier end-to-end test for RFC0001.10 (time_unix_nano preservation + range query).
crates/ourios-miner/tests/rfc_internal.rs Removes RFC0001.10 ignored stub; replaces with pointer to querier test.
crates/ourios-miner/tests/invariants.rs Removes §3.7.3 ignored stub; replaces with pointer to ingester test.
crates/ourios-ingester/tests/invariant_3_7_3_tenant_per_resource_logs.rs New ingester test asserting per-ResourceLogs tenant derivation through WAL + miner fan-out.

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

…ld-independent

The per-tenant template_count==2 assertion sat near the §6.2 similarity
threshold; make the two within-tenant lines clearly dissimilar so each
mines its own template regardless of widening config — the §3.7.3
scenario is tenancy, not widening.

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

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

Comment thread crates/ourios-querier/tests/rfc0001_time_preserved.rs Outdated
Comment thread crates/ourios-querier/tests/rfc0001_time_preserved.rs
…x [lo,hi) notation

Add an AT_HI control at the exclusive upper bound, whose hour partition
overlaps the window, so it is excluded only by the half-open row-level
ts<end predicate (not the partition prune) — the prior hour-earlier
control was partition-pruned. Correct the doc bracket to half-open [lo,hi).

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

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 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread crates/ourios-querier/tests/rfc0001_time_preserved.rs Outdated
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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

@jensholdgaard
jensholdgaard merged commit 484f8f3 into main Jun 9, 2026
11 checks passed
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