From 01569dff8bff4de2490ab6884582b53a5941f6b6 Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Thu, 24 Sep 2026 15:15:41 +0300 Subject: [PATCH 1/7] Record one cli.command telemetry event per run Each run builds one event from what the command reports as it runs (the parse, the token, each request, stdout bytes, the error code) and hands it to a sink at exit: a JSON line in ~/.mapbox/.telemetry/.jsonl by default, or Mapbox Events through a detached child with MAPBOX_CLI_TELEMETRY_SINK=api. Argument values leave only when they come from a fixed set; free strings are sent as a length, files as a size. Every request now goes through http::send, which records it; a source guard holds that. --- CHANGELOG.md | 7 + README.md | 6 + src/account_usage.rs | 17 +- src/agent_skills.rs | 4 +- src/auth.rs | 59 +- src/completion.rs | 1 + src/events.rs | 1297 +++++++++++++++++++++++++++++++++++++ src/executor.rs | 21 +- src/http.rs | 59 +- src/main.rs | 62 +- src/output.rs | 3 + src/telemetry.rs | 15 + src/tilesets_cli.rs | 25 +- src/update_check.rs | 8 +- tests/completion.rs | 14 +- tests/non_interactive.rs | 19 +- tests/source_guards.rs | 28 + tests/telemetry_events.rs | 337 ++++++++++ 18 files changed, 1910 insertions(+), 72 deletions(-) create mode 100644 src/events.rs create mode 100644 tests/telemetry_events.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 273b384..f6e3715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ that may never merge. They are not releases and are not listed here. ## Unreleased +- Each run records one `cli.command` telemetry event, appended to + `~/.mapbox/.telemetry/.jsonl` and kept for 7 days. Nothing is sent + anywhere by default. It never touches stdout, the exit code or how long a + command takes, so a script sees no difference; a CI job does find a + `.mapbox/.telemetry` directory it didn't before. `MAPBOX_CLI_NO_TELEMETRY=1` + turns it off. + - `MAPBOX_CLI_EXTRA_QUERY` appends raw query parameters to every request, in the same `k1=v1&k2=v2` shape as a URL's own query string — for an API parameter this CLI's specs don't declare a flag for. diff --git a/README.md b/README.md index f047655..bfc009e 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,12 @@ Every request sends `User-Agent: mapbox-cli/` and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` keeps even future markers out of that header. +Each run also appends one event to `~/.mapbox/.telemetry/.jsonl` +(kept for 7 days): the command's name, its options (a value only when it +comes from a fixed list, otherwise just its length or size), how it ended, +and how long it took — never a token, a file path or free text you typed. +It stays on this machine. `MAPBOX_CLI_NO_TELEMETRY=1` turns it off. + ### Agent skills ```sh diff --git a/src/account_usage.rs b/src/account_usage.rs index 461c3b9..f7157d4 100644 --- a/src/account_usage.rs +++ b/src/account_usage.rs @@ -507,8 +507,8 @@ fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { era * 146097 + doe - 719468 } -/// The inverse of [`days_from_civil`]. -fn civil_from_days(z: i64) -> (i64, u32, u32) { +/// The inverse of [`days_from_civil`]. Also [`crate::events`]'s calendar. +pub(crate) fn civil_from_days(z: i64) -> (i64, u32, u32) { let z = z + 719468; let era = z.div_euclid(146097); let doe = z - era * 146097; // [0, 146096] @@ -599,12 +599,13 @@ fn fetch( eprintln!("[debug] GET {}", redacted_url(&url, &query)); } - let response = client - .get(&url) - .query(&query) - .timeout(http::budget(timeout, http::Payload::Bounded)) - .send() - .map_err(|e| executor::transport_failure("Request failed", e))?; + let response = http::send( + client + .get(&url) + .query(&query) + .timeout(http::budget(timeout, http::Payload::Bounded)), + ) + .map_err(|e| executor::transport_failure("Request failed", e))?; let status = response.status(); // Before `text()` consumes the response: a 5xx here is worth escalating, diff --git a/src/agent_skills.rs b/src/agent_skills.rs index 63b45a4..83d2340 100644 --- a/src/agent_skills.rs +++ b/src/agent_skills.rs @@ -224,9 +224,7 @@ fn fetch(base: &str, git_ref: &str, debug: bool) -> Result> { eprintln!("[debug] GET {url}"); } - let response = http::client()? - .get(&url) - .send() + let response = http::send(http::client()?.get(&url)) .map_err(|e| executor::transport_failure("Could not reach GitHub", e))?; let status = response.status(); diff --git a/src/auth.rs b/src/auth.rs index bd81c80..820ca51 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -654,10 +654,7 @@ fn refresh_credentials(creds: &mut Credentials, debug: bool) -> Result<()> { eprintln!("[debug] POST {} (grant_type=refresh_token)", TOKEN_ENDPOINT); } - let resp = client - .post(TOKEN_ENDPOINT) - .form(¶ms) - .send() + let resp = crate::http::send(client.post(TOKEN_ENDPOINT).form(¶ms)) .context("Token refresh request failed")?; if !resp.status().is_success() { @@ -692,8 +689,11 @@ pub fn force_refresh(debug: bool, profile: Option<&str>, mode: Mode) -> Result<( let mut creds = load_credentials(profile) .ok_or_else(|| anyhow!("Not currently logged in. Run `mapbox auth login` first."))?; + crate::events::record_auth_step("refresh"); refresh_credentials(&mut creds, debug)?; + crate::events::record_auth_step("save_credentials"); save_credentials(&creds, profile)?; + crate::events::record_auth_step("done"); let expires_at = token_expires_at(&creds.access_token); let text = match expires_at { @@ -896,8 +896,10 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> { let path = credentials_path(profile)?; let had_credentials = path.exists(); if had_credentials { + crate::events::record_auth_step("remove_credentials"); std::fs::remove_file(&path)?; } + crate::events::record_auth_step("done"); let text = if had_credentials { "Logged out successfully." @@ -932,7 +934,7 @@ impl TokenSource { /// The name a program reads. The prose form is built in /// [`Identity::source_prose`], where it can name the variable or profile /// the bare word leaves out. - fn as_str(self) -> &'static str { + pub(crate) fn as_str(self) -> &'static str { match self { TokenSource::Flag => "flag", TokenSource::Environment => "environment", @@ -1136,17 +1138,18 @@ fn verify_token(token: &str, debug: bool, timeout: Option) -> Result"); } - let response = crate::http::client()? - .get(VALIDATION_ENDPOINT) - .query(&[("access_token", token)]) - // The one request `auth` makes to a Mapbox API rather than to the - // authorization server, so it is the one `--timeout` has to reach. - // The other three — refresh, registration, code exchange — carry a - // few hundred bytes each and keep the client's own budget. - .timeout(crate::http::budget(timeout, crate::http::Payload::Bounded)) - .send() - // `reqwest::Error`'s `Display` appends the URL, and the token is in it. - .map_err(|e| crate::executor::transport_failure("Token check failed", e))?; + let response = crate::http::send( + crate::http::client()? + .get(VALIDATION_ENDPOINT) + .query(&[("access_token", token)]) + // The one request `auth` makes to a Mapbox API rather than to the + // authorization server, so it is the one `--timeout` has to reach. + // The other three — refresh, registration, code exchange — carry a + // few hundred bytes each and keep the client's own budget. + .timeout(crate::http::budget(timeout, crate::http::Payload::Bounded)), + ) + // `reqwest::Error`'s `Display` appends the URL, and the token is in it. + .map_err(|e| crate::executor::transport_failure("Token check failed", e))?; let status = response.status(); // Before `text()` consumes the response — see `executor::request_id`. @@ -1436,12 +1439,13 @@ fn register_client(redirect_uri: &str, debug: bool, scopes: &str) -> Result, mode: Mode) -> Result<()> { // Computed once: register_client's ceiling and the authorize scope must agree. let scopes = default_scopes(); + crate::events::record_auth_step("register_client"); output::progress("Registering OAuth client with Mapbox..."); let registration = register_client(&redirect_uri, debug, scopes)?; @@ -1922,6 +1924,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { // Not discarded: with stderr redirected this is the only thing carrying // the run, so its failure is the difference between refusing now and // stalling for five minutes. See `login_can_be_completed`. + crate::events::record_auth_step("open_browser"); let browser_opened = open::that(&auth_url).is_ok(); if !login_can_be_completed(std::io::stderr().is_terminal(), browser_opened) { return Err(login_has_no_way_to_show_the_url()); @@ -1930,8 +1933,10 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { output::progress(&format!( "Waiting for authorization (listening on port {port})..." )); + crate::events::record_auth_step("wait_for_callback"); let code = wait_for_callback(port, &state, CALLBACK_TIMEOUT)?; + crate::events::record_auth_step("exchange_code"); output::progress("Exchanging authorization code for access token..."); let mut creds = exchange_code_for_token( &code, @@ -1943,7 +1948,9 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { )?; creds.client_id = Some(registration.client_id.clone()); + crate::events::record_auth_step("save_credentials"); save_credentials(&creds, profile)?; + crate::events::record_auth_step("done"); let profile_note = match profile { Some(name) if name != "default" => format!(" (profile: {name})"), diff --git a/src/completion.rs b/src/completion.rs index 0a33158..167d4f4 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -153,6 +153,7 @@ pub fn run(app: &Command, matches: &ArgMatches) -> Result<()> { // Straight to stdout rather than through `output::emit`: the script is // the result, and there is no rendering of it that is not itself. + crate::events::add_stdout_bytes(script.len()); let mut out = io::stdout().lock(); match out.write_all(&script).and_then(|()| out.flush()) { // A reader that stopped reading is `head`'s ordinary behavior, not a diff --git a/src/events.rs b/src/events.rs new file mode 100644 index 0000000..0b25c03 --- /dev/null +++ b/src/events.rs @@ -0,0 +1,1297 @@ +//! One `cli.command` event per run: what ran and how it ended. +//! +//! Modules report what they know as it happens — the parse, the token, each +//! request, the bytes on stdout, an error code — through the `record_*` +//! functions here, and `main` calls [`finish`] once on the way out. The +//! event is then handed to a sink: a JSON line under +//! `~/.mapbox/.telemetry/` by default, or Mapbox Events through a detached +//! child with `MAPBOX_CLI_TELEMETRY_SINK=api`. +//! +//! What this refuses to record is the point of it. Argument values leave +//! only when they come from a fixed set (an enum, a boolean, a number the +//! spec types, an allowlisted code); a free string is sent as its length, a +//! file as its size, a coordinate as its name alone. Command names come from +//! the command tree, never from argv. A token is read for its prefix and its +//! account claim and nothing else. +//! +//! Best-effort throughout: nothing here can change a command's output, its +//! exit code, or how long it takes to return. With telemetry off +//! (`MAPBOX_CLI_NO_TELEMETRY`), nothing is recorded or written. + +use std::collections::HashSet; +use std::io::{IsTerminal, Read, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command as Process, ExitCode, Stdio}; +use std::sync::{Mutex, OnceLock}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +use clap::parser::ValueSource; +use clap::{ArgMatches, Command}; +use serde::Serialize; + +use crate::spec::ServiceSpec; +use crate::{agent_detect, auth, confirm, executor, http, output, schema, telemetry}; + +const EVENT: &str = "cli.command"; +const SCHEMA_VERSION: &str = "2.0"; +const SDK_IDENTIFIER: &str = "mapbox-cli"; +const CURRENT: &str = env!("CARGO_PKG_VERSION"); + +/// `file` or `api`. Anything else, or nothing, is `file`. +const SINK_ENV: &str = "MAPBOX_CLI_TELEMETRY_SINK"; +/// Prints the event to stderr instead of handing it to the sink. +const DEBUG_ENV: &str = "MAPBOX_CLI_TELEMETRY_DEBUG"; + +/// Set on the child [`finish`] spawns for the `api` sink — a mode of this +/// binary for the reason `update_check`'s refresher is one: a hidden +/// subcommand would be in `--schema` and the generated skills. +const SENDER_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_SEND"; +/// Overrides for the compiled-in endpoint and token, for tests and for +/// pointing a dev build at staging. +const URL_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_URL"; +const TOKEN_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_TOKEN"; +const PRODUCTION_URL: &str = "https://events.mapbox.com/events/v2"; +const SEND_TIMEOUT: Duration = Duration::from_secs(5); +/// Far above one event (about 1 KB); a bound on what the child will read. +const MAX_PAYLOAD: u64 = 256 * 1024; + +const DIR: &str = ".telemetry"; +const USER_ID_FILE: &str = "user-id"; +const LAST_VERSION_FILE: &str = "last-version"; +/// Days of event files kept by the `file` sink. +const KEEP_DAYS: i64 = 7; + +// Bounds from the schema. An event over any of them is rejected whole at +// ingest, so they are enforced here rather than trusted. +const MAX_PARAMS: usize = 50; +const MAX_VALUE: usize = 200; +const MAX_NAME: usize = 64; +const MAX_KEYS: usize = 50; +const MAX_COMMAND_LEVELS: usize = 8; +const MAX_COMMAND_NAME: usize = 32; +const MAX_REQUEST_IDS: usize = 5; +const MAX_REQUEST_ID: usize = 128; +const MAX_CODE: usize = 64; +const MAX_VERSION: usize = 32; + +/// Options that are top-level fields or `invocation`, so never `params`. +const NOT_PARAMS: &[&str] = &[ + "token", + "profile", + "use-login", + "debug", + confirm::ARG, + http::TIMEOUT_ARG, + output::ARG, + schema::ARG, + executor::DRY_RUN_ARG, + "help", + "version", +]; + +/// Global options with no top-level field, sent as free strings. They are +/// read from the root matches: a leaf `Command` does not list the globals +/// it inherits. +const GLOBAL_PARAMS: &[&str] = &["username", output::FILTER_ARG]; + +/// Commands that record nothing. `completion` runs at every shell startup, +/// usually without anyone typing it, and is promised to touch nothing on +/// disk (`it_needs_no_token_and_touches_no_credentials`). +const NOT_RECORDED: &[&str] = &[crate::completion::COMMAND]; + +/// Free strings whose values are codes, not user data. +const ALLOWLISTED: &[&str] = &["language", "country", "types"]; + +/// Numbers that are sent by name only: together they are a location. +const COORDINATES: &[&str] = &["lon", "lat", "longitude", "latitude"]; + +/// The Python `tilesets` CLI's own commands (`mapbox_tilesets/scripts/cli.py`). +/// A forwarded first word outside this list is recorded as `other`, since it +/// is whatever the user typed. +const TILESETS_COMMANDS: &[&str] = &[ + "add-source", + "create", + "delete", + "delete-changeset", + "delete-source", + "estimate-area", + "job", + "jobs", + "list", + "list-activity", + "list-sources", + "publish", + "publish-changesets", + "status", + "tilejson", + "update", + "update-recipe", + "upload-changeset", + "upload-raster-source", + "upload-source", + "validate-recipe", + "validate-source", + "view-changeset", + "view-recipe", + "view-source", +]; + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +pub(crate) struct Param { + name: String, + #[serde(skip_serializing_if = "Option::is_none")] + value: Option, + #[serde(skip_serializing_if = "Option::is_none")] + length: Option, + #[serde(skip_serializing_if = "Option::is_none")] + bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + keys: Option>, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +struct Auth { + source: &'static str, + #[serde(rename = "type")] + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + account: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Network { + request_count: u32, + #[serde(skip_serializing_if = "Option::is_none")] + status: Option, + #[serde(skip_serializing_if = "Option::is_none")] + response_bytes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + request_body_bytes: Option, + network_ms: u64, + #[serde(skip_serializing_if = "Vec::is_empty")] + request_ids: Vec, + #[serde(skip_serializing_if = "std::ops::Not::not")] + more_pages: bool, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Cli { + #[serde(skip_serializing_if = "Option::is_none")] + build_channel: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + build_id: Option<&'static str>, + install_method: &'static str, +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Env { + arch: &'static str, + ci: bool, + #[serde(skip_serializing_if = "Option::is_none")] + agent: Option<&'static str>, + stdin_tty: bool, + stdout_tty: bool, +} + +/// The event as sent. Field order is the schema's. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Event { + event: &'static str, + version: &'static str, + created: String, + event_id: String, + user_id: String, + sdk_identifier: &'static str, + sdk_version: &'static str, + operating_system: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + invocation: Option<&'static str>, + #[serde(skip_serializing_if = "Vec::is_empty")] + params: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + usage_error: Option, + #[serde(skip_serializing_if = "Option::is_none")] + output: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + output_source: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + dry_run: Option, + #[serde(skip_serializing_if = "Option::is_none")] + debug: Option, + #[serde(skip_serializing_if = "Option::is_none")] + yes: Option, + #[serde(skip_serializing_if = "Option::is_none")] + profile: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + timeout_seconds: Option, + #[serde(skip_serializing_if = "Option::is_none")] + auth: Option, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + stdout_bytes: u64, + duration_ms: u64, + #[serde(skip_serializing_if = "Option::is_none")] + auth_step: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + update_notice: Option, + #[serde(skip_serializing_if = "Option::is_none")] + previous_version: Option, + #[serde(skip_serializing_if = "Option::is_none")] + network: Option, + cli: Cli, + env: Env, +} + +/// The global options a parsed run carries, all present or all absent. +#[derive(Debug, Clone, Default, PartialEq)] +struct Options { + output: &'static str, + output_source: &'static str, + dry_run: bool, + debug: bool, + yes: bool, + profile: &'static str, + timeout_seconds: Option, +} + +/// What the run has reported so far. +#[derive(Debug, Default)] +struct Run { + command: Option>, + invocation: Option<&'static str>, + params: Vec, + usage_error: Option, + options: Option, + auth: Option, + error_code: Option, + stdout_bytes: u64, + auth_step: Option<&'static str>, + update_notice: Option, + network: Option, + finished: bool, +} + +impl Run { + fn skip(&mut self) { + self.finished = true; + } + + const fn new() -> Self { + Run { + command: None, + invocation: None, + params: Vec::new(), + usage_error: None, + options: None, + auth: None, + error_code: None, + stdout_bytes: 0, + auth_step: None, + update_notice: None, + network: None, + finished: false, + } + } +} + +static RUN: Mutex = Mutex::new(Run::new()); +static STARTED: OnceLock = OnceLock::new(); +static ENABLED: OnceLock = OnceLock::new(); + +/// Read once: the answer must not change halfway through a run. +fn enabled() -> bool { + *ENABLED.get_or_init(telemetry::telemetry_allowed) +} + +/// A poisoned lock is a panic somewhere else; recording is not worth a +/// second one, so it is skipped. +fn with_run(f: impl FnOnce(&mut Run)) { + if !enabled() { + return; + } + if let Ok(mut run) = RUN.lock() { + f(&mut run); + } +} + +/// Marks the start of the run, for `durationMs`, and installs the panic +/// hook that records a crash as `errorCode: "panic"`. +pub fn start() { + STARTED.get_or_init(Instant::now); + + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + previous(info); + // `try_lock`: the panic may have happened while this thread held + // the lock, and waiting for it would hang instead of exiting. + if let Ok(mut run) = RUN.try_lock() { + run.error_code = Some("panic".to_string()); + deliver_locked(&mut run, Some(101)); + } + })); +} + +/// The command line clap parsed. `invocation` is `execute` or `schema`. +pub fn record_parsed( + app: &Command, + specs: &[ServiceSpec], + matches: &ArgMatches, + invocation: &'static str, +) { + with_run(|run| { + let (path, leaf_command, leaf_matches) = leaf(app, matches); + if path + .first() + .is_some_and(|top| NOT_RECORDED.contains(&top.as_str())) + { + run.skip(); + return; + } + run.command = command_field(&path, matches); + run.invocation = Some(invocation); + run.options = Some(options(matches, leaf_matches)); + if invocation == "execute" && path.first().map(String::as_str) != Some(TILESETS) { + let numeric = numeric_args(specs, &path); + let mut params = params(leaf_command, leaf_matches, &numeric); + params.extend(global_params(app, matches)); + params.truncate(MAX_PARAMS); + run.params = params; + } + }); +} + +/// A command line clap refused, or answered with help or the version. +/// Command names are recovered by walking the tree with argv's words, so +/// only names the tree already has can come out. +pub fn record_unparsed(app: &Command, argv: &[std::ffi::OsString], kind: clap::error::ErrorKind) { + use clap::error::ErrorKind; + with_run(|run| { + let path = command_from_argv(app, argv); + run.command = (!path.is_empty()).then(|| clip_command(path)); + match kind { + ErrorKind::DisplayHelp | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => { + run.invocation = Some("help"); + } + ErrorKind::DisplayVersion => run.invocation = Some("version"), + other => { + run.invocation = Some("execute"); + run.usage_error = Some(clip(&format!("{other:?}"), MAX_CODE)); + } + } + }); +} + +/// The token a command resolved, for `auth`. Read for its prefix and its +/// `u` claim; the token itself is not kept. +pub fn record_token(source: auth::TokenSource, token: &str) { + with_run(|run| run.auth = Some(auth_field(source, token))); +} + +/// [`record_token`] for the service arms' resolution: a typed `--token`, +/// then the environment unless `--use-login`, then the stored login. +pub fn record_resolved_token(matches: &ArgMatches, use_login: bool, token: &str) { + let source = if auth::typed_token(matches).is_some() { + auth::TokenSource::Flag + } else if !use_login && matches.get_one::("token").is_some() { + auth::TokenSource::Environment + } else { + auth::TokenSource::Login + }; + record_token(source, token); +} + +pub fn record_error_code(code: &str) { + with_run(|run| run.error_code = Some(clip(code, MAX_CODE))); +} + +pub fn record_auth_step(step: &'static str) { + with_run(|run| run.auth_step = Some(step)); +} + +pub fn record_update_notice(version: &str) { + with_run(|run| run.update_notice = Some(clip(version, MAX_VERSION))); +} + +pub fn add_stdout_bytes(bytes: usize) { + with_run(|run| run.stdout_bytes = run.stdout_bytes.saturating_add(bytes as u64)); +} + +pub fn record_more_pages() { + with_run(|run| run.network.get_or_insert_with(Network::default).more_pages = true); +} + +/// One request, from [`http::send`]. `status` is `None` when no response +/// came back; `request_id` is passed only for a Mapbox response. +pub fn record_request( + status: Option, + response_bytes: Option, + request_body_bytes: Option, + elapsed: Duration, + request_id: Option, +) { + with_run(|run| { + let network = run.network.get_or_insert_with(Network::default); + network.request_count = network.request_count.saturating_add(1); + network.status = status; + network.response_bytes = sum(network.response_bytes, response_bytes); + network.request_body_bytes = sum(network.request_body_bytes, request_body_bytes); + network.network_ms = network + .network_ms + .saturating_add(elapsed.as_millis() as u64); + if let Some(id) = request_id { + if network.request_ids.len() < MAX_REQUEST_IDS { + network.request_ids.push(clip(&id, MAX_REQUEST_ID)); + } + } + }); +} + +fn sum(total: Option, more: Option) -> Option { + match (total, more) { + (Some(a), Some(b)) => Some(a.saturating_add(b)), + (a, b) => a.or(b), + } +} + +/// Builds the event and hands it to the sink. Once per run; later calls do +/// nothing. `exit_code` is `None` for a `tilesets-cli` run that `exec`s. +pub fn finish(exit_code: Option) { + if !enabled() { + return; + } + if let Ok(mut run) = RUN.lock() { + deliver_locked(&mut run, exit_code); + } +} + +fn deliver_locked(run: &mut Run, exit_code: Option) { + if run.finished || !enabled() { + return; + } + run.finished = true; + let event = build(run, exit_code); + let Ok(line) = serde_json::to_string(&event) else { + return; + }; + if std::env::var_os(DEBUG_ENV).is_some_and(|value| !value.is_empty()) { + output::progress(&line); + return; + } + match std::env::var(SINK_ENV).ok().as_deref().map(str::trim) { + Some("api") => spawn_sender(&line), + _ => append_to_file(&line), + } +} + +fn build(run: &Run, exit_code: Option) -> Event { + let options = run.options.clone(); + let duration = STARTED.get().map_or(Duration::ZERO, Instant::elapsed); + Event { + event: EVENT, + version: SCHEMA_VERSION, + created: timestamp(SystemTime::now()), + event_id: uuid_v4(rand::random()), + user_id: user_id(), + sdk_identifier: SDK_IDENTIFIER, + sdk_version: CURRENT, + operating_system: std::env::consts::OS, + command: run.command.clone(), + invocation: run.invocation, + params: run.params.clone(), + usage_error: run.usage_error.clone(), + output: options.as_ref().map(|o| o.output), + output_source: options.as_ref().map(|o| o.output_source), + dry_run: options.as_ref().map(|o| o.dry_run), + debug: options.as_ref().map(|o| o.debug), + yes: options.as_ref().map(|o| o.yes), + profile: options.as_ref().map(|o| o.profile), + timeout_seconds: options.as_ref().and_then(|o| o.timeout_seconds), + auth: run.auth.clone(), + exit_code, + error_code: run.error_code.clone(), + stdout_bytes: run.stdout_bytes, + duration_ms: duration.as_millis() as u64, + auth_step: run.auth_step, + update_notice: run.update_notice.clone(), + previous_version: previous_version(), + network: run.network.clone(), + cli: Cli { + build_channel: option_env!("MAPBOX_CLI_BUILD_ENV"), + build_id: option_env!("MAPBOX_CLI_BUILD_ID"), + install_method: install_method(std::env::current_exe().ok().as_deref()), + }, + env: Env { + arch: telemetry::arch(), + ci: telemetry::in_ci(), + agent: agent_detect::detect_agent(), + stdin_tty: std::io::stdin().is_terminal(), + stdout_tty: telemetry::stdout_is_terminal(), + }, + } +} + +const TILESETS: &str = crate::tilesets_cli::COMMAND; + +/// The command path, the leaf `Command` and the leaf matches. +fn leaf<'a>( + app: &'a Command, + matches: &'a ArgMatches, +) -> (Vec, &'a Command, &'a ArgMatches) { + let mut path = vec![]; + let mut command = app; + let mut current = matches; + while let Some((name, sub)) = current.subcommand() { + path.push(name.to_string()); + if name == TILESETS && path.len() == 1 { + // Its forwarded words come back as subcommands of their own. + return (path, command.find_subcommand(name).unwrap_or(command), sub); + } + match command.find_subcommand(name) { + Some(found) => command = found, + None => break, + } + current = sub; + } + (path, command, current) +} + +fn command_field(path: &[String], matches: &ArgMatches) -> Option> { + if path.is_empty() { + return None; + } + if path[0] == TILESETS { + let word = matches + .subcommand_matches(TILESETS) + .map(crate::tilesets_cli::forwarded_args) + .and_then(|args| { + args.iter() + .map(|arg| arg.to_string_lossy().into_owned()) + .find(|arg| !arg.starts_with('-')) + }); + let mut command = vec![TILESETS.to_string()]; + if let Some(word) = word { + command.push(tilesets_word(&word).to_string()); + } + return Some(command); + } + Some(clip_command(path.to_vec())) +} + +fn tilesets_word(word: &str) -> &str { + TILESETS_COMMANDS + .iter() + .find(|known| **known == word) + .copied() + .unwrap_or("other") +} + +fn clip_command(path: Vec) -> Vec { + path.into_iter() + .take(MAX_COMMAND_LEVELS) + .map(|name| clip(&name, MAX_COMMAND_NAME)) + .collect() +} + +/// Subcommand names from argv, in order, for as long as each word names a +/// subcommand of the one before. Flags and their values are skipped; the +/// first word that is neither ends the walk. +fn command_from_argv(app: &Command, argv: &[std::ffi::OsString]) -> Vec { + let mut path = vec![]; + let mut command = app; + for word in argv.iter().skip(1).map(|w| w.to_string_lossy()) { + if word.starts_with('-') { + continue; + } + match command.find_subcommand(word.as_ref()) { + Some(found) => { + path.push(found.get_name().to_string()); + if found.get_name() == TILESETS { + break; + } + command = found; + } + None if path.is_empty() => continue, + None => break, + } + } + path +} + +fn options(matches: &ArgMatches, leaf_matches: &ArgMatches) -> Options { + let (output, output_source) = output_requested(matches); + let profile = match matches.get_one::("profile").map(String::as_str) { + None | Some("default") => "default", + Some(_) => "named", + }; + let timeout_seconds = (matches.value_source(http::TIMEOUT_ARG) + == Some(ValueSource::CommandLine)) + .then(|| matches.get_one::(http::TIMEOUT_ARG)) + .flatten() + .map(Duration::as_secs_f64); + Options { + output, + output_source, + dry_run: executor::wants_dry_run(leaf_matches), + debug: matches.get_flag("debug"), + yes: matches.get_flag(confirm::ARG), + profile, + timeout_seconds, + } +} + +/// `--output` as asked, in `Mode::from_matches`'s precedence, without its +/// warning — that has already been printed once by the time this runs. +fn output_requested(matches: &ArgMatches) -> (&'static str, &'static str) { + let known = |value: &str| { + [output::AUTO, output::TEXT, output::JSON] + .into_iter() + .find(|known| *known == value) + }; + if matches.value_source(output::ARG) == Some(ValueSource::CommandLine) { + let value = matches.get_one::(output::ARG).map(String::as_str); + return (value.and_then(known).unwrap_or(output::AUTO), "flag"); + } + match std::env::var(output::ENV) + .ok() + .map(|v| v.trim().to_string()) + { + Some(value) if !value.is_empty() => (known(&value).unwrap_or(output::AUTO), "env"), + _ => (output::AUTO, "default"), + } +} + +/// The spec parameters of the operation at `path` that are typed as numbers. +/// Only these send a numeric value: a free string that happens to be digits +/// (a postcode) is still a free string. +fn numeric_args(specs: &[ServiceSpec], path: &[String]) -> HashSet { + let Some((service, rest)) = path.split_first() else { + return HashSet::new(); + }; + specs + .iter() + .filter(|spec| &spec.name == service) + .flat_map(|spec| &spec.operations) + .filter(|op| op.command_path == rest) + .flat_map(|op| op.path_params.iter().chain(&op.query_params)) + .filter(|param| param.numeric.is_some()) + .map(|param| param.arg_name.clone()) + .collect() +} + +fn params(command: &Command, matches: &ArgMatches, numeric: &HashSet) -> Vec { + let mut out = vec![]; + for arg in command.get_arguments() { + let id = arg.get_id().as_str(); + if NOT_PARAMS.contains(&id) { + continue; + } + match matches.value_source(id) { + Some(ValueSource::CommandLine) | Some(ValueSource::EnvVariable) => {} + _ => continue, + } + let Some(raw) = matches.get_raw(id) else { + continue; + }; + let values: Vec = raw.map(|v| v.to_string_lossy().into_owned()).collect(); + let name = arg.get_long().unwrap_or(id); + let takes_values = arg.get_action().takes_values(); + let enumerated = !arg.get_possible_values().is_empty(); + out.push(classify( + name, + &values, + takes_values, + enumerated, + numeric.contains(id), + )); + if out.len() == MAX_PARAMS { + break; + } + } + out +} + +fn global_params(app: &Command, matches: &ArgMatches) -> Vec { + app.get_arguments() + .filter(|arg| GLOBAL_PARAMS.contains(&arg.get_id().as_str())) + .filter_map(|arg| { + let id = arg.get_id().as_str(); + match matches.value_source(id) { + Some(ValueSource::CommandLine) | Some(ValueSource::EnvVariable) => {} + _ => return None, + } + let value = matches.get_one::(id)?; + Some(classify( + arg.get_long().unwrap_or(id), + std::slice::from_ref(value), + true, + false, + false, + )) + }) + .collect() +} + +fn classify( + name: &str, + values: &[String], + takes_values: bool, + enumerated: bool, + numeric: bool, +) -> Param { + let joined = values.join(","); + let mut param = Param { + name: clip(name, MAX_NAME), + ..Param::default() + }; + if COORDINATES.contains(&name) { + return param; + } + if !takes_values || enumerated || numeric || ALLOWLISTED.contains(&name) { + param.value = Some(clip(&joined, MAX_VALUE)); + return param; + } + match name { + "file" => { + param.bytes = values + .iter() + .filter_map(|path| std::fs::metadata(path).ok()) + .map(|meta| meta.len()) + .reduce(u64::saturating_add); + } + "data" => { + let (bytes, keys) = data_shape(&joined); + param.bytes = bytes; + param.keys = keys; + } + _ => param.length = Some(joined.chars().count() as u64), + } + param +} + +/// Size and top-level keys of a `--data` body: inline JSON, `@`, or +/// `@-` (stdin, which is not read twice, so nothing but the name). +fn data_shape(value: &str) -> (Option, Option>) { + if value == "@-" { + return (None, None); + } + let text = match value.strip_prefix('@') { + Some(path) => { + let Ok(meta) = std::fs::metadata(path) else { + return (None, None); + }; + // Parsed only when small enough to be a request body worth + // describing; the size alone is still recorded above that. + if meta.len() > MAX_PAYLOAD { + return (Some(meta.len()), None); + } + match std::fs::read_to_string(path) { + Ok(text) => text, + Err(_) => return (Some(meta.len()), None), + } + } + None => value.to_string(), + }; + let keys = serde_json::from_str::(&text) + .ok() + .and_then(|json| { + json.as_object().map(|object| { + object + .keys() + .take(MAX_KEYS) + .map(|key| clip(key, MAX_NAME)) + .collect() + }) + }); + (Some(text.len() as u64), keys) +} + +fn auth_field(source: auth::TokenSource, token: &str) -> Auth { + let kind = match token.split('.').next() { + Some("pk") => "pk", + Some("sk") => "sk", + Some("tk") => "tk", + _ => "other", + }; + Auth { + source: source.as_str(), + kind, + account: auth::token_account(token).map(|account| clip(&account, MAX_NAME)), + } +} + +/// How this binary was installed, from where it lives. Only the category +/// leaves; the path does not. `MAPBOX_INSTALL_DIR` moves an install-script +/// binary anywhere, so those read as `other`. +fn install_method(exe: Option<&Path>) -> &'static str { + let Some(exe) = exe else { + return "other"; + }; + let path = exe + .to_string_lossy() + .replace('\\', "/") + .to_ascii_lowercase(); + if path.contains("/cellar/") || path.contains("/homebrew/") { + "homebrew" + } else if path.contains("/.cargo/bin/") { + "cargo" + } else if path.contains("/.local/bin/") || path.contains("/programs/mapbox/") { + "install-script" + } else { + "other" + } +} + +fn clip(text: &str, max_chars: usize) -> String { + text.chars().take(max_chars).collect() +} + +fn telemetry_dir() -> Option { + Some(auth::config_dir_path()?.join(DIR)) +} + +/// The directory, created `0700` inside a config directory `auth` has +/// created and hardened, as it would for credentials. `None` when it cannot be. +fn prepared_dir() -> Option { + auth::config_dir().ok()?; + let dir = telemetry_dir()?; + std::fs::create_dir_all(&dir).ok()?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); + } + Some(dir) +} + +fn open_private(path: &Path, append: bool) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + if append { + options.append(true).create(true); + } else { + options.write(true).create_new(true); + } + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + +/// The installation's random id, created on first use. Two first runs in +/// parallel can each create one; `create_new` makes the second read the +/// first's instead of replacing it. When nothing can be stored, the run +/// still gets an id — just not one the next run will share. +fn user_id() -> String { + let fresh = uuid_v4(rand::random()); + let Some(dir) = prepared_dir() else { + return fresh; + }; + let path = dir.join(USER_ID_FILE); + if let Some(existing) = read_user_id(&path) { + return existing; + } + match open_private(&path, false) { + Ok(mut file) => { + let _ = file.write_all(fresh.as_bytes()); + fresh + } + Err(_) => read_user_id(&path).unwrap_or(fresh), + } +} + +fn read_user_id(path: &Path) -> Option { + let text = std::fs::read_to_string(path).ok()?; + let id = text.trim(); + is_uuid(id).then(|| id.to_string()) +} + +fn is_uuid(text: &str) -> bool { + text.len() == 36 + && text.char_indices().all(|(i, c)| match i { + 8 | 13 | 18 | 23 => c == '-', + _ => c.is_ascii_hexdigit(), + }) +} + +/// The version the last run recorded, when it differs from this one — the +/// first run after an upgrade. Checked against the same shape the update +/// check trusts, since it is read back from disk. +fn previous_version() -> Option { + let dir = prepared_dir()?; + let path = dir.join(LAST_VERSION_FILE); + let last = std::fs::read_to_string(&path) + .ok() + .map(|text| text.trim().to_string()); + if last.as_deref() == Some(CURRENT) { + return None; + } + let _ = std::fs::remove_file(&path); + if let Ok(mut file) = open_private(&path, false) { + let _ = file.write_all(CURRENT.as_bytes()); + } + last.filter(|version| is_version(version)) + .map(|version| clip(&version, MAX_VERSION)) +} + +fn is_version(text: &str) -> bool { + !text.is_empty() + && text.len() <= MAX_VERSION + && text + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | '+')) +} + +fn uuid_v4(mut bytes: [u8; 16]) -> String { + bytes[6] = (bytes[6] & 0x0f) | 0x40; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + let hex: String = bytes.iter().map(|b| format!("{b:02x}")).collect(); + format!( + "{}-{}-{}-{}-{}", + &hex[0..8], + &hex[8..12], + &hex[12..16], + &hex[16..20], + &hex[20..32] + ) +} + +/// RFC 3339 in UTC, to the millisecond. +fn timestamp(at: SystemTime) -> String { + let since = at.duration_since(UNIX_EPOCH).unwrap_or_default(); + let (date, secs) = utc_date(since.as_secs()); + format!( + "{date}T{:02}:{:02}:{:02}.{:03}Z", + secs / 3600, + secs % 3600 / 60, + secs % 60, + since.subsec_millis() + ) +} + +/// `YYYY-MM-DD` and the seconds into that day. +fn utc_date(unix_secs: u64) -> (String, u64) { + let days = (unix_secs / 86_400) as i64; + let (y, m, d) = crate::account_usage::civil_from_days(days); + (format!("{y:04}-{m:02}-{d:02}"), unix_secs % 86_400) +} + +fn now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |d| d.as_secs()) +} + +/// Appends one line to today's file. A line this short goes out in one +/// `write`, which `O_APPEND` keeps whole against parallel runs. +fn append_to_file(line: &str) { + let Some(dir) = prepared_dir() else { + return; + }; + let (today, _) = utc_date(now_secs()); + let path = dir.join(format!("{today}.jsonl")); + let is_new_day = !path.exists(); + if let Ok(mut file) = open_private(&path, true) { + let _ = file.write_all(format!("{line}\n").as_bytes()); + } + if is_new_day { + prune(&dir, now_secs()); + } +} + +/// Deletes event files older than [`KEEP_DAYS`]. Only names that are +/// exactly `YYYY-MM-DD.jsonl` are considered, and only inside the telemetry +/// directory itself, so nothing else there can be matched. +fn prune(dir: &Path, now: u64) { + let (cutoff, _) = utc_date(now.saturating_sub(KEEP_DAYS as u64 * 86_400)); + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name().to_string_lossy().into_owned(); + if let Some(date) = event_file_date(&name) { + if date < cutoff.as_str() { + let _ = std::fs::remove_file(dir.join(&name)); + } + } + } +} + +fn event_file_date(name: &str) -> Option<&str> { + let date = name.strip_suffix(".jsonl")?; + let shape = date.len() == 10 + && date.char_indices().all(|(i, c)| match i { + 4 | 7 => c == '-', + _ => c.is_ascii_digit(), + }); + shape.then_some(date) +} + +/// The endpoint and token for the `api` sink. `None` without a token, which +/// a build outside the release pipeline has none of. +fn api_target() -> Option<(String, String)> { + let from_env = |name: &str| std::env::var(name).ok().filter(|v| !v.trim().is_empty()); + let token = from_env(TOKEN_ENV) + .or_else(|| option_env!("MAPBOX_CLI_TELEMETRY_TOKEN").map(String::from))?; + let url = from_env(URL_ENV) + .or_else(|| option_env!("MAPBOX_CLI_TELEMETRY_URL").map(String::from)) + .unwrap_or_else(|| PRODUCTION_URL.to_string()); + Some((url, token)) +} + +/// Starts the sender with the event on its stdin, and forgets it. Detached +/// the way `update_check` detaches its refresher, and for the same reasons. +/// The event goes on stdin rather than argv, which other users can read in +/// `ps`. +fn spawn_sender(line: &str) { + if api_target().is_none() { + return; + } + let Ok(exe) = std::env::current_exe() else { + return; + }; + let mut command = Process::new(exe); + command + .env(SENDER_ENV, "1") + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + command.process_group(0); + } + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + const DETACHED_PROCESS: u32 = 0x0000_0008; + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + command.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW); + } + + if let Ok(mut child) = command.spawn() { + // One event fits in the pipe's buffer, so this does not wait on the + // child; dropping stdin closes it, which is the child's end of input. + if let Some(mut stdin) = child.stdin.take() { + let _ = stdin.write_all(line.as_bytes()); + } + } +} + +/// Whether this process is the `api` sink's sender rather than a command. +pub fn is_sender_child() -> bool { + std::env::var_os(SENDER_ENV).is_some_and(|value| !value.is_empty()) +} + +/// The whole of the sender: read one event, POST it once, exit. Its result +/// is read by nobody, so there is nothing to report and nothing to retry. +pub fn run_sender_child() -> ExitCode { + let mut line = String::new(); + let _ = std::io::stdin().take(MAX_PAYLOAD).read_to_string(&mut line); + if line.trim().is_empty() || !enabled() { + return ExitCode::SUCCESS; + } + let Some((url, token)) = api_target() else { + return ExitCode::SUCCESS; + }; + let Ok(client) = http::bare_client(SEND_TIMEOUT) else { + return ExitCode::SUCCESS; + }; + let request = client + .post(url) + .query(&[("access_token", token)]) + .header(reqwest::header::CONTENT_TYPE, "application/json") + .body(format!("[{}]", line.trim())); + let _ = http::send(request); + ExitCode::SUCCESS +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_uuid_is_version_4_and_well_formed() { + let id = uuid_v4([0xff; 16]); + assert!(is_uuid(&id), "{id}"); + assert_eq!(&id[14..15], "4"); + assert!(matches!(&id[19..20], "8" | "9" | "a" | "b"), "{id}"); + assert!(!is_uuid("not-a-uuid")); + } + + #[test] + fn timestamps_are_utc_to_the_millisecond() { + let at = UNIX_EPOCH + Duration::from_millis(1_790_000_000_123); + assert_eq!(timestamp(at), "2026-09-21T14:13:20.123Z"); + assert_eq!(timestamp(UNIX_EPOCH), "1970-01-01T00:00:00.000Z"); + } + + #[test] + fn free_strings_send_their_length_and_codes_send_their_value() { + let values = |v: &str| vec![v.to_string()]; + let q = classify("q", &values("1600 Pennsylvania Ave"), true, false, false); + assert_eq!(q.value, None); + assert_eq!(q.length, Some(21)); + + // Digits are still a free string unless the spec types them. + let postcode = classify("postcode", &values("10001"), true, false, false); + assert_eq!((postcode.value, postcode.length), (None, Some(5))); + + let limit = classify("limit", &values("5"), true, false, true); + assert_eq!(limit.value.as_deref(), Some("5")); + + let language = classify("language", &values("en"), true, false, false); + assert_eq!(language.value.as_deref(), Some("en")); + + let flag = classify("download", &values("true"), false, false, false); + assert_eq!(flag.value.as_deref(), Some("true")); + } + + #[test] + fn coordinates_send_their_name_only() { + for name in COORDINATES { + let param = classify(name, &["12.5".to_string()], true, false, true); + assert_eq!( + param, + Param { + name: name.to_string(), + ..Param::default() + } + ); + } + } + + #[test] + fn a_data_body_sends_its_size_and_top_level_keys_only() { + let (bytes, keys) = data_shape(r#"{"name":"secret","layers":[]}"#); + assert_eq!(bytes, Some(29)); + assert_eq!(keys, Some(vec!["layers".to_string(), "name".to_string()])); + + assert_eq!(data_shape("[1,2]"), (Some(5), None)); + assert_eq!(data_shape("@-"), (None, None)); + } + + #[test] + fn a_long_value_is_clipped_to_the_schema_bound() { + let param = classify("types", &["x".repeat(500)], true, false, false); + assert_eq!(param.value.map(|v| v.len()), Some(MAX_VALUE)); + } + + #[test] + fn an_unknown_tilesets_word_is_other() { + assert_eq!(tilesets_word("upload-source"), "upload-source"); + assert_eq!(tilesets_word("my-secret-tileset"), "other"); + } + + #[test] + fn the_install_method_is_a_category_never_the_path() { + let method = |p: &str| install_method(Some(Path::new(p))); + assert_eq!( + method("/opt/homebrew/Cellar/mapbox/0.3.0/bin/mapbox"), + "homebrew" + ); + assert_eq!(method("/Users/a/.cargo/bin/mapbox"), "cargo"); + assert_eq!(method("/home/a/.local/bin/mapbox"), "install-script"); + assert_eq!( + method(r"C:\Users\a\AppData\Local\Programs\mapbox\mapbox.exe"), + "install-script" + ); + assert_eq!(method("/srv/tools/mapbox"), "other"); + assert_eq!(install_method(None), "other"); + } + + #[test] + fn only_dated_event_files_are_pruned() { + assert_eq!(event_file_date("2026-09-24.jsonl"), Some("2026-09-24")); + for name in [ + "user-id", + "last-version", + "2026-09-24.json", + "notes.jsonl", + "2026-9-24.jsonl", + ] { + assert_eq!(event_file_date(name), None, "{name}"); + } + } + + #[test] + fn prune_removes_old_event_files_and_nothing_else() { + let dir = + std::env::temp_dir().join(format!("mapbox-events-prune-{}", uuid_v4(rand::random()))); + std::fs::create_dir_all(&dir).unwrap(); + for name in [ + "2026-09-01.jsonl", + "2026-09-20.jsonl", + "user-id", + "2026-09-01.txt", + ] { + std::fs::write(dir.join(name), "x").unwrap(); + } + // 2026-09-24T00:00:00Z + prune(&dir, 1_790_208_000); + let mut left: Vec = std::fs::read_dir(&dir) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + left.sort(); + std::fs::remove_dir_all(&dir).unwrap(); + assert_eq!(left, ["2026-09-01.txt", "2026-09-20.jsonl", "user-id"]); + } + + #[test] + fn a_token_is_read_for_its_prefix_and_account_only() { + let payload = base64::Engine::encode( + &base64::engine::general_purpose::URL_SAFE_NO_PAD, + br#"{"u":"example-user","a":"x"}"#, + ); + let token = format!("sk.{payload}.signature"); + let auth = auth_field(auth::TokenSource::Login, &token); + assert_eq!( + auth, + Auth { + source: "login", + kind: "sk", + account: Some("example-user".to_string()) + } + ); + let serialized = serde_json::to_string(&auth).unwrap(); + assert!(!serialized.contains("signature"), "{serialized}"); + + assert_eq!(auth_field(auth::TokenSource::Flag, "garbage").kind, "other"); + } + + #[test] + fn command_names_come_from_the_tree_not_from_argv() { + let app = Command::new("mapbox").subcommand( + Command::new("styles") + .subcommand(Command::new("draft").subcommand(Command::new("get"))), + ); + let argv = |words: &[&str]| { + words + .iter() + .map(std::ffi::OsString::from) + .collect::>() + }; + assert_eq!( + command_from_argv( + &app, + &argv(&["mapbox", "-o", "json", "styles", "draft", "get", "my-style"]) + ), + ["styles", "draft", "get"] + ); + assert_eq!( + command_from_argv(&app, &argv(&["mapbox", "styles", "typo", "draft"])), + ["styles"] + ); + assert!(command_from_argv(&app, &argv(&["mapbox", "/secret/path"])).is_empty()); + } +} diff --git a/src/executor.rs b/src/executor.rs index 7bdadff..2ff7299 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -283,9 +283,7 @@ fn dispatch( req = attach_body(req, source)?; } - let response = req - .send() - .map_err(|e| transport_failure("Request failed", e))?; + let response = crate::http::send(req).map_err(|e| transport_failure("Request failed", e))?; let status = response.status(); // Must read headers before `bytes()` consumes the response — anything // not taken here is gone after. For a long time only `Content-Type` @@ -348,6 +346,9 @@ fn dispatch( .next_page .as_deref() .map(|next| NextPage::of(&op.query_params, next)); + if next_page.is_some() { + crate::events::record_more_pages(); + } match as_text { Some(text) => match serde_json::from_str::(&text) { @@ -1392,6 +1393,7 @@ fn write_binary(body: &[u8], content_type: &str) -> Result<()> { .into()); } + crate::events::add_stdout_bytes(body.len()); stdout .write_all(body) .and_then(|()| stdout.flush()) @@ -1882,12 +1884,13 @@ mod tests { let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("a loopback port"); let addr = listener.local_addr().expect("the bound address"); - let failure = crate::http::client() - .expect("a client") - .get(format!("http://{addr}/")) - .timeout(std::time::Duration::from_millis(250)) - .send() - .expect_err("a server that never answers cannot have answered"); + let failure = crate::http::send( + crate::http::client() + .expect("a client") + .get(format!("http://{addr}/")) + .timeout(std::time::Duration::from_millis(250)), + ) + .expect_err("a server that never answers cannot have answered"); let reported = super::transport_failure("Request failed", failure); assert_eq!(reported.code, "request_timed_out"); diff --git a/src/http.rs b/src/http.rs index 26cd087..f039a6d 100644 --- a/src/http.rs +++ b/src/http.rs @@ -20,7 +20,7 @@ use std::time::Duration; use anyhow::{Context, Result}; use clap::ArgMatches; -use crate::telemetry; +use crate::{events, executor, telemetry}; /// The flag and the variable a caller moves the budget with. pub const TIMEOUT_ARG: &str = "timeout"; @@ -232,14 +232,69 @@ pub fn client_for(command_group: Option<&str>) -> Result) -> Result { + build_with(telemetry::user_agent(command_group), timeout) +} + +/// A client whose `User-Agent` is [`telemetry::PRODUCT_TOKEN`] alone, for +/// the telemetry sender. Mapbox Events stores the `User-Agent` of the upload +/// in every record, so the markers — `agent/` among them — must not +/// ride along on it. +pub fn bare_client(timeout: Duration) -> Result { + build_with(telemetry::PRODUCT_TOKEN.to_string(), timeout) +} + +fn build_with(user_agent: String, timeout: Duration) -> Result { reqwest::blocking::Client::builder() - .user_agent(telemetry::user_agent(command_group)) + .user_agent(user_agent) .connect_timeout(CONNECT_TIMEOUT) .timeout(timeout) .build() .context("Could not start an HTTP client") } +/// Sends `request`, recording it for the run's telemetry event. +/// +/// Every request goes through here rather than `RequestBuilder::send`, +/// because a `reqwest` client has no response hook: a request sent anywhere +/// else is one `network` does not count. `only_http_sends_requests` in +/// `tests/source_guards.rs` holds that. +/// +/// The request id is kept only for a Mapbox host — it is what joins a row to +/// Mapbox's own logs, and another service's id joins to nothing we hold. +pub fn send( + request: reqwest::blocking::RequestBuilder, +) -> reqwest::Result { + let (client, request) = request.build_split(); + let request = request?; + let body_bytes = request + .body() + .and_then(|body| body.as_bytes()) + .map(|bytes| bytes.len() as u64); + let mapbox = request.url().host_str().is_some_and(is_mapbox_host); + + let started = std::time::Instant::now(); + let result = client.execute(request); + let elapsed = started.elapsed(); + + match &result { + Ok(response) => events::record_request( + Some(response.status().as_u16()), + response.content_length(), + body_bytes, + elapsed, + mapbox + .then(|| executor::request_id(response.headers())) + .flatten(), + ), + Err(_) => events::record_request(None, None, body_bytes, elapsed, None), + } + result +} + +fn is_mapbox_host(host: &str) -> bool { + host == "mapbox.com" || host.ends_with(".mapbox.com") +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/main.rs b/src/main.rs index a3a7961..1b637f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,6 +20,7 @@ mod completion; mod config; mod confirm; mod deprecation; +mod events; mod executor; mod generate_skills; mod http; @@ -645,13 +646,14 @@ fn no_stored_credentials(profile: Option<&str>) -> anyhow::Error { } /// Answers `--schema`, from either of the two places it can be noticed. -fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> ExitCode { +fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 { + events::record_parsed(app, specs, matches, "schema"); let mode = Mode::from_matches(matches); match schema::emit(mode, app, specs, matches) { - Ok(()) => ExitCode::SUCCESS, + Ok(()) => 0, Err(e) => { output::emit_error(mode, &e); - ExitCode::FAILURE + 1 } } } @@ -673,16 +675,25 @@ fn main() -> ExitCode { if update_check::is_refresh_child() { return update_check::run_refresh_child(); } + if events::is_sender_child() { + return events::run_sender_child(); + } + events::start(); let code = cli(); update_check::notify(); - code + // After the notice, which the event reports. + events::finish(Some(u32::from(code))); + ExitCode::from(code) } /// Every failure leaves through here, so that one `--output` decision covers /// results and errors alike. `run` does the work; `cli` only chooses how /// what comes back is rendered. -fn cli() -> ExitCode { +/// +/// The exit code is returned as a number rather than an `ExitCode`, which +/// cannot be read back, so `main` can report it. +fn cli() -> u8 { // Kept whole for the pre-parse fallback: `escape_passthrough_args` // rewrites the line for clap, and a failure needs to see what the caller // actually typed. @@ -694,7 +705,7 @@ fn cli() -> ExitCode { // cannot be until the specs it is parsed against exist. Err(e) => { output::emit_error(Mode::early(&raw_argv), &e); - return ExitCode::FAILURE; + return 1; } }; @@ -711,7 +722,10 @@ fn cli() -> ExitCode { // scan of argv — say whether `--schema` was really what was written. Err(e) => match schema::requested(&app, argv) { Some(matches) => return emit_schema(&app, &specs, &matches), - None => return report_parse_result(e, &raw_argv), + None => { + events::record_unparsed(&app, &raw_argv, e.kind()); + return report_parse_result(e, &raw_argv); + } }, }; @@ -722,12 +736,13 @@ fn cli() -> ExitCode { return emit_schema(&app, &specs, &matches); } + events::record_parsed(&app, &specs, &matches, "execute"); let mode = Mode::from_matches(&matches); match run(&app, &specs, &matches, mode) { - Ok(()) => ExitCode::SUCCESS, + Ok(()) => 0, Err(e) => { output::emit_error(mode, &e); - ExitCode::FAILURE + 1 } } } @@ -743,7 +758,7 @@ fn cli() -> ExitCode { /// The mode cannot come from the parse that just failed, so `Mode::early` /// reads `--output` off argv itself — an explicit choice has to survive the /// error that makes it matter most. -fn report_parse_result(err: clap::Error, raw_argv: &[std::ffi::OsString]) -> ExitCode { +fn report_parse_result(err: clap::Error, raw_argv: &[std::ffi::OsString]) -> u8 { let err = drop_subcommand_from_short_circuit_usage(err); // Clap uses 2 for a usage error and 0 for help/version; preserving that @@ -770,8 +785,12 @@ fn report_parse_result(err: clap::Error, raw_argv: &[std::ffi::OsString]) -> Exi // says nothing the message above it didn't. Text mode deserves the same // one-line-plus-suggestion treatment json already gets. if is_help || (!mode.is_json() && err.kind() != ErrorKind::MissingSubcommand) { + if !err.use_stderr() { + // Help and the version: clap writes these to stdout itself. + events::add_stdout_bytes(err.render().to_string().len()); + } let _ = err.print(); - return ExitCode::from(code); + return code; } // Clap's rendering is an error paragraph, then a blank line, then usage @@ -821,7 +840,7 @@ fn report_parse_result(err: clap::Error, raw_argv: &[std::ffi::OsString]) -> Exi }); output::emit_error(mode, &error.into()); - ExitCode::from(code) + code } /// The `tip: …` line clap's own suggester renders for an unrecognized @@ -997,6 +1016,19 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - if use_login && token.is_none() { return Err(no_stored_credentials(profile)); } + match &token { + Some(tilesets_cli::ChildToken::Flag(t)) => { + events::record_token(auth::TokenSource::Flag, t) + } + Some(tilesets_cli::ChildToken::Stored(t)) => { + events::record_token(auth::TokenSource::Login, t) + } + None => { + if let Some((_, t)) = auth::environment_token() { + events::record_token(auth::TokenSource::Environment, &t); + } + } + } if token.is_none() { // Falling through to whatever the environment holds. If that // shadows a login for a different account, say so: the @@ -1070,6 +1102,9 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - if use_login && token.is_none() { return Err(no_stored_credentials(profile)); } + if let Some(token) = &token { + events::record_resolved_token(matches, use_login, token); + } account_usage::run( usage_matches, @@ -1182,6 +1217,9 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - if use_login && token.is_none() { return Err(no_stored_credentials(profile)); } + if let Some(token) = &token { + events::record_resolved_token(matches, use_login, token); + } let username: Option = matches .get_one::("username") .cloned() diff --git a/src/output.rs b/src/output.rs index b06a953..b673046 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1483,6 +1483,7 @@ fn error_payload(e: &CliError) -> Value { /// no `state` field: the streams already separate the two cases. pub fn emit_error(mode: Mode, err: &anyhow::Error) { let cli = err.downcast_ref::(); + crate::events::record_error_code(cli.map_or(GENERIC_CODE, |e| e.code.as_str())); if mode.is_json() { let payload = match cli { @@ -1588,6 +1589,8 @@ fn adds_detail(body: &Value) -> bool { } fn write_stdout(line: &str) -> Result<()> { + // The line and its newline. + crate::events::add_stdout_bytes(line.len() + 1); let mut out = std::io::stdout().lock(); writeln!(out, "{line}")?; out.flush()?; diff --git a/src/telemetry.rs b/src/telemetry.rs index 16aade0..d00deff 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -51,6 +51,21 @@ fn arch_marker() -> String { format!("arch/{}", spelled_arch(std::env::consts::ARCH)) } +/// The CPU architecture as the event's `env.arch` spells it. +pub(crate) fn arch() -> &'static str { + spelled_arch(std::env::consts::ARCH) +} + +/// Whether this run is in CI, by the same rule as the `env/ci` marker. +pub(crate) fn in_ci() -> bool { + ci_marker().is_some() +} + +/// For `crate::events`, which may not reach for stdout itself. +pub(crate) fn stdout_is_terminal() -> bool { + std::io::stdout().is_terminal() +} + fn spelled_arch(arch: &str) -> &str { match arch { "aarch64" => "arm64", diff --git a/src/tilesets_cli.rs b/src/tilesets_cli.rs index d4e9058..998ed22 100644 --- a/src/tilesets_cli.rs +++ b/src/tilesets_cli.rs @@ -494,15 +494,38 @@ pub fn run(args: &[OsString], token: Option, debug: bool) -> Result< #[cfg(unix)] fn handoff(mut cmd: std::process::Command) -> std::io::Error { use std::os::unix::process::CommandExt; + // Now or never: after `exec` this process is the child, and `main` never + // gets to send the event, so it goes without an exit code. Only when the + // binary resolves — a missing `tilesets` is a failure `main` reports, + // and the event should carry it. + if resolves(std::path::Path::new(cmd.get_program())) { + crate::events::finish(None); + } cmd.exec() } +/// Whether `exec` would find `program`: as given if it has a directory in +/// it, otherwise on `PATH`. +#[cfg(unix)] +fn resolves(program: &std::path::Path) -> bool { + if program.components().count() > 1 { + return program.is_file(); + } + std::env::var_os("PATH") + .is_some_and(|path| std::env::split_paths(&path).any(|dir| dir.join(program).is_file())) +} + #[cfg(not(unix))] fn handoff(mut cmd: std::process::Command) -> std::io::Error { match cmd.status() { // 130 is the conventional "killed by SIGINT" code; on Windows a // `None` code means the child was terminated rather than exiting. - Ok(status) => std::process::exit(status.code().unwrap_or(130)), + Ok(status) => { + let code = status.code().unwrap_or(130); + // `exit` skips `main`'s way out, where the event is sent. + crate::events::finish(u32::try_from(code).ok()); + std::process::exit(code) + } Err(err) => err, } } diff --git a/src/update_check.rs b/src/update_check.rs index a515d4f..d902f48 100644 --- a/src/update_check.rs +++ b/src/update_check.rs @@ -392,12 +392,7 @@ pub fn run_refresh_child() -> ExitCode { /// The channel manifest documents the shape; `version` is the only field /// this reads, and it carries no leading `v`. fn fetch_latest(url: &str) -> Option { - let response = http::client() - .ok()? - .get(url) - .timeout(FETCH_TIMEOUT) - .send() - .ok()?; + let response = http::send(http::client().ok()?.get(url).timeout(FETCH_TIMEOUT)).ok()?; if !response.status().is_success() { return None; } @@ -432,6 +427,7 @@ pub fn notify() { // it had, rather than racing a child that may finish first. if let Some(latest) = should_notify(cache.as_ref(), CURRENT, now) { output::progress(¬ice(latest, CURRENT, cfg!(windows))); + crate::events::record_update_notice(latest); let mut updated = cache.clone().unwrap_or_default(); updated.notified_at = now; write_cache(&updated); diff --git a/tests/completion.rs b/tests/completion.rs index 2c5dc1f..73a4d80 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -354,11 +354,21 @@ fn a_missing_or_unknown_shell_is_a_usage_error() { /// refresh round-trip would be unusable. #[test] fn it_needs_no_token_and_touches_no_credentials() { - let home = sandbox_home(); + // A home of its own: every other test here shares `sandbox_home`, and + // any of them — a usage error, `--schema` — writes a telemetry event + // into its `.mapbox` while this one is looking. + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("completion-home-untouched"); let config = home.join(".mapbox"); let _ = std::fs::remove_dir_all(&config); + std::fs::create_dir_all(&home).expect("create the home"); - let out = run(&["completion", "zsh"]); + let out = command() + .env("HOME", &home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", &config) + .args(["completion", "zsh"]) + .output() + .expect("run mapbox"); assert!(out.status.success(), "{}", stderr(&out)); assert!( !config.exists(), diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index bdb02f6..207531d 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -165,7 +165,8 @@ fn the_refusal_carries_a_code_and_a_fix() { } /// The check runs before `config_dir`, which creates the store as a side -/// effect. A CI job that tried to log in should leave nothing behind. +/// effect. A CI job that tried to log in should leave nothing behind but +/// the run's telemetry event, which every command writes. #[test] fn the_refusal_creates_no_credential_directory() { let home = scratch("no-dir"); @@ -175,9 +176,21 @@ fn the_refusal_creates_no_credential_directory() { .expect("run mapbox"); assert!(!out.status.success()); + let left: Vec = std::fs::read_dir(home.join(".mapbox")) + .map(|entries| { + entries + .map(|e| { + e.expect("an entry") + .file_name() + .to_string_lossy() + .into_owned() + }) + .collect() + }) + .unwrap_or_default(); assert!( - !home.join(".mapbox").exists(), - "the refusal created the credential store anyway" + left.iter().all(|name| name == ".telemetry"), + "the refusal created the credential store anyway: {left:?}" ); } diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 0be47ea..5484d38 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -43,6 +43,9 @@ fn sources() -> Vec<(String, String)> { /// - `agent_skills` — the staging directory it renames skills out of, and the /// skill directory `install --force` replaces. /// - `auth` — `logout`, and the scratch file `write_private` renames from. +/// - `events` — its own dated event files past the retention window, matched +/// by exact `YYYY-MM-DD.jsonl` names inside `~/.mapbox/.telemetry`, and the +/// `last-version` file it rewrites. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. @@ -50,6 +53,7 @@ fn sources() -> Vec<(String, String)> { const MAY_DELETE: &[&str] = &[ "agent_skills.rs", "auth.rs", + "events.rs", "executor.rs", "generate_skills.rs", "skill_dest.rs", @@ -171,6 +175,30 @@ fn only_output_completion_and_binary_responses_write_to_stdout() { ); } +/// Every request goes through `http::send`, which is what records it for the +/// run's telemetry event. +/// +/// A `reqwest` client has no response hook, so a request sent with +/// `RequestBuilder::send` anywhere else is one the event's `network` never +/// counts — silently, since nothing fails. `http.rs` is exempt: it is where +/// `send` is defined, and its own tests call the builder directly. +#[test] +fn only_http_sends_requests() { + let unexpected: Vec = sources() + .into_iter() + .filter(|(name, source)| name != "http.rs" && source.contains(".send()")) + .map(|(name, _)| format!("src/{name}")) + .collect(); + + assert!( + unexpected.is_empty(), + "these modules call `.send()` directly:\n {}\n\n\ + Wrap the request builder in `http::send(...)` instead, so the request is \ + counted in the telemetry event.", + unexpected.join("\n ") + ); +} + /// Modules that turn a Mapbox API failure into a `CliError::http`, and so /// must carry the response's `X-Request-Id` into it. const CARRIES_A_REQUEST_ID: &[&str] = &["account_usage.rs", "auth.rs", "executor.rs"]; diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs new file mode 100644 index 0000000..5ecffe8 --- /dev/null +++ b/tests/telemetry_events.rs @@ -0,0 +1,337 @@ +//! End-to-end tests for the run's `cli.command` event. +//! +//! The unit tests in `src/events.rs` cover the pure parts — how an argument +//! is classified, what a timestamp looks like, which files pruning may +//! touch. What they cannot show is what a real run leaves behind: that the +//! event lands where it should, carries what the command did and nothing the +//! user typed, disappears when telemetry is off, and never changes stdout. + +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::time::{Duration, Instant}; + +use serde_json::Value; + +/// A token whose payload claims the account `example-user`. The signature +/// is the part that must never appear in an event. +const TOKEN: &str = "pk.eyJ1IjoiZXhhbXBsZS11c2VyIiwiYSI6IngifQ.SIGNATURE-NOT-FOR-EVENTS"; +const ADDRESS: &str = "1600 Pennsylvania Ave"; + +fn scratch(name: &str) -> PathBuf { + let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("events-{name}")); + let _ = std::fs::remove_dir_all(&home); + std::fs::create_dir_all(&home).expect("create the scratch home"); + home +} + +fn config_dir(home: &Path) -> PathBuf { + home.join(".mapbox") +} + +fn command(home: &Path) -> Command { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_mapbox")); + cmd.env_remove("MAPBOX_ACCESS_TOKEN") + .env_remove("MapboxAccessToken") + .env_remove("MAPBOX_USERNAME") + .env_remove("MAPBOX_OUTPUT") + .env_remove("MAPBOX_CLI_NO_TELEMETRY") + .env_remove("MAPBOX_CLI_TELEMETRY_SINK") + .env_remove("MAPBOX_CLI_TELEMETRY_DEBUG") + .env("MAPBOX_NO_UPDATE_CHECK", "1") + .env("HOME", home) + .env("XDG_CONFIG_HOME", home.join(".config")) + .env("MAPBOX_CONFIG_DIR", config_dir(home)); + cmd +} + +fn run(home: &Path, args: &[&str]) -> Output { + command(home).args(args).output().expect("run mapbox") +} + +/// Every event written under `home`, oldest first. +fn events(home: &Path) -> Vec { + let dir = config_dir(home).join(".telemetry"); + let Ok(entries) = std::fs::read_dir(&dir) else { + return vec![]; + }; + let mut files: Vec = entries + .map(|e| e.expect("an entry").path()) + .filter(|p| p.extension().is_some_and(|ext| ext == "jsonl")) + .collect(); + files.sort(); + files + .iter() + .flat_map(|f| { + std::fs::read_to_string(f) + .expect("read an event file") + .lines() + .map(|l| serde_json::from_str(l).expect("an event line is JSON")) + .collect::>() + }) + .collect() +} + +#[test] +fn a_run_writes_one_event_with_what_it_did() { + let home = scratch("one"); + let data = r#"{"name":"secret-style-name","layers":[]}"#; + let out = run( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + data, + "--dry-run", + "-t", + TOKEN, + ], + ); + assert!( + out.status.success(), + "{}", + String::from_utf8_lossy(&out.stderr) + ); + + let events = events(&home); + assert_eq!(events.len(), 1, "{events:?}"); + let event = &events[0]; + assert_eq!(event["event"], "cli.command"); + assert_eq!(event["sdkIdentifier"], "mapbox-cli"); + assert_eq!(event["command"], serde_json::json!(["styles", "create"])); + assert_eq!(event["invocation"], "execute"); + assert_eq!(event["exitCode"], 0); + assert_eq!(event["dryRun"], true); + assert_eq!(event["auth"]["source"], "flag"); + assert_eq!(event["auth"]["type"], "pk"); + assert_eq!(event["auth"]["account"], "example-user"); + assert_eq!( + event["stdoutBytes"].as_u64(), + Some(out.stdout.len() as u64), + "stdoutBytes should be what was written" + ); + let params = event["params"].as_array().expect("params"); + assert!(params.contains(&serde_json::json!({ + "name": "data", "bytes": data.len(), "keys": ["layers", "name"] + }))); + assert!(params.contains(&serde_json::json!({ "name": "username", "length": 7 }))); +} + +#[test] +fn nothing_the_user_typed_reaches_the_event() { + let home = scratch("private"); + // No network needed: a usage error still records, and `--dry-run` sends + // nothing. + let _ = run( + &home, + &[ + "styles", + "create", + "--username", + "someone", + "--data", + "{\"x\":1}", + "--dry-run", + "-t", + TOKEN, + ], + ); + let _ = run( + &home, + &[ + "geocoder", + "forward", + "--q", + ADDRESS, + "--no-such-flag", + "-t", + TOKEN, + ], + ); + let _ = run(&home, &["/Users/someone/secret/path"]); + + let written = std::fs::read_dir(config_dir(&home).join(".telemetry")) + .expect("the telemetry directory") + .map(|e| std::fs::read_to_string(e.expect("an entry").path()).unwrap_or_default()) + .collect::(); + for secret in [ + "SIGNATURE-NOT-FOR-EVENTS", + ADDRESS, + "someone", + "secret/path", + ] { + assert!( + !written.contains(secret), + "`{secret}` reached the event files:\n{written}" + ); + } + assert_eq!(events(&home).len(), 3); +} + +#[test] +fn help_version_and_usage_errors_record_their_invocation() { + let home = scratch("invocation"); + let _ = run(&home, &["--version"]); + let _ = run(&home, &["styles", "--help"]); + let _ = run(&home, &["styles", "list", "--schema"]); + let _ = run(&home, &["nosuchcommand"]); + + let events = events(&home); + let seen: Vec<(&str, &Value)> = events + .iter() + .map(|e| (e["invocation"].as_str().unwrap_or(""), &e["command"])) + .collect(); + assert_eq!( + seen, + [ + ("version", &Value::Null), + ("help", &serde_json::json!(["styles"])), + ("schema", &serde_json::json!(["styles", "list"])), + ("execute", &Value::Null), + ] + ); + assert_eq!(events[3]["usageError"], "InvalidSubcommand"); + assert_eq!(events[3]["errorCode"], "usage"); + assert_eq!(events[3]["exitCode"], 2); +} + +#[test] +fn the_opt_out_records_nothing() { + let home = scratch("opt-out-env"); + let out = command(&home) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "MAPBOX_CLI_NO_TELEMETRY=1 still wrote telemetry" + ); +} + +#[test] +fn stdout_is_identical_with_telemetry_on_and_off() { + let on = run(&scratch("stdout-on"), &["-o", "json", "config", "list"]); + let off = command(&scratch("stdout-off")) + .env("MAPBOX_CLI_NO_TELEMETRY", "1") + .args(["-o", "json", "config", "list"]) + .output() + .expect("run mapbox"); + assert_eq!(on.stdout, off.stdout); + assert_eq!(on.stderr, off.stderr); +} + +#[test] +fn completion_records_nothing() { + let home = scratch("completion"); + assert!(run(&home, &["completion", "zsh"]).status.success()); + assert!( + !config_dir(&home).exists(), + "`completion` created {}", + config_dir(&home).display() + ); +} + +/// The `api` sink, against a loopback server standing in for Mapbox Events: +/// one POST, the event wrapped in an array, the token in the query and the +/// bare `User-Agent` — no `agent/` marker, even when an agent is detected. +#[test] +fn the_api_sink_posts_one_event_with_a_bare_user_agent() { + let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); + let addr = listener.local_addr().expect("the bound address"); + listener.set_nonblocking(true).expect("nonblocking"); + + let home = scratch("api"); + let out = command(&home) + .env("MAPBOX_CLI_TELEMETRY_SINK", "api") + .env( + "MAPBOX_INTERNAL_TELEMETRY_URL", + format!("http://{addr}/events/v2"), + ) + .env("MAPBOX_INTERNAL_TELEMETRY_TOKEN", "pk.upload-token") + .env("CLAUDECODE", "1") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + + // The sender is detached and outlives the command, so wait for it. + let deadline = Instant::now() + Duration::from_secs(10); + let mut stream = loop { + match listener.accept() { + Ok((stream, _)) => break stream, + Err(_) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(50)), + Err(e) => panic!("the sender never connected: {e}"), + } + }; + stream.set_nonblocking(false).expect("blocking"); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .expect("a read timeout"); + + let mut request = Vec::new(); + let mut buf = [0u8; 4096]; + loop { + let n = stream.read(&mut buf).unwrap_or(0); + request.extend_from_slice(&buf[..n]); + let text = String::from_utf8_lossy(&request); + if let Some((head, body)) = text.split_once("\r\n\r\n") { + let length = head + .lines() + .find_map(|l| { + l.to_ascii_lowercase() + .strip_prefix("content-length:") + .map(|v| v.trim().parse::().unwrap_or(0)) + }) + .unwrap_or(0); + if body.len() >= length { + break; + } + } + if n == 0 { + break; + } + } + let _ = stream.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n"); + + let text = String::from_utf8_lossy(&request).into_owned(); + let (head, body) = text + .split_once("\r\n\r\n") + .expect("a request head and body"); + let first_line = head.lines().next().unwrap_or_default(); + assert!( + first_line.starts_with("POST /events/v2?access_token=pk.upload-token "), + "{first_line}" + ); + let user_agent = head + .lines() + .find_map(|l| { + l.to_ascii_lowercase() + .strip_prefix("user-agent:") + .map(|v| v.trim().to_string()) + }) + .expect("a User-Agent"); + assert_eq!( + user_agent, + format!("mapbox-cli/{}", env!("CARGO_PKG_VERSION")) + ); + + let sent: Value = serde_json::from_str(body).expect("the body is JSON"); + let sent = sent.as_array().expect("an array of events"); + assert_eq!(sent.len(), 1); + assert_eq!(sent[0]["command"], serde_json::json!(["config", "list"])); + assert_eq!(sent[0]["env"]["agent"], "claude-code"); + assert!( + !config_dir(&home) + .join(".telemetry") + .read_dir() + .is_ok_and(|mut d| d + .any(|e| e.is_ok_and(|e| e.path().extension().is_some_and(|x| x == "jsonl")))), + "the api sink also wrote to the file sink" + ); +} From ea6641a38e92cdfbf6a0c1bf3a2498a78ffd5534 Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Thu, 24 Sep 2026 15:36:02 +0300 Subject: [PATCH 2/7] Name event functions by what they do: set_* overwrites, add_* accumulates --- src/auth.rs | 22 +++++++++++----------- src/events.rs | 27 ++++++++++++++------------- src/executor.rs | 2 +- src/http.rs | 4 ++-- src/main.rs | 16 ++++++++-------- src/output.rs | 2 +- src/update_check.rs | 2 +- 7 files changed, 38 insertions(+), 37 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 820ca51..2e8b74f 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -689,11 +689,11 @@ pub fn force_refresh(debug: bool, profile: Option<&str>, mode: Mode) -> Result<( let mut creds = load_credentials(profile) .ok_or_else(|| anyhow!("Not currently logged in. Run `mapbox auth login` first."))?; - crate::events::record_auth_step("refresh"); + crate::events::set_auth_step("refresh"); refresh_credentials(&mut creds, debug)?; - crate::events::record_auth_step("save_credentials"); + crate::events::set_auth_step("save_credentials"); save_credentials(&creds, profile)?; - crate::events::record_auth_step("done"); + crate::events::set_auth_step("done"); let expires_at = token_expires_at(&creds.access_token); let text = match expires_at { @@ -896,10 +896,10 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> { let path = credentials_path(profile)?; let had_credentials = path.exists(); if had_credentials { - crate::events::record_auth_step("remove_credentials"); + crate::events::set_auth_step("remove_credentials"); std::fs::remove_file(&path)?; } - crate::events::record_auth_step("done"); + crate::events::set_auth_step("done"); let text = if had_credentials { "Logged out successfully." @@ -1896,7 +1896,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { // Computed once: register_client's ceiling and the authorize scope must agree. let scopes = default_scopes(); - crate::events::record_auth_step("register_client"); + crate::events::set_auth_step("register_client"); output::progress("Registering OAuth client with Mapbox..."); let registration = register_client(&redirect_uri, debug, scopes)?; @@ -1924,7 +1924,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { // Not discarded: with stderr redirected this is the only thing carrying // the run, so its failure is the difference between refusing now and // stalling for five minutes. See `login_can_be_completed`. - crate::events::record_auth_step("open_browser"); + crate::events::set_auth_step("open_browser"); let browser_opened = open::that(&auth_url).is_ok(); if !login_can_be_completed(std::io::stderr().is_terminal(), browser_opened) { return Err(login_has_no_way_to_show_the_url()); @@ -1933,10 +1933,10 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { output::progress(&format!( "Waiting for authorization (listening on port {port})..." )); - crate::events::record_auth_step("wait_for_callback"); + crate::events::set_auth_step("wait_for_callback"); let code = wait_for_callback(port, &state, CALLBACK_TIMEOUT)?; - crate::events::record_auth_step("exchange_code"); + crate::events::set_auth_step("exchange_code"); output::progress("Exchanging authorization code for access token..."); let mut creds = exchange_code_for_token( &code, @@ -1948,9 +1948,9 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { )?; creds.client_id = Some(registration.client_id.clone()); - crate::events::record_auth_step("save_credentials"); + crate::events::set_auth_step("save_credentials"); save_credentials(&creds, profile)?; - crate::events::record_auth_step("done"); + crate::events::set_auth_step("done"); let profile_note = match profile { Some(name) if name != "default" => format!(" (profile: {name})"), diff --git a/src/events.rs b/src/events.rs index 0b25c03..f639f73 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1,8 +1,9 @@ //! One `cli.command` event per run: what ran and how it ended. //! //! Modules report what they know as it happens — the parse, the token, each -//! request, the bytes on stdout, an error code — through the `record_*` -//! functions here, and `main` calls [`finish`] once on the way out. The +//! request, the bytes on stdout, an error code — and `main` calls [`finish`] +//! once on the way out; nothing is written before then. `set_*` functions +//! overwrite a field, last call wins; `add_*` functions accumulate. The //! event is then handed to a sink: a JSON line under //! `~/.mapbox/.telemetry/` by default, or Mapbox Events through a detached //! child with `MAPBOX_CLI_TELEMETRY_SINK=api`. @@ -340,7 +341,7 @@ pub fn start() { } /// The command line clap parsed. `invocation` is `execute` or `schema`. -pub fn record_parsed( +pub fn set_parsed( app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, @@ -371,7 +372,7 @@ pub fn record_parsed( /// A command line clap refused, or answered with help or the version. /// Command names are recovered by walking the tree with argv's words, so /// only names the tree already has can come out. -pub fn record_unparsed(app: &Command, argv: &[std::ffi::OsString], kind: clap::error::ErrorKind) { +pub fn set_unparsed(app: &Command, argv: &[std::ffi::OsString], kind: clap::error::ErrorKind) { use clap::error::ErrorKind; with_run(|run| { let path = command_from_argv(app, argv); @@ -391,13 +392,13 @@ pub fn record_unparsed(app: &Command, argv: &[std::ffi::OsString], kind: clap::e /// The token a command resolved, for `auth`. Read for its prefix and its /// `u` claim; the token itself is not kept. -pub fn record_token(source: auth::TokenSource, token: &str) { +pub fn set_token(source: auth::TokenSource, token: &str) { with_run(|run| run.auth = Some(auth_field(source, token))); } -/// [`record_token`] for the service arms' resolution: a typed `--token`, +/// [`set_token`] for the service arms' resolution: a typed `--token`, /// then the environment unless `--use-login`, then the stored login. -pub fn record_resolved_token(matches: &ArgMatches, use_login: bool, token: &str) { +pub fn set_resolved_token(matches: &ArgMatches, use_login: bool, token: &str) { let source = if auth::typed_token(matches).is_some() { auth::TokenSource::Flag } else if !use_login && matches.get_one::("token").is_some() { @@ -405,18 +406,18 @@ pub fn record_resolved_token(matches: &ArgMatches, use_login: bool, token: &str) } else { auth::TokenSource::Login }; - record_token(source, token); + set_token(source, token); } -pub fn record_error_code(code: &str) { +pub fn set_error_code(code: &str) { with_run(|run| run.error_code = Some(clip(code, MAX_CODE))); } -pub fn record_auth_step(step: &'static str) { +pub fn set_auth_step(step: &'static str) { with_run(|run| run.auth_step = Some(step)); } -pub fn record_update_notice(version: &str) { +pub fn set_update_notice(version: &str) { with_run(|run| run.update_notice = Some(clip(version, MAX_VERSION))); } @@ -424,13 +425,13 @@ pub fn add_stdout_bytes(bytes: usize) { with_run(|run| run.stdout_bytes = run.stdout_bytes.saturating_add(bytes as u64)); } -pub fn record_more_pages() { +pub fn set_more_pages() { with_run(|run| run.network.get_or_insert_with(Network::default).more_pages = true); } /// One request, from [`http::send`]. `status` is `None` when no response /// came back; `request_id` is passed only for a Mapbox response. -pub fn record_request( +pub fn add_request( status: Option, response_bytes: Option, request_body_bytes: Option, diff --git a/src/executor.rs b/src/executor.rs index 2ff7299..d5f0876 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -347,7 +347,7 @@ fn dispatch( .as_deref() .map(|next| NextPage::of(&op.query_params, next)); if next_page.is_some() { - crate::events::record_more_pages(); + crate::events::set_more_pages(); } match as_text { diff --git a/src/http.rs b/src/http.rs index f039a6d..62f4c3f 100644 --- a/src/http.rs +++ b/src/http.rs @@ -277,7 +277,7 @@ pub fn send( let elapsed = started.elapsed(); match &result { - Ok(response) => events::record_request( + Ok(response) => events::add_request( Some(response.status().as_u16()), response.content_length(), body_bytes, @@ -286,7 +286,7 @@ pub fn send( .then(|| executor::request_id(response.headers())) .flatten(), ), - Err(_) => events::record_request(None, None, body_bytes, elapsed, None), + Err(_) => events::add_request(None, None, body_bytes, elapsed, None), } result } diff --git a/src/main.rs b/src/main.rs index 1b637f8..6861d22 100644 --- a/src/main.rs +++ b/src/main.rs @@ -647,7 +647,7 @@ fn no_stored_credentials(profile: Option<&str>) -> anyhow::Error { /// Answers `--schema`, from either of the two places it can be noticed. fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 { - events::record_parsed(app, specs, matches, "schema"); + events::set_parsed(app, specs, matches, "schema"); let mode = Mode::from_matches(matches); match schema::emit(mode, app, specs, matches) { Ok(()) => 0, @@ -723,7 +723,7 @@ fn cli() -> u8 { Err(e) => match schema::requested(&app, argv) { Some(matches) => return emit_schema(&app, &specs, &matches), None => { - events::record_unparsed(&app, &raw_argv, e.kind()); + events::set_unparsed(&app, &raw_argv, e.kind()); return report_parse_result(e, &raw_argv); } }, @@ -736,7 +736,7 @@ fn cli() -> u8 { return emit_schema(&app, &specs, &matches); } - events::record_parsed(&app, &specs, &matches, "execute"); + events::set_parsed(&app, &specs, &matches, "execute"); let mode = Mode::from_matches(&matches); match run(&app, &specs, &matches, mode) { Ok(()) => 0, @@ -1018,14 +1018,14 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - } match &token { Some(tilesets_cli::ChildToken::Flag(t)) => { - events::record_token(auth::TokenSource::Flag, t) + events::set_token(auth::TokenSource::Flag, t) } Some(tilesets_cli::ChildToken::Stored(t)) => { - events::record_token(auth::TokenSource::Login, t) + events::set_token(auth::TokenSource::Login, t) } None => { if let Some((_, t)) = auth::environment_token() { - events::record_token(auth::TokenSource::Environment, &t); + events::set_token(auth::TokenSource::Environment, &t); } } } @@ -1103,7 +1103,7 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - return Err(no_stored_credentials(profile)); } if let Some(token) = &token { - events::record_resolved_token(matches, use_login, token); + events::set_resolved_token(matches, use_login, token); } account_usage::run( @@ -1218,7 +1218,7 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - return Err(no_stored_credentials(profile)); } if let Some(token) = &token { - events::record_resolved_token(matches, use_login, token); + events::set_resolved_token(matches, use_login, token); } let username: Option = matches .get_one::("username") diff --git a/src/output.rs b/src/output.rs index b673046..638e0c1 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1483,7 +1483,7 @@ fn error_payload(e: &CliError) -> Value { /// no `state` field: the streams already separate the two cases. pub fn emit_error(mode: Mode, err: &anyhow::Error) { let cli = err.downcast_ref::(); - crate::events::record_error_code(cli.map_or(GENERIC_CODE, |e| e.code.as_str())); + crate::events::set_error_code(cli.map_or(GENERIC_CODE, |e| e.code.as_str())); if mode.is_json() { let payload = match cli { diff --git a/src/update_check.rs b/src/update_check.rs index d902f48..290c0ab 100644 --- a/src/update_check.rs +++ b/src/update_check.rs @@ -427,7 +427,7 @@ pub fn notify() { // it had, rather than racing a child that may finish first. if let Some(latest) = should_notify(cache.as_ref(), CURRENT, now) { output::progress(¬ice(latest, CURRENT, cfg!(windows))); - crate::events::record_update_notice(latest); + crate::events::set_update_notice(latest); let mut updated = cache.clone().unwrap_or_default(); updated.notified_at = now; write_cache(&updated); From 7be0bad140ec4081297924d39c097d955c2c0114 Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Thu, 24 Sep 2026 17:08:39 +0300 Subject: [PATCH 3/7] Rename events.rs to telemetry_event.rs telemetry.rs is the User-Agent markers; the event is a separate module, and its name should say it is telemetry rather than a generic "events". --- src/account_usage.rs | 2 +- src/auth.rs | 22 ++++++++++----------- src/completion.rs | 2 +- src/executor.rs | 4 ++-- src/http.rs | 6 +++--- src/main.rs | 28 +++++++++++++-------------- src/output.rs | 4 ++-- src/telemetry.rs | 2 +- src/{events.rs => telemetry_event.rs} | 0 src/tilesets_cli.rs | 4 ++-- src/update_check.rs | 2 +- tests/source_guards.rs | 4 ++-- tests/telemetry_events.rs | 2 +- 13 files changed, 41 insertions(+), 41 deletions(-) rename src/{events.rs => telemetry_event.rs} (100%) diff --git a/src/account_usage.rs b/src/account_usage.rs index f7157d4..862516b 100644 --- a/src/account_usage.rs +++ b/src/account_usage.rs @@ -507,7 +507,7 @@ fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { era * 146097 + doe - 719468 } -/// The inverse of [`days_from_civil`]. Also [`crate::events`]'s calendar. +/// The inverse of [`days_from_civil`]. Also [`crate::telemetry_event`]'s calendar. pub(crate) fn civil_from_days(z: i64) -> (i64, u32, u32) { let z = z + 719468; let era = z.div_euclid(146097); diff --git a/src/auth.rs b/src/auth.rs index 2e8b74f..b515ea2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -689,11 +689,11 @@ pub fn force_refresh(debug: bool, profile: Option<&str>, mode: Mode) -> Result<( let mut creds = load_credentials(profile) .ok_or_else(|| anyhow!("Not currently logged in. Run `mapbox auth login` first."))?; - crate::events::set_auth_step("refresh"); + crate::telemetry_event::set_auth_step("refresh"); refresh_credentials(&mut creds, debug)?; - crate::events::set_auth_step("save_credentials"); + crate::telemetry_event::set_auth_step("save_credentials"); save_credentials(&creds, profile)?; - crate::events::set_auth_step("done"); + crate::telemetry_event::set_auth_step("done"); let expires_at = token_expires_at(&creds.access_token); let text = match expires_at { @@ -896,10 +896,10 @@ pub fn logout(profile: Option<&str>, mode: Mode) -> Result<()> { let path = credentials_path(profile)?; let had_credentials = path.exists(); if had_credentials { - crate::events::set_auth_step("remove_credentials"); + crate::telemetry_event::set_auth_step("remove_credentials"); std::fs::remove_file(&path)?; } - crate::events::set_auth_step("done"); + crate::telemetry_event::set_auth_step("done"); let text = if had_credentials { "Logged out successfully." @@ -1896,7 +1896,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { // Computed once: register_client's ceiling and the authorize scope must agree. let scopes = default_scopes(); - crate::events::set_auth_step("register_client"); + crate::telemetry_event::set_auth_step("register_client"); output::progress("Registering OAuth client with Mapbox..."); let registration = register_client(&redirect_uri, debug, scopes)?; @@ -1924,7 +1924,7 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { // Not discarded: with stderr redirected this is the only thing carrying // the run, so its failure is the difference between refusing now and // stalling for five minutes. See `login_can_be_completed`. - crate::events::set_auth_step("open_browser"); + crate::telemetry_event::set_auth_step("open_browser"); let browser_opened = open::that(&auth_url).is_ok(); if !login_can_be_completed(std::io::stderr().is_terminal(), browser_opened) { return Err(login_has_no_way_to_show_the_url()); @@ -1933,10 +1933,10 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { output::progress(&format!( "Waiting for authorization (listening on port {port})..." )); - crate::events::set_auth_step("wait_for_callback"); + crate::telemetry_event::set_auth_step("wait_for_callback"); let code = wait_for_callback(port, &state, CALLBACK_TIMEOUT)?; - crate::events::set_auth_step("exchange_code"); + crate::telemetry_event::set_auth_step("exchange_code"); output::progress("Exchanging authorization code for access token..."); let mut creds = exchange_code_for_token( &code, @@ -1948,9 +1948,9 @@ pub fn login(debug: bool, profile: Option<&str>, mode: Mode) -> Result<()> { )?; creds.client_id = Some(registration.client_id.clone()); - crate::events::set_auth_step("save_credentials"); + crate::telemetry_event::set_auth_step("save_credentials"); save_credentials(&creds, profile)?; - crate::events::set_auth_step("done"); + crate::telemetry_event::set_auth_step("done"); let profile_note = match profile { Some(name) if name != "default" => format!(" (profile: {name})"), diff --git a/src/completion.rs b/src/completion.rs index 167d4f4..a917329 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -153,7 +153,7 @@ pub fn run(app: &Command, matches: &ArgMatches) -> Result<()> { // Straight to stdout rather than through `output::emit`: the script is // the result, and there is no rendering of it that is not itself. - crate::events::add_stdout_bytes(script.len()); + crate::telemetry_event::add_stdout_bytes(script.len()); let mut out = io::stdout().lock(); match out.write_all(&script).and_then(|()| out.flush()) { // A reader that stopped reading is `head`'s ordinary behavior, not a diff --git a/src/executor.rs b/src/executor.rs index d5f0876..1e74a25 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -347,7 +347,7 @@ fn dispatch( .as_deref() .map(|next| NextPage::of(&op.query_params, next)); if next_page.is_some() { - crate::events::set_more_pages(); + crate::telemetry_event::set_more_pages(); } match as_text { @@ -1393,7 +1393,7 @@ fn write_binary(body: &[u8], content_type: &str) -> Result<()> { .into()); } - crate::events::add_stdout_bytes(body.len()); + crate::telemetry_event::add_stdout_bytes(body.len()); stdout .write_all(body) .and_then(|()| stdout.flush()) diff --git a/src/http.rs b/src/http.rs index 62f4c3f..9e307f5 100644 --- a/src/http.rs +++ b/src/http.rs @@ -20,7 +20,7 @@ use std::time::Duration; use anyhow::{Context, Result}; use clap::ArgMatches; -use crate::{events, executor, telemetry}; +use crate::{executor, telemetry, telemetry_event}; /// The flag and the variable a caller moves the budget with. pub const TIMEOUT_ARG: &str = "timeout"; @@ -277,7 +277,7 @@ pub fn send( let elapsed = started.elapsed(); match &result { - Ok(response) => events::add_request( + Ok(response) => telemetry_event::add_request( Some(response.status().as_u16()), response.content_length(), body_bytes, @@ -286,7 +286,7 @@ pub fn send( .then(|| executor::request_id(response.headers())) .flatten(), ), - Err(_) => events::add_request(None, None, body_bytes, elapsed, None), + Err(_) => telemetry_event::add_request(None, None, body_bytes, elapsed, None), } result } diff --git a/src/main.rs b/src/main.rs index 6861d22..b7ce33e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,7 +20,6 @@ mod completion; mod config; mod confirm; mod deprecation; -mod events; mod executor; mod generate_skills; mod http; @@ -31,6 +30,7 @@ mod schema; mod skill_dest; mod spec; mod telemetry; +mod telemetry_event; mod tilesets_cli; mod uninstall; mod update_check; @@ -647,7 +647,7 @@ fn no_stored_credentials(profile: Option<&str>) -> anyhow::Error { /// Answers `--schema`, from either of the two places it can be noticed. fn emit_schema(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches) -> u8 { - events::set_parsed(app, specs, matches, "schema"); + telemetry_event::set_parsed(app, specs, matches, "schema"); let mode = Mode::from_matches(matches); match schema::emit(mode, app, specs, matches) { Ok(()) => 0, @@ -675,15 +675,15 @@ fn main() -> ExitCode { if update_check::is_refresh_child() { return update_check::run_refresh_child(); } - if events::is_sender_child() { - return events::run_sender_child(); + if telemetry_event::is_sender_child() { + return telemetry_event::run_sender_child(); } - events::start(); + telemetry_event::start(); let code = cli(); update_check::notify(); // After the notice, which the event reports. - events::finish(Some(u32::from(code))); + telemetry_event::finish(Some(u32::from(code))); ExitCode::from(code) } @@ -723,7 +723,7 @@ fn cli() -> u8 { Err(e) => match schema::requested(&app, argv) { Some(matches) => return emit_schema(&app, &specs, &matches), None => { - events::set_unparsed(&app, &raw_argv, e.kind()); + telemetry_event::set_unparsed(&app, &raw_argv, e.kind()); return report_parse_result(e, &raw_argv); } }, @@ -736,7 +736,7 @@ fn cli() -> u8 { return emit_schema(&app, &specs, &matches); } - events::set_parsed(&app, &specs, &matches, "execute"); + telemetry_event::set_parsed(&app, &specs, &matches, "execute"); let mode = Mode::from_matches(&matches); match run(&app, &specs, &matches, mode) { Ok(()) => 0, @@ -787,7 +787,7 @@ fn report_parse_result(err: clap::Error, raw_argv: &[std::ffi::OsString]) -> u8 if is_help || (!mode.is_json() && err.kind() != ErrorKind::MissingSubcommand) { if !err.use_stderr() { // Help and the version: clap writes these to stdout itself. - events::add_stdout_bytes(err.render().to_string().len()); + telemetry_event::add_stdout_bytes(err.render().to_string().len()); } let _ = err.print(); return code; @@ -1018,14 +1018,14 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - } match &token { Some(tilesets_cli::ChildToken::Flag(t)) => { - events::set_token(auth::TokenSource::Flag, t) + telemetry_event::set_token(auth::TokenSource::Flag, t) } Some(tilesets_cli::ChildToken::Stored(t)) => { - events::set_token(auth::TokenSource::Login, t) + telemetry_event::set_token(auth::TokenSource::Login, t) } None => { if let Some((_, t)) = auth::environment_token() { - events::set_token(auth::TokenSource::Environment, &t); + telemetry_event::set_token(auth::TokenSource::Environment, &t); } } } @@ -1103,7 +1103,7 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - return Err(no_stored_credentials(profile)); } if let Some(token) = &token { - events::set_resolved_token(matches, use_login, token); + telemetry_event::set_resolved_token(matches, use_login, token); } account_usage::run( @@ -1218,7 +1218,7 @@ fn run(app: &Command, specs: &[ServiceSpec], matches: &ArgMatches, mode: Mode) - return Err(no_stored_credentials(profile)); } if let Some(token) = &token { - events::set_resolved_token(matches, use_login, token); + telemetry_event::set_resolved_token(matches, use_login, token); } let username: Option = matches .get_one::("username") diff --git a/src/output.rs b/src/output.rs index 638e0c1..93e3532 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1483,7 +1483,7 @@ fn error_payload(e: &CliError) -> Value { /// no `state` field: the streams already separate the two cases. pub fn emit_error(mode: Mode, err: &anyhow::Error) { let cli = err.downcast_ref::(); - crate::events::set_error_code(cli.map_or(GENERIC_CODE, |e| e.code.as_str())); + crate::telemetry_event::set_error_code(cli.map_or(GENERIC_CODE, |e| e.code.as_str())); if mode.is_json() { let payload = match cli { @@ -1590,7 +1590,7 @@ fn adds_detail(body: &Value) -> bool { fn write_stdout(line: &str) -> Result<()> { // The line and its newline. - crate::events::add_stdout_bytes(line.len() + 1); + crate::telemetry_event::add_stdout_bytes(line.len() + 1); let mut out = std::io::stdout().lock(); writeln!(out, "{line}")?; out.flush()?; diff --git a/src/telemetry.rs b/src/telemetry.rs index d00deff..e9b56d3 100644 --- a/src/telemetry.rs +++ b/src/telemetry.rs @@ -61,7 +61,7 @@ pub(crate) fn in_ci() -> bool { ci_marker().is_some() } -/// For `crate::events`, which may not reach for stdout itself. +/// For `crate::telemetry_event`, which may not reach for stdout itself. pub(crate) fn stdout_is_terminal() -> bool { std::io::stdout().is_terminal() } diff --git a/src/events.rs b/src/telemetry_event.rs similarity index 100% rename from src/events.rs rename to src/telemetry_event.rs diff --git a/src/tilesets_cli.rs b/src/tilesets_cli.rs index 998ed22..98b144c 100644 --- a/src/tilesets_cli.rs +++ b/src/tilesets_cli.rs @@ -499,7 +499,7 @@ fn handoff(mut cmd: std::process::Command) -> std::io::Error { // binary resolves — a missing `tilesets` is a failure `main` reports, // and the event should carry it. if resolves(std::path::Path::new(cmd.get_program())) { - crate::events::finish(None); + crate::telemetry_event::finish(None); } cmd.exec() } @@ -523,7 +523,7 @@ fn handoff(mut cmd: std::process::Command) -> std::io::Error { Ok(status) => { let code = status.code().unwrap_or(130); // `exit` skips `main`'s way out, where the event is sent. - crate::events::finish(u32::try_from(code).ok()); + crate::telemetry_event::finish(u32::try_from(code).ok()); std::process::exit(code) } Err(err) => err, diff --git a/src/update_check.rs b/src/update_check.rs index 290c0ab..e9030a4 100644 --- a/src/update_check.rs +++ b/src/update_check.rs @@ -427,7 +427,7 @@ pub fn notify() { // it had, rather than racing a child that may finish first. if let Some(latest) = should_notify(cache.as_ref(), CURRENT, now) { output::progress(¬ice(latest, CURRENT, cfg!(windows))); - crate::events::set_update_notice(latest); + crate::telemetry_event::set_update_notice(latest); let mut updated = cache.clone().unwrap_or_default(); updated.notified_at = now; write_cache(&updated); diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 5484d38..677d86f 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -43,7 +43,7 @@ fn sources() -> Vec<(String, String)> { /// - `agent_skills` — the staging directory it renames skills out of, and the /// skill directory `install --force` replaces. /// - `auth` — `logout`, and the scratch file `write_private` renames from. -/// - `events` — its own dated event files past the retention window, matched +/// - `telemetry_event` — its own dated event files past the retention window, matched /// by exact `YYYY-MM-DD.jsonl` names inside `~/.mapbox/.telemetry`, and the /// `last-version` file it rewrites. /// - `executor` — nothing durable; the temp file a `--file` upload streams. @@ -53,7 +53,7 @@ fn sources() -> Vec<(String, String)> { const MAY_DELETE: &[&str] = &[ "agent_skills.rs", "auth.rs", - "events.rs", + "telemetry_event.rs", "executor.rs", "generate_skills.rs", "skill_dest.rs", diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index 5ecffe8..67c91da 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -1,6 +1,6 @@ //! End-to-end tests for the run's `cli.command` event. //! -//! The unit tests in `src/events.rs` cover the pure parts — how an argument +//! The unit tests in `src/telemetry_event.rs` cover the pure parts — how an argument //! is classified, what a timestamp looks like, which files pruning may //! touch. What they cannot show is what a real run leaves behind: that the //! event lands where it should, carries what the command did and nothing the From 473a20993c2ef42ede52d49175efb99bf6698ba2 Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Thu, 24 Sep 2026 17:15:48 +0300 Subject: [PATCH 4/7] Move delivery into telemetry_sink; leave HTTP as an interface telemetry_event decides what the event contains; telemetry_sink decides where it goes. FileSink writes the local file for testing; HttpSink is the interface for Mapbox Events, documented but not implemented, so the detached sender, http::bare_client and the sink/debug variables are gone. --- src/http.rs | 14 +- src/main.rs | 4 +- src/telemetry_event.rs | 298 +++----------------------------------- src/telemetry_sink.rs | 247 +++++++++++++++++++++++++++++++ tests/source_guards.rs | 8 +- tests/telemetry_events.rs | 104 ------------- 6 files changed, 277 insertions(+), 398 deletions(-) create mode 100644 src/telemetry_sink.rs diff --git a/src/http.rs b/src/http.rs index 9e307f5..f85af3e 100644 --- a/src/http.rs +++ b/src/http.rs @@ -232,20 +232,8 @@ pub fn client_for(command_group: Option<&str>) -> Result) -> Result { - build_with(telemetry::user_agent(command_group), timeout) -} - -/// A client whose `User-Agent` is [`telemetry::PRODUCT_TOKEN`] alone, for -/// the telemetry sender. Mapbox Events stores the `User-Agent` of the upload -/// in every record, so the markers — `agent/` among them — must not -/// ride along on it. -pub fn bare_client(timeout: Duration) -> Result { - build_with(telemetry::PRODUCT_TOKEN.to_string(), timeout) -} - -fn build_with(user_agent: String, timeout: Duration) -> Result { reqwest::blocking::Client::builder() - .user_agent(user_agent) + .user_agent(telemetry::user_agent(command_group)) .connect_timeout(CONNECT_TIMEOUT) .timeout(timeout) .build() diff --git a/src/main.rs b/src/main.rs index b7ce33e..e4cda70 100644 --- a/src/main.rs +++ b/src/main.rs @@ -31,6 +31,7 @@ mod skill_dest; mod spec; mod telemetry; mod telemetry_event; +mod telemetry_sink; mod tilesets_cli; mod uninstall; mod update_check; @@ -675,9 +676,6 @@ fn main() -> ExitCode { if update_check::is_refresh_child() { return update_check::run_refresh_child(); } - if telemetry_event::is_sender_child() { - return telemetry_event::run_sender_child(); - } telemetry_event::start(); let code = cli(); diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs index f639f73..9e888cf 100644 --- a/src/telemetry_event.rs +++ b/src/telemetry_event.rs @@ -4,9 +4,8 @@ //! request, the bytes on stdout, an error code — and `main` calls [`finish`] //! once on the way out; nothing is written before then. `set_*` functions //! overwrite a field, last call wins; `add_*` functions accumulate. The -//! event is then handed to a sink: a JSON line under -//! `~/.mapbox/.telemetry/` by default, or Mapbox Events through a detached -//! child with `MAPBOX_CLI_TELEMETRY_SINK=api`. +//! finished event goes to [`crate::telemetry_sink`], which decides where it +//! is delivered; this module decides only what it contains. //! //! What this refuses to record is the point of it. Argument values leave //! only when they come from a fixed set (an enum, a boolean, a number the @@ -20,9 +19,8 @@ //! (`MAPBOX_CLI_NO_TELEMETRY`), nothing is recorded or written. use std::collections::HashSet; -use std::io::{IsTerminal, Read, Write}; -use std::path::{Path, PathBuf}; -use std::process::{Command as Process, ExitCode, Stdio}; +use std::io::IsTerminal; +use std::path::Path; use std::sync::{Mutex, OnceLock}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; @@ -31,36 +29,21 @@ use clap::{ArgMatches, Command}; use serde::Serialize; use crate::spec::ServiceSpec; -use crate::{agent_detect, auth, confirm, executor, http, output, schema, telemetry}; +use crate::{ + agent_detect, auth, confirm, executor, http, output, schema, telemetry, telemetry_sink, +}; const EVENT: &str = "cli.command"; const SCHEMA_VERSION: &str = "2.0"; const SDK_IDENTIFIER: &str = "mapbox-cli"; const CURRENT: &str = env!("CARGO_PKG_VERSION"); -/// `file` or `api`. Anything else, or nothing, is `file`. -const SINK_ENV: &str = "MAPBOX_CLI_TELEMETRY_SINK"; -/// Prints the event to stderr instead of handing it to the sink. -const DEBUG_ENV: &str = "MAPBOX_CLI_TELEMETRY_DEBUG"; - -/// Set on the child [`finish`] spawns for the `api` sink — a mode of this -/// binary for the reason `update_check`'s refresher is one: a hidden -/// subcommand would be in `--schema` and the generated skills. -const SENDER_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_SEND"; -/// Overrides for the compiled-in endpoint and token, for tests and for -/// pointing a dev build at staging. -const URL_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_URL"; -const TOKEN_ENV: &str = "MAPBOX_INTERNAL_TELEMETRY_TOKEN"; -const PRODUCTION_URL: &str = "https://events.mapbox.com/events/v2"; -const SEND_TIMEOUT: Duration = Duration::from_secs(5); -/// Far above one event (about 1 KB); a bound on what the child will read. -const MAX_PAYLOAD: u64 = 256 * 1024; - -const DIR: &str = ".telemetry"; +/// The largest `--data @` file read for its top-level keys; above +/// it, only the size is recorded. +const MAX_DATA_TO_PARSE: u64 = 256 * 1024; + const USER_ID_FILE: &str = "user-id"; const LAST_VERSION_FILE: &str = "last-version"; -/// Days of event files kept by the `file` sink. -const KEEP_DAYS: i64 = 7; // Bounds from the schema. An event over any of them is rejected whole at // ingest, so they are enforced here rather than trusted. @@ -479,16 +462,8 @@ fn deliver_locked(run: &mut Run, exit_code: Option) { } run.finished = true; let event = build(run, exit_code); - let Ok(line) = serde_json::to_string(&event) else { - return; - }; - if std::env::var_os(DEBUG_ENV).is_some_and(|value| !value.is_empty()) { - output::progress(&line); - return; - } - match std::env::var(SINK_ENV).ok().as_deref().map(str::trim) { - Some("api") => spawn_sender(&line), - _ => append_to_file(&line), + if let Ok(line) = serde_json::to_string(&event) { + telemetry_sink::deliver(&line); } } @@ -790,7 +765,7 @@ fn data_shape(value: &str) -> (Option, Option>) { }; // Parsed only when small enough to be a request body worth // describing; the size alone is still recorded above that. - if meta.len() > MAX_PAYLOAD { + if meta.len() > MAX_DATA_TO_PARSE { return (Some(meta.len()), None); } match std::fs::read_to_string(path) { @@ -854,65 +829,23 @@ fn clip(text: &str, max_chars: usize) -> String { text.chars().take(max_chars).collect() } -fn telemetry_dir() -> Option { - Some(auth::config_dir_path()?.join(DIR)) -} - -/// The directory, created `0700` inside a config directory `auth` has -/// created and hardened, as it would for credentials. `None` when it cannot be. -fn prepared_dir() -> Option { - auth::config_dir().ok()?; - let dir = telemetry_dir()?; - std::fs::create_dir_all(&dir).ok()?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); - } - Some(dir) -} - -fn open_private(path: &Path, append: bool) -> std::io::Result { - let mut options = std::fs::OpenOptions::new(); - if append { - options.append(true).create(true); - } else { - options.write(true).create_new(true); - } - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - options.mode(0o600); - } - options.open(path) -} - /// The installation's random id, created on first use. Two first runs in /// parallel can each create one; `create_new` makes the second read the /// first's instead of replacing it. When nothing can be stored, the run /// still gets an id — just not one the next run will share. fn user_id() -> String { - let fresh = uuid_v4(rand::random()); - let Some(dir) = prepared_dir() else { - return fresh; - }; - let path = dir.join(USER_ID_FILE); - if let Some(existing) = read_user_id(&path) { + if let Some(existing) = read_user_id() { return existing; } - match open_private(&path, false) { - Ok(mut file) => { - let _ = file.write_all(fresh.as_bytes()); - fresh - } - Err(_) => read_user_id(&path).unwrap_or(fresh), + let fresh = uuid_v4(rand::random()); + if telemetry_sink::create_state(USER_ID_FILE, &fresh) { + return fresh; } + read_user_id().unwrap_or(fresh) } -fn read_user_id(path: &Path) -> Option { - let text = std::fs::read_to_string(path).ok()?; - let id = text.trim(); - is_uuid(id).then(|| id.to_string()) +fn read_user_id() -> Option { + telemetry_sink::read_state(USER_ID_FILE).filter(|id| is_uuid(id)) } fn is_uuid(text: &str) -> bool { @@ -927,18 +860,11 @@ fn is_uuid(text: &str) -> bool { /// first run after an upgrade. Checked against the same shape the update /// check trusts, since it is read back from disk. fn previous_version() -> Option { - let dir = prepared_dir()?; - let path = dir.join(LAST_VERSION_FILE); - let last = std::fs::read_to_string(&path) - .ok() - .map(|text| text.trim().to_string()); + let last = telemetry_sink::read_state(LAST_VERSION_FILE); if last.as_deref() == Some(CURRENT) { return None; } - let _ = std::fs::remove_file(&path); - if let Ok(mut file) = open_private(&path, false) { - let _ = file.write_all(CURRENT.as_bytes()); - } + telemetry_sink::replace_state(LAST_VERSION_FILE, CURRENT); last.filter(|version| is_version(version)) .map(|version| clip(&version, MAX_VERSION)) } @@ -968,7 +894,7 @@ fn uuid_v4(mut bytes: [u8; 16]) -> String { /// RFC 3339 in UTC, to the millisecond. fn timestamp(at: SystemTime) -> String { let since = at.duration_since(UNIX_EPOCH).unwrap_or_default(); - let (date, secs) = utc_date(since.as_secs()); + let (date, secs) = telemetry_sink::utc_date(since.as_secs()); format!( "{date}T{:02}:{:02}:{:02}.{:03}Z", secs / 3600, @@ -978,144 +904,6 @@ fn timestamp(at: SystemTime) -> String { ) } -/// `YYYY-MM-DD` and the seconds into that day. -fn utc_date(unix_secs: u64) -> (String, u64) { - let days = (unix_secs / 86_400) as i64; - let (y, m, d) = crate::account_usage::civil_from_days(days); - (format!("{y:04}-{m:02}-{d:02}"), unix_secs % 86_400) -} - -fn now_secs() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_or(0, |d| d.as_secs()) -} - -/// Appends one line to today's file. A line this short goes out in one -/// `write`, which `O_APPEND` keeps whole against parallel runs. -fn append_to_file(line: &str) { - let Some(dir) = prepared_dir() else { - return; - }; - let (today, _) = utc_date(now_secs()); - let path = dir.join(format!("{today}.jsonl")); - let is_new_day = !path.exists(); - if let Ok(mut file) = open_private(&path, true) { - let _ = file.write_all(format!("{line}\n").as_bytes()); - } - if is_new_day { - prune(&dir, now_secs()); - } -} - -/// Deletes event files older than [`KEEP_DAYS`]. Only names that are -/// exactly `YYYY-MM-DD.jsonl` are considered, and only inside the telemetry -/// directory itself, so nothing else there can be matched. -fn prune(dir: &Path, now: u64) { - let (cutoff, _) = utc_date(now.saturating_sub(KEEP_DAYS as u64 * 86_400)); - let Ok(entries) = std::fs::read_dir(dir) else { - return; - }; - for entry in entries.flatten() { - let name = entry.file_name().to_string_lossy().into_owned(); - if let Some(date) = event_file_date(&name) { - if date < cutoff.as_str() { - let _ = std::fs::remove_file(dir.join(&name)); - } - } - } -} - -fn event_file_date(name: &str) -> Option<&str> { - let date = name.strip_suffix(".jsonl")?; - let shape = date.len() == 10 - && date.char_indices().all(|(i, c)| match i { - 4 | 7 => c == '-', - _ => c.is_ascii_digit(), - }); - shape.then_some(date) -} - -/// The endpoint and token for the `api` sink. `None` without a token, which -/// a build outside the release pipeline has none of. -fn api_target() -> Option<(String, String)> { - let from_env = |name: &str| std::env::var(name).ok().filter(|v| !v.trim().is_empty()); - let token = from_env(TOKEN_ENV) - .or_else(|| option_env!("MAPBOX_CLI_TELEMETRY_TOKEN").map(String::from))?; - let url = from_env(URL_ENV) - .or_else(|| option_env!("MAPBOX_CLI_TELEMETRY_URL").map(String::from)) - .unwrap_or_else(|| PRODUCTION_URL.to_string()); - Some((url, token)) -} - -/// Starts the sender with the event on its stdin, and forgets it. Detached -/// the way `update_check` detaches its refresher, and for the same reasons. -/// The event goes on stdin rather than argv, which other users can read in -/// `ps`. -fn spawn_sender(line: &str) { - if api_target().is_none() { - return; - } - let Ok(exe) = std::env::current_exe() else { - return; - }; - let mut command = Process::new(exe); - command - .env(SENDER_ENV, "1") - .stdin(Stdio::piped()) - .stdout(Stdio::null()) - .stderr(Stdio::null()); - - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - command.process_group(0); - } - #[cfg(windows)] - { - use std::os::windows::process::CommandExt; - const DETACHED_PROCESS: u32 = 0x0000_0008; - const CREATE_NO_WINDOW: u32 = 0x0800_0000; - command.creation_flags(DETACHED_PROCESS | CREATE_NO_WINDOW); - } - - if let Ok(mut child) = command.spawn() { - // One event fits in the pipe's buffer, so this does not wait on the - // child; dropping stdin closes it, which is the child's end of input. - if let Some(mut stdin) = child.stdin.take() { - let _ = stdin.write_all(line.as_bytes()); - } - } -} - -/// Whether this process is the `api` sink's sender rather than a command. -pub fn is_sender_child() -> bool { - std::env::var_os(SENDER_ENV).is_some_and(|value| !value.is_empty()) -} - -/// The whole of the sender: read one event, POST it once, exit. Its result -/// is read by nobody, so there is nothing to report and nothing to retry. -pub fn run_sender_child() -> ExitCode { - let mut line = String::new(); - let _ = std::io::stdin().take(MAX_PAYLOAD).read_to_string(&mut line); - if line.trim().is_empty() || !enabled() { - return ExitCode::SUCCESS; - } - let Some((url, token)) = api_target() else { - return ExitCode::SUCCESS; - }; - let Ok(client) = http::bare_client(SEND_TIMEOUT) else { - return ExitCode::SUCCESS; - }; - let request = client - .post(url) - .query(&[("access_token", token)]) - .header(reqwest::header::CONTENT_TYPE, "application/json") - .body(format!("[{}]", line.trim())); - let _ = http::send(request); - ExitCode::SUCCESS -} - #[cfg(test)] mod tests { use super::*; @@ -1210,44 +998,6 @@ mod tests { assert_eq!(install_method(None), "other"); } - #[test] - fn only_dated_event_files_are_pruned() { - assert_eq!(event_file_date("2026-09-24.jsonl"), Some("2026-09-24")); - for name in [ - "user-id", - "last-version", - "2026-09-24.json", - "notes.jsonl", - "2026-9-24.jsonl", - ] { - assert_eq!(event_file_date(name), None, "{name}"); - } - } - - #[test] - fn prune_removes_old_event_files_and_nothing_else() { - let dir = - std::env::temp_dir().join(format!("mapbox-events-prune-{}", uuid_v4(rand::random()))); - std::fs::create_dir_all(&dir).unwrap(); - for name in [ - "2026-09-01.jsonl", - "2026-09-20.jsonl", - "user-id", - "2026-09-01.txt", - ] { - std::fs::write(dir.join(name), "x").unwrap(); - } - // 2026-09-24T00:00:00Z - prune(&dir, 1_790_208_000); - let mut left: Vec = std::fs::read_dir(&dir) - .unwrap() - .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) - .collect(); - left.sort(); - std::fs::remove_dir_all(&dir).unwrap(); - assert_eq!(left, ["2026-09-01.txt", "2026-09-20.jsonl", "user-id"]); - } - #[test] fn a_token_is_read_for_its_prefix_and_account_only() { let payload = base64::Engine::encode( diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs new file mode 100644 index 0000000..1dc6694 --- /dev/null +++ b/src/telemetry_sink.rs @@ -0,0 +1,247 @@ +//! Where the run's telemetry event goes once [`crate::telemetry_event`] has +//! built it. +//! +//! This module never decides what an event contains; it is handed one +//! finished JSON line. That keeps the privacy rules in one file and lets a +//! delivery change — a new endpoint, batching — happen without touching them. +//! +//! [`selected`] is the one place that picks the [`Sink`]. Today that is +//! [`FileSink`], which keeps events on this machine for local testing; +//! [`HttpSink`] is the interface for sending them, not implemented yet. +//! +//! It also owns the directory, `~/.mapbox/.telemetry` (or under +//! `$MAPBOX_CONFIG_DIR`), including the two small state files the event +//! reads — the installation id and the last version seen — so that every +//! write and delete under it is in this file. + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; + +use crate::auth; + +const DIR: &str = ".telemetry"; +/// Days of event files [`FileSink`] keeps, today included. +const KEEP_DAYS: u64 = 7; + +/// Something that takes one finished event line and delivers it. +/// +/// Best-effort by contract: `deliver` reports nothing and must not fail the +/// command, block its exit, or write to stdout. +pub(crate) trait Sink { + fn deliver(&self, line: &str); +} + +/// Appends each event to `.jsonl`, one line per run. +pub(crate) struct FileSink; + +/// Sends each event to Mapbox Events. **Not implemented yet**: `deliver` +/// drops the event. +/// +/// What an implementation is expected to do: +/// +/// - `POST https://events.mapbox.com/events/v2?access_token=` with +/// the body `[]`, using a CLI-owned `pk.` token compiled into +/// release builds — never the user's. +/// - Send from a detached child so the command never waits, the way +/// `update_check` detaches its refresher, with the event on the child's +/// stdin rather than argv (argv is visible in `ps`). +/// - One attempt with a short timeout; drop the event on any failure. +/// - Send through [`crate::http::send`] with a `User-Agent` of +/// `mapbox-cli/` alone: Mapbox Events stores it in every record, +/// so the `agent/` marker must not ride along. +// Not constructed until `selected` switches to it. +#[allow(dead_code)] +pub(crate) struct HttpSink; + +impl Sink for FileSink { + fn deliver(&self, line: &str) { + let Some(dir) = dir() else { + return; + }; + let (today, _) = utc_date(now_secs()); + let path = dir.join(format!("{today}.jsonl")); + let is_new_day = !path.exists(); + // One `write` per line. `O_APPEND` places each one at the end, but a + // line past the platform's atomic-write size (an event near the + // schema's bounds) is not guaranteed to stay whole against a + // parallel run writing at the same moment. + if let Ok(mut file) = open_private(&path, true) { + let _ = file.write_all(format!("{line}\n").as_bytes()); + } + if is_new_day { + prune(&dir, now_secs()); + } + } +} + +impl Sink for HttpSink { + fn deliver(&self, _line: &str) {} +} + +/// Hands `line` to the [`selected`] sink. +pub(crate) fn deliver(line: &str) { + selected().deliver(line); +} + +/// The sink every event goes to. `HttpSink` replaces `FileSink` here once +/// it is implemented and `cli.command` is registered with Mapbox Events. +fn selected() -> &'static dyn Sink { + &FileSink +} + +/// The contents of a state file, trimmed, if it is there. +pub(crate) fn read_state(name: &str) -> Option { + let text = std::fs::read_to_string(dir()?.join(name)).ok()?; + Some(text.trim().to_string()) +} + +/// Creates a state file, only if it does not exist yet. `false` when it +/// already did or could not be written — two first runs racing each get +/// one answer this way, rather than the second replacing the first. +pub(crate) fn create_state(name: &str, contents: &str) -> bool { + let Some(dir) = dir() else { + return false; + }; + match open_private(&dir.join(name), false) { + Ok(mut file) => file.write_all(contents.as_bytes()).is_ok(), + Err(_) => false, + } +} + +/// Replaces a state file's contents. +pub(crate) fn replace_state(name: &str, contents: &str) { + let Some(dir) = dir() else { + return; + }; + let path = dir.join(name); + let _ = std::fs::remove_file(&path); + if let Ok(mut file) = open_private(&path, false) { + let _ = file.write_all(contents.as_bytes()); + } +} + +/// The directory, created `0700` inside a config directory `auth` has +/// created and hardened, as it would for credentials. `None` when it +/// cannot be. +fn dir() -> Option { + auth::config_dir().ok()?; + let dir = auth::config_dir_path()?.join(DIR); + std::fs::create_dir_all(&dir).ok()?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)); + } + Some(dir) +} + +/// Opens `path` `0600`: for appending, or created new and failing if it +/// already exists. +fn open_private(path: &Path, append: bool) -> std::io::Result { + let mut options = std::fs::OpenOptions::new(); + if append { + options.append(true).create(true); + } else { + options.write(true).create_new(true); + } + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + options.open(path) +} + +/// Deletes event files outside the last [`KEEP_DAYS`] days. Only names that +/// are exactly `YYYY-MM-DD.jsonl` are considered, and only inside the +/// telemetry directory itself, so nothing else there can be matched. +fn prune(dir: &Path, now: u64) { + let (oldest_kept, _) = utc_date(now.saturating_sub((KEEP_DAYS - 1) * 86_400)); + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name().to_string_lossy().into_owned(); + if let Some(date) = event_file_date(&name) { + if date < oldest_kept.as_str() { + let _ = std::fs::remove_file(dir.join(&name)); + } + } + } +} + +fn event_file_date(name: &str) -> Option<&str> { + let date = name.strip_suffix(".jsonl")?; + let shape = date.len() == 10 + && date.char_indices().all(|(i, c)| match i { + 4 | 7 => c == '-', + _ => c.is_ascii_digit(), + }); + shape.then_some(date) +} + +/// `YYYY-MM-DD` and the seconds into that day, in UTC. +pub(crate) fn utc_date(unix_secs: u64) -> (String, u64) { + let days = (unix_secs / 86_400) as i64; + let (y, m, d) = crate::account_usage::civil_from_days(days); + (format!("{y:04}-{m:02}-{d:02}"), unix_secs % 86_400) +} + +fn now_secs() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |d| d.as_secs()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_dated_event_files_are_pruned() { + assert_eq!(event_file_date("2026-09-24.jsonl"), Some("2026-09-24")); + for name in [ + "user-id", + "last-version", + "2026-09-24.json", + "notes.jsonl", + "2026-9-24.jsonl", + ] { + assert_eq!(event_file_date(name), None, "{name}"); + } + } + + #[test] + fn prune_keeps_seven_days_and_nothing_else_is_touched() { + let dir = + std::env::temp_dir().join(format!("mapbox-telemetry-prune-{}", rand::random::())); + std::fs::create_dir_all(&dir).unwrap(); + for name in [ + "2026-09-17.jsonl", + "2026-09-18.jsonl", + "2026-09-24.jsonl", + "user-id", + "2026-09-01.txt", + ] { + std::fs::write(dir.join(name), "x").unwrap(); + } + // 2026-09-24T12:00:00Z: 09-18 through 09-24 is seven days. + prune(&dir, 1_790_251_200); + let mut left: Vec = std::fs::read_dir(&dir) + .unwrap() + .map(|e| e.unwrap().file_name().to_string_lossy().into_owned()) + .collect(); + left.sort(); + std::fs::remove_dir_all(&dir).unwrap(); + assert_eq!( + left, + [ + "2026-09-01.txt", + "2026-09-18.jsonl", + "2026-09-24.jsonl", + "user-id" + ] + ); + } +} diff --git a/tests/source_guards.rs b/tests/source_guards.rs index 677d86f..d83991b 100644 --- a/tests/source_guards.rs +++ b/tests/source_guards.rs @@ -43,9 +43,9 @@ fn sources() -> Vec<(String, String)> { /// - `agent_skills` — the staging directory it renames skills out of, and the /// skill directory `install --force` replaces. /// - `auth` — `logout`, and the scratch file `write_private` renames from. -/// - `telemetry_event` — its own dated event files past the retention window, matched -/// by exact `YYYY-MM-DD.jsonl` names inside `~/.mapbox/.telemetry`, and the -/// `last-version` file it rewrites. +/// - `telemetry_sink` — its own dated event files past the retention window, +/// matched by exact `YYYY-MM-DD.jsonl` names inside `~/.mapbox/.telemetry`, +/// and a state file there it replaces. /// - `executor` — nothing durable; the temp file a `--file` upload streams. /// - `generate_skills` — the staged skill directory it renames into place. /// - `skill_dest` — a test scratch directory. @@ -53,10 +53,10 @@ fn sources() -> Vec<(String, String)> { const MAY_DELETE: &[&str] = &[ "agent_skills.rs", "auth.rs", - "telemetry_event.rs", "executor.rs", "generate_skills.rs", "skill_dest.rs", + "telemetry_sink.rs", "uninstall.rs", ]; diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index 67c91da..05cf75d 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -6,11 +6,8 @@ //! event lands where it should, carries what the command did and nothing the //! user typed, disappears when telemetry is off, and never changes stdout. -use std::io::{Read, Write}; -use std::net::TcpListener; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; -use std::time::{Duration, Instant}; use serde_json::Value; @@ -37,8 +34,6 @@ fn command(home: &Path) -> Command { .env_remove("MAPBOX_USERNAME") .env_remove("MAPBOX_OUTPUT") .env_remove("MAPBOX_CLI_NO_TELEMETRY") - .env_remove("MAPBOX_CLI_TELEMETRY_SINK") - .env_remove("MAPBOX_CLI_TELEMETRY_DEBUG") .env("MAPBOX_NO_UPDATE_CHECK", "1") .env("HOME", home) .env("XDG_CONFIG_HOME", home.join(".config")) @@ -236,102 +231,3 @@ fn completion_records_nothing() { config_dir(&home).display() ); } - -/// The `api` sink, against a loopback server standing in for Mapbox Events: -/// one POST, the event wrapped in an array, the token in the query and the -/// bare `User-Agent` — no `agent/` marker, even when an agent is detected. -#[test] -fn the_api_sink_posts_one_event_with_a_bare_user_agent() { - let listener = TcpListener::bind("127.0.0.1:0").expect("a loopback port"); - let addr = listener.local_addr().expect("the bound address"); - listener.set_nonblocking(true).expect("nonblocking"); - - let home = scratch("api"); - let out = command(&home) - .env("MAPBOX_CLI_TELEMETRY_SINK", "api") - .env( - "MAPBOX_INTERNAL_TELEMETRY_URL", - format!("http://{addr}/events/v2"), - ) - .env("MAPBOX_INTERNAL_TELEMETRY_TOKEN", "pk.upload-token") - .env("CLAUDECODE", "1") - .args(["config", "list"]) - .output() - .expect("run mapbox"); - assert!(out.status.success()); - - // The sender is detached and outlives the command, so wait for it. - let deadline = Instant::now() + Duration::from_secs(10); - let mut stream = loop { - match listener.accept() { - Ok((stream, _)) => break stream, - Err(_) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(50)), - Err(e) => panic!("the sender never connected: {e}"), - } - }; - stream.set_nonblocking(false).expect("blocking"); - stream - .set_read_timeout(Some(Duration::from_secs(5))) - .expect("a read timeout"); - - let mut request = Vec::new(); - let mut buf = [0u8; 4096]; - loop { - let n = stream.read(&mut buf).unwrap_or(0); - request.extend_from_slice(&buf[..n]); - let text = String::from_utf8_lossy(&request); - if let Some((head, body)) = text.split_once("\r\n\r\n") { - let length = head - .lines() - .find_map(|l| { - l.to_ascii_lowercase() - .strip_prefix("content-length:") - .map(|v| v.trim().parse::().unwrap_or(0)) - }) - .unwrap_or(0); - if body.len() >= length { - break; - } - } - if n == 0 { - break; - } - } - let _ = stream.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n"); - - let text = String::from_utf8_lossy(&request).into_owned(); - let (head, body) = text - .split_once("\r\n\r\n") - .expect("a request head and body"); - let first_line = head.lines().next().unwrap_or_default(); - assert!( - first_line.starts_with("POST /events/v2?access_token=pk.upload-token "), - "{first_line}" - ); - let user_agent = head - .lines() - .find_map(|l| { - l.to_ascii_lowercase() - .strip_prefix("user-agent:") - .map(|v| v.trim().to_string()) - }) - .expect("a User-Agent"); - assert_eq!( - user_agent, - format!("mapbox-cli/{}", env!("CARGO_PKG_VERSION")) - ); - - let sent: Value = serde_json::from_str(body).expect("the body is JSON"); - let sent = sent.as_array().expect("an array of events"); - assert_eq!(sent.len(), 1); - assert_eq!(sent[0]["command"], serde_json::json!(["config", "list"])); - assert_eq!(sent[0]["env"]["agent"], "claude-code"); - assert!( - !config_dir(&home) - .join(".telemetry") - .read_dir() - .is_ok_and(|mut d| d - .any(|e| e.is_ok_and(|e| e.path().extension().is_some_and(|x| x == "jsonl")))), - "the api sink also wrote to the file sink" - ); -} From 1ba398dff61089e87f64e5dbb7ded81c67bf449d Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Fri, 25 Sep 2026 12:27:37 +0300 Subject: [PATCH 5/7] Add parentEventId and workflow to the telemetry event parentEventId is read from MAPBOX_CLI_PARENT_EVENT, which a workflow will set on the steps it launches, so a mapbox run inside a user's script can be tied to the workflow that started it rather than counted as a direct run. The event id is now fixed at start for that reason. workflow records where a workflow came from, and its name only when Mapbox named it: a custom workflow's name is dropped in the setter. No workflow command exists yet; this is the base the schema and Legal review cover. --- src/telemetry_event.rs | 95 ++++++++++++++++++++++++++++++++++++++- tests/telemetry_events.rs | 24 ++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs index 9e888cf..9c65e26 100644 --- a/src/telemetry_event.rs +++ b/src/telemetry_event.rs @@ -42,6 +42,10 @@ const CURRENT: &str = env!("CARGO_PKG_VERSION"); /// it, only the size is recorded. const MAX_DATA_TO_PARSE: u64 = 256 * 1024; +/// Set by a workflow on each step it launches, to its own [`event_id`], so +/// a `mapbox` run inside a workflow step records which run started it. +const PARENT_EVENT_ENV: &str = "MAPBOX_CLI_PARENT_EVENT"; + const USER_ID_FILE: &str = "user-id"; const LAST_VERSION_FILE: &str = "last-version"; @@ -133,6 +137,34 @@ pub(crate) struct Param { keys: Option>, } +/// Where a workflow came from. Only Mapbox names its `Builtin` and +/// `Marketplace` workflows; a `Custom` one is named by the user, so its name +/// is never recorded. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(dead_code)] // Used by the workflow commands, which don't exist yet. +pub(crate) enum WorkflowSource { + Builtin, + Marketplace, + Custom, +} + +impl WorkflowSource { + fn as_str(self) -> &'static str { + match self { + WorkflowSource::Builtin => "builtin", + WorkflowSource::Marketplace => "marketplace", + WorkflowSource::Custom => "custom", + } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize)] +struct Workflow { + source: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + name: Option, +} + #[derive(Debug, Clone, PartialEq, Serialize)] struct Auth { source: &'static str, @@ -188,6 +220,8 @@ struct Event { version: &'static str, created: String, event_id: String, + #[serde(skip_serializing_if = "Option::is_none")] + parent_event_id: Option, user_id: String, sdk_identifier: &'static str, sdk_version: &'static str, @@ -196,6 +230,8 @@ struct Event { command: Option>, #[serde(skip_serializing_if = "Option::is_none")] invocation: Option<&'static str>, + #[serde(skip_serializing_if = "Option::is_none")] + workflow: Option, #[serde(skip_serializing_if = "Vec::is_empty")] params: Vec, #[serde(skip_serializing_if = "Option::is_none")] @@ -251,6 +287,7 @@ struct Options { struct Run { command: Option>, invocation: Option<&'static str>, + workflow: Option, params: Vec, usage_error: Option, options: Option, @@ -272,6 +309,7 @@ impl Run { Run { command: None, invocation: None, + workflow: None, params: Vec::new(), usage_error: None, options: None, @@ -288,6 +326,7 @@ impl Run { static RUN: Mutex = Mutex::new(Run::new()); static STARTED: OnceLock = OnceLock::new(); +static EVENT_ID: OnceLock = OnceLock::new(); static ENABLED: OnceLock = OnceLock::new(); /// Read once: the answer must not change halfway through a run. @@ -310,6 +349,7 @@ fn with_run(f: impl FnOnce(&mut Run)) { /// hook that records a crash as `errorCode: "panic"`. pub fn start() { STARTED.get_or_init(Instant::now); + event_id(); let previous = std::panic::take_hook(); std::panic::set_hook(Box::new(move |info| { @@ -392,6 +432,38 @@ pub fn set_resolved_token(matches: &ArgMatches, use_login: bool, token: &str) { set_token(source, token); } +/// The workflow this run adds, removes or runs. `name` is dropped for a +/// `Custom` workflow, whatever the caller passes. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow(source: WorkflowSource, name: Option<&str>) { + with_run(|run| run.workflow = Some(workflow_field(source, name))); +} + +fn workflow_field(source: WorkflowSource, name: Option<&str>) -> Workflow { + let name = match source { + WorkflowSource::Custom => None, + _ => name.map(|name| clip(name, MAX_NAME)), + }; + Workflow { + source: source.as_str(), + name, + } +} + +/// This run's event id, fixed at [`start`] so a workflow can hand it to the +/// steps it launches before the event itself is built. +pub(crate) fn event_id() -> &'static str { + EVENT_ID.get_or_init(|| uuid_v4(rand::random())) +} + +/// The run that started this one, when a workflow step set it. Read back +/// only when it has the shape of an event id. +fn parent_event_id() -> Option { + let value = std::env::var(PARENT_EVENT_ENV).ok()?; + let value = value.trim(); + is_uuid(value).then(|| value.to_string()) +} + pub fn set_error_code(code: &str) { with_run(|run| run.error_code = Some(clip(code, MAX_CODE))); } @@ -474,13 +546,15 @@ fn build(run: &Run, exit_code: Option) -> Event { event: EVENT, version: SCHEMA_VERSION, created: timestamp(SystemTime::now()), - event_id: uuid_v4(rand::random()), + event_id: event_id().to_string(), + parent_event_id: parent_event_id(), user_id: user_id(), sdk_identifier: SDK_IDENTIFIER, sdk_version: CURRENT, operating_system: std::env::consts::OS, command: run.command.clone(), invocation: run.invocation, + workflow: run.workflow.clone(), params: run.params.clone(), usage_error: run.usage_error.clone(), output: options.as_ref().map(|o| o.output), @@ -908,6 +982,25 @@ fn timestamp(at: SystemTime) -> String { mod tests { use super::*; + #[test] + fn a_custom_workflow_never_records_its_name() { + assert_eq!( + workflow_field(WorkflowSource::Custom, Some("acme-client-export")), + Workflow { + source: "custom", + name: None + } + ); + assert_eq!( + workflow_field(WorkflowSource::Marketplace, Some("style-clone")), + Workflow { + source: "marketplace", + name: Some("style-clone".to_string()) + } + ); + assert_eq!(workflow_field(WorkflowSource::Builtin, None).name, None); + } + #[test] fn a_uuid_is_version_4_and_well_formed() { let id = uuid_v4([0xff; 16]); diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index 05cf75d..df2498b 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -231,3 +231,27 @@ fn completion_records_nothing() { config_dir(&home).display() ); } + +#[test] +fn a_run_started_by_a_workflow_step_records_its_parent() { + let parent = "5f0c1e9a-7b2d-4c1e-9f3a-2d8e6b1a0c47"; + let home = scratch("parent"); + let out = command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", parent) + .args(["config", "list"]) + .output() + .expect("run mapbox"); + assert!(out.status.success()); + // Anything that isn't an event id is ignored rather than recorded. + let _ = command(&home) + .env("MAPBOX_CLI_PARENT_EVENT", "/Users/someone/secret") + .args(["config", "list"]) + .output() + .expect("run mapbox"); + + let events = events(&home); + assert_eq!(events.len(), 2); + assert_eq!(events[0]["parentEventId"], parent); + assert_ne!(events[0]["eventId"], parent); + assert!(events[1].get("parentEventId").is_none(), "{:?}", events[1]); +} From 11faaae2f39460310f785dd4a9eebd2bb66567db Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Fri, 25 Sep 2026 12:42:11 +0300 Subject: [PATCH 6/7] Record workflow steps in the telemetry event workflow gains stepCount and up to 20 steps. A step is either a mapbox command, recorded by the names the command tree has plus its exit and error code, or one of the user's own scripts, recorded only by how it exited and how long it took. The two constructors are the only way to build a step, so a script step cannot carry a command name or an error category. --- src/telemetry_event.rs | 177 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 165 insertions(+), 12 deletions(-) diff --git a/src/telemetry_event.rs b/src/telemetry_event.rs index 9c65e26..1cb340c 100644 --- a/src/telemetry_event.rs +++ b/src/telemetry_event.rs @@ -60,6 +60,7 @@ const MAX_COMMAND_NAME: usize = 32; const MAX_REQUEST_IDS: usize = 5; const MAX_REQUEST_ID: usize = 128; const MAX_CODE: usize = 64; +const MAX_STEPS: usize = 20; const MAX_VERSION: usize = 32; /// Options that are top-level fields or `invocation`, so never `params`. @@ -159,10 +160,32 @@ impl WorkflowSource { } #[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] struct Workflow { source: &'static str, #[serde(skip_serializing_if = "Option::is_none")] name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + step_count: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + steps: Vec, +} + +/// One step of a workflow run. Built only by [`cli_step`] and +/// [`script_step`], so a script step can never carry a command name or an +/// error category: nothing about a user's script is recorded beyond how it +/// exited and how long it took. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +struct Step { + kind: &'static str, + #[serde(skip_serializing_if = "Option::is_none")] + command: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + exit_code: Option, + #[serde(skip_serializing_if = "Option::is_none")] + error_code: Option, + duration_ms: u64, } #[derive(Debug, Clone, PartialEq, Serialize)] @@ -447,6 +470,78 @@ fn workflow_field(source: WorkflowSource, name: Option<&str>) -> Workflow { Workflow { source: source.as_str(), name, + step_count: None, + steps: Vec::new(), + } +} + +/// How many steps the running workflow has, which can be more than the +/// [`MAX_STEPS`] recorded. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn set_workflow_step_count(count: usize) { + with_run(|run| { + if let Some(workflow) = run.workflow.as_mut() { + workflow.step_count = Some(u32::try_from(count).unwrap_or(u32::MAX)); + } + }); +} + +/// A step that ran a `mapbox` command. `words` is the step's command line; +/// only the names the command tree has are kept, so an argument can't pass +/// for a command name. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) { + let step = cli_step(app, words, exit_code, error_code, duration); + with_run(|run| push_step(run, step)); +} + +/// A step that ran one of the user's own scripts. +#[allow(dead_code)] // Called by the workflow commands, which don't exist yet. +pub(crate) fn add_script_step(exit_code: Option, duration: Duration) { + let step = script_step(exit_code, duration); + with_run(|run| push_step(run, step)); +} + +fn cli_step( + app: &Command, + words: &[&str], + exit_code: Option, + error_code: Option<&str>, + duration: Duration, +) -> Step { + let path = tree_path(app, words.iter().map(|word| word.to_string())); + Step { + kind: "cli", + command: (!path.is_empty()).then(|| clip_command(path)), + exit_code, + error_code: error_code.map(|code| clip(code, MAX_CODE)), + duration_ms: duration.as_millis() as u64, + } +} + +fn script_step(exit_code: Option, duration: Duration) -> Step { + Step { + kind: "script", + command: None, + exit_code, + error_code: None, + duration_ms: duration.as_millis() as u64, + } +} + +/// Steps belong to a workflow: one reported before [`set_workflow`] has +/// nowhere to go and is dropped. +fn push_step(run: &mut Run, step: Step) { + if let Some(workflow) = run.workflow.as_mut() { + if workflow.steps.len() < MAX_STEPS { + workflow.steps.push(step); + } } } @@ -654,13 +749,25 @@ fn clip_command(path: Vec) -> Vec { /// subcommand of the one before. Flags and their values are skipped; the /// first word that is neither ends the walk. fn command_from_argv(app: &Command, argv: &[std::ffi::OsString]) -> Vec { + tree_path( + app, + argv.iter() + .skip(1) + .map(|word| word.to_string_lossy().into_owned()), + ) +} + +/// Subcommand names from `words`, in order, for as long as each word names +/// a subcommand of the one before. Flags and their values are skipped; the +/// first word that is neither ends the walk. +fn tree_path(app: &Command, words: impl IntoIterator) -> Vec { let mut path = vec![]; let mut command = app; - for word in argv.iter().skip(1).map(|w| w.to_string_lossy()) { + for word in words { if word.starts_with('-') { continue; } - match command.find_subcommand(word.as_ref()) { + match command.find_subcommand(&word) { Some(found) => { path.push(found.get_name().to_string()); if found.get_name() == TILESETS { @@ -982,21 +1089,67 @@ fn timestamp(at: SystemTime) -> String { mod tests { use super::*; + #[test] + fn a_script_step_records_only_how_it_exited_and_how_long_it_took() { + let step = script_step(Some(3), Duration::from_millis(2300)); + assert_eq!( + serde_json::to_value(&step).unwrap(), + serde_json::json!({ "kind": "script", "exitCode": 3, "durationMs": 2300 }) + ); + } + + #[test] + fn a_cli_step_keeps_only_names_the_command_tree_has() { + let app = Command::new("mapbox") + .subcommand(Command::new("styles").subcommand(Command::new("get"))); + let step = cli_step( + &app, + &["styles", "get", "my-secret-style"], + Some(0), + None, + Duration::ZERO, + ); + assert_eq!( + step.command, + Some(vec!["styles".to_string(), "get".to_string()]) + ); + + let unknown = cli_step( + &app, + &["/Users/someone/run.sh"], + Some(1), + Some("error"), + Duration::ZERO, + ); + assert_eq!(unknown.command, None); + assert_eq!(unknown.error_code.as_deref(), Some("error")); + } + + #[test] + fn steps_need_a_workflow_and_stop_at_the_bound() { + let mut run = Run::new(); + push_step(&mut run, script_step(Some(0), Duration::ZERO)); + assert!( + run.workflow.is_none(), + "a step without a workflow is dropped" + ); + + run.workflow = Some(workflow_field(WorkflowSource::Builtin, Some("style-clone"))); + for _ in 0..MAX_STEPS + 5 { + push_step(&mut run, script_step(Some(0), Duration::ZERO)); + } + assert_eq!(run.workflow.unwrap().steps.len(), MAX_STEPS); + } + #[test] fn a_custom_workflow_never_records_its_name() { assert_eq!( - workflow_field(WorkflowSource::Custom, Some("acme-client-export")), - Workflow { - source: "custom", - name: None - } + workflow_field(WorkflowSource::Custom, Some("acme-client-export")).name, + None ); assert_eq!( - workflow_field(WorkflowSource::Marketplace, Some("style-clone")), - Workflow { - source: "marketplace", - name: Some("style-clone".to_string()) - } + workflow_field(WorkflowSource::Marketplace, Some("style-clone")).name, + Some("style-clone".to_string()) ); assert_eq!(workflow_field(WorkflowSource::Builtin, None).name, None); } From bc5fadf04b913d02f3f88ca2981d21692af289db Mon Sep 17 00:00:00 2001 From: Mofei Zhu <13761509829@163.com> Date: Fri, 25 Sep 2026 12:51:28 +0300 Subject: [PATCH 7/7] Never create or chmod the config directory to record telemetry main's auth profile tests hold read-only commands to leaving ~/.mapbox alone: not creating it, not changing its permissions. The file sink called auth::config_dir on every run, which does both. It now writes only when the config directory already exists, and creates nothing but its own .telemetry inside it. That also restores tests/non_interactive.rs and tests/completion.rs to main's versions. doctor.rs, new on main, sent a request directly; it now goes through http::send like every other. --- src/doctor.rs | 6 +----- src/telemetry_sink.rs | 18 ++++++++++++------ tests/completion.rs | 14 ++------------ tests/non_interactive.rs | 19 +++---------------- tests/telemetry_events.rs | 19 +++++++++++++++++-- 5 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/doctor.rs b/src/doctor.rs index 0350178..790a9f8 100644 --- a/src/doctor.rs +++ b/src/doctor.rs @@ -312,11 +312,7 @@ impl ConnectivityReport { /// of this crate uses `Option` for, and what the docs promise. fn check(debug: bool, host: &str, timeout: Duration) -> Self { let outcome = http::client().and_then(|client| { - client - .get(host) - .timeout(timeout) - .send() - .map_err(anyhow::Error::from) + http::send(client.get(host).timeout(timeout)).map_err(anyhow::Error::from) }); match outcome { diff --git a/src/telemetry_sink.rs b/src/telemetry_sink.rs index 1dc6694..a393859 100644 --- a/src/telemetry_sink.rs +++ b/src/telemetry_sink.rs @@ -10,7 +10,8 @@ //! [`HttpSink`] is the interface for sending them, not implemented yet. //! //! It also owns the directory, `~/.mapbox/.telemetry` (or under -//! `$MAPBOX_CONFIG_DIR`), including the two small state files the event +//! `$MAPBOX_CONFIG_DIR`), written only once the config directory exists, +//! including the two small state files the event //! reads — the installation id and the last version seen — so that every //! write and delete under it is in this file. @@ -121,12 +122,17 @@ pub(crate) fn replace_state(name: &str, contents: &str) { } } -/// The directory, created `0700` inside a config directory `auth` has -/// created and hardened, as it would for credentials. `None` when it -/// cannot be. +/// The directory, created `0700` inside the config directory — only when +/// that already exists. Recording must not be what creates `~/.mapbox` or +/// changes its permissions: read-only commands promise to leave it alone +/// (`tests/auth_profiles.rs`, `tests/non_interactive.rs`). `None` otherwise, +/// and nothing is written. fn dir() -> Option { - auth::config_dir().ok()?; - let dir = auth::config_dir_path()?.join(DIR); + let config = auth::config_dir_path()?; + if !config.is_dir() { + return None; + } + let dir = config.join(DIR); std::fs::create_dir_all(&dir).ok()?; #[cfg(unix)] { diff --git a/tests/completion.rs b/tests/completion.rs index 73a4d80..2c5dc1f 100644 --- a/tests/completion.rs +++ b/tests/completion.rs @@ -354,21 +354,11 @@ fn a_missing_or_unknown_shell_is_a_usage_error() { /// refresh round-trip would be unusable. #[test] fn it_needs_no_token_and_touches_no_credentials() { - // A home of its own: every other test here shares `sandbox_home`, and - // any of them — a usage error, `--schema` — writes a telemetry event - // into its `.mapbox` while this one is looking. - let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("completion-home-untouched"); + let home = sandbox_home(); let config = home.join(".mapbox"); let _ = std::fs::remove_dir_all(&config); - std::fs::create_dir_all(&home).expect("create the home"); - let out = command() - .env("HOME", &home) - .env("XDG_CONFIG_HOME", home.join(".config")) - .env("MAPBOX_CONFIG_DIR", &config) - .args(["completion", "zsh"]) - .output() - .expect("run mapbox"); + let out = run(&["completion", "zsh"]); assert!(out.status.success(), "{}", stderr(&out)); assert!( !config.exists(), diff --git a/tests/non_interactive.rs b/tests/non_interactive.rs index 207531d..bdb02f6 100644 --- a/tests/non_interactive.rs +++ b/tests/non_interactive.rs @@ -165,8 +165,7 @@ fn the_refusal_carries_a_code_and_a_fix() { } /// The check runs before `config_dir`, which creates the store as a side -/// effect. A CI job that tried to log in should leave nothing behind but -/// the run's telemetry event, which every command writes. +/// effect. A CI job that tried to log in should leave nothing behind. #[test] fn the_refusal_creates_no_credential_directory() { let home = scratch("no-dir"); @@ -176,21 +175,9 @@ fn the_refusal_creates_no_credential_directory() { .expect("run mapbox"); assert!(!out.status.success()); - let left: Vec = std::fs::read_dir(home.join(".mapbox")) - .map(|entries| { - entries - .map(|e| { - e.expect("an entry") - .file_name() - .to_string_lossy() - .into_owned() - }) - .collect() - }) - .unwrap_or_default(); assert!( - left.iter().all(|name| name == ".telemetry"), - "the refusal created the credential store anyway: {left:?}" + !home.join(".mapbox").exists(), + "the refusal created the credential store anyway" ); } diff --git a/tests/telemetry_events.rs b/tests/telemetry_events.rs index df2498b..2b28cf3 100644 --- a/tests/telemetry_events.rs +++ b/tests/telemetry_events.rs @@ -19,7 +19,9 @@ const ADDRESS: &str = "1600 Pennsylvania Ave"; fn scratch(name: &str) -> PathBuf { let home = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join(format!("events-{name}")); let _ = std::fs::remove_dir_all(&home); - std::fs::create_dir_all(&home).expect("create the scratch home"); + // With `.mapbox` already there, as on any machine that has logged in: + // the file sink never creates it. + std::fs::create_dir_all(config_dir(&home)).expect("create the scratch config dir"); home } @@ -225,9 +227,22 @@ fn stdout_is_identical_with_telemetry_on_and_off() { fn completion_records_nothing() { let home = scratch("completion"); assert!(run(&home, &["completion", "zsh"]).status.success()); + assert!( + !config_dir(&home).join(".telemetry").exists(), + "`completion` wrote telemetry" + ); +} + +/// Recording never creates the config directory: a machine that has never +/// logged in or set a config keeps no `~/.mapbox` at all. +#[test] +fn without_a_config_directory_nothing_is_created() { + let home = scratch("no-config-dir"); + std::fs::remove_dir(config_dir(&home)).expect("remove the scratch config dir"); + assert!(run(&home, &["styles", "--help"]).status.success()); assert!( !config_dir(&home).exists(), - "`completion` created {}", + "recording created {}", config_dir(&home).display() ); }