diff --git a/crates/obs/src/metrics/collectors/audit.rs b/crates/obs/src/metrics/collectors/audit.rs index 13700b1e1..35a8129e2 100644 --- a/crates/obs/src/metrics/collectors/audit.rs +++ b/crates/obs/src/metrics/collectors/audit.rs @@ -72,6 +72,7 @@ pub fn collect_audit_metrics(stats: &[AuditTargetStats]) -> Vec Vec { mod tests { use super::*; use crate::metrics::report::report_metrics; + use crate::metrics::schema::MetricType; #[test] fn test_collect_iam_metrics() { @@ -125,4 +126,25 @@ mod tests { assert!(metric.labels.is_empty()); } } + + #[test] + fn iam_metric_descriptors_preserve_gauge_vs_counter_semantics() { + let gauge_descriptors = [ + &LAST_SYNC_DURATION_MILLIS_MD, + &PLUGIN_AUTHN_SERVICE_FAILED_REQUESTS_MINUTE_MD, + &PLUGIN_AUTHN_SERVICE_LAST_FAIL_SECONDS_MD, + &PLUGIN_AUTHN_SERVICE_LAST_SUCC_SECONDS_MD, + &PLUGIN_AUTHN_SERVICE_SUCC_AVG_RTT_MS_MINUTE_MD, + &PLUGIN_AUTHN_SERVICE_SUCC_MAX_RTT_MS_MINUTE_MD, + &PLUGIN_AUTHN_SERVICE_TOTAL_REQUESTS_MINUTE_MD, + &SINCE_LAST_SYNC_MILLIS_MD, + ]; + + for descriptor in gauge_descriptors { + assert_eq!(descriptor.metric_type, MetricType::Gauge); + } + + assert_eq!(SYNC_FAILURES_MD.metric_type, MetricType::Counter); + assert_eq!(SYNC_SUCCESSES_MD.metric_type, MetricType::Counter); + } } diff --git a/crates/obs/src/metrics/collectors/notification_target.rs b/crates/obs/src/metrics/collectors/notification_target.rs index 58d5d5309..e3073e577 100644 --- a/crates/obs/src/metrics/collectors/notification_target.rs +++ b/crates/obs/src/metrics/collectors/notification_target.rs @@ -63,6 +63,7 @@ pub fn collect_notification_target_metrics(stats: &[NotificationTargetStats]) -> #[cfg(test)] mod tests { use super::*; + use crate::metrics::schema::MetricType; #[test] fn test_collect_notification_target_metrics() { @@ -89,4 +90,11 @@ mod tests { .any(|(key, value)| *key == TARGET_TYPE && value == "webhook") })); } + + #[test] + fn notification_target_totals_are_exported_as_gauges() { + assert_eq!(NOTIFICATION_TARGET_FAILED_MESSAGES_MD.metric_type, MetricType::Gauge); + assert_eq!(NOTIFICATION_TARGET_QUEUE_LENGTH_MD.metric_type, MetricType::Gauge); + assert_eq!(NOTIFICATION_TARGET_TOTAL_MESSAGES_MD.metric_type, MetricType::Gauge); + } } diff --git a/crates/obs/src/metrics/report.rs b/crates/obs/src/metrics/report.rs index d2befb1d2..18c076259 100644 --- a/crates/obs/src/metrics/report.rs +++ b/crates/obs/src/metrics/report.rs @@ -38,6 +38,18 @@ fn into_static_str(cache: &OnceLock>>, value intern_string(cache, value) } +fn counter_value_from_f64(value: f64) -> Option { + if !value.is_finite() || value < 0.0 { + return None; + } + + if value >= u64::MAX as f64 { + Some(u64::MAX) + } else { + Some(value as u64) + } +} + pub fn report_metrics(metrics: &[PrometheusMetric]) { for metric in metrics { let name = into_static_str(&NAME_CACHE, &metric.name); @@ -53,8 +65,10 @@ pub fn report_metrics(metrics: &[PrometheusMetric]) { match metric.metric_type { MetricType::Counter => { - let counter = counter!(name, &labels); - counter.absolute(metric.value as u64); + if let Some(value) = counter_value_from_f64(metric.value) { + let counter = counter!(name, &labels); + counter.absolute(value); + } } MetricType::Gauge => { let gauge = gauge!(name, &labels); @@ -168,4 +182,17 @@ mod tests { assert_eq!(metric.metric_type, metric_type); } } + + #[test] + fn counter_value_from_f64_rejects_negative_and_nan_inputs() { + assert_eq!(counter_value_from_f64(-1.0), None); + assert_eq!(counter_value_from_f64(f64::NAN), None); + assert_eq!(counter_value_from_f64(f64::NEG_INFINITY), None); + } + + #[test] + fn counter_value_from_f64_clamps_large_values() { + assert_eq!(counter_value_from_f64(42.0), Some(42)); + assert_eq!(counter_value_from_f64(u64::MAX as f64 * 2.0), Some(u64::MAX)); + } } diff --git a/crates/obs/src/metrics/schema/audit.rs b/crates/obs/src/metrics/schema/audit.rs index 2d6855842..a5d738c6a 100644 --- a/crates/obs/src/metrics/schema/audit.rs +++ b/crates/obs/src/metrics/schema/audit.rs @@ -14,7 +14,7 @@ #![allow(dead_code)] -use crate::{MetricDescriptor, MetricName, new_counter_md, new_gauge_md, subsystems}; +use crate::{MetricDescriptor, MetricName, new_gauge_md, subsystems}; use std::sync::LazyLock; const TARGET_ID: &str = "target_id"; @@ -25,7 +25,7 @@ pub const SUCCESS: &str = "success"; pub const FAILURE: &str = "failure"; pub static AUDIT_FAILED_MESSAGES_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::AuditFailedMessages, "Total number of messages that failed to send since start", &[TARGET_ID], @@ -43,7 +43,7 @@ pub static AUDIT_TARGET_QUEUE_LENGTH_MD: LazyLock = LazyLock:: }); pub static AUDIT_TOTAL_MESSAGES_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::AuditTotalMessages, "Total number of messages sent since start", &[TARGET_ID], diff --git a/crates/obs/src/metrics/schema/cluster_iam.rs b/crates/obs/src/metrics/schema/cluster_iam.rs index 9f8077259..7c0ff00a3 100644 --- a/crates/obs/src/metrics/schema/cluster_iam.rs +++ b/crates/obs/src/metrics/schema/cluster_iam.rs @@ -14,11 +14,11 @@ #![allow(dead_code)] -use crate::{MetricDescriptor, MetricName, new_counter_md, subsystems}; +use crate::{MetricDescriptor, MetricName, new_counter_md, new_gauge_md, subsystems}; use std::sync::LazyLock; pub static LAST_SYNC_DURATION_MILLIS_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::LastSyncDurationMillis, "Last successful IAM data sync duration in milliseconds", &[], @@ -27,7 +27,7 @@ pub static LAST_SYNC_DURATION_MILLIS_MD: LazyLock = LazyLock:: }); pub static PLUGIN_AUTHN_SERVICE_FAILED_REQUESTS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceFailedRequestsMinute, "When plugin authentication is configured, returns failed requests count in the last full minute", &[], @@ -36,7 +36,7 @@ pub static PLUGIN_AUTHN_SERVICE_FAILED_REQUESTS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceLastFailSeconds, "When plugin authentication is configured, returns time (in seconds) since the last failed request to the service", &[], @@ -45,7 +45,7 @@ pub static PLUGIN_AUTHN_SERVICE_LAST_FAIL_SECONDS_MD: LazyLock }); pub static PLUGIN_AUTHN_SERVICE_LAST_SUCC_SECONDS_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceLastSuccSeconds, "When plugin authentication is configured, returns time (in seconds) since the last successful request to the service", &[], @@ -54,7 +54,7 @@ pub static PLUGIN_AUTHN_SERVICE_LAST_SUCC_SECONDS_MD: LazyLock }); pub static PLUGIN_AUTHN_SERVICE_SUCC_AVG_RTT_MS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceSuccAvgRttMsMinute, "When plugin authentication is configured, returns average round-trip-time of successful requests in the last full minute", &[], @@ -63,7 +63,7 @@ pub static PLUGIN_AUTHN_SERVICE_SUCC_AVG_RTT_MS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceSuccMaxRttMsMinute, "When plugin authentication is configured, returns maximum round-trip-time of successful requests in the last full minute", &[], @@ -72,7 +72,7 @@ pub static PLUGIN_AUTHN_SERVICE_SUCC_MAX_RTT_MS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::PluginAuthnServiceTotalRequestsMinute, "When plugin authentication is configured, returns total requests count in the last full minute", &[], @@ -81,7 +81,7 @@ pub static PLUGIN_AUTHN_SERVICE_TOTAL_REQUESTS_MINUTE_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::SinceLastSyncMillis, "Time (in milliseconds) since last successful IAM data sync.", &[], diff --git a/crates/obs/src/metrics/schema/notification_target.rs b/crates/obs/src/metrics/schema/notification_target.rs index f3af7d89f..ceb801c13 100644 --- a/crates/obs/src/metrics/schema/notification_target.rs +++ b/crates/obs/src/metrics/schema/notification_target.rs @@ -14,7 +14,7 @@ #![allow(dead_code)] -use crate::{MetricDescriptor, MetricName, new_counter_md, new_gauge_md, subsystems}; +use crate::{MetricDescriptor, MetricName, new_gauge_md, subsystems}; use std::sync::LazyLock; pub const TARGET_ID: &str = "target_id"; @@ -23,7 +23,7 @@ pub const TARGET_TYPE: &str = "target_type"; const NOTIFICATION_TARGET_LABELS: [&str; 2] = [TARGET_ID, TARGET_TYPE]; pub static NOTIFICATION_TARGET_FAILED_MESSAGES_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::NotificationTargetFailedMessages, "Total number of notification messages that permanently failed to send", &NOTIFICATION_TARGET_LABELS, @@ -41,7 +41,7 @@ pub static NOTIFICATION_TARGET_QUEUE_LENGTH_MD: LazyLock = Laz }); pub static NOTIFICATION_TARGET_TOTAL_MESSAGES_MD: LazyLock = LazyLock::new(|| { - new_counter_md( + new_gauge_md( MetricName::NotificationTargetTotalMessages, "Total number of notification messages successfully delivered", &NOTIFICATION_TARGET_LABELS,