add tenant id to metrics aggregation#1568
Merged
nitisht merged 3 commits intoparseablehq:mainfrom Mar 6, 2026
Merged
Conversation
Contributor
WalkthroughBilling 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
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
📒 Files selected for processing (1)
src/handlers/http/cluster/mod.rs
Contributor
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/storage/store_metadata.rssrc/tenants/mod.rs
7b38399 to
390890e
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/tenants/mod.rs (1)
66-83:⚠️ Potential issue | 🟠 MajorPrevent 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
📒 Files selected for processing (3)
src/handlers/http/cluster/mod.rssrc/storage/store_metadata.rssrc/tenants/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/handlers/http/cluster/mod.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Refactor
New Features