Merge pull request #47 from mapbox/telemetry-config

Add mapbox config set telemetry off

Author: zmofei

CHANGELOG.md

@@ -269,6 +269,15 @@ that may never merge. They are not releases and are not listed here.
   code; it does find a `~/.mapbox/history` directory it didn't before, and
   `mapbox config list` now reports a second key, `history`.
 
+- `mapbox config set telemetry off` turns telemetry off for good, in every
+  shell, the way `MAPBOX_CLI_NO_TELEMETRY=1` does for one: the run's event,
+  the `User-Agent` markers and the update notice. The run that turns it off
+  records nothing either. `mapbox config list` now also reports `telemetry`,
+  and `mapbox doctor` reports it as `telemetry_persisted`; its
+  `telemetry_allowed` still means the environment variable alone. Version
+  0.3.0 doesn't know this key, and its `mapbox config set` drops it from the
+  file, so with two installs, set it again after using the older one.
+
 - Each run sends one `cli.command` telemetry event to Mapbox, from a
   background process the command doesn't wait for, with your own token
   (`--token`, `MAPBOX_ACCESS_TOKEN` or your login) or, when you have none,

README.md

@@ -282,8 +282,9 @@ deeper where that reads better, as in `mapbox styles draft get`.
 parameters and sample output.
 
 Every API request a command makes sends `User-Agent: mapbox-cli/<version>`
-and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1`
-keeps even future markers out of that header.
+and nothing else about you or your machine. `MAPBOX_CLI_NO_TELEMETRY=1` or
+`mapbox config set telemetry off` keeps even future markers out of that
+header.
 
 Each run also sends one event to Mapbox:
 
@@ -305,7 +306,7 @@ or your login) when you have one, and with a token built into the CLI
 otherwise; Mapbox Events keeps the token an event was sent with, and the
 account it belongs to, alongside the event. A build from source has no
 built-in token, so with no token of your own the event is dropped.
-`MAPBOX_CLI_NO_TELEMETRY=1` turns it off.
+`MAPBOX_CLI_NO_TELEMETRY=1` or `mapbox config set telemetry off` turns it off.
 
 ### Diagnostics and settings
 
@@ -572,7 +573,7 @@ kept narrow:
 | What it sends | A `GET` for the channel's `latest/manifest.json`, with no token, no account, no command, and nothing about you or your machine beyond `User-Agent: mapbox-cli/<version>` |
 | When | At most once a day, and only when stderr is a terminal, so CI and piped runs never check and never print |
 | Where | A detached background process. Your command never waits on it: offline, the timing is unchanged and nothing is printed |
-| Off | `MAPBOX_NO_UPDATE_CHECK=1`, or `MAPBOX_CLI_NO_TELEMETRY=1`, which silences this too, for the shell session it's set in |
+| Off | `MAPBOX_NO_UPDATE_CHECK=1`, or `MAPBOX_CLI_NO_TELEMETRY=1`, which silences this too, for the shell session it's set in; `mapbox config set telemetry off` silences it for good |
 
 `~/.mapbox/update-check.json` (or `$MAPBOX_CONFIG_DIR`) holds the answer
 between runs. A build that names no release channel never checks at all, and
@@ -601,7 +602,7 @@ mapbox history show be40d711    # or one run, by any prefix of its id
 are not recorded. `mapbox config set history off` turns history off for
 good, and `MAPBOX_HISTORY=0` for one shell; with it off, nothing is written
 and no directory is created, but what was already recorded stays until you
-delete `~/.mapbox/history`. `MAPBOX_CLI_NO_TELEMETRY` does not affect it.
+delete `~/.mapbox/history`. Neither telemetry opt-out affects it.
 
 ### Diagnostic logs
 
@@ -669,6 +670,12 @@ our CLIs by setting
 MAPBOX_CLI_NO_TELEMETRY=1
 ```
 
