Fix a few upgrades that failed due to overflowing u32::MAX (#6826)

backend/canisters/community/CHANGELOG.md

@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Reduce size of metrics in memory ([#6765](https://github.com/open-chat-labs/open-chat/pull/6765))
 - Run cycles check (+ other background tasks) when executing timer jobs ([#6815](https://github.com/open-chat-labs/open-chat/pull/6815))
 - Add cycles balance check to more timer jobs ([#6822](https://github.com/open-chat-labs/open-chat/pull/6822))
+- Handle metric counters that overflow `u32::MAX` ([#6826](https://github.com/open-chat-labs/open-chat/pull/6826))
 - Add the `BotCommand` access token type ([#6830](https://github.com/open-chat-labs/open-chat/pull/6830))
 - Add `bot_api_gateway` canisterId to the canister state ([#6842](https://github.com/open-chat-labs/open-chat/pull/6842))
 - Simplify `inspect_message` ([#6847](https://github.com/open-chat-labs/open-chat/pull/6847))

backend/canisters/group/CHANGELOG.md

@@ -21,6 +21,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 - Remove code to migrate events to stable memory ([#6837](https://github.com/open-chat-labs/open-chat/pull/6837))
 
+### Fixed
+
+- Fix a few upgrades that failed due to overflowing `u32::MAX` ([#6826](https://github.com/open-chat-labs/open-chat/pull/6826))
+
 ## [[2.0.1453](https://github.com/open-chat-labs/open-chat/releases/tag/v2.0.1453-group)] - 2024-11-14
 
 ### Changed

backend/canisters/user/CHANGELOG.md

@@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Reduce size of metrics in memory ([#6765](https://github.com/open-chat-labs/open-chat/pull/6765))
 - Run cycles check (+ other background tasks) when executing timer jobs ([#6815](https://github.com/open-chat-labs/open-chat/pull/6815))
 - Add cycles balance check to more timer jobs ([#6822](https://github.com/open-chat-labs/open-chat/pull/6822))
+- Handle metric counters that overflow `u32::MAX` ([#6826](https://github.com/open-chat-labs/open-chat/pull/6826))
 - Avoid iterating all events when migrating private replies after group import ([#6827](https://github.com/open-chat-labs/open-chat/pull/6827))
 - Add the `BotCommand` access token type ([#6830](https://github.com/open-chat-labs/open-chat/pull/6830))
 - Avoid storing any events on the heap within direct chats ([#6838](https://github.com/open-chat-labs/open-chat/pull/6838))

backend/libraries/chat_events/src/metrics.rs

@@ -1,5 +1,6 @@
 use serde::{Deserialize, Serialize};
 use std::cmp::{max, min};
+use tracing::error;
 use types::{is_default, ChatMetrics, TimestampMillis};
 
 #[derive(Serialize, Deserialize, Debug, Default, Clone)]
@@ -112,59 +113,59 @@ struct ChatMetricsInternalCombined {
     #[serde(rename = "m", default)]
     metrics: Vec<MetricCounter>,
     #[serde(rename = "t", default, skip_serializing_if = "is_default")]
-    pub text_messages: u32,
+    pub text_messages: u64,
     #[serde(rename = "i", default, skip_serializing_if = "is_default")]
-    pub image_messages: u32,
+    pub image_messages: u64,
     #[serde(rename = "v", default, skip_serializing_if = "is_default")]
-    pub video_messages: u32,
+    pub video_messages: u64,
     #[serde(rename = "a", default, skip_serializing_if = "is_default")]
-    pub audio_messages: u32,
+    pub audio_messages: u64,
     #[serde(rename = "f", default, skip_serializing_if = "is_default")]
-    pub file_messages: u32,
+    pub file_messages: u64,
     #[serde(rename = "p", default, skip_serializing_if = "is_default")]
-    pub polls: u32,
+    pub polls: u64,
     #[serde(rename = "pv", default, skip_serializing_if = "is_default")]
-    pub poll_votes: u32,
+    pub poll_votes: u64,
     #[serde(rename = "icp", default, skip_serializing_if = "is_default")]
-    pub icp_messages: u32,
+    pub icp_messages: u64,
     #[serde(rename = "sns1", default, skip_serializing_if = "is_default")]
-    pub sns1_messages: u32,
+    pub sns1_messages: u64,
     #[serde(rename = "ckbtc", default, skip_serializing_if = "is_default")]
-    pub ckbtc_messages: u32,
+    pub ckbtc_messages: u64,
     #[serde(rename = "chat", default, skip_serializing_if = "is_default")]
-    pub chat_messages: u32,
+    pub chat_messages: u64,
     #[serde(rename = "kinic", default, skip_serializing_if = "is_default")]
-    pub kinic_messages: u32,
+    pub kinic_messages: u64,
     #[serde(rename = "o", default, skip_serializing_if = "is_default")]
-    pub other_crypto_messages: u32,
+    pub other_crypto_messages: u64,
     #[serde(rename = "d", default, skip_serializing_if = "is_default")]
-    pub deleted_messages: u32,
+    pub deleted_messages: u64,
     #[serde(rename = "g", default, skip_serializing_if = "is_default")]
-    pub giphy_messages: u32,
+    pub giphy_messages: u64,
     #[serde(rename = "pz", default, skip_serializing_if = "is_default")]
-    pub prize_messages: u32,
+    pub prize_messages: u64,
     #[serde(rename = "pzw", default, skip_serializing_if = "is_default")]
-    pub prize_winner_messages: u32,
+    pub prize_winner_messages: u64,
     #[serde(rename = "rp", default, skip_serializing_if = "is_default")]
-    pub replies: u32,
+    pub replies: u64,
     #[serde(rename = "e", default, skip_serializing_if = "is_default")]
-    pub edits: u32,
+    pub edits: u64,
     #[serde(rename = "rt", default, skip_serializing_if = "is_default")]
-    pub reactions: u32,
+    pub reactions: u64,
     #[serde(rename = "ti", default, skip_serializing_if = "is_default")]
-    pub tips: u32,
+    pub tips: u64,
     #[serde(rename = "pr", default, skip_serializing_if = "is_default")]
-    pub proposals: u32,
+    pub proposals: u64,
     #[serde(rename = "rpt", default, skip_serializing_if = "is_default")]
-    pub reported_messages: u32,
+    pub reported_messages: u64,
     #[serde(rename = "mr", default, skip_serializing_if = "is_default")]
-    pub message_reminders: u32,
+    pub message_reminders: u64,
     #[serde(rename = "p2p", default, skip_serializing_if = "is_default")]
-    pub p2p_swaps: u32,
+    pub p2p_swaps: u64,
     #[serde(rename = "vc", default, skip_serializing_if = "is_default")]
-    pub video_calls: u32,
+    pub video_calls: u64,
     #[serde(rename = "cu", default, skip_serializing_if = "is_default")]
-    pub custom_type_messages: u32,
+    pub custom_type_messages: u64,
     #[serde(rename = "la", alias = "l")]
     pub last_active: TimestampMillis,
 }
@@ -183,117 +184,97 @@ impl From<ChatMetricsInternalCombined> for ChatMetricsInternal {
             last_active: value.last_active,
         };
 
-        if value.text_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::TextMessages, value.text_messages));
+        if let Some(count) = try_convert_to_metric_count(value.text_messages, "text_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::TextMessages, count));
         }
-        if value.image_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::ImageMessages, value.image_messages));
+        if let Some(count) = try_convert_to_metric_count(value.image_messages, "image_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::ImageMessages, count));
         }
-        if value.video_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::VideoMessages, value.video_messages));
+        if let Some(count) = try_convert_to_metric_count(value.video_messages, "video_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::VideoMessages, count));
         }
-        if value.audio_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::AudioMessages, value.audio_messages));
+        if let Some(count) = try_convert_to_metric_count(value.audio_messages, "audio_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::AudioMessages, count));
         }
-        if value.file_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::FileMessages, value.file_messages));
+        if let Some(count) = try_convert_to_metric_count(value.file_messages, "file_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::FileMessages, count));
         }
-        if value.polls > 0 {
-            metrics.metrics.push(MetricCounter::new(MetricKey::Polls, value.polls));
+        if let Some(count) = try_convert_to_metric_count(value.polls, "polls") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Polls, count));
         }
-        if value.poll_votes > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::PollVotes, value.poll_votes));
+        if let Some(count) = try_convert_to_metric_count(value.poll_votes, "poll_votes") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::PollVotes, count));
         }
         let crypto_messages = value.icp_messages
             + value.sns1_messages
             + value.ckbtc_messages
             + value.chat_messages
             + value.kinic_messages
             + value.other_crypto_messages;
-        if crypto_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::CryptoMessages, crypto_messages));
+        if let Some(count) = try_convert_to_metric_count(crypto_messages, "crypto_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::CryptoMessages, count));
         }
-        if value.deleted_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::DeletedMessages, value.deleted_messages));
+        if let Some(count) = try_convert_to_metric_count(value.deleted_messages, "deleted_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::DeletedMessages, count));
         }
-        if value.giphy_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::GiphyMessages, value.giphy_messages));
+        if let Some(count) = try_convert_to_metric_count(value.giphy_messages, "giphy_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::GiphyMessages, count));
+        }
+        if let Some(count) = try_convert_to_metric_count(value.prize_messages, "prize_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::PrizeMessages, count));
         }
-        if value.prize_messages > 0 {
+        if let Some(count) = try_convert_to_metric_count(value.prize_winner_messages, "prize_winner_messages") {
             metrics
                 .metrics
-                .push(MetricCounter::new(MetricKey::PrizeMessages, value.prize_messages));
+                .push(MetricCounter::new(MetricKey::PrizeWinnerMessages, count));
         }
-        if value.prize_winner_messages > 0 {
-            metrics.metrics.push(MetricCounter::new(
-                MetricKey::PrizeWinnerMessages,
-                value.prize_winner_messages,
-            ));
+        if let Some(count) = try_convert_to_metric_count(value.replies, "replies") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Replies, count));
         }
-        if value.replies > 0 {
-            metrics.metrics.push(MetricCounter::new(MetricKey::Replies, value.replies));
+        if let Some(count) = try_convert_to_metric_count(value.edits, "edits") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Edits, count));
         }
-        if value.edits > 0 {
-            metrics.metrics.push(MetricCounter::new(MetricKey::Edits, value.edits));
+        if let Some(count) = try_convert_to_metric_count(value.reactions, "reactions") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Reactions, count));
         }
-        if value.reactions > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::Reactions, value.reactions));
+        if let Some(count) = try_convert_to_metric_count(value.tips, "tips") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Tips, count));
         }
