Coderabbit suggestions

Author: parmesant

src/alerts/alert_types.rs

@@ -179,7 +179,7 @@ impl AlertTrait for ThresholdAlert {
 
     fn get_eval_window(&self) -> String {
         match &self.eval_config {
-            EvalConfig::RollingWindow(rolling_window) => rolling_window.eval_start.clone()
+            EvalConfig::RollingWindow(rolling_window) => rolling_window.eval_start.clone(),
         }
     }
 

src/alerts/alerts_utils.rs

@@ -25,7 +25,7 @@ use datafusion::{
     prelude::{Expr, lit},
 };
 use itertools::Itertools;
-use tracing::{trace, warn};
+use tracing::trace;
 
 use crate::{
     alerts::{AlertTrait, Conditions, LogicalOperator, WhereConfigOperator},
@@ -135,17 +135,16 @@ async fn execute_remote_query(query: &str, time_range: &TimeRange) -> Result<f64
 
 /// Convert JSON result value to f64
 fn convert_result_to_f64(result_value: serde_json::Value) -> Result<f64, AlertError> {
-    warn!(result_value=?result_value);
     // due to the previous validations, we can be sure that we get an array of objects with just one entry
     // [{"countField": Number(1120.251)}]
-    if let Some(array_val) = result_value.as_array()
+    if let Some(array_val) = result_value.as_array().filter(|arr| !arr.is_empty())
         && let Some(object) = array_val[0].as_object()
     {
         let values = object.values().map(|v| v.as_f64().unwrap()).collect_vec();
         Ok(values[0])
     } else {
         Err(AlertError::CustomError(
-            "Query result is not a number".to_string(),
+            "Query result is not a number or response is empty".to_string(),
         ))
     }
 }

src/alerts/mod.rs

@@ -499,16 +499,13 @@ impl AlertState {
                     return Err(AlertError::InvalidStateChange(msg));
                 } else {
                     // update state on disk and in memory
-                    let guard = ALERTS.write().await;
-                    if let Some(alerts) = guard.as_ref() {
-                        alerts
-                            .update_state(alert_id, new_state, Some("".into()))
-                            .await?;
-                    }
-
-                    // ALERTS
-                    //     .update_state(alert_id, new_state, Some("".into()))
-                    //     .await?;
+                    let guard = ALERTS.read().await;
+                    let alerts = guard.as_ref().ok_or_else(|| {
+                        AlertError::CustomError("Alert manager not initialized".into())
+                    })?;
+                    alerts
+                        .update_state(alert_id, new_state, Some("".into()))
+                        .await?;
                 }
             }
             AlertState::Silenced => {

src/alerts/target.rs

@@ -101,7 +101,7 @@ impl TargetConfigs {
 
     pub async fn delete(&self, target_id: &Ulid) -> Result<Target, AlertError> {
         // ensure that the target is not being used by any alert
-        let guard = ALERTS.write().await;
+        let guard = ALERTS.read().await;
         let alerts = if let Some(alerts) = guard.as_ref() {
             alerts
         } else {
@@ -293,7 +293,9 @@ impl Target {
             let alerts = if let Some(alerts) = guard.as_ref() {
                 alerts
             } else {
-                panic!("No AlertManager set");
+                error!("No AlertManager set for alert_id: {alert_id}, stopping timeout task");
+                *state.lock().unwrap() = TimeoutState::default();
+                return;
             };
             match retry {
                 Retry::Infinite => loop {

src/handlers/http/alerts.rs

@@ -75,8 +75,12 @@ pub async fn post(
 ) -> Result<impl Responder, AlertError> {
     let alert: AlertConfig = alert.into().await?;
 
+    let threshold_alert;
     let alert: &dyn AlertTrait = match &alert.alert_type {
-        AlertType::Threshold => &*(Box::new(ThresholdAlert::from(alert)) as Box<dyn AlertTrait>),
+        AlertType::Threshold => {
+            threshold_alert = ThresholdAlert::from(alert);
+            &threshold_alert
+        }
         AlertType::Anomaly => {
             return Err(AlertError::CustomError(
                 "Get Parseable Enterprise for Anomaly alerts".into(),