Skip to content

add tenant id to metrics aggregation #1568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nikhilsinhaparseable wants to merge 2 commits into
base: main
Choose a base branch
from
+62 −5
Open

add tenant id to metrics aggregation #1568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc920e6
add tenant id to metrics aggregation
nikhilsinhaparseable Mar 4, 2026
d874de1
update related to tenant creation and updation
nikhilsinhaparseable Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/handlers/http/cluster/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,16 @@ pub struct BillingMetricEvent {
pub provider: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub model: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub tenant_id: Option<String>,
pub event_type: String,
pub event_time: chrono::NaiveDateTime,
}

// Internal structure for collecting metrics from prometheus
#[derive(Debug, Default)]
struct BillingMetricsCollector {
pub tenant_id: Option<String>,
pub node_address: String,
pub node_type: String,
pub total_events_ingested_by_date: HashMap<String, u64>,
Expand All @@ -116,8 +119,9 @@ struct BillingMetricsCollector {
}

impl BillingMetricsCollector {
pub fn new(node_address: String, node_type: String) -> Self {
pub fn new(node_address: String, node_type: String, tenant_id: Option<String>) -> Self {
Self {
tenant_id,
node_address,
node_type,
event_time: Utc::now().naive_utc(),
Expand Down Expand Up @@ -153,6 +157,7 @@ impl BillingMetricsCollector {
method: None,
provider: None,
model: None,
tenant_id: self.tenant_id.clone(),
event_type: "billing-metrics".to_string(),
event_time: self.event_time,
});
Expand Down Expand Up @@ -282,6 +287,7 @@ impl BillingMetricsCollector {
method: Some(method.clone()),
provider: None,
model: None,
tenant_id: self.tenant_id.clone(),
event_type: "billing-metrics".to_string(),
event_time: self.event_time,
});
Expand Down Expand Up @@ -320,6 +326,7 @@ impl BillingMetricsCollector {
method: None,
provider: Some(provider.clone()),
model: Some(model.clone()),
tenant_id: self.tenant_id.clone(),
event_type: "billing-metrics".to_string(),
event_time: self.event_time,
});
Expand Down Expand Up @@ -1382,16 +1389,31 @@ fn extract_billing_metrics_from_samples(
node_address: String,
node_type: String,
) -> Vec<BillingMetricEvent> {
let mut collector = BillingMetricsCollector::new(node_address, node_type);
// Group samples by tenant_id so each tenant gets its own collector
let mut collectors: HashMap<Option<String>, BillingMetricsCollector> = HashMap::new();

for sample in samples {
if let prometheus_parse::Value::Counter(val) = sample.value {
process_sample(&mut collector, &sample, val);
// Extract tenant_id from labels; treat "DEFAULT_TENANT" or absent as None (single-tenant compat)
let tenant_id = sample
.labels
.get("tenant_id")
.filter(|t| *t != "DEFAULT_TENANT")
.map(|t| t.to_string());
Comment on lines +1397 to +1402
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 4, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Find where DEFAULT_TENANT is defined and its value
echo "=== DEFAULT_TENANT definition ==="
rg -n --type=rust 'const\s+DEFAULT_TENANT|pub\s+const\s+DEFAULT_TENANT|DEFAULT_TENANT\s*='

# 2) Read the specific lines in cluster/mod.rs to see the hardcoded string
echo -e "\n=== Code at lines 1397-1402 in cluster/mod.rs ==="
sed -n '1390,1410p' src/handlers/http/cluster/mod.rs

# 3) Check current imports in cluster/mod.rs
echo -e "\n=== Imports in cluster/mod.rs ==="
head -50 src/handlers/http/cluster/mod.rs | grep -E '^use|^import'

Repository: parseablehq/parseable

Length of output: 2171

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 4) Check how DEFAULT_TENANT is used in other tenant normalization contexts
echo "=== DEFAULT_TENANT usage patterns in codebase ==="
rg -B2 -A2 'DEFAULT_TENANT' --type=rust | head -100

# 5) Search for other similar tenant_id normalizations
echo -e "\n=== Other tenant_id label/filter patterns ==="
rg -n 'tenant_id.*filter|filter.*tenant_id' --type=rust src/

Repository: parseablehq/parseable

Length of output: 8066

Use the DEFAULT_TENANT constant instead of hardcoding the string literal.

Line 1401 hardcodes "DEFAULT_TENANT" in the filter. To maintain consistency with the codebase and prevent normalization drift if the constant changes, use the shared constant from crate::parseable.

♻️ Proposed change 
-use crate::parseable::PARSEABLE;
+use crate::parseable::{DEFAULT_TENANT, PARSEABLE};
...
-                .filter(|t| *t != "DEFAULT_TENANT")
+                .filter(|t| t.as_str() != DEFAULT_TENANT)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Extract tenant_id from labels; treat "DEFAULT_TENANT" or absent as None (single-tenant compat)
let tenant_id = sample
.labels
.get("tenant_id")
.filter(|t| *t != "DEFAULT_TENANT")
.map(|t| t.to_string());
// Extract tenant_id from labels; treat "DEFAULT_TENANT" or absent as None (single-tenant compat)
let tenant_id = sample
.labels
.get("tenant_id")
.filter(|t| t.as_str() != DEFAULT_TENANT)
.map(|t| t.to_string());
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/handlers/http/cluster/mod.rs` around lines 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>.


let collector = collectors.entry(tenant_id.clone()).or_insert_with(|| {
BillingMetricsCollector::new(node_address.clone(), node_type.clone(), tenant_id)
});

process_sample(collector, &sample, val);
}
}

// Convert to flattened events, excluding empty collections
collector.into_events()
// Convert all collectors to flattened events
collectors
.into_values()
.flat_map(|c| c.into_events())
.collect()
}

/// Process a single prometheus sample and update the collector
Expand Down
12 changes: 12 additions & 0 deletions src/storage/store_metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ pub struct StorageMetadata {
pub default_role: Option<String>,
pub suspended_services: Option<HashSet<Service>>,
pub global_query_auth: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub customer_name: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub start_date: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub end_date: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub plan: Option<String>,
}

impl Default for StorageMetadata {
Expand All @@ -88,6 +96,10 @@ impl Default for StorageMetadata {
default_role: None,
suspended_services: None,
global_query_auth: None,
customer_name: None,
start_date: None,
end_date: None,
plan: None,
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/tenants/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,29 @@ impl TenantMetadata {
);
}

pub fn get_tenant_meta(&self, tenant_id: &str) -> Option<StorageMetadata> {
self.tenants.get(tenant_id).map(|t| t.meta.clone())
}

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;
true
} else {
false
}
}
Comment on lines +66 to +83
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid clobbering existing metadata on partial updates.

On Line 75–78, assigning each field directly from Option<String> erases existing values whenever callers pass None (including omitted fields in patch-like APIs). That can cause silent metadata loss.

💡 Proposed 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 clearing fields is also required, use Option<Option<String>> at the API boundary to distinguish “unset” vs “explicitly clear”.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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;
true
} else {
false
}
}
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) {
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
}
}
🤖 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, 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.


pub fn get_global_query_auth(&self, tenant_id: &str) -> Option<String> {
if let Some(tenant) = self.tenants.get(tenant_id) {
tenant.meta.global_query_auth.clone()
Expand Down
Loading