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

blva · blva · commit 4591c262eee0 · 2025-08-12T14:51:53.000+01:00
diff --git a/src/common/session.ts b/src/common/session.ts
@@ -147,4 +147,4 @@ export class Session extends EventEmitter<SessionEvents> {
     get connectedAtlasCluster(): AtlasClusterConnectionInfo | undefined {
         return this.connectionManager.currentConnectionState.connectedAtlasCluster;
     }
-}
+}
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 { 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 = (): Promise<string> => 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,35 +49,7 @@ export class Telemetry {
         if (!this.isTelemetryEnabled()) {
             return;
         }
-        this.setupPromise = Promise.all([
-            getDeviceId({
-                getMachineId: () => this.getRawMachineId(),
-                onError: (reason, error) => {
-                    switch (reason) {
-                        case "resolutionError":
-                            this.session.logger.debug({
-                                id: LogId.telemetryDeviceIdFailure,
-                                context: "telemetry",
-                                message: String(error),
-                            });
-                            break;
-                        case "timeout":
-                            this.session.logger.debug({
-                                id: LogId.telemetryDeviceIdTimeout,
-                                context: "telemetry",
-                                message: "Device ID retrieval timed out",
-                                noRedaction: true,
-                            });
-                            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;
 
@@ -96,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/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts
@@ -1,18 +1,15 @@
-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";
 import { CompositeLogger } from "../../src/common/logger.js";
 import { ConnectionManager } from "../../src/common/connectionManager.js";
 import { ExportsManager } from "../../src/common/exportsManager.js";
 
 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 logger = new CompositeLogger();
         const telemetry = Telemetry.create(
@@ -30,7 +27,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
@@ -7,6 +7,10 @@ import { ConnectionManager } from "../../../src/common/connectionManager.js";
 import { ExportsManager } from "../../../src/common/exportsManager.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", () => {
@@ -59,23 +63,51 @@ 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");
                 }
             });
         }
 
+<<<<<<< HEAD
         it("should configure the proxy to use environment variables", async () => {
             await session.connectToMongoDB({ connectionString: "mongodb://localhost" });
+=======
+        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);
+>>>>>>> c5c91e9 (feat: update connectionString appName param - [MCP-68])
             expect(session.serviceProvider).toBeDefined();
 
             const connectMock = MockNodeDriverServiceProvider.connect;
             expect(connectMock).toHaveBeenCalledOnce();
+<<<<<<< HEAD
 
             const connectionConfig = connectMock.mock.calls[0]?.[1];
             expect(connectionConfig?.proxy).toEqual({ useEnvironmentVariableProxies: true });
             expect(connectionConfig?.applyProxyToOIDC).toEqual(true);
+=======
+            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");
+>>>>>>> c5c91e9 (feat: update connectionString appName param - [MCP-68])
         });
     });
 });
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

Original file line number	Diff line number	Diff line change
`@@ -147,4 +147,4 @@ export class Session extends EventEmitter<SessionEvents> {`
`147`	`147`	`get connectedAtlasCluster(): AtlasClusterConnectionInfo \| undefined {`
`148`	`148`	`return this.connectionManager.currentConnectionState.connectedAtlasCluster;`
`149`	`149`	`}`
`150`		`-}`
	`150`	`+}`