W-21146754 Fix telemetry event firing issues (#105)

allison-grady · web-flow · commit 4ca62576028a · 2026-02-11T10:28:22.000-08:00
* Bug fixes one and two

* Server stopped fix

* Fix Tool_called log

* Expand dev.js files for env vars

* Update dev.js and fix error logs

* Revert env change

* Re-add disable by default

* Fix flush and align features with other repo

* Fix logic to read env var from mcp.json

* Fix failing tests

* Fix dev.js logic

* Fix som cli error, update relevant unit tests

* Remove unneeded code and update README

* Remove debug code

* Remove debug code again

* Simpllify README, run.js, and combine send event and flush

* Revert file

* Update jsdoc for clarity
diff --git a/packages/b2c-cli/bin/dev.js b/packages/b2c-cli/bin/dev.js
@@ -5,16 +5,26 @@
  * For full license text, see the license.txt file in the repo root or http://www.apache.org/licenses/LICENSE-2.0
  */
 
-// Disable telemetry in development mode to avoid polluting production data
-process.env.SFCC_DISABLE_TELEMETRY = process.env.SFCC_DISABLE_TELEMETRY || 'true';
-
-// Load .env file if present (Node.js native support)
+// Load .env file if present (Node.js native support; same as main run.js)
+// For development, place .env in the package directory and run from there.
 try {
   process.loadEnvFile();
 } catch {
   // .env file not found or not readable, continue without it
 }
 
+// Disable telemetry by default in development when both vars are unset.
+// Set SFCC_DISABLE_TELEMETRY=false (or SF_DISABLE_TELEMETRY=false) in .env to enable COMMAND_START/COMMAND_SUCCESS.
+const userWantsTelemetryEnabled =
+  process.env.SF_DISABLE_TELEMETRY === 'false' || process.env.SFCC_DISABLE_TELEMETRY === 'false';
+if (userWantsTelemetryEnabled) {
+  process.env.SF_DISABLE_TELEMETRY = 'false';
+  process.env.SFCC_DISABLE_TELEMETRY = 'false';
+} else {
+  process.env.SF_DISABLE_TELEMETRY = 'true';
+  process.env.SFCC_DISABLE_TELEMETRY = 'true';
+}
+
 import {execute} from '@oclif/core';
 
 await execute({development: true, dir: import.meta.url});
diff --git a/packages/b2c-dx-mcp/README.md b/packages/b2c-dx-mcp/README.md
@@ -304,23 +304,12 @@ The MCP server collects anonymous usage telemetry to help improve the developer
 
 **Development mode**: Telemetry is automatically disabled when using `bin/dev.js`, so local development and testing won't pollute production data.
 
-### Disabling Telemetry
+### Configuring telemetry
 
-Set one of these environment variables to disable telemetry:
+Set options in the `env` object of your server entry in `.cursor/mcp.json` or `~/.cursor/mcp.json` (the client injects these when it starts the server):
 
-```bash
-# Salesforce CLI standard (recommended)
-SF_DISABLE_TELEMETRY=true
-
-# Or SFCC-specific
-SFCC_DISABLE_TELEMETRY=true
-```
-
-You can also override the telemetry connection string for testing:
-
-```bash
-SFCC_APP_INSIGHTS_KEY=your-connection-string
-```
+- **Disable**: `SF_DISABLE_TELEMETRY=true` or `SFCC_DISABLE_TELEMETRY=true`
+- **Custom endpoint**: `SFCC_APP_INSIGHTS_KEY=your-key`
 
 ### What We Collect
 
diff --git a/packages/b2c-dx-mcp/bin/dev.js b/packages/b2c-dx-mcp/bin/dev.js
@@ -5,14 +5,18 @@
  * For full license text, see the license.txt file in the repo root or http://www.apache.org/licenses/LICENSE-2.0
  */
 
-// Disable telemetry in development mode to avoid polluting production data
-process.env.SFCC_DISABLE_TELEMETRY = process.env.SFCC_DISABLE_TELEMETRY || 'true';
-
-// Load .env file if present (Node.js native support)
-try {
-  process.loadEnvFile();
-} catch {
-  // .env file not found or not readable, continue without it
+// Disable telemetry in development to avoid polluting production data.
+// Honor SF_DISABLE_TELEMETRY (sf CLI) and SFCC_DISABLE_TELEMETRY.
+// If user explicitly enables (either is 'false'), respect that and sync both to 'false'.
+// Otherwise, default to disabling in dev.
+const userWantsTelemetryEnabled =
+  process.env.SF_DISABLE_TELEMETRY === 'false' || process.env.SFCC_DISABLE_TELEMETRY === 'false';
+if (userWantsTelemetryEnabled) {
+  process.env.SF_DISABLE_TELEMETRY = 'false';
+  process.env.SFCC_DISABLE_TELEMETRY = 'false';
+} else {
+  process.env.SF_DISABLE_TELEMETRY = 'true';
+  process.env.SFCC_DISABLE_TELEMETRY = 'true';
 }
 
 import {execute} from '@oclif/core';
diff --git a/packages/b2c-dx-mcp/src/commands/mcp.ts b/packages/b2c-dx-mcp/src/commands/mcp.ts
@@ -219,6 +219,32 @@ export default class McpServerCommand extends BaseCommand<typeof McpServerComman
     }),
   };
 
