Skip to content

add tenant id to metrics aggregation#1568

Merged
nitisht merged 3 commits intoparseablehq:mainfrom
nikhilsinhaparseable:tenant-metrics
Mar 6, 2026
Merged

add tenant id to metrics aggregation#1568
nitisht merged 3 commits intoparseablehq:mainfrom
nikhilsinhaparseable:tenant-metrics

Conversation

@nikhilsinhaparseable
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Mar 4, 2026

Summary by CodeRabbit

  • Refactor

    • Billing metrics are now collected and processed per-tenant so emitted metrics include tenant context for accurate per-tenant reporting.
  • New Features

    • Storage metadata accepts optional customer name, start/end dates, and plan.
    • Tenant metadata API: new methods to read and update a tenant's customer name, dates, and plan.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 4, 2026

Walkthrough

Billing events and the collector now carry an optional tenant_id; Prometheus sample extraction groups samples by tenant and instantiates per-tenant collectors. StorageMetadata gained optional billing fields and TenantMetadata received read/update APIs for those fields.

Changes

Cohort / File(s) Summary
Tenant-scoped billing metrics
src/handlers/http/cluster/mod.rs
Added tenant_id: Option<String> to BillingMetricEvent and BillingMetricsCollector; expanded BillingMetricsCollector::new(...) to accept tenant_id; metric construction now attaches tenant_id; extract_billing_metrics_from_samples() groups samples by tenant_id, maintains per-tenant collectors, and returns flattened per-tenant events.
Storage metadata fields
src/storage/store_metadata.rs
Added optional fields to StorageMetadata: customer_name, start_date, end_date, plan with #[serde(default, skip_serializing_if = "Option::is_none")]; updated Default to set them to None.
Tenant metadata accessors
src/tenants/mod.rs
Added get_tenant_meta(&self, tenant_id: &str) -> Option<StorageMetadata> and update_tenant_meta(&self, tenant_id: &str, customer_name: Option<String>, start_date: Option<String>, end_date: Option<String>, plan: Option<String>) -> bool to read and update tenant storage metadata.

Sequence Diagram(s)

sequenceDiagram
  participant Prom as "Prometheus/Samples"
  participant Extract as "extract_billing_metrics_from_samples"
  participant Map as "Per-tenant Collector Map"
  participant Collector as "BillingMetricsCollector"
  participant Emitter as "Event Output"

  Prom->>Extract: push samples (with optional tenant_id)
  Extract->>Map: lookup/create collector for tenant_id
  Map->>Collector: instantiate Collector (node_address, node_type, tenant_id) rgba(100,150,240,0.5)
  Extract->>Collector: feed samples (metric extraction)
  Collector->>Emitter: emit BillingMetricEvent(s) (include tenant_id)
  Collector-->>Map: stored/updated per-tenant collector
  Extract->>Emitter: flatten and return all per-tenant events
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hop through metrics, light and neat,
Tenants tucked under each little beat,
Collectors multiply, events take flight,
Prom samples bloom in tenanted light,
A rabbit cheers: scoped billing feels right!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely empty, missing all required template sections including description, rationale, key changes, and testing checklist. Add a comprehensive pull request description following the repository template, including the goal, rationale, key changes, and confirmation of testing and documentation.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'add tenant id to metrics aggregation' accurately reflects the main change: adding tenant_id support throughout the metrics collection system across multiple files.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/handlers/http/cluster/mod.rs`:
- Around line 1397-1402: Replace the hardcoded string "DEFAULT_TENANT" with the
shared constant from crate::parseable: import or reference
crate::parseable::DEFAULT_TENANT and use it in the closure inside the filter on
sample.labels.get("tenant_id") so the tenant_id extraction uses DEFAULT_TENANT
instead of the literal; update the filter to compare against DEFAULT_TENANT
(e.g., .filter(|t| *t != DEFAULT_TENANT)) while keeping the surrounding logic
that maps to Option<String>.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0e6ee6ec-7df2-4c5c-9844-a5025ed53076

📥 Commits

Reviewing files that changed from the base of the PR and between 08073d6 and cc920e6.

📒 Files selected for processing (1)
  • src/handlers/http/cluster/mod.rs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/tenants/mod.rs`:
