From 52beaeded2003108b43bafc20bbfcd911594f067 Mon Sep 17 00:00:00 2001
From: Koen Vlaswinkel <koesie10@github.com>
Date: Wed, 6 Nov 2024 16:32:34 +0100
Subject: [PATCH 1/4] Remove extension telemetry setting and use VS Code
 setting instead

---
 extensions/ql-vscode/package.json             |  10 -
 .../ql-vscode/src/common/vscode/dialog.ts     |  46 +--
 .../ql-vscode/src/common/vscode/telemetry.ts  | 165 +---------
 extensions/ql-vscode/src/config.ts            |   4 -
 .../test/e2e/docker/User/settings.json        |   3 +-
 .../no-workspace/common/vscode/dialog.test.ts |  52 ---
 .../test/vscode-tests/no-workspace/index.ts   |   4 +-
 .../no-workspace/telemetry.test.ts            | 304 +-----------------
 8 files changed, 22 insertions(+), 566 deletions(-)

diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json
index c116c0c14ad..bd10a1f39d9 100644
--- a/extensions/ql-vscode/package.json
+++ b/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,
diff --git a/extensions/ql-vscode/src/common/vscode/dialog.ts b/extensions/ql-vscode/src/common/vscode/dialog.ts
index b6ad555fe0d..f580ea5f340 100644
--- a/extensions/ql-vscode/src/common/vscode/dialog.ts
+++ b/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.
diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts
index 451eff486a2..8bab7b38dde 100644
--- a/extensions/ql-vscode/src/common/vscode/telemetry.ts
+++ b/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;
 }
diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts
index 6df153d9ddb..cfabd9ea609 100644
--- a/extensions/ql-vscode/src/config.ts
+++ b/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);
diff --git a/extensions/ql-vscode/test/e2e/docker/User/settings.json b/extensions/ql-vscode/test/e2e/docker/User/settings.json
index 8dfd44b281c..bd40904834a 100644
--- a/extensions/ql-vscode/test/e2e/docker/User/settings.json
+++ b/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"
 }
diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/common/vscode/dialog.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/common/vscode/dialog.test.ts
index 895928b6368..c843787dda8 100644
--- a/extensions/ql-vscode/test/vscode-tests/no-workspace/common/vscode/dialog.test.ts
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/common/vscode/dialog.test.ts
@@ -1,7 +1,6 @@
 import { window } from "vscode";
 import {
   showBinaryChoiceDialog,
-  showBinaryChoiceWithUrlDialog,
   showInformationMessageWithAction,
   showNeverAskAgainDialog,
 } from "../../../../../src/common/vscode/dialog";
@@ -68,57 +67,6 @@ describe("showInformationMessageWithAction", () => {
   });
 });
 
