Remove extension telemetry setting and use VS Code setting instead

Author: koesie10

extensions/ql-vscode/package.json

@@ -497,16 +497,6 @@
         "title": "Telemetry",
         "order": 11,
         "properties": {
-          "codeQL.telemetry.enableTelemetry": {
-            "type": "boolean",
-            "default": false,
-            "scope": "application",
-            "markdownDescription": "Specifies whether to send CodeQL usage telemetry. This setting AND the one of the global telemetry settings (`#telemetry.enableTelemetry#` or `#telemetry.telemetryLevel#`) must be enabled for telemetry to be sent to GitHub. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code)",
-            "tags": [
-              "telemetry",
-              "usesOnlineServices"
-            ]
-          },
           "codeQL.telemetry.logTelemetry": {
             "type": "boolean",
             "default": false,

extensions/ql-vscode/src/common/vscode/dialog.ts

@@ -1,4 +1,4 @@
-import { env, Uri, window } from "vscode";
+import { window } from "vscode";
 
 /**
  * Opens a modal dialog for the user to make a yes/no choice.
@@ -34,50 +34,6 @@ export async function showBinaryChoiceDialog(
   return chosenItem?.title === yesItem.title;
 }
 
-/**
- * Opens a modal dialog for the user to make a yes/no choice.
- *
- * @param message The message to show.
- * @param modal If true (the default), show a modal dialog box, otherwise dialog is non-modal and can
- *        be closed even if the user does not make a choice.
- *
- * @return
- *  `true` if the user clicks 'Yes',
- *  `false` if the user clicks 'No' or cancels the dialog,
- *  `undefined` if the dialog is closed without the user making a choice.
- */
-export async function showBinaryChoiceWithUrlDialog(
-  message: string,
-  url: string,
-): Promise<boolean | undefined> {
-  const urlItem = { title: "More Information", isCloseAffordance: false };
-  const yesItem = { title: "Yes", isCloseAffordance: false };
-  const noItem = { title: "No", isCloseAffordance: true };
-  let chosenItem;
-
-  // Keep the dialog open as long as the user is clicking the 'more information' option.
-  // To prevent an infinite loop, if the user clicks 'more information' 5 times, close the dialog and return cancelled
-  let count = 0;
-  do {
-    chosenItem = await window.showInformationMessage(
-      message,
-      { modal: true },
-      urlItem,
-      yesItem,
-      noItem,
-    );
-    if (chosenItem === urlItem) {
-      await env.openExternal(Uri.parse(url, true));
-    }
-    count++;
-  } while (chosenItem === urlItem && count < 5);
-
-  if (!chosenItem || chosenItem.title === urlItem.title) {
-    return undefined;
-  }
-  return chosenItem.title === yesItem.title;
-}
-
 /**
  * Show an information message with a customisable action.
  * @param message The message to show.

extensions/ql-vscode/src/common/vscode/telemetry.ts

@@ -1,26 +1,14 @@
-import type {
-  Extension,
-  ExtensionContext,
-  ConfigurationChangeEvent,
-} from "vscode";
-import { ConfigurationTarget, env } from "vscode";
+import type { Extension, ExtensionContext } from "vscode";
 import TelemetryReporter from "vscode-extension-telemetry";
-import {
-  ConfigListener,
-  CANARY_FEATURES,
-  ENABLE_TELEMETRY,
-  LOG_TELEMETRY,
-  isIntegrationTestMode,
-  isCanary,
-} from "../../config";
+import { LOG_TELEMETRY, isCanary } from "../../config";
 import type { TelemetryClient } from "applicationinsights";
 import { extLogger } from "../logging/vscode";
 import { UserCancellationException } from "./progress";
-import { showBinaryChoiceWithUrlDialog } from "./dialog";
 import type { RedactableError } from "../errors";
 import type { SemVer } from "semver";
 import type { AppTelemetry } from "../telemetry";
 import type { EnvelopeTelemetry } from "applicationinsights/out/Declarations/Contracts";
+import type { Disposable } from "../disposable-object";
 
 // Key is injected at build time through the APP_INSIGHTS_KEY environment variable.
 const key = "REPLACE-APP-INSIGHTS-KEY";
@@ -55,80 +43,25 @@ const baseDataPropertiesToRemove = [
 
 const NOT_SET_CLI_VERSION = "not-set";
 
-export class ExtensionTelemetryListener
-  extends ConfigListener
-  implements AppTelemetry
-{
-  private reporter?: TelemetryReporter;
+export class ExtensionTelemetryListener implements AppTelemetry, Disposable {
+  private readonly reporter: TelemetryReporter;
 
   private cliVersionStr = NOT_SET_CLI_VERSION;
 
-  constructor(
-    private readonly id: string,
-    private readonly version: string,
-    private readonly key: string,
-    private readonly ctx: ExtensionContext,
-  ) {
-    super();
-
-    env.onDidChangeTelemetryEnabled(async () => {
-      await this.initialize();
-    });
-  }
-
-  /**
-   * This function handles changes to relevant configuration elements. There are 2 configuration
-   * ids that this function cares about:
-   *
-   *     * `codeQL.telemetry.enableTelemetry`: If this one has changed, then we need to re-initialize
-   *        the reporter and the reporter may wind up being removed.
-   *     * `codeQL.canary`: A change here could possibly re-trigger a dialog popup.
-   *
-   * Note that the global telemetry setting also gate-keeps whether or not to send telemetry events
-   * to Application Insights. However, this gatekeeping happens inside of the vscode-extension-telemetry
-   * package. So, this does not need to be handled here.
-   *
-   * @param e the configuration change event
-   */
-  async handleDidChangeConfiguration(
-    e: ConfigurationChangeEvent,
-  ): Promise<void> {
-    if (e.affectsConfiguration(ENABLE_TELEMETRY.qualifiedName)) {
-      await this.initialize();
-    }
-
-    // Re-request telemetry so that users can see the dialog again.
-    // Re-request if codeQL.canary is being set to `true` and telemetry
-    // is not currently enabled.
-    if (
-      e.affectsConfiguration(CANARY_FEATURES.qualifiedName) &&
-      CANARY_FEATURES.getValue() &&
-      !ENABLE_TELEMETRY.getValue()
-    ) {
-      await this.setTelemetryRequested(false);
-      await this.requestTelemetryPermission();
-    }
-  }
-
-  async initialize() {
-    await this.requestTelemetryPermission();
-
-    this.disposeReporter();
-
-    if (ENABLE_TELEMETRY.getValue<boolean>()) {
-      this.createReporter();
-    }
-  }
-
-  private createReporter() {
+  constructor(id: string, version: string, key: string) {
+    // We can always initialize this and send events using it because the vscode-extension-telemetry will check
+    // whether the `telemetry.telemetryLevel` setting is enabled.
     this.reporter = new TelemetryReporter(
-      this.id,
-      this.version,
-      this.key,
+      id,
+      version,
+      key,
       /* anonymize stack traces */ true,
     );
-    this.push(this.reporter);
 
+    this.addTelemetryProcessor();
+  }
+
+  private addTelemetryProcessor() {
     // The appInsightsClient field is private but we want to access it anyway
     const client = this.reporter["appInsightsClient"] as TelemetryClient;
     if (client) {
@@ -151,14 +84,10 @@ export class ExtensionTelemetryListener
   }
 
   dispose() {
-    super.dispose();
-    void this.reporter?.dispose();
+    void this.reporter.dispose();
   }
 
   sendCommandUsage(name: string, executionTime: number, error?: Error): void {
-    if (!this.reporter) {
-      return;
-    }
     const status = !error
       ? CommandCompletion.Success
       : error instanceof UserCancellationException
@@ -178,10 +107,6 @@ export class ExtensionTelemetryListener
   }
 
   sendUIInteraction(name: string): void {
-    if (!this.reporter) {
-      return;
-    }
-
     this.reporter.sendTelemetryEvent(
       "ui-interaction",
       {
@@ -197,10 +122,6 @@ export class ExtensionTelemetryListener
     error: RedactableError,
     extraProperties?: { [key: string]: string },
   ): void {
-    if (!this.reporter) {
-      return;
-    }
-
     const properties: { [key: string]: string } = {
       isCanary: isCanary().toString(),
       cliVersion: this.cliVersionStr,
@@ -215,10 +136,6 @@ export class ExtensionTelemetryListener
   }
 
   sendConfigInformation(config: Record<string, string>): void {
-    if (!this.reporter) {
-      return;
-    }
-
     this.reporter.sendTelemetryEvent(
       "config",
       {
@@ -230,37 +147,6 @@ export class ExtensionTelemetryListener
     );
   }
 
-  /**
-   * Displays a popup asking the user if they want to enable telemetry
-   * for this extension.
-   */
-  async requestTelemetryPermission() {
-    if (!this.wasTelemetryRequested()) {
-      // if global telemetry is disabled, avoid showing the dialog or making any changes
-      let result = undefined;
-      if (
-        env.isTelemetryEnabled &&
-        // Avoid showing the dialog if we are in integration test mode.
-        !isIntegrationTestMode()
-      ) {
-        // Extension won't start until this completes.
-        result = await showBinaryChoiceWithUrlDialog(
-          "Does the CodeQL Extension by GitHub have your permission to collect usage data and metrics to help us improve CodeQL for VSCode?",
-          "https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code",
-        );
-      }
-      if (result !== undefined) {
-        await Promise.all([
-          this.setTelemetryRequested(true),
-          ENABLE_TELEMETRY.updateValue<boolean>(
-            result,
-            ConfigurationTarget.Global,
-          ),
-        ]);
-      }
-    }
-  }
-
   /**
    * Exposed for testing
    */
@@ -271,21 +157,6 @@ export class ExtensionTelemetryListener
   set cliVersion(version: SemVer | undefined) {
     this.cliVersionStr = version ? version.toString() : NOT_SET_CLI_VERSION;
   }
-
-  private disposeReporter() {
-    if (this.reporter) {
-      void this.reporter.dispose();
-      this.reporter = undefined;
-    }
-  }
-
-  private wasTelemetryRequested(): boolean {
-    return !!this.ctx.globalState.get<boolean>("telemetry-request-viewed");
-  }
-
-  private async setTelemetryRequested(newValue: boolean): Promise<void> {
-    await this.ctx.globalState.update("telemetry-request-viewed", newValue);
-  }
 }
 
 /**
@@ -305,11 +176,7 @@ export async function initializeTelemetry(
     extension.id,
     extension.packageJSON.version,
     key,
-    ctx,
   );
-  // do not await initialization, since doing so will sometimes cause a modal popup.
-  // this is a particular problem during integration tests, which will hang if a modal popup is displayed.
-  void telemetryListener.initialize();
   ctx.subscriptions.push(telemetryListener);
   return telemetryListener;
 }

extensions/ql-vscode/src/config.ts

@@ -165,10 +165,6 @@ const ROOT_SETTING = new Setting("codeQL");
 const TELEMETRY_SETTING = new Setting("telemetry", ROOT_SETTING);
 
 export const LOG_TELEMETRY = new Setting("logTelemetry", TELEMETRY_SETTING);
-export const ENABLE_TELEMETRY = new Setting(
-  "enableTelemetry",
-  TELEMETRY_SETTING,
-);
 
 // Distribution configuration
 const DISTRIBUTION_SETTING = new Setting("cli", ROOT_SETTING);

extensions/ql-vscode/test/e2e/docker/User/settings.json

@@ -1,6 +1,5 @@
 {
   "workbench.startupEditor": "none",
   "security.workspace.trust.enabled": false,
-  "codeQL.cli.executablePath": "/opt/codeql/codeql",
-  "codeQL.telemetry.enableTelemetry": false
+  "codeQL.cli.executablePath": "/opt/codeql/codeql"
 }