- Around line 66-83: The update_tenant_meta function currently overwrites
tenant.meta fields with whichever Option<String> values are passed, clobbering
existing metadata when callers pass None; change the logic in update_tenant_meta
so that for each incoming parameter (customer_name, start_date, end_date, plan)
you only assign to tenant.meta.<field> when the corresponding Option is
Some(value) and leave the existing value untouched when it is None, and if you
need to support explicit clearing distinguish "unset" vs "clear" at the API
boundary by using Option<Option<String>> instead.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a3e5d05a-8487-4593-82bd-00187c0b9066

📥 Commits

Reviewing files that changed from the base of the PR and between cc920e6 and d874de1.

📒 Files selected for processing (2)
  • src/storage/store_metadata.rs
  • src/tenants/mod.rs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/tenants/mod.rs (1)

66-83: ⚠️ Potential issue | 🟠 Major

Prevent partial-update clobbering in update_tenant_meta.

Line 75–78 overwrites stored values with None, which can silently erase existing metadata during patch-like updates.

Suggested fix
 pub fn update_tenant_meta(
     &self,
     tenant_id: &str,
     customer_name: Option<String>,
     start_date: Option<String>,
     end_date: Option<String>,
     plan: Option<String>,
 ) -> bool {
     if let Some(mut tenant) = self.tenants.get_mut(tenant_id) {
-        tenant.meta.customer_name = customer_name;
-        tenant.meta.start_date = start_date;
-        tenant.meta.end_date = end_date;
-        tenant.meta.plan = plan;
+        if let Some(customer_name) = customer_name {
+            tenant.meta.customer_name = Some(customer_name);
+        }
+        if let Some(start_date) = start_date {
+            tenant.meta.start_date = Some(start_date);
+        }
+        if let Some(end_date) = end_date {
+            tenant.meta.end_date = Some(end_date);
+        }
+        if let Some(plan) = plan {
+            tenant.meta.plan = Some(plan);
+        }
         true
     } else {
         false
     }
 }

If explicit clearing is required, use Option<Option<String>> at the API boundary to distinguish “not provided” vs “clear value”.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/tenants/mod.rs` around lines 66 - 83, update_tenant_meta currently
overwrites existing metadata with None when callers omit fields; change the
implementation so it only mutates tenant.meta fields when the corresponding
parameter is Some(value) (or alternatively change the function signature to
accept Option<Option<String>> at the API boundary to distinguish "not provided"
vs "clear"), i.e., locate update_tenant_meta and the assignments to
tenant.meta.customer_name, tenant.meta.start_date, tenant.meta.end_date,
tenant.meta.plan and replace them with conditional updates that only set the
stored field when the incoming parameter is Some(...) (or adapt callers to
supply Option<Option<String>> and handle None/Some(None)/Some(Some(val))
accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/tenants/mod.rs`:
- Around line 66-83: update_tenant_meta currently overwrites existing metadata
with None when callers omit fields; change the implementation so it only mutates
tenant.meta fields when the corresponding parameter is Some(value) (or
alternatively change the function signature to accept Option<Option<String>> at
the API boundary to distinguish "not provided" vs "clear"), i.e., locate
update_tenant_meta and the assignments to tenant.meta.customer_name,
tenant.meta.start_date, tenant.meta.end_date, tenant.meta.plan and replace them
with conditional updates that only set the stored field when the incoming
parameter is Some(...) (or adapt callers to supply Option<Option<String>> and
handle None/Some(None)/Some(Some(val)) accordingly).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3452f09e-c824-4efb-bb31-0f3426afa309

📥 Commits

Reviewing files that changed from the base of the PR and between 7b38399 and 390890e.

📒 Files selected for processing (3)
  • src/handlers/http/cluster/mod.rs
  • src/storage/store_metadata.rs
  • src/tenants/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/handlers/http/cluster/mod.rs

@nitisht nitisht merged commit 0c9c438 into parseablehq:main Mar 6, 2026
12 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