+  /** Signal that triggered shutdown (if any) - used to exit process after finally() */
+  private shutdownSignal?: string;
+
+  /** Promise that resolves when stdin closes (MCP client disconnects) */
+  private stdinClosePromise?: Promise<void>;
+
+  /**
+   * Override finally() to wait for stdin close before stopping telemetry.
+   * This ensures SERVER_STOPPED is sent before telemetry.stop() is called.
+   */
+  protected async finally(err: Error | undefined): Promise<void> {
+    // Wait for stdin to close and SERVER_STOPPED to be sent
+    // This keeps the command "running" until the MCP client disconnects
+    await this.stdinClosePromise;
+
+    // Now call super.finally() which sends COMMAND_SUCCESS and stops telemetry
+    await super.finally(err);
+
+    // Exit process if shutdown was triggered by a signal (SIGINT/SIGTERM)
+    // Catching these signals prevents Node's default exit behavior
+    if (this.shutdownSignal === 'SIGINT' || this.shutdownSignal === 'SIGTERM') {
+      // eslint-disable-next-line n/no-process-exit, unicorn/no-process-exit
+      process.exit(0);
+    }
+  }
+
   /**
    * Loads configuration from flags, environment variables, and config files.
    *
@@ -261,7 +287,7 @@ export default class McpServerCommand extends BaseCommand<typeof McpServerComman
    * 4. Create Services via Services.fromResolvedConfig() using already-resolved config
    * 5. Register tools based on --toolsets and --tools flags
    * 6. Connect to stdio transport (JSON-RPC over stdin/stdout)
-   * 7. Log startup message to stderr
+   * 7. Log startup message via structured logger
    *
    * @throws Never throws - invalid toolsets are filtered, not rejected
    *
@@ -320,14 +346,28 @@ export default class McpServerCommand extends BaseCommand<typeof McpServerComman
     const transport = new StdioServerTransport();
     await server.connect(transport);
 
-    // Track server stop when stdin closes (MCP client disconnects)
-    // Note: The 'close' event has no arguments - it's just a signal that the stream closed
-    process.stdin.on('close', () => {
-      this.telemetry?.sendEvent('SERVER_STOPPED');
-      // Don't call stop() here - let finally() handle telemetry cleanup
+    // Create promise that resolves when server stops (stdin close or signal)
+    // This allows finally() to wait for SERVER_STOPPED before stopping telemetry
+    this.stdinClosePromise = new Promise((resolve) => {
+      const sendStopAndResolve = (signal: string): void => {
+        this.shutdownSignal = signal;
+        this.telemetry?.sendEvent('SERVER_STOPPED', {signal});
+        // Flush telemetry before resolving to ensure SERVER_STOPPED is sent
+        // before finally() proceeds to stop telemetry
+        const flushPromise = this.telemetry?.flush() ?? Promise.resolve();
+        flushPromise.then(() => resolve()).catch(() => resolve());
+      };
+
+      // Handle stdin close (MCP client disconnects normally)
+      process.stdin.on('close', () => sendStopAndResolve('stdin_close'));
+
+      // Handle Ctrl+C
+      process.on('SIGINT', () => sendStopAndResolve('SIGINT'));
+
+      // Handle kill signal
+      process.on('SIGTERM', () => sendStopAndResolve('SIGTERM'));
     });
 
-    // Log startup message using the structured logger
     this.logger.info({version: this.config.version}, 'MCP Server running on stdio');
   }
 }
diff --git a/packages/b2c-dx-mcp/src/server.ts b/packages/b2c-dx-mcp/src/server.ts
@@ -83,21 +83,25 @@ export class B2CDxMcpServer extends McpServer {
         const result = await handler(args);
         const runTimeMs = Date.now() - startTime;
 
-        this.telemetry?.sendEvent('TOOL_CALLED', {
-          toolName: name,
-          runTimeMs,
-          isError: result.isError ?? false,
-        });
+        await this.telemetry
+          ?.sendEventAndFlush('TOOL_CALLED', {
+            toolName: name,
+            runTimeMs,
+            isError: result.isError ?? false,
+          })
+          .catch(() => {});
 
         return result;
       } catch (error) {
         const runTimeMs = Date.now() - startTime;
 
-        this.telemetry?.sendEvent('TOOL_CALLED', {
-          toolName: name,
-          runTimeMs,
-          isError: true,
-        });
+        await this.telemetry
+          ?.sendEventAndFlush('TOOL_CALLED', {
+            toolName: name,
+            runTimeMs,
+            isError: true,
+          })
+          .catch(() => {});
 
         throw error;
       }
@@ -113,19 +117,19 @@ export class B2CDxMcpServer extends McpServer {
   public override async connect(transport: Transport): Promise<void> {
     try {
       await super.connect(transport);
-      if (this.isConnected()) {
-        this.telemetry?.sendEvent('SERVER_STATUS', {status: 'started'});
-      } else {
-        this.telemetry?.sendEvent('SERVER_STATUS', {
-          status: 'error',
-          errorMessage: 'Server not connected after connect() call',
-        });
-      }
+      const statusPromise = this.isConnected()
+        ? this.telemetry?.sendEventAndFlush('SERVER_STATUS', {status: 'started'})
+        : this.telemetry?.sendEventAndFlush('SERVER_STATUS', {
+            status: 'error',
+            errorMessage: 'Server not connected after connect() call',
+          });
+      await (statusPromise ?? Promise.resolve()).catch(() => {});
     } catch (error) {
-      this.telemetry?.sendEvent('SERVER_STATUS', {
+      const errorPromise = this.telemetry?.sendEventAndFlush('SERVER_STATUS', {
         status: 'error',
         errorMessage: error instanceof Error ? error.message : String(error),
       });
+      await (errorPromise ?? Promise.resolve()).catch(() => {});
       throw error;
     }
   }
diff --git a/packages/b2c-dx-mcp/test/commands/mcp.test.ts b/packages/b2c-dx-mcp/test/commands/mcp.test.ts
@@ -330,62 +330,78 @@ describe('McpServerCommand', () => {
       });
 
       it('returns project default when no env override', () => {
-        const originalDisable = process.env.SFCC_DISABLE_TELEMETRY;
+        const originalSfDisable = process.env.SF_DISABLE_TELEMETRY;
+        const originalSfccDisable = process.env.SFCC_DISABLE_TELEMETRY;
         const originalKey = process.env.SFCC_APP_INSIGHTS_KEY;
         try {
+          delete process.env.SF_DISABLE_TELEMETRY;
           delete process.env.SFCC_DISABLE_TELEMETRY;
           delete process.env.SFCC_APP_INSIGHTS_KEY;
           expect(Telemetry.getConnectionString('default-key')).to.equal('default-key');
         } finally {
-          if (originalDisable !== undefined) process.env.SFCC_DISABLE_TELEMETRY = originalDisable;
-          if (originalKey !== undefined) process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
+          if (originalSfDisable === undefined) delete process.env.SF_DISABLE_TELEMETRY;
+          else process.env.SF_DISABLE_TELEMETRY = originalSfDisable;
+          if (originalSfccDisable === undefined) delete process.env.SFCC_DISABLE_TELEMETRY;
+          else process.env.SFCC_DISABLE_TELEMETRY = originalSfccDisable;
+          if (originalKey === undefined) delete process.env.SFCC_APP_INSIGHTS_KEY;
+          else process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
         }
       });
 
       it('returns env override when SFCC_APP_INSIGHTS_KEY is set', () => {
-        const originalDisable = process.env.SFCC_DISABLE_TELEMETRY;
+        const originalSfDisable = process.env.SF_DISABLE_TELEMETRY;
+        const originalSfccDisable = process.env.SFCC_DISABLE_TELEMETRY;
         const originalKey = process.env.SFCC_APP_INSIGHTS_KEY;
         try {
+          delete process.env.SF_DISABLE_TELEMETRY;
           delete process.env.SFCC_DISABLE_TELEMETRY;
           process.env.SFCC_APP_INSIGHTS_KEY = 'env-override-key';
           expect(Telemetry.getConnectionString('default-key')).to.equal('env-override-key');
         } finally {
-          if (originalDisable !== undefined) process.env.SFCC_DISABLE_TELEMETRY = originalDisable;
-          if (originalKey === undefined) {
-            delete process.env.SFCC_APP_INSIGHTS_KEY;
-          } else {
-            process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
-          }
+          if (originalSfDisable === undefined) delete process.env.SF_DISABLE_TELEMETRY;
+          else process.env.SF_DISABLE_TELEMETRY = originalSfDisable;
+          if (originalSfccDisable === undefined) delete process.env.SFCC_DISABLE_TELEMETRY;
+          else process.env.SFCC_DISABLE_TELEMETRY = originalSfccDisable;
+          if (originalKey === undefined) delete process.env.SFCC_APP_INSIGHTS_KEY;
+          else process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
         }
       });
 
       it('returns undefined when no default and no env override', () => {
-        const originalDisable = process.env.SFCC_DISABLE_TELEMETRY;
+        const originalSfDisable = process.env.SF_DISABLE_TELEMETRY;
+        const originalSfccDisable = process.env.SFCC_DISABLE_TELEMETRY;
         const originalKey = process.env.SFCC_APP_INSIGHTS_KEY;
         try {
+          delete process.env.SF_DISABLE_TELEMETRY;
           delete process.env.SFCC_DISABLE_TELEMETRY;
           delete process.env.SFCC_APP_INSIGHTS_KEY;
           expect(Telemetry.getConnectionString()).to.be.undefined;
         } finally {
-          if (originalDisable !== undefined) process.env.SFCC_DISABLE_TELEMETRY = originalDisable;
-          if (originalKey !== undefined) process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
+          if (originalSfDisable === undefined) delete process.env.SF_DISABLE_TELEMETRY;
+          else process.env.SF_DISABLE_TELEMETRY = originalSfDisable;
+          if (originalSfccDisable === undefined) delete process.env.SFCC_DISABLE_TELEMETRY;
+          else process.env.SFCC_DISABLE_TELEMETRY = originalSfccDisable;
+          if (originalKey === undefined) delete process.env.SFCC_APP_INSIGHTS_KEY;
+          else process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
         }
       });
 
       it('returns env override even without project default', () => {
-        const originalDisable = process.env.SFCC_DISABLE_TELEMETRY;
+        const originalSfDisable = process.env.SF_DISABLE_TELEMETRY;
+        const originalSfccDisable = process.env.SFCC_DISABLE_TELEMETRY;
         const originalKey = process.env.SFCC_APP_INSIGHTS_KEY;
         try {
+          delete process.env.SF_DISABLE_TELEMETRY;
           delete process.env.SFCC_DISABLE_TELEMETRY;
           process.env.SFCC_APP_INSIGHTS_KEY = 'env-only-key';
           expect(Telemetry.getConnectionString()).to.equal('env-only-key');
         } finally {
-          if (originalDisable !== undefined) process.env.SFCC_DISABLE_TELEMETRY = originalDisable;
-          if (originalKey === undefined) {
-            delete process.env.SFCC_APP_INSIGHTS_KEY;
-          } else {
-            process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
-          }
+          if (originalSfDisable === undefined) delete process.env.SF_DISABLE_TELEMETRY;
+          else process.env.SF_DISABLE_TELEMETRY = originalSfDisable;
+          if (originalSfccDisable === undefined) delete process.env.SFCC_DISABLE_TELEMETRY;
+          else process.env.SFCC_DISABLE_TELEMETRY = originalSfccDisable;
+          if (originalKey === undefined) delete process.env.SFCC_APP_INSIGHTS_KEY;
+          else process.env.SFCC_APP_INSIGHTS_KEY = originalKey;
         }
       });
     });
diff --git a/packages/b2c-dx-mcp/test/server.test.ts b/packages/b2c-dx-mcp/test/server.test.ts
@@ -22,6 +22,10 @@ class MockTelemetry {
     this.attributes = {...this.attributes, ...attrs};
   }
 
+  async flush(): Promise<void> {
+    // Mock flush - does nothing but allows tests to pass
+  }
+
   sendEvent(name: string, attributes: Record<string, unknown> = {}): void {
     this.events.push({name, attributes});
   }
diff --git a/packages/b2c-tooling-sdk/src/cli/base-command.ts b/packages/b2c-tooling-sdk/src/cli/base-command.ts
@@ -206,7 +206,7 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
   }): Promise<Telemetry | undefined> {
     // If telemetry was already initialized by initTelemetryFromConfig, stop it first
     if (this.telemetry) {
-      this.telemetry.stop();
+      await this.telemetry.stop();
     }
 
     const connectionString = Telemetry.getConnectionString(options.appInsightsKey);
@@ -472,9 +472,20 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
     const exitCode = err.exitCode ?? 1;
     const duration = this.commandStartTime ? Date.now() - this.commandStartTime : undefined;
 
-    // Send exception to telemetry if initialized
-    this.telemetry?.sendException(err, {command: this.id, exitCode, duration});
-    this.telemetry?.stop();
+    // Send exception and COMMAND_ERROR event so the error appears in custom events (same view as COMMAND_START)
+    // Flush explicitly before stop to ensure events are sent before process exits
+    if (this.telemetry) {
+      this.telemetry.sendException(err, {command: this.id, exitCode, duration});
+      this.telemetry.sendEvent('COMMAND_ERROR', {
+        command: this.id,
+        exitCode,
+        duration,
+        errorMessage: err.message,
+        ...(err.cause ? {errorCause: String(err.cause)} : {}),
+      });
+      await this.telemetry.flush();
+      await this.telemetry.stop();
+    }
 
     // Log if logger is available (may not be if error during init)
     if (this.logger) {
@@ -507,7 +518,7 @@ export abstract class BaseCommand<T extends typeof Command> extends Command {
     if (!err && this.telemetry) {
       const duration = this.commandStartTime ? Date.now() - this.commandStartTime : undefined;
       this.telemetry.sendEvent('COMMAND_SUCCESS', {command: this.id, duration});
-      this.telemetry.stop();
+      await this.telemetry.stop();
     }
     await super.finally(err);
   }
diff --git a/packages/b2c-tooling-sdk/src/telemetry/telemetry.ts b/packages/b2c-tooling-sdk/src/telemetry/telemetry.ts
diff --git a/packages/b2c-tooling-sdk/test/cli/base-command.test.ts b/packages/b2c-tooling-sdk/test/cli/base-command.test.ts
diff --git a/packages/b2c-tooling-sdk/test/telemetry/telemetry.test.ts b/packages/b2c-tooling-sdk/test/telemetry/telemetry.test.ts

Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,10 @@ class MockTelemetry {`
`22`	`22`	`this.attributes = {...this.attributes, ...attrs};`
`23`	`23`	`}`
`24`	`24`
	`25`	`+ async flush(): Promise<void> {`
	`26`	`+ // Mock flush - does nothing but allows tests to pass`
	`27`	`+ }`
	`28`	`+`
`25`	`29`	`sendEvent(name: string, attributes: Record<string, unknown> = {}): void {`
`26`	`30`	`this.events.push({name, attributes});`
`27`	`31`	`}`