-        if value.tips > 0 {
-            metrics.metrics.push(MetricCounter::new(MetricKey::Tips, value.tips));
+        if let Some(count) = try_convert_to_metric_count(value.proposals, "proposals") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::Proposals, count));
         }
-        if value.proposals > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::Proposals, value.proposals));
+        if let Some(count) = try_convert_to_metric_count(value.reported_messages, "reported_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::ReportedMessages, count));
         }
-        if value.reported_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::ReportedMessages, value.reported_messages));
-        }
-        if value.message_reminders > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::MessageReminders, value.message_reminders));
+        if let Some(count) = try_convert_to_metric_count(value.message_reminders, "message_reminders") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::MessageReminders, count));
         }
-        if value.p2p_swaps > 0 {
-            metrics.metrics.push(MetricCounter::new(MetricKey::P2pSwaps, value.p2p_swaps));
+        if let Some(count) = try_convert_to_metric_count(value.p2p_swaps, "p2p_swaps") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::P2pSwaps, count));
         }
-        if value.video_calls > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::VideoCalls, value.video_calls));
+        if let Some(count) = try_convert_to_metric_count(value.video_calls, "video_calls") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::VideoCalls, count));
         }
-        if value.custom_type_messages > 0 {
-            metrics
-                .metrics
-                .push(MetricCounter::new(MetricKey::CustomTypeMessages, value.custom_type_messages));
+        if let Some(count) = try_convert_to_metric_count(value.custom_type_messages, "custom_type_messages") {
+            metrics.metrics.push(MetricCounter::new(MetricKey::CustomTypeMessages, count));
         }
         metrics
     }
 }
 
+fn try_convert_to_metric_count(value: u64, name: &str) -> Option<u32> {
+    if let Ok(count) = u32::try_from(value) {
+        if count > 0 {
+            Some(count)
+        } else {
+            None
+        }
+    } else {
+        error!(name, value, "Metric value exceeded limit");
+        None
+    }
+}
+
 impl ChatMetricsInternal {
     pub fn incr(&mut self, key: MetricKey, count: u32) {
         if let Some(m) = self.metrics.iter_mut().find(|m| m.key() == key) {