From 7fabd82baeedda7872802c530626e360a39dd5b7 Mon Sep 17 00:00:00 2001 From: Ruben Hensen Date: Mon, 18 May 2026 17:36:07 +0200 Subject: [PATCH 1/2] feat(metrics): pre-seed known channels at zero on startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this, Prometheus only creates a series for cryptify_uploads_total {channel="X"} when the first upload from channel X lands. PromQL increase(...[1h]) over that series returns nothing because the earliest sample is already non-zero — dashboards under-report low-volume channels for an hour after each pod restart. Pre-seeds website, staging-website, outlook, thunderbird, api, unknown to 0 in Metrics::new(), so the full label set is visible from the first scrape and increase() can compute deltas honestly. The previous render-time "if empty, emit unknown=0" fallback is now effectively dead (the map is never empty after new()), but left in place defensively against any future code path that constructs Metrics via Default rather than new(). --- src/metrics.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index ed4b57f..c81b274 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -20,6 +20,20 @@ use rocket::http::HeaderMap; /// Channel label used when no other source information is present. pub const CHANNEL_UNKNOWN: &str = "unknown"; +/// Channels pre-seeded at value 0 on startup so dashboards see the full +/// label set from the first scrape, rather than each channel popping into +/// existence the first time a request from it lands. Without this, PromQL +/// `increase()` over a window can read `0` for a channel whose first +/// observed sample is already non-zero — see #102 follow-up discussion. +pub const KNOWN_CHANNELS: &[&str] = &[ + "website", + "staging-website", + "outlook", + "thunderbird", + "api", + CHANNEL_UNKNOWN, +]; + /// Header clients can set to identify themselves (`outlook`, `thunderbird`, /// `api`, ...). Leading whitespace is trimmed and the value is lowercased /// and restricted to `[a-z0-9_-]` so it cannot inject Prometheus syntax. @@ -36,7 +50,16 @@ pub struct Metrics { impl Metrics { pub fn new() -> Self { - Self::default() + let m = Self::default(); + { + let mut uploads = m.uploads.lock().unwrap(); + let mut bytes = m.upload_bytes.lock().unwrap(); + for c in KNOWN_CHANNELS { + uploads.insert((*c).to_string(), 0); + bytes.insert((*c).to_string(), 0); + } + } + m } /// Record a successfully finalized upload. @@ -325,11 +348,19 @@ mod tests { } #[test] - fn render_emits_zero_counters_when_empty() { + fn render_preseeds_known_channels_at_zero() { let m = Metrics::new(); let text = m.render(); - assert!(text.contains("cryptify_uploads_total{channel=\"unknown\"} 0")); - assert!(text.contains("cryptify_upload_bytes_total{channel=\"unknown\"} 0")); + for c in KNOWN_CHANNELS { + assert!( + text.contains(&format!("cryptify_uploads_total{{channel=\"{c}\"}} 0")), + "missing zero-seed for uploads channel={c} in:\n{text}" + ); + assert!( + text.contains(&format!("cryptify_upload_bytes_total{{channel=\"{c}\"}} 0")), + "missing zero-seed for upload_bytes channel={c} in:\n{text}" + ); + } assert!(text.contains("cryptify_storage_bytes 0")); assert!(text.contains("cryptify_active_files 0")); assert!(text.contains("cryptify_expired_files_total 0")); From 928104c26356ff6d49c4b0159c507128d1aeb244 Mon Sep 17 00:00:00 2001 From: Ruben Hensen Date: Mon, 18 May 2026 17:51:48 +0200 Subject: [PATCH 2/2] refactor(metrics): route Default through new() so they can't diverge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address dobby's non-blocking observation on #165: with #[derive(Default)] still in place, Metrics::default() produced an empty-channels object while Metrics::new() pre-seeded — a footgun for a future contributor who reaches for ::default() in a generic context or test helper. Drops the derive, refactors new() to build the maps directly (no internal lock dance), and adds a manual `impl Default for Metrics` that just calls Self::new(). The two construction paths now produce identical objects. All 13 metrics tests still pass. --- src/metrics.rs | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index c81b274..188106f 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -39,7 +39,6 @@ pub const KNOWN_CHANNELS: &[&str] = &[ /// and restricted to `[a-z0-9_-]` so it cannot inject Prometheus syntax. pub const SOURCE_HEADER: &str = "X-Cryptify-Source"; -#[derive(Default)] pub struct Metrics { uploads: Mutex>, upload_bytes: Mutex>, @@ -48,18 +47,32 @@ pub struct Metrics { expired_files: AtomicU64, } +// `Default` is implemented manually (not derived) so it goes through +// `Metrics::new()` and pre-seeds `KNOWN_CHANNELS`. A derived `Default` +// would silently produce an empty-channel object, which diverges from +// `new()` and re-introduces the missing-baseline problem this module +// exists to solve. +impl Default for Metrics { + fn default() -> Self { + Self::new() + } +} + impl Metrics { pub fn new() -> Self { - let m = Self::default(); - { - let mut uploads = m.uploads.lock().unwrap(); - let mut bytes = m.upload_bytes.lock().unwrap(); - for c in KNOWN_CHANNELS { - uploads.insert((*c).to_string(), 0); - bytes.insert((*c).to_string(), 0); - } + let mut uploads = BTreeMap::new(); + let mut bytes = BTreeMap::new(); + for c in KNOWN_CHANNELS { + uploads.insert((*c).to_string(), 0u64); + bytes.insert((*c).to_string(), 0u64); + } + Self { + uploads: Mutex::new(uploads), + upload_bytes: Mutex::new(bytes), + storage_bytes: AtomicI64::new(0), + active_files: AtomicI64::new(0), + expired_files: AtomicU64::new(0), } - m } /// Record a successfully finalized upload.