+or, to turn it off in every shell,
+
+```sh
+mapbox config set telemetry off
+```
+
 For additional information on our data processing activities and your
 related rights, please see our Mapbox
 [Privacy Policy](https://www.mapbox.com/legal/privacy).

docs/commands.md

@@ -4265,6 +4265,7 @@ was set in, and stays in every future shell instead.
 | `update-check` | `on` | The update notice; mirrors `MAPBOX_NO_UPDATE_CHECK` (see [Update notices](../README.md#update-notices)) |
 | `history` | `on` | [Command history](../README.md#command-history), read by `mapbox history`; `MAPBOX_HISTORY=0` or `=1` overrides it for a session |
 | `log` | `off` | [Diagnostic logs](../README.md#diagnostic-logs), shown by `mapbox history show`; `MAPBOX_LOG=1` or `=0` overrides it for a session. Needs `history` on: `config set log on` with history off fails with `history_required` |
+| `telemetry` | `on` | Telemetry — the run's event, the `User-Agent` markers and the update notice; mirrors `MAPBOX_CLI_NO_TELEMETRY` (see [Privacy](../README.md#privacy)) |
 
 ### `mapbox config get`
 
@@ -4276,7 +4277,7 @@ than failing, the same forgiving read the update-check cache itself uses.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to read: `update-check`, `history` or `log`. |
+| `<key>` | Which setting to read: `update-check`, `history`, `log` or `telemetry`. |
 
 #### Examples
 
@@ -4315,7 +4316,7 @@ without an environment variable.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to change: `update-check`, `history` or `log`. |
+| `<key>` | Which setting to change: `update-check`, `history`, `log` or `telemetry`. |
 | `<value>` | `on` or `off`. |
 
 #### Examples
@@ -4374,6 +4375,7 @@ mapbox config list
 update-check  on
 history       on
 log           off
+telemetry     on
 ```
 
 </td><td>
@@ -4391,6 +4393,10 @@ log           off
   {
     "key": "log",
     "value": false
+  },
+  {
+    "key": "telemetry",
+    "value": true
   }
 ]
 ```
@@ -4409,7 +4415,7 @@ default, a key explicitly set to the old default value does not.
 
 | Parameter | Effect |
 | --- | --- |
-| `<key>` | Which setting to clear: `update-check`, `history` or `log`. |
+| `<key>` | Which setting to clear: `update-check`, `history`, `log` or `telemetry`. |
 
 #### Examples
 
@@ -4703,6 +4709,7 @@ Telemetry:     on
   "proxy": { "active": [] },
   "switches": {
     "telemetry_allowed": true,
+    "telemetry_persisted": true,
     "update_check_env_opt_out": false,
     "update_check_persisted": true
   },
@@ -4719,6 +4726,10 @@ Telemetry:     on
 </td></tr>
 </table>
 
+In `switches`, `telemetry_allowed` is `MAPBOX_CLI_NO_TELEMETRY` alone and
+`telemetry_persisted` is `mapbox config set telemetry`; telemetry is on only
+when both are `true`.
+
 With `--verify`, a `connectivity` object joins the JSON and a `Reachable:`
 line joins the text — `{ "reachable": true, "status": 200 }`, or `{
 "reachable": false }` (plus an `error` field under `--debug`) when the

src/config.rs

@@ -19,8 +19,8 @@ use std::path::PathBuf;
 use anyhow::{Context, Result};
 use clap::builder::PossibleValuesParser;
 use clap::{Arg, ArgMatches, Command};
-use serde::{Deserialize, Serialize};
-use serde_json::{json, Value};
+use serde::Serialize;
+use serde_json::{json, Map, Value};
 
 use crate::auth;
 use crate::output::{self, CliError, Mode};
@@ -33,7 +33,8 @@ const CONFIG_FILE: &str = "config.json";
 const UPDATE_CHECK_KEY: &str = "update-check";
 const HISTORY_KEY: &str = "history";
 const LOG_KEY: &str = "log";
-const KEYS: &[&str] = &[UPDATE_CHECK_KEY, HISTORY_KEY, LOG_KEY];
+const TELEMETRY_KEY: &str = "telemetry";
+const KEYS: &[&str] = &[UPDATE_CHECK_KEY, HISTORY_KEY, LOG_KEY, TELEMETRY_KEY];
 
 const ON: &str = "on";
 const OFF: &str = "off";
@@ -42,14 +43,43 @@ const OFF: &str = "off";
 /// default is `on` — distinct from `Some(true)`, which is someone turning it
 /// back on after having turned it off, but read identically by
 /// [`update_check_setting`].
-#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize)]
 struct Config {
     #[serde(default, skip_serializing_if = "Option::is_none")]
     update_check: Option<bool>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     history: Option<bool>,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     log: Option<bool>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    telemetry: Option<bool>,
+    /// Keys this version doesn't know, kept so that writing the file doesn't
+    /// erase a setting a newer version saved.
+    #[serde(flatten)]
+    rest: Map<String, Value>,
+}
+
+impl Config {
+    /// Reads each key on its own, so one malformed value resets only that
+    /// key. Parsed as a whole, a single bad value would reset every key,
+    /// `telemetry` included, and an opt-out would quietly turn back on.
+    fn parse(text: &str) -> Config {
+        let Ok(Value::Object(mut rest)) = serde_json::from_str(text) else {
+            return Config::default();
+        };
+        let mut flag = |key: &str| rest.remove(key).and_then(|value| value.as_bool());
+        let update_check = flag("update_check");
+        let history = flag("history");
+        let log = flag("log");
+        let telemetry = flag("telemetry");
+        Config {
+            update_check,
+            history,
+            log,
+            telemetry,
+            rest,
+        }
+    }
 }
 
 fn config_path() -> Option<PathBuf> {
@@ -64,7 +94,7 @@ fn config_path() -> Option<PathBuf> {
 fn read_config() -> Config {
     config_path()
         .and_then(|path| std::fs::read_to_string(path).ok())
-        .and_then(|text| serde_json::from_str(&text).ok())
+        .map(|text| Config::parse(&text))
         .unwrap_or_default()
 }
 
@@ -102,6 +132,12 @@ pub fn log_enabled() -> bool {
     read_config().log.unwrap_or(false)
 }
 
+/// Whether telemetry is on, per the persisted setting. Read through
+/// [`crate::telemetry::telemetry_allowed`], alongside `MAPBOX_CLI_NO_TELEMETRY`.
+pub fn telemetry_enabled() -> bool {
+    read_config().telemetry.unwrap_or(true)
+}
+
 fn on_off(enabled: bool) -> &'static str {
     if enabled {
         ON
@@ -118,6 +154,7 @@ fn resolve(config: &Config, key: &str) -> bool {
         UPDATE_CHECK_KEY => update_check_setting(config),
         HISTORY_KEY => config.history.unwrap_or(true),
         LOG_KEY => config.log.unwrap_or(false),
+        TELEMETRY_KEY => config.telemetry.unwrap_or(true),
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
 }
@@ -132,6 +169,7 @@ fn clear(config: &mut Config, key: &str) {
         UPDATE_CHECK_KEY => config.update_check = None,
         HISTORY_KEY => config.history = None,
         LOG_KEY => config.log = None,
+        TELEMETRY_KEY => config.telemetry = None,
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
 }
@@ -214,6 +252,7 @@ pub fn set(matches: &ArgMatches, mode: Mode) -> Result<()> {
         UPDATE_CHECK_KEY => config.update_check = Some(enabled),
         HISTORY_KEY => config.history = Some(enabled),
         LOG_KEY => config.log = Some(enabled),
+        TELEMETRY_KEY => config.telemetry = Some(enabled),
         _ => unreachable!("clap's value_parser restricts `key` to {KEYS:?}"),
     }
     write_config(&config)?;
@@ -273,15 +312,32 @@ mod tests {
         };
         let text = serde_json::to_string(&off).expect("serialize");
         assert_eq!(text, r#"{"update_check":false}"#);
-        let read: Config = serde_json::from_str(&text).expect("deserialize");
-        assert_eq!(read, off);
+        assert_eq!(Config::parse(&text), off);
 
         // A file from before this key existed, or one with nothing set yet.
-        let empty: Config = serde_json::from_str("{}").expect("an empty object");
+        let empty = Config::parse("{}");
         assert_eq!(empty.update_check, None);
         assert!(update_check_setting(&empty));
     }
 
+    #[test]
+    fn a_malformed_value_resets_only_its_own_key() {
+        let config = Config::parse(r#"{"telemetry":false,"history":"off"}"#);
+        assert_eq!(config.telemetry, Some(false));
+        assert_eq!(config.history, None);
+        assert_eq!(Config::parse("not json"), Config::default());
+    }
+
+    #[test]
+    fn keys_from_a_newer_version_survive_a_write() {
+        let mut config = Config::parse(r#"{"history":true,"future_key":[1]}"#);
+        config.history = Some(false);
+        assert_eq!(
+            serde_json::to_string(&config).expect("serialize"),
+            r#"{"history":false,"future_key":[1]}"#
+        );
+    }
+
     #[test]
     fn on_and_off_round_trip_through_on_off() {
         assert_eq!(on_off(true), ON);

src/doctor.rs

@@ -238,7 +238,11 @@ impl ProxyReport {
 
 #[derive(Serialize)]
 struct SwitchesReport {
+    /// `MAPBOX_CLI_NO_TELEMETRY` alone, as before `telemetry_persisted`
+    /// existed; a script that read it keeps reading the same fact.
     telemetry_allowed: bool,
+    /// `mapbox config set telemetry`; the event needs this and the above.
+    telemetry_persisted: bool,
     update_check_env_opt_out: bool,
     update_check_persisted: bool,
 }
@@ -248,7 +252,8 @@ impl SwitchesReport {
         let env_opted_out =
             std::env::var(update_check::NO_UPDATE_CHECK_ENV).is_ok_and(|v| !v.trim().is_empty());
         SwitchesReport {
-            telemetry_allowed: telemetry::telemetry_allowed(),
+            telemetry_allowed: telemetry::env_allows_telemetry(),
+            telemetry_persisted: config::telemetry_enabled(),
             update_check_env_opt_out: env_opted_out,
             update_check_persisted: config::update_check_enabled(),
         }
@@ -264,14 +269,19 @@ impl SwitchesReport {
     /// and `update_check_env_opt_out` alone, so `MAPBOX_CLI_NO_TELEMETRY=1`
     /// printed "Update check: on" for a check that would not run.
     fn update_check_on(&self) -> bool {
-        self.update_check_persisted && !self.update_check_env_opt_out && self.telemetry_allowed
+        self.update_check_persisted
+            && !self.update_check_env_opt_out
+            && self.telemetry_allowed
+            && self.telemetry_persisted
     }
 
     fn update_check_field(&self) -> (&'static str, String) {
         let reason = if self.update_check_env_opt_out {
             format!(" ({} is set)", update_check::NO_UPDATE_CHECK_ENV)
         } else if !self.telemetry_allowed {
             " (MAPBOX_CLI_NO_TELEMETRY silences this too)".to_string()
+        } else if !self.telemetry_persisted {
+            " (mapbox config set telemetry off silences this too)".to_string()
         } else if !self.update_check_persisted {
             " (mapbox config set update-check off)".to_string()
         } else {
@@ -282,8 +292,14 @@ impl SwitchesReport {
     }
 
     fn telemetry_field(&self) -> (&'static str, String) {
-        let state = if self.telemetry_allowed { "on" } else { "off" };
-        ("Telemetry:", state.to_string())
+        let value = if !self.telemetry_allowed {
+            "off (MAPBOX_CLI_NO_TELEMETRY is set)"
+        } else if !self.telemetry_persisted {
+            "off (mapbox config set telemetry off)"
+        } else {
+            "on"
+        };
+        ("Telemetry:", value.to_string())
     }
 }