From e8be67d41f0a4b092bebae98910e282d72ac9cb9 Mon Sep 17 00:00:00 2001 From: aecsocket Date: Mon, 27 Apr 2026 13:25:57 +0100 Subject: [PATCH] Fix how analytics writes are serialized (#5926) --- Cargo.lock | 39 +++++++++++++------- Cargo.toml | 2 +- apps/labrinth/src/models/v3/analytics.rs | 19 ++++++---- apps/labrinth/src/queue/analytics/mod.rs | 9 +++++ apps/labrinth/src/routes/analytics.rs | 3 +- apps/labrinth/src/routes/internal/admin.rs | 32 +++++++++++----- apps/labrinth/src/routes/v3/analytics_get.rs | 2 + 7 files changed, 74 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d1abfa59..517ab17e1 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index 54865d6d3..0e9a5ad67 100644 --- a/Cargo.toml +++ b/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" diff --git a/apps/labrinth/src/models/v3/analytics.rs b/apps/labrinth/src/models/v3/analytics.rs index eb028a03f..c10cbb723 100644 --- a/apps/labrinth/src/models/v3/analytics.rs +++ b/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 - pub reason: Option, - pub game_version: Option, - pub loader: Option, + // added retroactively - may be empty + pub reason: String, + pub game_version: 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 - pub country: Option, + // added retroactively - may be empty + pub country: String, } #[derive(Row, Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Hash)] diff --git a/apps/labrinth/src/queue/analytics/mod.rs b/apps/labrinth/src/queue/analytics/mod.rs index 1f04d81a8..fc271fcf4 100644 --- a/apps/labrinth/src/queue/analytics/mod.rs +++ b/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 = HashMap::new(); let mut project_downloads: HashMap = 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?; } diff --git a/apps/labrinth/src/routes/analytics.rs b/apps/labrinth/src/routes/analytics.rs index 5f84fd330..4bd524b56 100644 --- a/apps/labrinth/src/routes/analytics.rs +++ b/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(), }); } } diff --git a/apps/labrinth/src/routes/internal/admin.rs b/apps/labrinth/src/routes/internal/admin.rs index 8d14ce4a6..87d98b989 100644 --- a/apps/labrinth/src/routes/internal/admin.rs +++ b/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 - .headers - .get(DOWNLOAD_META_HEADER) - .and_then(|v| serde_json::from_str::(v).ok()); + let meta = + if let Some(meta) = download_body.headers.get(DOWNLOAD_META_HEADER) { + serde_json::from_str::(meta) + .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), - game_version: meta.as_ref().map(|m| m.game_version.clone()), - loader: meta.as_ref().map(|m| m.loader.clone()), - }); + reason: meta + .as_ref() + .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("")) } diff --git a/apps/labrinth/src/routes/v3/analytics_get.rs b/apps/labrinth/src/routes/v3/analytics_get.rs index 43bd84b26..756c467e0 100644 --- a/apps/labrinth/src/routes/v3/analytics_get.rs +++ b/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, }