From 0b382fea90c6041d29c8319cc19f70e4a0b93c60 Mon Sep 17 00:00:00 2001 From: David Laban Date: Mon, 9 Sep 2024 17:43:42 +0100 Subject: [PATCH] report stats to new server, with status (#281) * report stats to new server, with status This requires us to move the reporting to happen after we've done the install, but let's not quibble about a few milliseconds here or there, when we may have saved the user multiple minutes. * drop unknown query params in server * also send agent as url param * lump other error types together --- cargo-quickinstall/src/install_error.rs | 34 +++++++++++++++++++ cargo-quickinstall/src/lib.rs | 43 +++++++++++++++++++++---- cargo-quickinstall/src/main.rs | 5 +-- stats-server/src/main.rs | 6 +++- 4 files changed, 78 insertions(+), 10 deletions(-) diff --git a/cargo-quickinstall/src/install_error.rs b/cargo-quickinstall/src/install_error.rs index bdf688b52d..9b4f33d531 100644 --- a/cargo-quickinstall/src/install_error.rs +++ b/cargo-quickinstall/src/install_error.rs @@ -107,3 +107,37 @@ impl From for InstallError { InstallError::JsonErr(err) } } + +#[derive(Debug)] +pub enum InstallSuccess { + InstalledFromTarball, + BuiltFromSource, +} + +/** + * Returns a status string for reporting to our stats server. + * + * The return type is a static string to encourage us to keep the cardinality vaguely small-ish, + * and avoid us accidentally dumping personally identifiable information into influxdb. + * + * If we find ourselves getting a lot of a particular genre of error, we can always make a new + * release to split things out a bit more. + * + * There is no requirement for cargo-quickinstall and cargo-binstall to agree on the status strings, + * but it is probably a good idea to keep at least the first two in sync. + */ +pub fn install_result_to_status_str(result: &Result) -> &'static str { + match result { + Ok(InstallSuccess::InstalledFromTarball) => "installed-from-tarball", + Ok(InstallSuccess::BuiltFromSource) => "built-from-source", + Err(InstallError::CargoInstallFailed) => "cargo-install-failed", + Err(InstallError::NoFallback(_)) => "no-fallback", + Err(InstallError::MissingCrateNameArgument(_)) + | Err(InstallError::CommandFailed(_)) + | Err(InstallError::IoError(_)) + | Err(InstallError::CrateDoesNotExist { .. }) + | Err(InstallError::InvalidJson { .. }) + | Err(InstallError::JsonErr(_)) + | Err(InstallError::FailToParseRustcOutput { .. }) => "other-error", + } +} diff --git a/cargo-quickinstall/src/lib.rs b/cargo-quickinstall/src/lib.rs index b6c845a696..9d78b64bf3 100644 --- a/cargo-quickinstall/src/lib.rs +++ b/cargo-quickinstall/src/lib.rs @@ -112,7 +112,10 @@ pub fn get_cargo_binstall_version() -> Option { String::from_utf8(output.stdout).ok() } -pub fn install_crate_curl(details: &CrateDetails, fallback: bool) -> Result<(), InstallError> { +pub fn install_crate_curl( + details: &CrateDetails, + fallback: bool, +) -> Result { let urls = get_quickinstall_download_urls(details); let res = match curl_and_untar(&urls[0]) { @@ -135,7 +138,7 @@ pub fn install_crate_curl(details: &CrateDetails, fallback: bool) -> Result<(), version = details.version, bin_dir = bin_dir.display(), ); - Ok(()) + Ok(InstallSuccess::InstalledFromTarball) } Err(err) if err.is_curl_404() => { if !fallback { @@ -153,7 +156,7 @@ pub fn install_crate_curl(details: &CrateDetails, fallback: bool) -> Result<(), let status = prepare_cargo_install_cmd(details).status()?; if status.success() { - Ok(()) + Ok(InstallSuccess::BuiltFromSource) } else { Err(InstallError::CargoInstallFailed) } @@ -237,17 +240,24 @@ fn get_target_triple_from_rustc() -> Result { Ok(parts.join("-")) } -pub fn report_stats_in_background(details: &CrateDetails) { +pub fn report_stats_in_background( + details: &CrateDetails, + result: &Result, +) { let stats_url = format!( - "https://warehouse-clerk-tmp.vercel.app/api/crate/{}-{}-{}.tar.gz", - details.crate_name, details.version, details.target + "https://cargo-quickinstall-stats-server.fly.dev/record-install?crate={crate}&version={version}&target={target}&agent={agent}&status={status}", + crate = url_encode(&details.crate_name), + version = url_encode(&details.version), + target = url_encode(&details.target), + agent = url_encode(concat!("cargo-quickinstall/", env!("CARGO_PKG_VERSION"))), + status = install_result_to_status_str(result), ); // Simply spawn the curl command to report stat. // // It's ok for it to fail and we would let the init process reap // the `curl` process. - prepare_curl_head_cmd(&stats_url) + prepare_curl_post_cmd(&stats_url) .stdin(process::Stdio::null()) .stdout(process::Stdio::null()) .stderr(process::Stdio::null()) @@ -255,6 +265,19 @@ pub fn report_stats_in_background(details: &CrateDetails) { .ok(); } +fn url_encode(input: &str) -> String { + let mut encoded = String::with_capacity(input.len()); + + for c in input.chars() { + match c { + 'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '_' | '.' | '~' => encoded.push(c), + _ => encoded.push_str(&format!("%{:02X}", c as u8)), + } + } + + encoded +} + fn format_curl_and_untar_cmd(url: &str, bin_dir: &Path) -> String { format!( "{curl_cmd} | {untar_cmd}", @@ -319,6 +342,12 @@ fn prepare_curl_head_cmd(url: &str) -> std::process::Command { cmd } +fn prepare_curl_post_cmd(url: &str) -> std::process::Command { + let mut cmd = prepare_curl_cmd(); + cmd.args(["-X", "POST"]).arg(url); + cmd +} + fn curl_head(url: &str) -> Result, InstallError> { prepare_curl_head_cmd(url) .output_checked_status() diff --git a/cargo-quickinstall/src/main.rs b/cargo-quickinstall/src/main.rs index e78e523ea7..8737d7b4b7 100644 --- a/cargo-quickinstall/src/main.rs +++ b/cargo-quickinstall/src/main.rs @@ -91,8 +91,9 @@ fn do_main_curl( let shell_cmd = do_dry_run_curl(&crate_details, args.fallback)?; println!("{}", shell_cmd); } else { - report_stats_in_background(&crate_details); - install_crate_curl(&crate_details, args.fallback)?; + let result = install_crate_curl(&crate_details, args.fallback); + report_stats_in_background(&crate_details, &result); + result?; } } diff --git a/stats-server/src/main.rs b/stats-server/src/main.rs index 0c81655d31..3660ad49cd 100644 --- a/stats-server/src/main.rs +++ b/stats-server/src/main.rs @@ -47,7 +47,11 @@ async fn record_install(Query(params): Query>) -> Strin let mut point = Measurement::builder("counts").field("count", 1); for (tag, value) in ¶ms { - point = point.tag(tag, &**value) + if !["crate", "version", "target", "agent", "status"].contains(&tag.as_str()) { + println!("Skipping unknown query param: {tag}={value}"); + continue; + } + point = point.tag(tag, value.as_str()) } INFLUX_CLIENT .write(&INFLUX_BUCKET, &[point.build().unwrap()])