feat: update connectionString appName param - [MCP-68]

blva · blva · commit c5c91e9fe4ac · 2025-07-28T16:05:23.000+01:00
diff --git a/src/common/session.ts b/src/common/session.ts
@@ -98,10 +98,15 @@ export class Session extends EventEmitter<{
     }
 
     async connectToMongoDB(connectionString: string, connectOptions: ConnectOptions): Promise<void> {
-        connectionString = setAppNameParamIfMissing({
+        // Use the extended appName format with deviceId and clientName
+        connectionString = await setAppNameParamIfMissing({
             connectionString,
-            defaultAppName: `${packageInfo.mcpServerName} ${packageInfo.version}`,
+            components: {
+                appName: `${packageInfo.mcpServerName} ${packageInfo.version}`,
+                clientName: this.agentRunner?.name || "unknown",
+            },
         });
+
         this.serviceProvider = await NodeDriverServiceProvider.connect(connectionString, {
             productDocsLink: "https://github.com/mongodb-js/mongodb-mcp-server/",
             productName: "MongoDB MCP",
diff --git a/src/helpers/connectionOptions.ts b/src/helpers/connectionOptions.ts
@@ -1,20 +1,51 @@
 import { MongoClientOptions } from "mongodb";
 import ConnectionString from "mongodb-connection-string-url";
+import { getDeviceIdForConnection } from "./deviceId.js";
 
-export function setAppNameParamIfMissing({
+export interface AppNameComponents {
+    appName: string;
+    deviceId?: string;
+    clientName?: string;
+}
+
+/**
+ * Sets the appName parameter with the extended format: appName--deviceId--clientName
+ * Only sets the appName if it's not already present in the connection string
+ * @param connectionString - The connection string to modify
+ * @param components - The components to build the appName from
+ * @returns The modified connection string
+ */
+export async function setAppNameParamIfMissing({
     connectionString,
-    defaultAppName,
+    components,
 }: {
     connectionString: string;
-    defaultAppName?: string;
-}): string {
+    components: AppNameComponents;
+}): Promise<string> {
     const connectionStringUrl = new ConnectionString(connectionString);
-
     const searchParams = connectionStringUrl.typedSearchParams<MongoClientOptions>();
 
-    if (!searchParams.has("appName") && defaultAppName !== undefined) {
-        searchParams.set("appName", defaultAppName);
+    // Only set appName if it's not already present
+    if (searchParams.has("appName")) {
+        return connectionStringUrl.toString();
+    }
+
+    // Get deviceId if not provided
+    let deviceId = components.deviceId;
+    if (!deviceId) {
+        deviceId = await getDeviceIdForConnection();
     }
 
+    // Get clientName if not provided
+    let clientName = components.clientName;
+    if (!clientName) {
+        clientName = "unknown";
+    }
+
+    // Build the extended appName format: appName--deviceId--clientName
+    const extendedAppName = `${components.appName}--${deviceId}--${clientName}`;
+
+    searchParams.set("appName", extendedAppName);
+
     return connectionStringUrl.toString();
 }
diff --git a/src/helpers/deviceId.ts b/src/helpers/deviceId.ts
@@ -0,0 +1,48 @@
+import { getDeviceId } from "@mongodb-js/device-id";
+import nodeMachineId from "node-machine-id";
+import logger, { LogId } from "../common/logger.js";
+
+export const DEVICE_ID_TIMEOUT = 3000;
+
+/**
+ * Sets the appName parameter with the extended format: appName--deviceId--clientName
+ * Only sets the appName if it's not already present in the connection string
+ *
+ * @param connectionString - The connection string to modify
+ * @param components - The components to build the appName from
+ * @returns Promise that resolves to the modified connection string
+ *
+ * @example
+ * ```typescript
+ * const result = await setExtendedAppNameParam({
+ *   connectionString: "mongodb://localhost:27017",
+ *   components: { appName: "MyApp", clientName: "Cursor" }
+ * });
+ * // Result: "mongodb://localhost:27017/?appName=MyApp--deviceId--Cursor"
+ * ```
+ */
+export async function getDeviceIdForConnection(): Promise<string> {
+    try {
+        const deviceId = await getDeviceId({
+            getMachineId: () => nodeMachineId.machineId(true),
+            onError: (reason, error) => {
+                switch (reason) {
+                    case "resolutionError":
+                        logger.debug(LogId.telemetryDeviceIdFailure, "deviceId", String(error));
+                        break;
+                    case "timeout":
+                        logger.debug(LogId.telemetryDeviceIdTimeout, "deviceId", "Device ID retrieval timed out");
+                        break;
+                    case "abort":
+                        // No need to log in the case of aborts
+                        break;
+                }
+            },
+            abortSignal: new AbortController().signal,
+        });
+        return deviceId;
+    } catch (error) {
+        logger.debug(LogId.telemetryDeviceIdFailure, "deviceId", `Failed to get device ID: ${String(error)}`);
+        return "unknown";
+    }
+}
diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts
@@ -5,33 +5,27 @@ import logger, { LogId } from "../common/logger.js";
 import { ApiClient } from "../common/atlas/apiClient.js";
 import { MACHINE_METADATA } from "./constants.js";
 import { EventCache } from "./eventCache.js";
-import nodeMachineId from "node-machine-id";
-import { getDeviceId } from "@mongodb-js/device-id";
+import { getDeviceIdForConnection } from "../helpers/deviceId.js";
 import { detectContainerEnv } from "../helpers/container.js";
 
 type EventResult = {
     success: boolean;
     error?: Error;
 };
 
-export const DEVICE_ID_TIMEOUT = 3000;
-
 export class Telemetry {
     private isBufferingEvents: boolean = true;
     /** Resolves when the setup is complete or a timeout occurs */
     public setupPromise: Promise<[string, boolean]> | undefined;
-    private deviceIdAbortController = new AbortController();
     private eventCache: EventCache;
-    private getRawMachineId: () => Promise<string>;
 
     private constructor(
         private readonly session: Session,
         private readonly userConfig: UserConfig,
         private readonly commonProperties: CommonProperties,
-        { eventCache, getRawMachineId }: { eventCache: EventCache; getRawMachineId: () => Promise<string> }
+        { eventCache }: { eventCache: EventCache }
     ) {
         this.eventCache = eventCache;
-        this.getRawMachineId = getRawMachineId;
     }
 
     static create(
@@ -40,14 +34,12 @@ export class Telemetry {
         {
             commonProperties = { ...MACHINE_METADATA },
             eventCache = EventCache.getInstance(),
-            getRawMachineId = () => nodeMachineId.machineId(true),
         }: {
             eventCache?: EventCache;
-            getRawMachineId?: () => Promise<string>;
             commonProperties?: CommonProperties;
         } = {}
     ): Telemetry {
-        const instance = new Telemetry(session, userConfig, commonProperties, { eventCache, getRawMachineId });
+        const instance = new Telemetry(session, userConfig, commonProperties, { eventCache });
 
         void instance.setup();
         return instance;
@@ -57,26 +49,7 @@ export class Telemetry {
         if (!this.isTelemetryEnabled()) {
             return;
         }
-        this.setupPromise = Promise.all([
-            getDeviceId({
-                getMachineId: () => this.getRawMachineId(),
-                onError: (reason, error) => {
-                    switch (reason) {
-                        case "resolutionError":
-                            logger.debug(LogId.telemetryDeviceIdFailure, "telemetry", String(error));
-                            break;
-                        case "timeout":
-                            logger.debug(LogId.telemetryDeviceIdTimeout, "telemetry", "Device ID retrieval timed out");
-                            break;
-                        case "abort":
-                            // No need to log in the case of aborts
-                            break;
-                    }
-                },
-                abortSignal: this.deviceIdAbortController.signal,
-            }),
-            detectContainerEnv(),
-        ]);
+        this.setupPromise = Promise.all([getDeviceIdForConnection(), detectContainerEnv()]);
 
         const [deviceId, containerEnv] = await this.setupPromise;
 
@@ -87,8 +60,6 @@ export class Telemetry {
     }
 
     public async close(): Promise<void> {
-        this.deviceIdAbortController.abort();
-        this.isBufferingEvents = false;
         await this.emitEvents(this.eventCache.getEvents());
     }
 
diff --git a/src/tools/atlas/connect/connectCluster.ts b/src/tools/atlas/connect/connectCluster.ts
@@ -6,6 +6,8 @@ import { generateSecurePassword } from "../../../helpers/generatePassword.js";
 import logger, { LogId } from "../../../common/logger.js";
 import { inspectCluster } from "../../../common/atlas/cluster.js";
 import { ensureCurrentIpInAccessList } from "../../../common/atlas/accessListUtils.js";
+import { setAppNameParamIfMissing } from "../../../helpers/connectionOptions.js";
+import { packageInfo } from "../../../common/packageInfo.js";
 
 const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours
 
@@ -120,7 +122,15 @@ export class ConnectClusterTool extends AtlasToolBase {
         cn.username = username;
         cn.password = password;
         cn.searchParams.set("authSource", "admin");
-        return cn.toString();
+
+        const connectionStringWithAuth = cn.toString();
+        return await setAppNameParamIfMissing({
+            connectionString: connectionStringWithAuth,
+            components: {
+                appName: `${packageInfo.mcpServerName} ${packageInfo.version}`,
+                clientName: this.session.agentRunner?.name || "unknown",
+            },
+        });
     }
 
     private async connectToCluster(projectId: string, clusterName: string, connectionString: string): Promise<void> {
diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts
@@ -1,15 +1,12 @@
-import { createHmac } from "crypto";
 import { Telemetry } from "../../src/telemetry/telemetry.js";
 import { Session } from "../../src/common/session.js";
 import { config } from "../../src/common/config.js";
-import nodeMachineId from "node-machine-id";
+import { getDeviceIdForConnection } from "../../src/helpers/deviceId.js";
 import { describe, expect, it } from "vitest";
 
 describe("Telemetry", () => {
-    it("should resolve the actual machine ID", async () => {
-        const actualId: string = await nodeMachineId.machineId(true);
-
-        const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex");
+    it("should resolve the actual device ID", async () => {
+        const actualDeviceId = await getDeviceIdForConnection();
 
         const telemetry = Telemetry.create(
             new Session({
@@ -23,7 +20,7 @@ describe("Telemetry", () => {
 
         await telemetry.setupPromise;
 
-        expect(telemetry.getCommonProperties().device_id).toBe(actualHashedId);
+        expect(telemetry.getCommonProperties().device_id).toBe(actualDeviceId);
         expect(telemetry["isBufferingEvents"]).toBe(false);
     });
 });
diff --git a/tests/integration/tools/mongodb/connect/connect.test.ts b/tests/integration/tools/mongodb/connect/connect.test.ts
@@ -7,7 +7,12 @@ import {
 } from "../../../helpers.js";
 import { config } from "../../../../../src/common/config.js";
 import { defaultTestConfig, setupIntegrationTest } from "../../../helpers.js";
-import { beforeEach, describe, expect, it } from "vitest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+// Mock the deviceId utility for consistent testing
+vi.mock("../../../../../src/helpers/deviceId.js", () => ({
+    getDeviceIdForConnection: vi.fn().mockResolvedValue("test-device-id"),
+}));
 
 describeWithMongoDB(
     "SwitchConnection tool",
diff --git a/tests/unit/common/session.test.ts b/tests/unit/common/session.test.ts
@@ -4,6 +4,10 @@ import { Session } from "../../../src/common/session.js";
 import { config } from "../../../src/common/config.js";
 
 vi.mock("@mongosh/service-provider-node-driver");
+vi.mock("../../../src/helpers/deviceId.js", () => ({
+    getDeviceIdForConnection: vi.fn().mockResolvedValue("test-device-id"),
+}));
+
 const MockNodeDriverServiceProvider = vi.mocked(NodeDriverServiceProvider);
 
 describe("Session", () => {
@@ -50,11 +54,39 @@ describe("Session", () => {
                 expect(connectMock).toHaveBeenCalledOnce();
                 const connectionString = connectMock.mock.calls[0]?.[0];
                 if (testCase.expectAppName) {
-                    expect(connectionString).toContain("appName=MongoDB+MCP+Server");
+                    // Check for the extended appName format: appName--deviceId--clientName
+                    expect(connectionString).toContain("appName=MongoDB+MCP+Server+");
+                    expect(connectionString).toContain("--test-device-id--");
                 } else {
                     expect(connectionString).not.toContain("appName=MongoDB+MCP+Server");
                 }
             });
         }
+
+        it("should include client name when agent runner is set", async () => {
+            session.setAgentRunner({ name: "test-client", version: "1.0.0" });
+
+            await session.connectToMongoDB("mongodb://localhost:27017", config.connectOptions);
+            expect(session.serviceProvider).toBeDefined();
+
+            const connectMock = MockNodeDriverServiceProvider.connect;
+            expect(connectMock).toHaveBeenCalledOnce();
+            const connectionString = connectMock.mock.calls[0]?.[0];
+
+            // Should include the client name in the appName
+            expect(connectionString).toContain("--test-device-id--test-client");
+        });
+
+        it("should use 'unknown' for client name when agent runner is not set", async () => {
+            await session.connectToMongoDB("mongodb://localhost:27017", config.connectOptions);
+            expect(session.serviceProvider).toBeDefined();
+
+            const connectMock = MockNodeDriverServiceProvider.connect;
+            expect(connectMock).toHaveBeenCalledOnce();
+            const connectionString = connectMock.mock.calls[0]?.[0];
+
+            // Should use 'unknown' for client name when agent runner is not set
+            expect(connectionString).toContain("--test-device-id--unknown");
+        });
     });
 });
diff --git a/tests/unit/helpers/connectionOptions.test.ts b/tests/unit/helpers/connectionOptions.test.ts
diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts
