Fix how analytics writes are serialized (#5926)

Cargo.lock

@@ -27,7 +27,7 @@ checksum = "daa239b93927be1ff123eebada5a3ff23e89f0124ccb8609234e5103d5a5ae6d"
 dependencies = [
  "actix-utils",
  "actix-web",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "futures-util",
  "log",
  "once_cell",
@@ -46,7 +46,7 @@ dependencies = [
  "actix-web",
  "bitflags 2.9.4",
  "bytes",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "futures-core",
  "http-range",
  "log",
@@ -72,7 +72,7 @@ dependencies = [
  "brotli",
  "bytes",
  "bytestring",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "encoding_rs",
  "flate2",
  "foldhash",
@@ -226,7 +226,7 @@ dependencies = [
  "bytestring",
  "cfg-if",
  "cookie 0.16.2",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "encoding_rs",
  "foldhash",
  "futures-core",
@@ -1902,6 +1902,15 @@ dependencies = [
  "unicode-segmentation",
 ]
 
+[[package]]
+name = "convert_case"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "633458d4ef8c78b72454de2d54fd6ab2e60f9e02be22f3c6104cdc8a4e0fceb9"
+dependencies = [
+ "unicode-segmentation",
+]
+
 [[package]]
 name = "cookie"
 version = "0.16.2"
@@ -2450,21 +2459,23 @@ dependencies = [
 
 [[package]]
 name = "derive_more"
-version = "2.0.1"
+version = "2.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "093242cf7570c207c83073cf82f79706fe7b8317e98620a47d5be7c3d8497678"
+checksum = "d751e9e49156b02b44f9c1815bcb94b984cdcc4396ecc32521c739452808b134"
 dependencies = [
  "derive_more-impl",
 ]
 
 [[package]]
 name = "derive_more-impl"
-version = "2.0.1"
+version = "2.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3"
+checksum = "799a97264921d8623a957f6c3b9011f3b5492f557bbb7a5a19b7fa6d06ba8dcb"
 dependencies = [
+ "convert_case 0.10.0",
  "proc-macro2",
  "quote",
+ "rustc_version",
  "syn 2.0.106",
  "unicode-xid",
 ]
@@ -4914,7 +4925,7 @@ dependencies = [
  "const_format",
  "dashmap",
  "deadpool-redis",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "dotenv-build",
  "dotenvy",
  "either",
@@ -5516,7 +5527,7 @@ name = "modrinth-util"
 version = "0.0.0"
 dependencies = [
  "actix-web",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "dotenvy",
  "eyre",
  "modrinth-log",
@@ -5582,7 +5593,7 @@ dependencies = [
  "arc-swap",
  "bytes",
  "chrono",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "reqwest 0.12.24",
  "rust_decimal",
  "rust_iso3166",
@@ -6593,7 +6604,7 @@ checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a"
 name = "path-util"
 version = "0.0.0"
 dependencies = [
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "itertools 0.14.0",
  "serde",
  "typed-path",
@@ -9275,7 +9286,7 @@ dependencies = [
 name = "sqlx-tracing"
 version = "0.2.0"
 dependencies = [
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "futures",
  "opentelemetry",
  "opentelemetry-testing",
@@ -10148,7 +10159,7 @@ dependencies = [
  "daedalus",
  "dashmap",
  "data-url",
- "derive_more 2.0.1",
+ "derive_more 2.1.1",
  "dirs",
  "discord-rich-presence",
  "dotenvy",

Cargo.toml

@@ -64,7 +64,7 @@ darling = { version = "0.23" }
 dashmap = "6.1.0"
 data-url = "0.3.2"
 deadpool-redis = { git = "https://github.com/modrinth/deadpool", rev = "db5fb00b036ecc8fe5f18853c559b745ffe47bde", version = "0.22.1" }
-derive_more = "2.0.1"
+derive_more = "2.1.1"
 directories = "6.0.0"
 dirs = "6.0.0"
 discord-rich-presence = "1.0.0"

apps/labrinth/src/models/v3/analytics.rs

@@ -1,4 +1,5 @@
 use clickhouse::Row;
+use derive_more::Display;
 use serde::{Deserialize, Serialize};
 use std::hash::Hash;
 use std::net::Ipv6Addr;
@@ -24,14 +25,18 @@ pub struct Download {
     pub user_agent: String,
     pub headers: Vec<(String, String)>,
 
-    // added retroactively - may be missing
+    // added retroactively - may be empty
-    pub reason: Option<DownloadReason>,
+    pub reason: String,
-    pub game_version: Option<String>,
+    pub game_version: String,
-    pub loader: Option<String>,
+    pub loader: String,
 }
 
 /// Why a project was downloaded.
-#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[derive(
+    Debug, Display, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize,
+)]
+#[serde(rename_all = "snake_case")]
+#[display(rename_all = "snake_case")]
 pub enum DownloadReason {
     /// Project was downloaded directly by the user.
     Standalone,
@@ -95,8 +100,8 @@ pub struct Playtime {
     /// Parent modpack this playtime was recorded in
     pub parent: u64,
 
-    // added retroactively - may be missing
+    // added retroactively - may be empty
-    pub country: Option<String>,
+    pub country: String,
 }
 
 #[derive(Row, Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash)]

apps/labrinth/src/queue/analytics/mod.rs

@@ -9,6 +9,7 @@ use crate::routes::analytics::MINECRAFT_SERVER_PLAYS;
 use dashmap::{DashMap, DashSet};
 use redis::cmd;
 use std::collections::HashMap;
+use tracing::trace;
 
 pub mod cache;
 
@@ -262,6 +263,7 @@ impl AnalyticsQueue {
         }
 
         if !downloads_queue.is_empty() {
+            let downloads_count = downloads_queue.len();
             let mut downloads_keys = Vec::new();
             let raw_downloads = DashMap::new();
 
@@ -319,6 +321,11 @@ impl AnalyticsQueue {
             let mut version_downloads: HashMap<i64, i32> = HashMap::new();
             let mut project_downloads: HashMap<i64, i32> = HashMap::new();
 
+            trace!(
+                "inserting {} raw downloads out of {downloads_count} downloads",
+                raw_downloads.len()
+            );
+
             for (_, download) in raw_downloads {
                 *version_downloads
                     .entry(download.version_id as i64)
@@ -327,6 +334,8 @@ impl AnalyticsQueue {
                     .entry(download.project_id as i64)
                     .or_default() += 1;
 
+                trace!("writing download {download:?}");
+
                 downloads.write(&download).await?;
             }
 

apps/labrinth/src/routes/analytics.rs

@@ -234,7 +234,8 @@ async fn playtime_ingest(
                 parent: playtime.parent.map_or(0, |x| x.0),
                 country: headers
                     .get("cf-ipcountry")
-                    .and_then(|c| c.to_str().map(|s| s.to_string()).ok()),
+                    .and_then(|c| c.to_str().map(|s| s.to_string()).ok())
+                    .unwrap_or_default(),
             });
         }
     }

apps/labrinth/src/routes/internal/admin.rs

@@ -16,6 +16,7 @@ use serde::Deserialize;
 use std::collections::HashMap;
 use std::net::Ipv4Addr;
 use std::sync::Arc;
+use tracing::trace;
 
 pub fn config(cfg: &mut utoipa_actix_web::service_config::ServiceConfig) {
     cfg.service(
@@ -129,12 +130,16 @@ pub async fn count_download(
     let ip = crate::util::ip::convert_to_ip_v6(&download_body.ip)
         .unwrap_or_else(|_| Ipv4Addr::new(127, 0, 0, 1).to_ipv6_mapped());
 
-    let meta = download_body
+    let meta =
-        .headers
+        if let Some(meta) = download_body.headers.get(DOWNLOAD_META_HEADER) {
-        .get(DOWNLOAD_META_HEADER)
+            serde_json::from_str::<DownloadMeta>(meta)
-        .and_then(|v| serde_json::from_str::<DownloadMeta>(v).ok());
+                .map(Some)
+                .wrap_request_err("invalid download meta")?
+        } else {
+            None
+        };
 
-    analytics_queue.add_download(Download {
+    let download = Download {
         recorded: get_current_tenths_of_ms(),
         domain: url.host_str().unwrap_or_default().to_string(),
         site_path: url.path().to_string(),
@@ -169,10 +174,19 @@ pub async fn count_download(
                     .contains(&&*x.0.to_lowercase())
             })
             .collect(),
-        reason: meta.as_ref().map(|m| m.reason),
+        reason: meta
-        game_version: meta.as_ref().map(|m| m.game_version.clone()),
+            .as_ref()
-        loader: meta.as_ref().map(|m| m.loader.clone()),
+            .map(|m| m.reason.to_string())
-    });
+            .unwrap_or_default(),
+        game_version: meta
+            .as_ref()
+            .map(|m| m.game_version.clone())
+            .unwrap_or_default(),
+        loader: meta.as_ref().map(|m| m.loader.clone()).unwrap_or_default(),
+    };
+    trace!("added download {download:#?}");
+
+    analytics_queue.add_download(download);
 
     Ok(HttpResponse::NoContent().body(""))
 }

apps/labrinth/src/routes/v3/analytics_get.rs

@@ -54,10 +54,12 @@ pub struct GetRequest {
     /// What time range to return statistics for.
     pub time_range: TimeRange,
     /// What analytics metrics to return data for.
+    #[serde(default)]
     pub return_metrics: ReturnMetrics,
     /// What project IDs to return data for.
     ///
     /// If this is empty, all of the user's projects will be included.
+    #[serde(default)]
     pub project_ids: Vec<ProjectId>,
 }