update telemetry after discussion

packages/typespec-vscode/src/extension.ts

@@ -86,21 +86,21 @@ export async function activate(context: ExtensionContext) {
               async (tel) => {
                 if (args?.forceRecreate === true) {
                   logger.info("Forcing to recreate TypeSpec LSP server...");
-                  const c = await recreateLSPClient(context, tel.activityId);
-                  tel.lastStep = "Recreate LSP client";
-                  return getResultFromLSPClient(c);
+                  tel.lastStep = "Recreate LSP client in force";
+                  return await recreateLSPClient(context, tel.activityId);
                 }
                 if (client && client.state === State.Running) {
-                  await client.restart(tel.activityId);
                   tel.lastStep = "Restart LSP client";
-                  return getResultFromLSPClient(client);
+                  await client.restart();
+                  return client.state === State.Running
+                    ? { code: ResultCode.Success, value: client }
+                    : { code: ResultCode.Fail, details: "TspLanguageClient is not running." };
                 } else {
                   logger.info(
                     "TypeSpec LSP server is not running which is not expected, try to recreate and start...",
                   );
-                  const c = await recreateLSPClient(context, tel.activityId);
-                  tel.lastStep = "Create LSP client";
-                  return getResultFromLSPClient(c);
+                  tel.lastStep = "Recreate LSP client";
+                  return await recreateLSPClient(context, tel.activityId);
                 }
               },
               args?.activityId,
@@ -136,7 +136,12 @@ export async function activate(context: ExtensionContext) {
     vscode.workspace.onDidChangeConfiguration(async (e: vscode.ConfigurationChangeEvent) => {
       if (e.affectsConfiguration(SettingName.TspServerPath)) {
         logger.info("TypeSpec server path changed, restarting server...");
-        await recreateLSPClient(context);
+        await telemetryClient.doOperationWithTelemetry(
+          TelemetryEventName.ServerPathSettingChanged,
+          async (tel) => {
+            return await recreateLSPClient(context, tel.activityId);
+          },
+        );
       }
     }),
   );
@@ -181,8 +186,7 @@ export async function activate(context: ExtensionContext) {
         await telemetryClient.doOperationWithTelemetry(
           TelemetryEventName.StartExtension,
           async (tel: OperationTelemetryEvent) => {
-            await recreateLSPClient(context, tel.activityId);
-            return getResultFromLSPClient(client);
+            return await recreateLSPClient(context, tel.activityId);
           },
         );
       },
@@ -197,27 +201,18 @@ export async function deactivate() {
   await client?.stop();
 }
 
-async function recreateLSPClient(context: ExtensionContext, activityId?: string) {
+async function recreateLSPClient(
+  context: ExtensionContext,
+  activityId: string,
+): Promise<Result<TspLanguageClient>> {
   logger.info("Recreating TypeSpec LSP server...");
   const oldClient = client;
-  client = await TspLanguageClient.create(context, outputChannel);
+  client = await TspLanguageClient.create(activityId, context, outputChannel);
   await oldClient?.stop();
   await client.start(activityId);
-  return client;
-}
-
-function getResultFromLSPClient(c: TspLanguageClient | undefined): Result<TspLanguageClient> {
-  if (c?.state === State.Running) {
-    return {
-      code: ResultCode.Success,
-      value: c,
-    };
-  } else {
-    return {
-      code: ResultCode.Fail,
-      details: "TspLanguageClient is not running. Please check previous log for details.",
-    };
-  }
+  return client.state === State.Running
+    ? { code: ResultCode.Success, value: client }
+    : { code: ResultCode.Fail, details: "TspLanguageClient is not running." };
 }
 
 function showStartUpMessages(stateManager: ExtensionStateManager) {

packages/typespec-vscode/src/telemetry/telemetry-client.ts

@@ -1,11 +1,13 @@
 import TelemetryReporter from "@vscode/extension-telemetry";
 import pkgJson from "../../package.json" assert { type: "json" };
 import logger from "../log/logger.js";
-import { Result } from "../types.js";
+import { ResultCode } from "../types.js";
 import { isWhitespaceStringOrUndefined } from "../utils.js";
 import {
   createOperationTelemetryEvent,
   emptyActivityId,
+  OperationDetailProperties,
+  OperationDetailTelemetryEvent,
   OperationTelemetryEvent,
   TelemetryEventName,
 } from "./telemetry-event.js";
@@ -18,8 +20,17 @@ class TelemetryClient {
   private _logTelemetryErrorCount = 0;
 
   constructor() {
-    const cs = `InstrumentationKey=${pkgJson.telemetryKey}`;
-    this._client = new (TelemetryReporter as any)(cs);
+    this._client = new (TelemetryReporter as any)(this.getConnectionString());
+  }
+
+  private getConnectionString() {
+    return `InstrumentationKey=${pkgJson.telemetryKey}`;
+  }
+
+  public async flush() {
+    // flush function is not exposed by the telemetry client, so we leverage dispose to trigger the flush and recreate the client
+    await this._client?.dispose();
+    this._client = new (TelemetryReporter as any)(this.getConnectionString());
   }
 
   private sendEvent(
@@ -48,17 +59,42 @@ class TelemetryClient {
 
   public async doOperationWithTelemetry<T>(
     eventName: TelemetryEventName,
-    operation: (opTelemetryEvent: OperationTelemetryEvent) => Promise<Result<T>>,
+    operation: (
+      opTelemetryEvent: OperationTelemetryEvent,
+      /**
+       * Call this function to send the telemetry event if you don't want to wait until the end of the operation for some reason
+       */
+      sendTelemetryEvent: (result: ResultCode) => void,
+    ) => Promise<T>,
     activityId?: string,
-  ): Promise<Result<T>> {
+  ): Promise<T> {
     const opTelemetryEvent = createOperationTelemetryEvent(eventName, activityId);
+    let eventSent = false;
+    const sendTelemetryEvent = (result?: ResultCode) => {
+      if (!eventSent) {
+        eventSent = true;
+        opTelemetryEvent.endTime ??= new Date();
+        if (result) {
+          opTelemetryEvent.result = result;
+        }
+        this.logOperationTelemetryEvent(opTelemetryEvent);
+      }
+    };
     try {
-      const result = await operation(opTelemetryEvent);
-      opTelemetryEvent.result ??= result.code;
+      const result = await operation(opTelemetryEvent, (result) => sendTelemetryEvent(result));
+      const isResultCode = (v: any) => Object.values(ResultCode).includes(v as ResultCode);
+      if (result) {
+        if (isResultCode(result)) {
+          // TODO: test
+          opTelemetryEvent.result ??= result as ResultCode;
+        } else if (typeof result === "object" && "code" in result && isResultCode(result.code)) {
+          // TODO: test
+          opTelemetryEvent.result ??= result.code as ResultCode;
+        }
+      }
       return result;
     } finally {
-      opTelemetryEvent.endTime ??= new Date();
-      this.logOperationTelemetryEvent(opTelemetryEvent);
+      sendTelemetryEvent();
     }
   }
 
@@ -79,24 +115,41 @@ class TelemetryClient {
     });
   }
 
+  public logOperationDetailTelemetry(
+    activityId: string,
+    detail: Partial<Record<keyof OperationDetailProperties, string>>,
+  ) {
+    const data: OperationDetailTelemetryEvent = {
+      activityId: activityId,
+      eventName: TelemetryEventName.OperationDetail,
+      ...detail,
+    };
+
+    if (detail.error !== undefined) {
+      this.sendErrorEvent(TelemetryEventName.OperationDetail, {
+        ...data,
+      });
+    } else {
+      this.sendEvent(TelemetryEventName.OperationDetail, {
+        ...data,
+      });
+    }
+  }
+
   /**
-   * Use this method to send log to telemetry.
+   * Use this method to send error to telemetry.
    * IMPORTANT: make sure to:
    *   - Collect as *little* telemetry as possible.
    *   - Do not include any personal or sensitive information.
    * Detail guidance can be found at: https://code.visualstudio.com/api/extension-guides/telemetry
-   * @param level
-   * @param message
-   * @param activityId
    */
-  public log(level: "error", message: string, activityId?: string) {
-    const telFunc = level === "error" ? this.sendErrorEvent : this.sendEvent;
-    telFunc.call(this, TelemetryEventName.Log, {
-      activityId: isWhitespaceStringOrUndefined(activityId) ? emptyActivityId : activityId!,
-      level: level,
-      message: message,
-    });
-  }
+  // public logError(error: string, activityId?: string) {
+  //   this.sendErrorEvent(TelemetryEventName.Error, {
+  //     activityId: isWhitespaceStringOrUndefined(activityId) ? emptyActivityId : activityId!,
+  //     timestamp: new Date().toISOString(),
+  //     error: error,
+  //   });
+  // }
 
   private logErrorWhenLoggingTelemetry(error: any) {
     if (this._logTelemetryErrorCount++ < this.MAX_LOG_TELEMETRY_ERROR) {

packages/typespec-vscode/src/telemetry/telemetry-event.ts

@@ -8,38 +8,31 @@ export enum TelemetryEventName {
   RestartServer = "restart-server",
   GenerateCode = "generate-code",
   ImportFromOpenApi3 = "import-from-openapi3",
-  /** For extra log we need in telemetry.
-   *  IMPORTANT: make sure to:
-   *   - Collect as *little* telemetry as possible.
-   *   - Do not include any personal or sensitive information.
-   *  Detail guidance can be found at: https://code.visualstudio.com/api/extension-guides/telemetry
-   */
-  Log = "typespec/log",
+  ServerPathSettingChanged = "server-path-changed",
+  OperationDetail = "operation-detail",
+}
+export class OperationDetailProperties {
+  error = "error";
+  emitterPackage = "emitterPackage";
+  compilerLocation = "compilerLocation";
 }
 
-export interface BaseTelemetryEvent {
-  /**
-   * all the telemetry events from the same activity should have the same activityId
-   * if not provided, a new activityId will be generated
-   */
+export interface TelemetryEventBase {
   activityId: string;
-  /**
-   * the name of the event
-   */
   eventName: TelemetryEventName;
 }
 
-export interface OperationTelemetryEvent extends BaseTelemetryEvent {
-  eventName: TelemetryEventName;
+export interface OperationTelemetryEvent extends TelemetryEventBase {
   startTime: Date;
   endTime?: Date;
   result?: ResultCode;
-  /**
-   * the last step when the operation finish successfully or not
-   */
   lastStep?: string;
 }
 
+export interface OperationDetailTelemetryEvent
+  extends TelemetryEventBase,
+    Partial<Record<keyof OperationDetailProperties, string>> {}
+
 /**
  * Create a operation telemetry event with following default values.
  * Please make sure the default values are updated properly as needed

packages/typespec-vscode/src/tsp-executable-resolver.ts

@@ -2,6 +2,7 @@ import { dirname, isAbsolute, join } from "path";
 import { ExtensionContext, workspace } from "vscode";
 import { Executable, ExecutableOptions } from "vscode-languageclient/node.js";
 import logger from "./log/logger.js";
+import telemetryClient from "./telemetry/telemetry-client.js";
 import { SettingName } from "./types.js";
 import { isFile, loadModule, useShellInExec } from "./utils.js";
 import { VSCodeVariableResolver } from "./vscode-variable-resolver.js";
@@ -42,7 +43,10 @@ export async function resolveTypeSpecCli(
   }
 }
 
-export async function resolveTypeSpecServer(context: ExtensionContext): Promise<Executable> {
+export async function resolveTypeSpecServer(
+  activityId: string,
+  context: ExtensionContext,
+): Promise<Executable> {
   const nodeOptions = process.env.TYPESPEC_SERVER_NODE_OPTIONS;
   const args = ["--stdio"];
 
@@ -75,6 +79,9 @@ export async function resolveTypeSpecServer(context: ExtensionContext): Promise<
   // @typespec/compiler` in a vanilla setup.
   if (serverPath) {
     logger.info(`Server path loaded from TypeSpec extension configuration: ${serverPath}`);
+    telemetryClient.logOperationDetailTelemetry(activityId, {
+      compilerLocation: "customized-compiler",
+    });
   } else {
     logger.info(
       "Server path not configured in TypeSpec extension configuration, trying to resolve locally within current workspace.",
@@ -87,8 +94,14 @@ export async function resolveTypeSpecServer(context: ExtensionContext): Promise<
     logger.warning(
       `Can't resolve server path from either TypeSpec extension configuration or workspace, try to use default value ${executable}.`,
     );
+    telemetryClient.logOperationDetailTelemetry(activityId, {
+      compilerLocation: "global-compiler",
+    });
     return useShellInExec({ command: executable, args, options });
   }
+  telemetryClient.logOperationDetailTelemetry(activityId, {
+    compilerLocation: "local-compiler",
+  });
   const variableResolver = new VSCodeVariableResolver({
     workspaceFolder,
     workspaceRoot: workspaceFolder, // workspaceRoot is deprecated but we still support it for backwards compatibility.

packages/typespec-vscode/src/tsp-language-client.ts

@@ -5,6 +5,7 @@ import type {
   InitProjectTemplate,
   ServerInitializeResult,
 } from "@typespec/compiler";
+import { inspect } from "util";
 import { ExtensionContext, LogOutputChannel, RelativePattern, workspace } from "vscode";
 import { Executable, LanguageClient, LanguageClientOptions } from "vscode-languageclient/node.js";
 import { TspConfigFileName } from "./const.js";
@@ -113,7 +114,7 @@ export class TspLanguageClient {
     }
   }
 
-  async restart(activityId?: string): Promise<void> {
+  async restart(): Promise<void> {
     try {
       if (this.client.needsStop()) {
         await this.client.restart();
@@ -125,11 +126,6 @@ export class TspLanguageClient {
         logger.error(
           `Unexpected state when restarting TypeSpec server. state = ${this.client.state}.`,
         );
-        telemetryClient.log(
-          "error",
-          `Unexpected state when restarting TypeSpec server. state = ${this.client.state}.`,
-          activityId,
-        );
       }
     } catch (e) {
       logger.error("Error restarting TypeSpec server", [e]);
@@ -149,7 +145,7 @@ export class TspLanguageClient {
     }
   }
 
-  async start(activityId?: string): Promise<void> {
+  async start(activityId: string): Promise<void> {
     try {
       if (this.client.needsStart()) {
         // please be aware that this method would popup error notification in vscode directly
@@ -173,13 +169,15 @@ export class TspLanguageClient {
           { showOutput: false, showPopup: true },
         );
         logger.error("Error detail", [e]);
-        telemetryClient.log("error", "TypeSpec server executable not found", activityId);
       } else {
         logger.error("Unexpected error when starting TypeSpec server", [e], {
           showOutput: false,
           showPopup: true,
         });
       }
+      telemetryClient.logOperationDetailTelemetry(activityId, {
+        error: `Error when starting TypeSpec server: ${inspect(e)}`,
+      });
     }
   }
 
@@ -190,10 +188,11 @@ export class TspLanguageClient {
   }
 
   static async create(
+    activityId: string,
     context: ExtensionContext,
     outputChannel: LogOutputChannel,
   ): Promise<TspLanguageClient> {
-    const exe = await resolveTypeSpecServer(context);
+    const exe = await resolveTypeSpecServer(activityId, context);
     logger.debug("TypeSpec server resolved as ", [exe]);
     const watchers = [
       workspace.createFileSystemWatcher("**/*.cadl"),