Add a persisted telemetry setting to mapbox config

mapbox config set telemetry off stops the run's telemetry event in every
shell, beside MAPBOX_CLI_NO_TELEMETRY for one. The setting is read again
at exit, so the run that turns it off does not report itself.

Author: zmofei

CHANGELOG.md

@@ -260,6 +260,10 @@ 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 the telemetry event off for good,
+  in every shell, the way `MAPBOX_CLI_NO_TELEMETRY=1` does for one. The run
+  that turns it off records nothing either. `mapbox config list` now also
+  reports `telemetry`.
 - 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

@@ -295,7 +295,7 @@ it, and dropped if it can't be delivered. It is sent with your own token
 (`--token`, `MAPBOX_ACCESS_TOKEN` or your login) when you have one, and with
 a token built into the CLI otherwise. 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
 

docs/commands.md

@@ -4129,6 +4129,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` | The run's telemetry event; mirrors `MAPBOX_CLI_NO_TELEMETRY` (see [Privacy](../README.md#privacy)) |
 
 ### `mapbox config get`
 
@@ -4140,7 +4141,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
 
@@ -4179,7 +4180,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
@@ -4238,6 +4239,7 @@ mapbox config list
 update-check  on
 history       on
 log           off
+telemetry     on
 ```
 
 </td><td>
@@ -4255,6 +4257,10 @@ log           off
   {
     "key": "log",
     "value": false
+  },
+  {
+    "key": "telemetry",
+    "value": true
   }
 ]
 ```
@@ -4273,7 +4279,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
 

src/config.rs

@@ -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";
@@ -50,6 +51,8 @@ struct Config {
     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>,
 }
 
 fn config_path() -> Option<PathBuf> {
@@ -102,6 +105,12 @@ pub fn log_enabled() -> bool {
     read_config().log.unwrap_or(false)
 }
 
+/// Whether the run's telemetry event may be recorded, per the persisted
+/// setting. [`crate::telemetry_event`] checks it 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 +127,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 +142,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 +225,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)?;

src/telemetry_event.rs

@@ -18,7 +18,8 @@
 //!
 //! 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 built or written.
+//! (`MAPBOX_CLI_NO_TELEMETRY`, or `mapbox config set telemetry off`),
+//! nothing is built or written.
 
 use std::io::IsTerminal;
 use std::path::Path;
@@ -332,7 +333,12 @@ fn with_workflow(f: impl FnOnce(&mut Option<Workflow>)) {
 /// of them — a fresh `userId` each time. Skipped for the same reason
 /// `run_history` skips it (aws/aws-cli#10031).
 pub(crate) fn deliver(record: &Record) {
-    if !telemetry::telemetry_allowed() || std::env::var_os("SUDO_USER").is_some() {
+    // The config setting is read at the end of the run: the run that turns
+    // telemetry off is one that should not report itself.
+    if !telemetry::telemetry_allowed()
+        || !crate::config::telemetry_enabled()
+        || std::env::var_os("SUDO_USER").is_some()
+    {
         return;
     }
     let event = build(record);

tests/config.rs

@@ -138,7 +138,7 @@ fn list_reports_every_setting_including_an_unset_one() {
     assert!(empty.status.success());
     assert_eq!(
         stdout(&empty),
-        r#"[{"key":"update-check","value":true},{"key":"history","value":true},{"key":"log","value":false}]"#
+        r#"[{"key":"update-check","value":true},{"key":"history","value":true},{"key":"log","value":false},{"key":"telemetry","value":true}]"#
     );
 
     let set = command(&home)
@@ -154,7 +154,7 @@ fn list_reports_every_setting_including_an_unset_one() {
     assert!(after.status.success());
     assert_eq!(
         stdout(&after),
-        r#"[{"key":"update-check","value":false},{"key":"history","value":true},{"key":"log","value":false}]"#
+        r#"[{"key":"update-check","value":false},{"key":"history","value":true},{"key":"log","value":false},{"key":"telemetry","value":true}]"#
     );
 
     let text = command(&home)
@@ -164,7 +164,7 @@ fn list_reports_every_setting_including_an_unset_one() {
     assert!(text.status.success());
     assert_eq!(
         stdout(&text),
-        "update-check  off\nhistory       on\nlog           off"
+        "update-check  off\nhistory       on\nlog           off\ntelemetry     on"
     );
 }
 

tests/telemetry_events.rs

@@ -201,7 +201,7 @@ fn help_version_and_usage_errors_record_their_invocation() {
 }
 
 #[test]
-fn the_opt_out_records_nothing() {
+fn either_opt_out_records_nothing() {
     let home = scratch("opt-out-env");
     let out = command(&home)
         .env("MAPBOX_CLI_NO_TELEMETRY", "1")
@@ -213,6 +213,28 @@ fn the_opt_out_records_nothing() {
         !config_dir(&home).join(".telemetry").exists(),
         "MAPBOX_CLI_NO_TELEMETRY=1 still wrote telemetry"
     );
+
+    // With a token and somewhere to send, so that nothing arriving is the
+    // setting's doing.
+    let home = scratch("opt-out-config");
+    let (received, url) = events_server(true);
+    let with_a_token = |args: &[&str]| {
+        command(&home)
+            .env("MAPBOX_INTERNAL_TELEMETRY_URL", &url)
+            .env("MAPBOX_CLI_TOKEN", "pk.cli")
+            .args(args)
+            .output()
+            .expect("run mapbox")
+    };
+    assert!(with_a_token(&["config", "set", "telemetry", "off"])
+        .status
+        .success());
+    // That run sends nothing either: the setting it wrote is read at exit.
+    let _ = with_a_token(&["config", "list"]);
+    assert!(
+        received.recv_timeout(Duration::from_secs(3)).is_err(),
+        "`telemetry off` still sent an event"
+    );
 }
 
 #[test]