From 6916cb3242617711fdfc107162ace38a11710aaa Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Mon, 10 Nov 2025 22:18:52 -0500 Subject: [PATCH 1/4] remove created from reserved fields --- src/alerts/alert_structs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/alerts/alert_structs.rs b/src/alerts/alert_structs.rs index 894fd1bc4..ba20f0028 100644 --- a/src/alerts/alert_structs.rs +++ b/src/alerts/alert_structs.rs @@ -41,7 +41,6 @@ const RESERVED_FIELDS: &[&str] = &[ "tags", "state", "notificationState", - "created", "lastTriggeredAt", ]; From e080b0f17382f1c557270a46f312b86afc598fbd Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Mon, 10 Nov 2025 22:25:33 -0500 Subject: [PATCH 2/4] remove reserved fields --- src/alerts/alert_structs.rs | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/src/alerts/alert_structs.rs b/src/alerts/alert_structs.rs index ba20f0028..70e7e4f8a 100644 --- a/src/alerts/alert_structs.rs +++ b/src/alerts/alert_structs.rs @@ -24,26 +24,6 @@ use serde_json::Value; use tokio::sync::{RwLock, mpsc}; use ulid::Ulid; -const RESERVED_FIELDS: &[&str] = &[ - "id", - "version", - "severity", - "title", - "query", - "datasets", - "alertType", - "anomalyConfig", - "forecastConfig", - "thresholdConfig", - "notificationConfig", - "evalConfig", - "targets", - "tags", - "state", - "notificationState", - "lastTriggeredAt", -]; - use crate::{ alerts::{ AlertError, CURRENT_ALERTS_VERSION, @@ -335,15 +315,6 @@ impl AlertRequest { other_fields.len() ))); } - - for key in other_fields.keys() { - if RESERVED_FIELDS.contains(&key.as_str()) { - return Err(AlertError::ValidationFailure(format!( - "Field '{}' cannot be in other_fields as it's a reserved field name", - key - ))); - } - } } // Validate that all target IDs exist From dc796e58285336e9531a230b0927001d3b6c2173 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Mon, 10 Nov 2025 22:47:10 -0500 Subject: [PATCH 3/4] remove default created --- src/alerts/alert_structs.rs | 56 +------------------------------------ src/alerts/alert_types.rs | 9 +----- src/alerts/mod.rs | 2 +- 3 files changed, 3 insertions(+), 64 deletions(-) diff --git a/src/alerts/alert_structs.rs b/src/alerts/alert_structs.rs index 70e7e4f8a..0e3ee382a 100644 --- a/src/alerts/alert_structs.rs +++ b/src/alerts/alert_structs.rs @@ -19,7 +19,7 @@ use std::{collections::HashMap, time::Duration}; use chrono::{DateTime, Utc}; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Serialize}; use serde_json::Value; use tokio::sync::{RwLock, mpsc}; use ulid::Ulid; @@ -39,52 +39,6 @@ use crate::{ storage::object_storage::{alert_json_path, alert_state_json_path}, }; -/// Custom deserializer for DateTime that handles legacy empty strings -/// -/// This is a compatibility layer for migrating old alerts that stored empty strings -/// instead of valid timestamps. In production, this should log warnings to help -/// identify data quality issues. -/// -/// # Migration Path -/// - Empty strings → Default to current time with a warning -/// - Missing fields → Default to current time -/// - Valid timestamps → Parse normally -pub fn deserialize_datetime_with_empty_string_fallback<'de, D>( - deserializer: D, -) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - #[derive(Deserialize)] - #[serde(untagged)] - enum DateTimeOrString { - DateTime(DateTime), - String(String), - } - - match DateTimeOrString::deserialize(deserializer)? { - DateTimeOrString::DateTime(dt) => Ok(dt), - DateTimeOrString::String(s) => { - if s.is_empty() { - // Log warning about data quality issue - tracing::warn!( - "Alert has empty 'created' field - this indicates a data quality issue. \ - Defaulting to current timestamp. Please investigate and fix the data source." - ); - Ok(Utc::now()) - } else { - s.parse::>().map_err(serde::de::Error::custom) - } - } - } -} - -/// Default function for created timestamp - returns current time -/// This handles the case where created field is missing in deserialization -pub fn default_created_time() -> DateTime { - Utc::now() -} - /// Helper struct for basic alert fields during migration pub struct BasicAlertFields { pub id: Ulid, @@ -394,10 +348,6 @@ pub struct AlertConfig { pub state: AlertState, pub notification_state: NotificationState, pub notification_config: NotificationConfig, - #[serde( - default = "default_created_time", - deserialize_with = "deserialize_datetime_with_empty_string_fallback" - )] pub created: DateTime, pub tags: Option>, pub last_triggered_at: Option>, @@ -426,10 +376,6 @@ pub struct AlertConfigResponse { pub state: AlertState, pub notification_state: NotificationState, pub notification_config: NotificationConfig, - #[serde( - default = "default_created_time", - deserialize_with = "deserialize_datetime_with_empty_string_fallback" - )] pub created: DateTime, pub tags: Option>, pub last_triggered_at: Option>, diff --git a/src/alerts/alert_types.rs b/src/alerts/alert_types.rs index 1bef0e3a0..bca078e18 100644 --- a/src/alerts/alert_types.rs +++ b/src/alerts/alert_types.rs @@ -29,10 +29,7 @@ use crate::{ AlertConfig, AlertError, AlertState, AlertType, AlertVersion, EvalConfig, Severity, ThresholdConfig, alert_enums::NotificationState, - alert_structs::{ - AlertStateEntry, GroupResult, default_created_time, - deserialize_datetime_with_empty_string_fallback, - }, + alert_structs::{AlertStateEntry, GroupResult}, alert_traits::{AlertTrait, MessageCreation}, alerts_utils::{evaluate_condition, execute_alert_query, extract_time_range}, get_number_of_agg_exprs, @@ -65,10 +62,6 @@ pub struct ThresholdAlert { pub state: AlertState, pub notification_state: NotificationState, pub notification_config: NotificationConfig, - #[serde( - default = "default_created_time", - deserialize_with = "deserialize_datetime_with_empty_string_fallback" - )] pub created: DateTime, pub tags: Option>, pub datasets: Vec, diff --git a/src/alerts/mod.rs b/src/alerts/mod.rs index 4df7bfd78..19a9d1a0b 100644 --- a/src/alerts/mod.rs +++ b/src/alerts/mod.rs @@ -51,7 +51,7 @@ pub use crate::alerts::alert_enums::{ pub use crate::alerts::alert_structs::{ AlertConfig, AlertInfo, AlertRequest, AlertStateEntry, Alerts, AlertsInfo, AlertsInfoByState, AlertsSummary, BasicAlertFields, Context, DeploymentInfo, RollingWindow, StateTransition, - ThresholdConfig, default_created_time, deserialize_datetime_with_empty_string_fallback, + ThresholdConfig, }; use crate::alerts::alert_traits::{AlertManagerTrait, AlertTrait}; use crate::alerts::alert_types::ThresholdAlert; From 920be48dd35acab07f7cf851e1f2dca9c7023fc8 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Tue, 11 Nov 2025 04:52:49 -0500 Subject: [PATCH 4/4] sanitise and remove reserved field from other_fields --- src/alerts/alert_structs.rs | 73 +++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 4 deletions(-) diff --git a/src/alerts/alert_structs.rs b/src/alerts/alert_structs.rs index 0e3ee382a..f484ac9c3 100644 --- a/src/alerts/alert_structs.rs +++ b/src/alerts/alert_structs.rs @@ -39,6 +39,35 @@ use crate::{ storage::object_storage::{alert_json_path, alert_state_json_path}, }; +const RESERVED_FIELDS: &[&str] = &[ + "version", + "id", + "severity", + "title", + "query", + "datasets", + "alertType", + "alert_type", + "anomalyConfig", + "anomaly_config", + "forecastConfig", + "forecast_config", + "thresholdConfig", + "threshold_config", + "evalConfig", + "eval_config", + "targets", + "state", + "notificationState", + "notification_state", + "notificationConfig", + "notification_config", + "created", + "tags", + "lastTriggeredAt", + "last_triggered_at", +]; + /// Helper struct for basic alert fields during migration pub struct BasicAlertFields { pub id: Ulid, @@ -261,7 +290,7 @@ pub struct AlertRequest { impl AlertRequest { pub async fn into(self) -> Result { // Validate that other_fields doesn't contain reserved field names - if let Some(ref other_fields) = self.other_fields { + let other_fields = if let Some(mut other_fields) = self.other_fields { // Limit other_fields to maximum 10 fields if other_fields.len() > 10 { return Err(AlertError::ValidationFailure(format!( @@ -269,7 +298,21 @@ impl AlertRequest { other_fields.len() ))); } - } + + for reserved in RESERVED_FIELDS { + if other_fields.remove(*reserved).is_some() { + tracing::warn!("Removed reserved field '{}' from other_fields", reserved); + } + } + + if other_fields.is_empty() { + None + } else { + Some(other_fields) + } + } else { + None + }; // Validate that all target IDs exist for id in &self.targets { @@ -283,6 +326,8 @@ impl AlertRequest { ))); } + let created_timestamp = Utc::now(); + let config = AlertConfig { version: AlertVersion::from(CURRENT_ALERTS_VERSION), id: Ulid::new(), @@ -320,11 +365,12 @@ impl AlertRequest { state: AlertState::default(), notification_state: NotificationState::Notify, notification_config: self.notification_config, - created: Utc::now(), + created: created_timestamp, tags: self.tags, last_triggered_at: None, - other_fields: self.other_fields, + other_fields, }; + Ok(config) } } @@ -384,6 +430,25 @@ pub struct AlertConfigResponse { } impl AlertConfig { + /// Filters out reserved field names from other_fields + /// This prevents conflicts when flattening other_fields during serialization + pub fn sanitize_other_fields(&mut self) { + if let Some(ref mut other_fields) = self.other_fields { + for reserved in RESERVED_FIELDS { + if other_fields.remove(*reserved).is_some() { + tracing::warn!( + "Removed reserved field '{}' from other_fields during sanitization", + reserved + ); + } + } + + if other_fields.is_empty() { + self.other_fields = None; + } + } + } + pub fn to_response(self) -> AlertConfigResponse { AlertConfigResponse { version: self.version,