From a73de1f0bd8fd6159441407b0caad8d6ffb2bfcf Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 5 May 2025 13:41:52 +0200 Subject: [PATCH 1/5] chore: switch to `@mongodb-js/device-id` This package is meant to minimize code replication and to make sure the approach we're taking with hashing is consistent. --- package-lock.json | 7 +++++++ package.json | 1 + src/telemetry/telemetry.ts | 27 +++++++-------------------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index e71274d2..0998e03f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,6 +10,7 @@ "license": "Apache-2.0", "dependencies": { "@modelcontextprotocol/sdk": "^1.8.0", + "@mongodb-js/device-id": "^0.2.0", "@mongodb-js/devtools-connect": "^3.7.2", "@mongosh/service-provider-node-driver": "^3.6.0", "bson": "^6.10.3", @@ -2766,6 +2767,12 @@ "node": ">=16.20.0" } }, + "node_modules/@mongodb-js/device-id": { + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@mongodb-js/device-id/-/device-id-0.2.0.tgz", + "integrity": "sha512-auEMkQc6hpSQSQziK5AbeuJeVnI7OQvWmaoMIWcXrMm+RA6pF0ADXZPS6kBtBIrRhWElV6PVYiq+Gfzsss2RYQ==", + "license": "Apache-2.0" + }, "node_modules/@mongodb-js/devtools-connect": { "version": "3.7.2", "resolved": "https://registry.npmjs.org/@mongodb-js/devtools-connect/-/devtools-connect-3.7.2.tgz", diff --git a/package.json b/package.json index 6e77412f..81917242 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ }, "dependencies": { "@modelcontextprotocol/sdk": "^1.8.0", + "@mongodb-js/device-id": "^0.2.0", "@mongodb-js/devtools-connect": "^3.7.2", "@mongosh/service-provider-node-driver": "^3.6.0", "bson": "^6.10.3", diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 30a0363b..c9aa201b 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -5,9 +5,9 @@ import logger, { LogId } from "../logger.js"; import { ApiClient } from "../common/atlas/apiClient.js"; import { MACHINE_METADATA } from "./constants.js"; import { EventCache } from "./eventCache.js"; -import { createHmac } from "crypto"; import nodeMachineId from "node-machine-id"; import { DeferredPromise } from "../deferred-promise.js"; +import { getDeviceId as extractDeviceId } from "@mongodb-js/device-id"; type EventResult = { success: boolean; @@ -80,26 +80,13 @@ export class Telemetry { * @returns A hashed, unique identifier for the running device or `"unknown"` if not known. */ private async getDeviceId(): Promise { - try { - if (this.commonProperties.device_id) { - return this.commonProperties.device_id; - } - - const originalId: string = await this.getRawMachineId(); - - // Create a hashed format from the all uppercase version of the machine ID - // to match it exactly with the denisbrodbeck/machineid library that Atlas CLI uses. - const hmac = createHmac("sha256", originalId.toUpperCase()); - - /** This matches the message used to create the hashes in Atlas CLI */ - const DEVICE_ID_HASH_MESSAGE = "atlascli"; - - hmac.update(DEVICE_ID_HASH_MESSAGE); - return hmac.digest("hex"); - } catch (error) { - logger.debug(LogId.telemetryDeviceIdFailure, "telemetry", String(error)); - return "unknown"; + if (this.commonProperties.device_id) { + return this.commonProperties.device_id; } + return extractDeviceId({ + getMachineId: () => this.getRawMachineId(), + isNodeMachineId: true, + }).value; } /** From 22c99917479f3cdbaa9ea65595162a61fba50846 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 5 May 2025 14:15:26 +0200 Subject: [PATCH 2/5] refactor: remove timeout and deferred promise logic --- src/deferred-promise.ts | 58 -------------------------------------- src/telemetry/telemetry.ts | 35 ++++++++--------------- 2 files changed, 11 insertions(+), 82 deletions(-) delete mode 100644 src/deferred-promise.ts diff --git a/src/deferred-promise.ts b/src/deferred-promise.ts deleted file mode 100644 index 1eb3f6e0..00000000 --- a/src/deferred-promise.ts +++ /dev/null @@ -1,58 +0,0 @@ -type DeferredPromiseOptions = { - timeout?: number; - onTimeout?: (resolve: (value: T) => void, reject: (reason: Error) => void) => void; -}; - -/** Creates a promise and exposes its resolve and reject methods, with an optional timeout. */ -export class DeferredPromise extends Promise { - resolve: (value: T) => void; - reject: (reason: unknown) => void; - private timeoutId?: NodeJS.Timeout; - - constructor( - executor: (resolve: (value: T) => void, reject: (reason: Error) => void) => void, - { timeout, onTimeout }: DeferredPromiseOptions = {} - ) { - let resolveFn: (value: T) => void; - let rejectFn: (reason?: unknown) => void; - - super((resolve, reject) => { - resolveFn = resolve; - rejectFn = reject; - }); - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.resolve = resolveFn!; - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - this.reject = rejectFn!; - - if (timeout !== undefined && onTimeout) { - this.timeoutId = setTimeout(() => { - onTimeout(this.resolve, this.reject); - }, timeout); - } - - executor( - (value: T) => { - if (this.timeoutId) clearTimeout(this.timeoutId); - this.resolve(value); - }, - (reason: Error) => { - if (this.timeoutId) clearTimeout(this.timeoutId); - this.reject(reason); - } - ); - } - - static fromPromise(promise: Promise, options: DeferredPromiseOptions = {}): DeferredPromise { - return new DeferredPromise((resolve, reject) => { - promise - .then((value) => { - resolve(value); - }) - .catch((reason) => { - reject(reason as Error); - }); - }, options); - } -} diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index c9aa201b..6aaaaa06 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -6,8 +6,7 @@ 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 { DeferredPromise } from "../deferred-promise.js"; -import { getDeviceId as extractDeviceId } from "@mongodb-js/device-id"; +import { getDeviceId } from "@mongodb-js/device-id"; type EventResult = { success: boolean; @@ -19,7 +18,8 @@ export const DEVICE_ID_TIMEOUT = 3000; export class Telemetry { private isBufferingEvents: boolean = true; /** Resolves when the device ID is retrieved or timeout occurs */ - public deviceIdPromise: DeferredPromise | undefined; + public deviceIdPromise: Promise | undefined; + public resolveDeviceId: ((value: string) => void) | undefined; private eventCache: EventCache; private getRawMachineId: () => Promise; @@ -58,37 +58,24 @@ export class Telemetry { if (!this.isTelemetryEnabled()) { return; } - this.deviceIdPromise = DeferredPromise.fromPromise(this.getDeviceId(), { - timeout: DEVICE_ID_TIMEOUT, - onTimeout: (resolve) => { - resolve("unknown"); - logger.debug(LogId.telemetryDeviceIdTimeout, "telemetry", "Device ID retrieval timed out"); - }, + const { value: deviceId, resolve: resolveDeviceId } = getDeviceId({ + getMachineId: () => this.getRawMachineId(), + isNodeMachineId: true, + onError: (error) => logger.debug(LogId.telemetryDeviceIdFailure, "telemetry", String(error)), }); - this.commonProperties.device_id = await this.deviceIdPromise; + + this.resolveDeviceId = resolveDeviceId; + this.commonProperties.device_id = await deviceId; this.isBufferingEvents = false; } public async close(): Promise { - this.deviceIdPromise?.resolve("unknown"); + this.resolveDeviceId?.("unknown"); this.isBufferingEvents = false; await this.emitEvents(this.eventCache.getEvents()); } - /** - * @returns A hashed, unique identifier for the running device or `"unknown"` if not known. - */ - private async getDeviceId(): Promise { - if (this.commonProperties.device_id) { - return this.commonProperties.device_id; - } - return extractDeviceId({ - getMachineId: () => this.getRawMachineId(), - isNodeMachineId: true, - }).value; - } - /** * Emits events through the telemetry pipeline * @param events - The events to emit From 0681b6fa42cbcaa5e6370296172acc9b436157f8 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 5 May 2025 14:26:09 +0200 Subject: [PATCH 3/5] fix: delete unused tests --- src/telemetry/telemetry.ts | 2 - tests/integration/telemetry.test.ts | 1 - tests/unit/deferred-promise.test.ts | 72 ----------------------------- 3 files changed, 75 deletions(-) delete mode 100644 tests/unit/deferred-promise.test.ts diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 6aaaaa06..f61f6a24 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -39,8 +39,6 @@ export class Telemetry { { commonProperties = { ...MACHINE_METADATA }, eventCache = EventCache.getInstance(), - - // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access getRawMachineId = () => nodeMachineId.machineId(true), }: { eventCache?: EventCache; diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index fe8e51ff..522c1154 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -6,7 +6,6 @@ import nodeMachineId from "node-machine-id"; describe("Telemetry", () => { it("should resolve the actual machine ID", async () => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access const actualId: string = await nodeMachineId.machineId(true); const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex"); diff --git a/tests/unit/deferred-promise.test.ts b/tests/unit/deferred-promise.test.ts deleted file mode 100644 index c6011af1..00000000 --- a/tests/unit/deferred-promise.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { DeferredPromise } from "../../src/deferred-promise.js"; -import { jest } from "@jest/globals"; - -describe("DeferredPromise", () => { - beforeEach(() => { - jest.useFakeTimers(); - }); - afterEach(() => { - jest.useRealTimers(); - }); - - it("should resolve with the correct value", async () => { - const deferred = new DeferredPromise((resolve) => { - resolve("resolved value"); - }); - - await expect(deferred).resolves.toEqual("resolved value"); - }); - - it("should reject with the correct error", async () => { - const deferred = new DeferredPromise((_, reject) => { - reject(new Error("rejected error")); - }); - - await expect(deferred).rejects.toThrow("rejected error"); - }); - - it("should timeout if not resolved or rejected within the specified time", async () => { - const deferred = new DeferredPromise( - () => { - // Do not resolve or reject - }, - { timeout: 100, onTimeout: (resolve, reject) => reject(new Error("Promise timed out")) } - ); - - jest.advanceTimersByTime(100); - - await expect(deferred).rejects.toThrow("Promise timed out"); - }); - - it("should clear the timeout when resolved", async () => { - const deferred = new DeferredPromise( - (resolve) => { - setTimeout(() => resolve("resolved value"), 100); - }, - { timeout: 200 } - ); - - const promise = deferred.then((value) => { - expect(value).toBe("resolved value"); - }); - - jest.advanceTimersByTime(100); - await promise; - }); - - it("should clear the timeout when rejected", async () => { - const deferred = new DeferredPromise( - (_, reject) => { - setTimeout(() => reject(new Error("rejected error")), 100); - }, - { timeout: 200, onTimeout: (resolve, reject) => reject(new Error("Promise timed out")) } - ); - - const promise = deferred.catch((error) => { - expect(error).toEqual(new Error("rejected error")); - }); - - jest.advanceTimersByTime(100); - await promise; - }); -}); From 24c181063d5f2fd8ae03d43010315768cc5b4af1 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 5 May 2025 15:21:06 +0200 Subject: [PATCH 4/5] fix: use jest.cjs and fix tests --- eslint.config.js | 2 +- jest.config.ts => jest.config.cjs | 2 +- src/telemetry/telemetry.ts | 5 +++++ 3 files changed, 7 insertions(+), 2 deletions(-) rename jest.config.ts => jest.config.cjs (97%) diff --git a/eslint.config.js b/eslint.config.js index b42518a5..7416d1b0 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -48,7 +48,7 @@ export default defineConfig([ "coverage", "global.d.ts", "eslint.config.js", - "jest.config.ts", + "jest.config.cjs", ]), eslintPluginPrettierRecommended, ]); diff --git a/jest.config.ts b/jest.config.cjs similarity index 97% rename from jest.config.ts rename to jest.config.cjs index 7fb7ce67..f9a34b53 100644 --- a/jest.config.ts +++ b/jest.config.cjs @@ -1,5 +1,5 @@ /** @type {import('ts-jest').JestConfigWithTsJest} **/ -export default { +module.exports = { preset: "ts-jest/presets/default-esm", testEnvironment: "node", extensionsToTreatAsEsm: [".ts"], diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index f61f6a24..1b6f5bd3 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -60,8 +60,13 @@ export class Telemetry { getMachineId: () => this.getRawMachineId(), isNodeMachineId: true, onError: (error) => logger.debug(LogId.telemetryDeviceIdFailure, "telemetry", String(error)), + onTimeout: (resolve) => { + logger.debug(LogId.telemetryDeviceIdTimeout, "telemetry", "Device ID retrieval timed out"); + resolve("unknown"); + }, }); + this.deviceIdPromise = deviceId; this.resolveDeviceId = resolveDeviceId; this.commonProperties.device_id = await deviceId; From d2fc72db90b3d00298f50d87b0eed44bc7612808 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Thu, 8 May 2025 15:56:00 +0200 Subject: [PATCH 5/5] fix lint --- src/telemetry/telemetry.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 1b87471f..ccf0eb41 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -66,6 +66,9 @@ export class Telemetry { 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,