-describe("showBinaryChoiceWithUrlDialog", () => {
-  let showInformationMessageSpy: jest.SpiedFunction<
-    typeof window.showInformationMessage
-  >;
-
-  beforeEach(() => {
-    showInformationMessageSpy = jest
-      .spyOn(window, "showInformationMessage")
-      .mockResolvedValue(undefined);
-  });
-
-  const resolveArg =
-    (index: number) =>
-    (...args: any[]) =>
-      Promise.resolve(args[index]);
-
-  it("should show a binary choice dialog with a url and return `yes`", async () => {
-    // pretend user clicks on the url twice and then clicks 'yes'
-    showInformationMessageSpy
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(3));
-    const val = await showBinaryChoiceWithUrlDialog("xxx", "invalid:url");
-    expect(val).toBe(true);
-  });
-
-  it("should show a binary choice dialog with a url and return `no`", async () => {
-    // pretend user clicks on the url twice and then clicks 'no'
-    showInformationMessageSpy
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(4));
-    const val = await showBinaryChoiceWithUrlDialog("xxx", "invalid:url");
-    expect(val).toBe(false);
-  });
-
-  it("should show a binary choice dialog and exit after clcking `more info` 5 times", async () => {
-    // pretend user clicks on the url twice and then clicks 'no'
-    showInformationMessageSpy
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2))
-      .mockImplementation(resolveArg(2));
-    const val = await showBinaryChoiceWithUrlDialog("xxx", "invalid:url");
-    // No choice was made
-    expect(val).toBeUndefined();
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(5);
-  });
-});
-
 describe("showNeverAskAgainDialog", () => {
   let showInformationMessageSpy: jest.SpiedFunction<
     typeof window.showInformationMessage
diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/index.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/index.ts
index 50f2deeccea..c39abbbaf72 100644
--- a/extensions/ql-vscode/test/vscode-tests/no-workspace/index.ts
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/index.ts
@@ -3,9 +3,7 @@ import type { ExtensionContext } from "vscode";
 export function createMockExtensionContext(): ExtensionContext {
   return {
     globalState: {
-      _state: {
-        "telemetry-request-viewed": true,
-      } as Record<string, any>,
+      _state: {} as Record<string, any>,
       get(key: string) {
         return this._state[key];
       },
diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts
index 5d797b78918..d1f6f525019 100644
--- a/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/telemetry.test.ts
@@ -1,14 +1,10 @@
 import TelemetryReporter from "vscode-extension-telemetry";
-import type { ExtensionContext } from "vscode";
-import { workspace, ConfigurationTarget, window, env } from "vscode";
+import { workspace, env } from "vscode";
 import {
   ExtensionTelemetryListener,
   telemetryListener as globalTelemetryListener,
 } from "../../../src/common/vscode/telemetry";
 import { UserCancellationException } from "../../../src/common/vscode/progress";
-import { ENABLE_TELEMETRY } from "../../../src/config";
-import { createMockExtensionContext } from "./index";
-import { vscodeGetConfigurationMock } from "../test-config";
 import { redactableError } from "../../../src/common/errors";
 import { SemVer } from "semver";
 
@@ -17,10 +13,7 @@ import { SemVer } from "semver";
 jest.setTimeout(10000);
 
 describe("telemetry reporting", () => {
-  let originalTelemetryExtension: boolean | undefined;
-  let originalTelemetryGlobal: string | undefined;
   let isCanary: string;
-  let ctx: ExtensionContext;
   let telemetryListener: ExtensionTelemetryListener;
 
   let sendTelemetryEventSpy: jest.SpiedFunction<
@@ -29,22 +22,8 @@ describe("telemetry reporting", () => {
   let sendTelemetryErrorEventSpy: jest.SpiedFunction<
     typeof TelemetryReporter.prototype.sendTelemetryErrorEvent
   >;
-  let disposeSpy: jest.SpiedFunction<
-    typeof TelemetryReporter.prototype.dispose
-  >;
-
-  let isTelemetryEnabledSpy: jest.SpyInstance<
-    typeof env.isTelemetryEnabled,
-    []
-  >;
-
-  let showInformationMessageSpy: jest.SpiedFunction<
-    typeof window.showInformationMessage
-  >;
 
   beforeEach(async () => {
-    vscodeGetConfigurationMock.mockRestore();
-
     try {
       // in case a previous test has accidentally activated this extension,
       // need to disable it first.
@@ -52,44 +31,24 @@ describe("telemetry reporting", () => {
       // specified in the package.json.
       globalTelemetryListener?.dispose();
 
-      ctx = createMockExtensionContext();
-
       sendTelemetryEventSpy = jest
         .spyOn(TelemetryReporter.prototype, "sendTelemetryEvent")
         .mockReturnValue(undefined);
       sendTelemetryErrorEventSpy = jest
         .spyOn(TelemetryReporter.prototype, "sendTelemetryErrorEvent")
         .mockReturnValue(undefined);
-      disposeSpy = jest
-        .spyOn(TelemetryReporter.prototype, "dispose")
-        .mockResolvedValue(undefined);
 
-      showInformationMessageSpy = jest
-        .spyOn(window, "showInformationMessage")
-        .mockResolvedValue(undefined);
-
-      originalTelemetryExtension = workspace
-        .getConfiguration()
-        .get<boolean>("codeQL.telemetry.enableTelemetry");
-      originalTelemetryGlobal = workspace
-        .getConfiguration()
-        .get<string>("telemetry.telemetryLevel");
       isCanary = (!!workspace
         .getConfiguration()
         .get<boolean>("codeQL.canary")).toString();
 
       // each test will default to telemetry being enabled
-      isTelemetryEnabledSpy = jest
-        .spyOn(env, "isTelemetryEnabled", "get")
-        .mockReturnValue(true);
-      await setTelemetryLevel("telemetry", "all");
-      await enableTelemetry("codeQL.telemetry", true);
+      jest.spyOn(env, "isTelemetryEnabled", "get").mockReturnValue(true);
 
       telemetryListener = new ExtensionTelemetryListener(
         "my-id",
         "1.2.3",
         "fake-key",
-        ctx,
       );
       await wait(100);
     } catch (e) {
@@ -99,18 +58,9 @@ describe("telemetry reporting", () => {
 
   afterEach(async () => {
     telemetryListener?.dispose();
-    // await wait(100);
-    try {
-      await setTelemetryLevel("telemetry", originalTelemetryGlobal);
-      await enableTelemetry("codeQL.telemetry", originalTelemetryExtension);
-    } catch (e) {
-      console.error(e);
-    }
   });
 
-  it("should initialize telemetry when 'codeQL.telemetry.enableTelemetry' is enabled and global 'telemetry.telemetryLevel' is 'all'", async () => {
-    await telemetryListener.initialize();
-
+  it("should initialize telemetry", async () => {
     expect(telemetryListener._reporter).toBeDefined();
     const reporter: any = telemetryListener._reporter;
     expect(reporter.extensionId).toBe("my-id");
@@ -118,80 +68,7 @@ describe("telemetry reporting", () => {
     expect(reporter.userOptIn).toBe(true); // enabled
   });
 
-  it("should initialize telemetry when global 'telemetry.telemetryLevel' is 'off'", async () => {
-    isTelemetryEnabledSpy.mockReturnValue(false);
-    await setTelemetryLevel("telemetry", "off");
-    await telemetryListener.initialize();
-    expect(telemetryListener._reporter).toBeDefined();
-
-    const reporter: any = telemetryListener._reporter;
-    expect(reporter.userOptIn).toBe(false); // disabled
-  });
-
-  it("should not initialize telemetry when extension option disabled", async () => {
-    await enableTelemetry("codeQL.telemetry", false);
-    await telemetryListener.initialize();
-
-    expect(telemetryListener._reporter).toBeUndefined();
-  });
-
-  it("should not initialize telemetry when both options disabled", async () => {
-    await enableTelemetry("codeQL.telemetry", false);
-    isTelemetryEnabledSpy.mockReturnValue(false);
-    await setTelemetryLevel("telemetry", "off");
-    await telemetryListener.initialize();
-    expect(telemetryListener._reporter).toBeUndefined();
-  });
-
-  it("should dispose telemetry object when re-initializing and should not add multiple", async () => {
-    await telemetryListener.initialize();
-    expect(telemetryListener._reporter).toBeDefined();
-    const firstReporter = telemetryListener._reporter;
-    await telemetryListener.initialize();
-    expect(telemetryListener._reporter).toBeDefined();
-    expect(telemetryListener._reporter).not.toBe(firstReporter);
-
-    expect(disposeSpy).toHaveBeenCalledTimes(1);
-
-    // initializing a third time continues to dispose
-    await telemetryListener.initialize();
-    expect(disposeSpy).toHaveBeenCalledTimes(2);
-  });
-
-  it("should reinitialize reporter when extension setting changes", async () => {
-    await telemetryListener.initialize();
-
-    expect(disposeSpy).not.toHaveBeenCalled();
-    expect(telemetryListener._reporter).toBeDefined();
-
-    // this disables the reporter
-    await enableTelemetry("codeQL.telemetry", false);
-
-    expect(telemetryListener._reporter).toBeUndefined();
-
-    expect(disposeSpy).toHaveBeenCalledTimes(1);
-
-    // creates a new reporter, but does not dispose again
-    await enableTelemetry("codeQL.telemetry", true);
-
-    expect(telemetryListener._reporter).toBeDefined();
-    expect(disposeSpy).toHaveBeenCalledTimes(1);
-  });
-
-  it("should set userOptIn to false when global setting changes", async () => {
-    await telemetryListener.initialize();
-
-    const reporter: any = telemetryListener._reporter;
-    expect(reporter.userOptIn).toBe(true); // enabled
-
-    isTelemetryEnabledSpy.mockReturnValue(false);
-    await setTelemetryLevel("telemetry", "off");
-    expect(reporter.userOptIn).toBe(false); // disabled
-  });
-
   it("should send an event", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendCommandUsage("command-id", 1234, undefined);
 
     expect(sendTelemetryEventSpy).toHaveBeenCalledWith(
@@ -208,8 +85,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send a command usage event with an error", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendCommandUsage(
       "command-id",
       1234,
@@ -230,7 +105,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send a command usage event with a cli version", async () => {
-    await telemetryListener.initialize();
     telemetryListener.cliVersion = new SemVer("1.2.3");
 
     telemetryListener.sendCommandUsage(
@@ -274,39 +148,7 @@ describe("telemetry reporting", () => {
     expect(sendTelemetryErrorEventSpy).not.toHaveBeenCalled();
   });
 
-  it("should avoid sending an event when telemetry is disabled", async () => {
-    await telemetryListener.initialize();
-    await enableTelemetry("codeQL.telemetry", false);
-
-    telemetryListener.sendCommandUsage("command-id", 1234, undefined);
-    telemetryListener.sendCommandUsage("command-id", 1234, new Error());
-
-    expect(sendTelemetryEventSpy).not.toHaveBeenCalled();
-    expect(sendTelemetryErrorEventSpy).not.toHaveBeenCalled();
-  });
-
-  it("should send an event when telemetry is re-enabled", async () => {
-    await telemetryListener.initialize();
-    await enableTelemetry("codeQL.telemetry", false);
-    await enableTelemetry("codeQL.telemetry", true);
-
-    telemetryListener.sendCommandUsage("command-id", 1234, undefined);
-
-    expect(sendTelemetryEventSpy).toHaveBeenCalledWith(
-      "command-usage",
-      {
-        name: "command-id",
-        status: "Success",
-        isCanary,
-        cliVersion: "not-set",
-      },
-      { executionTime: 1234 },
-    );
-    expect(sendTelemetryErrorEventSpy).not.toHaveBeenCalled();
-  });
-
   it("should filter undesired properties from telemetry payload", async () => {
-    await telemetryListener.initialize();
     // Reach into the internal appInsights client to grab our telemetry processor.
     const telemetryProcessor: Function = (telemetryListener._reporter as any)
       .appInsightsClient._telemetryProcessors[0];
@@ -340,118 +182,7 @@ describe("telemetry reporting", () => {
     });
   });
 
-  const resolveArg =
-    (index: number) =>
-    (...args: any[]) =>
-      Promise.resolve(args[index]);
-
-  it("should request permission if popup has never been seen before", async () => {
-    showInformationMessageSpy.mockImplementation(
-      resolveArg(3 /* "yes" item */),
-    );
-    await ctx.globalState.update("telemetry-request-viewed", false);
-    expect(env.isTelemetryEnabled).toBe(true);
-
-    await enableTelemetry("codeQL.telemetry", false);
-
-    await telemetryListener.initialize();
-
-    // Wait for user's selection to propagate in settings.
-    await wait(500);
-
-    // Dialog opened, user clicks "yes" and telemetry enabled
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(1);
-    expect(ENABLE_TELEMETRY.getValue()).toBe(true);
-    expect(ctx.globalState.get("telemetry-request-viewed")).toBe(true);
-  });
-
-  it("should prevent telemetry if permission is denied", async () => {
-    showInformationMessageSpy.mockImplementation(resolveArg(4 /* "no" item */));
-    await ctx.globalState.update("telemetry-request-viewed", false);
-    await enableTelemetry("codeQL.telemetry", true);
-
-    await telemetryListener.initialize();
-
-    // Dialog opened, user clicks "no" and telemetry disabled
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(1);
-    expect(ENABLE_TELEMETRY.getValue()).toBe(false);
-    expect(ctx.globalState.get("telemetry-request-viewed")).toBe(true);
-  });
-
-  it("should unchange telemetry if permission dialog is dismissed", async () => {
-    showInformationMessageSpy.mockResolvedValue(undefined /* cancelled */);
-    await ctx.globalState.update("telemetry-request-viewed", false);
-
-    // this causes requestTelemetryPermission to be called
-    await enableTelemetry("codeQL.telemetry", false);
-
-    // Dialog opened, and user closes without interacting with it
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(1);
-    expect(ENABLE_TELEMETRY.getValue()).toBe(false);
-    // dialog was canceled, so should not have marked as viewed
-    expect(ctx.globalState.get("telemetry-request-viewed")).toBe(false);
-  });
-
-  it("should unchange telemetry if permission dialog is cancelled if starting as true", async () => {
-    await enableTelemetry("codeQL.telemetry", false);
-
-    // as before, except start with telemetry enabled. It should _stay_ enabled if the
-    // dialog is canceled.
-    showInformationMessageSpy.mockResolvedValue(undefined /* cancelled */);
-    await ctx.globalState.update("telemetry-request-viewed", false);
-
-    // this causes requestTelemetryPermission to be called
-    await enableTelemetry("codeQL.telemetry", true);
-
-    // Dialog opened, and user closes without interacting with it
-    // Telemetry state should not have changed
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(1);
-    expect(ENABLE_TELEMETRY.getValue()).toBe(true);
-    // dialog was canceled, so should not have marked as viewed
-    expect(ctx.globalState.get("telemetry-request-viewed")).toBe(false);
-  });
-
-  it("should avoid showing dialog if global telemetry is disabled", async () => {
-    // when telemetry is disabled globally, we never want to show the
-    // opt in/out dialog. We just assume that codeql telemetry should
-    // remain disabled as well.
-    // If the user ever turns global telemetry back on, then we can
-    // show the dialog.
-
-    isTelemetryEnabledSpy.mockReturnValue(false);
-    await setTelemetryLevel("telemetry", "off");
-    await ctx.globalState.update("telemetry-request-viewed", false);
-
-    await telemetryListener.initialize();
-
-    // popup should not be shown even though we have initialized telemetry
-    expect(showInformationMessageSpy).not.toHaveBeenCalled();
-  });
-
-  // This test is failing because codeQL.canary is not a registered configuration.
-  // We do not want to have it registered because we don't want this item
-  // appearing in the settings page. It needs to only be set by users we tell
-  // about it to.
-  // At this point, I see no other way of testing re-requesting permission.
-  xit("should request permission again when user changes canary setting", async () => {
-    // initially, both canary and telemetry are false
-    await workspace.getConfiguration().update("codeQL.canary", false);
-    await enableTelemetry("codeQL.telemetry", false);
-    await ctx.globalState.update("telemetry-request-viewed", true);
-    await telemetryListener.initialize();
-    showInformationMessageSpy.mockResolvedValue(undefined /* cancelled */);
-
-    // set canary to true
-    await workspace.getConfiguration().update("codeQL.canary", true);
-
-    // now, we should have to click through the telemetry requestor again
-    expect(ctx.globalState.get("telemetry-request-viewed")).toBe(false);
-    expect(showInformationMessageSpy).toHaveBeenCalledTimes(1);
-  });
-
   it("should send a ui-interaction telemetry event", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendUIInteraction("test");
 
     expect(sendTelemetryEventSpy).toHaveBeenCalledWith(
@@ -467,8 +198,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send a ui-interaction telemetry event with a cli version", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.cliVersion = new SemVer("1.2.3");
     telemetryListener.sendUIInteraction("test");
 
@@ -485,8 +214,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send an error telemetry event", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendError(redactableError`test`);
 
     expect(sendTelemetryEventSpy).not.toHaveBeenCalled();
@@ -503,7 +230,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send an error telemetry event with a cli version", async () => {
-    await telemetryListener.initialize();
     telemetryListener.cliVersion = new SemVer("1.2.3");
 
     telemetryListener.sendError(redactableError`test`);
@@ -522,8 +248,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should redact error message contents", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendError(
       redactableError`test message with secret information: ${42} and more ${"secret"} parts`,
     );
@@ -543,8 +267,6 @@ describe("telemetry reporting", () => {
   });
 
   it("should send config telemetry event", async () => {
-    await telemetryListener.initialize();
-
     telemetryListener.sendConfigInformation({
       testKey: "testValue",
       testKey2: "42",
@@ -563,26 +285,6 @@ describe("telemetry reporting", () => {
     expect(sendTelemetryErrorEventSpy).not.toHaveBeenCalled();
   });
 
-  async function enableTelemetry(section: string, value: boolean | undefined) {
-    await workspace
-      .getConfiguration(section)
-      .update("enableTelemetry", value, ConfigurationTarget.Global);
-
-    // Need to wait some time since the onDidChangeConfiguration listeners fire
-    // asynchronously. Must ensure they to complete in order to have a successful test.
-    await wait(100);
-  }
-
-  async function setTelemetryLevel(section: string, value: string | undefined) {
-    await workspace
-      .getConfiguration(section)
-      .update("telemetryLevel", value, ConfigurationTarget.Global);
-
-    // Need to wait some time since the onDidChangeConfiguration listeners fire
-    // asynchronously. Must ensure they to complete in order to have a successful test.
-    await wait(100);
-  }
-
   async function wait(ms = 0) {
     return new Promise((resolve) => setTimeout(resolve, ms));
   }

From 036c60b494a29dcb9ad7f5517f1ebd080961bbb6 Mon Sep 17 00:00:00 2001
From: Koen Vlaswinkel <koesie10@github.com>
Date: Thu, 5 Dec 2024 11:47:30 +0100
Subject: [PATCH 2/4] Add notification when enabling telemetry

---
 .../ql-vscode/src/common/vscode/telemetry.ts  | 59 ++++++++++++++++++-
 extensions/ql-vscode/src/config.ts            |  6 ++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/extensions/ql-vscode/src/common/vscode/telemetry.ts b/extensions/ql-vscode/src/common/vscode/telemetry.ts
index 8bab7b38dde..d8c6f54c6c8 100644
--- a/extensions/ql-vscode/src/common/vscode/telemetry.ts
+++ b/extensions/ql-vscode/src/common/vscode/telemetry.ts
@@ -1,6 +1,7 @@
 import type { Extension, ExtensionContext } from "vscode";
+import { ConfigurationTarget, env, Uri, window } from "vscode";
 import TelemetryReporter from "vscode-extension-telemetry";
-import { LOG_TELEMETRY, isCanary } from "../../config";
+import { ENABLE_TELEMETRY, isCanary, LOG_TELEMETRY } from "../../config";
 import type { TelemetryClient } from "applicationinsights";
 import { extLogger } from "../logging/vscode";
 import { UserCancellationException } from "./progress";
@@ -159,6 +160,45 @@ export class ExtensionTelemetryListener implements AppTelemetry, Disposable {
   }
 }
 
+async function notifyTelemetryChange() {
+  const continueItem = { title: "Continue", isCloseAffordance: false };
+  const vsCodeTelemetryItem = {
+    title: "More Information about VS Code Telemetry",
+    isCloseAffordance: false,
+  };
+  const codeqlTelemetryItem = {
+    title: "More Information about CodeQL Telemetry",
+    isCloseAffordance: false,
+  };
+  let chosenItem;
+
+  do {
+    chosenItem = await window.showInformationMessage(
+      "The CodeQL extension now follows VS Code's telemetry settings. VS Code telemetry is currently enabled. Learn how to update your telemetry settings by clicking the links below.",
+      { modal: true },
+      continueItem,
+      vsCodeTelemetryItem,
+      codeqlTelemetryItem,
+    );
+    if (chosenItem === vsCodeTelemetryItem) {
+      await env.openExternal(
+        Uri.parse(
+          "https://code.visualstudio.com/docs/getstarted/telemetry",
+          true,
+        ),
+      );
+    }
+    if (chosenItem === codeqlTelemetryItem) {
+      await env.openExternal(
+        Uri.parse(
+          "https://docs.github.com/en/code-security/codeql-for-vs-code/using-the-advanced-functionality-of-the-codeql-for-vs-code-extension/telemetry-in-codeql-for-visual-studio-code",
+          true,
+        ),
+      );
+    }
+  } while (chosenItem !== continueItem);
+}
+
 /**
  * The global Telemetry instance
  */
@@ -172,11 +212,28 @@ export async function initializeTelemetry(
   if (telemetryListener !== undefined) {
     throw new Error("Telemetry is already initialized");
   }
+
+  if (ENABLE_TELEMETRY.getValue<boolean | undefined>() === false) {
+    if (env.isTelemetryEnabled) {
+      // Await this so that the user is notified before any telemetry is sent
+      await notifyTelemetryChange();
+    }
+
+    // Remove the deprecated telemetry setting
+    ENABLE_TELEMETRY.updateValue(undefined, ConfigurationTarget.Global);
+    ENABLE_TELEMETRY.updateValue(undefined, ConfigurationTarget.Workspace);
+    ENABLE_TELEMETRY.updateValue(
+      undefined,
+      ConfigurationTarget.WorkspaceFolder,
+    );
+  }
+
   telemetryListener = new ExtensionTelemetryListener(
     extension.id,
     extension.packageJSON.version,
     key,
   );
   ctx.subscriptions.push(telemetryListener);
+
   return telemetryListener;
 }
diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts
index cfabd9ea609..989e4931bcf 100644
--- a/extensions/ql-vscode/src/config.ts
+++ b/extensions/ql-vscode/src/config.ts
@@ -166,6 +166,12 @@ const TELEMETRY_SETTING = new Setting("telemetry", ROOT_SETTING);
 
 export const LOG_TELEMETRY = new Setting("logTelemetry", TELEMETRY_SETTING);
 
+// Legacy setting that is no longer used, but is used for showing a message when the user upgrades.
+export const ENABLE_TELEMETRY = new Setting(
+  "enableTelemetry",
+  TELEMETRY_SETTING,
+);
+
 // Distribution configuration
 const DISTRIBUTION_SETTING = new Setting("cli", ROOT_SETTING);
 export const CUSTOM_CODEQL_PATH_SETTING = new Setting(

From 9506c17a7972662b2b1d37578364ad70db9f35e2 Mon Sep 17 00:00:00 2001
From: Koen Vlaswinkel <koesie10@github.com>
Date: Thu, 5 Dec 2024 11:53:20 +0100
Subject: [PATCH 3/4] Update CHANGELOG

---
 extensions/ql-vscode/CHANGELOG.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md
index 4e93bfd5531..5e1c5ac390f 100644
--- a/extensions/ql-vscode/CHANGELOG.md
+++ b/extensions/ql-vscode/CHANGELOG.md
@@ -2,7 +2,8 @@
 
 ## [UNRELEASED]
 
-- Add a palette command that allows importing all databases directly inside of a parent folder. [3797](https://github.com/github/vscode-codeql/pull/3797)
+- Add a palette command that allows importing all databases directly inside of a parent folder. [#3797](https://github.com/github/vscode-codeql/pull/3797)
+- Only use VS Code telemetry settings instead of using `codeQL.telemetry.enableTelemetry` [#3853](https://github.com/github/vscode-codeql/pull/3853)
 
 ## 1.16.1 - 6 November 2024
 

From de9fd276d1df23afe18b43f6359ebac28c74a9b1 Mon Sep 17 00:00:00 2001
From: Koen Vlaswinkel <koesie10@github.com>
Date: Thu, 5 Dec 2024 12:12:02 +0100
Subject: [PATCH 4/4] Remove telemetry step from E2E tests

---
 extensions/ql-vscode/test/e2e/run-query.spec.ts | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/extensions/ql-vscode/test/e2e/run-query.spec.ts b/extensions/ql-vscode/test/e2e/run-query.spec.ts
index 6007a279fd6..1bec2591355 100644
--- a/extensions/ql-vscode/test/e2e/run-query.spec.ts
+++ b/extensions/ql-vscode/test/e2e/run-query.spec.ts
@@ -5,11 +5,6 @@ test("run query and open it from history", async ({ page }) => {
 
   await page.getByRole("tab", { name: "CodeQL" }).locator("a").click();
 
-  // decline extension telemetry
-  await page.getByRole("button", { name: "No", exact: true }).click({
-    timeout: 60000,
-  });
-
   await page.keyboard.press("Control+Shift+P");
   await page.keyboard.type("Create Query");
   await page.keyboard.press("Enter");