From 5ac96a832ed44b578114745a2ebafec43b54ebaa Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 19 Mar 2025 14:01:00 +0100 Subject: [PATCH 01/17] feat(telemetry): add device ID logging --- package-lock.json | 8 ++++--- packages/logging/package.json | 3 ++- packages/logging/src/analytics-helpers.ts | 2 ++ .../logging/src/logging-and-telemetry.spec.ts | 24 +++++++++++++++++++ packages/logging/src/logging-and-telemetry.ts | 19 ++++++++++++++- packages/logging/src/types.ts | 2 ++ 6 files changed, 53 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 69fe85236..8b71d48d9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21994,8 +21994,9 @@ }, "node_modules/node-machine-id": { "version": "1.1.12", - "license": "MIT", - "optional": true + "resolved": "https://registry.npmjs.org/node-machine-id/-/node-machine-id-1.1.12.tgz", + "integrity": "sha512-QNABxbrPa3qEIfrE6GOJ7BYIuignnJw7iQ2YPbc3Nla1HzRJjXzZOiikfF8m7eAMfichLt3M4VgLOetqgDmgGQ==", + "license": "MIT" }, "node_modules/node-preload": { "version": "0.2.1", @@ -29828,7 +29829,8 @@ "@mongosh/history": "2.4.6", "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", - "mongodb-redact": "^1.1.5" + "mongodb-redact": "^1.1.5", + "node-machine-id": "^1.1.12" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/package.json b/packages/logging/package.json index 9f684d34a..dd543b7b1 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -22,7 +22,8 @@ "@mongosh/history": "2.4.6", "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", - "mongodb-redact": "^1.1.5" + "mongodb-redact": "^1.1.5", + "node-machine-id": "^1.1.12" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/src/analytics-helpers.ts b/packages/logging/src/analytics-helpers.ts index b1f922936..db529e90d 100644 --- a/packages/logging/src/analytics-helpers.ts +++ b/packages/logging/src/analytics-helpers.ts @@ -3,10 +3,12 @@ import path from 'path'; export type MongoshAnalyticsIdentity = | { + deviceId?: string; userId: string; anonymousId?: never; } | { + deviceId?: string; userId?: never; anonymousId: string; }; diff --git a/packages/logging/src/logging-and-telemetry.spec.ts b/packages/logging/src/logging-and-telemetry.spec.ts index bfcfd8af7..2eee4c122 100644 --- a/packages/logging/src/logging-and-telemetry.spec.ts +++ b/packages/logging/src/logging-and-telemetry.spec.ts @@ -40,6 +40,7 @@ describe('MongoshLoggingAndTelemetry', function () { loggingAndTelemetry = setupLoggingAndTelemetry({ bus, analytics, + deviceId: 'test-device', userTraits: { platform: process.platform, arch: process.arch, @@ -105,6 +106,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'identify', { anonymousId: userId, + deviceId: 'test-device', traits: { arch: process.arch, platform: process.platform, @@ -116,6 +118,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: userId, + deviceId: 'test-device', event: 'New Connection', properties: { mongosh_version: '1.0.0', @@ -163,6 +166,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'identify', { anonymousId: userId, + deviceId: 'test-device', traits: { arch: process.arch, platform: process.platform, @@ -174,6 +178,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: userId, + deviceId: 'test-device', event: 'New Connection', properties: { mongosh_version: '1.0.0', @@ -576,6 +581,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'identify', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', traits: { platform: process.platform, arch: process.arch, @@ -587,6 +593,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'identify', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', traits: { platform: process.platform, arch: process.arch, @@ -598,6 +605,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Startup Time', properties: { is_interactive: true, @@ -613,6 +621,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Error', properties: { mongosh_version: '1.0.0', @@ -628,6 +637,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Error', properties: { mongosh_version: '1.0.0', @@ -643,6 +653,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Use', properties: { mongosh_version: '1.0.0', @@ -654,6 +665,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Show', properties: { mongosh_version: '1.0.0', @@ -673,6 +685,7 @@ describe('MongoshLoggingAndTelemetry', function () { shell: true, }, anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', }, ], [ @@ -685,6 +698,7 @@ describe('MongoshLoggingAndTelemetry', function () { nested: false, }, anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', }, ], [ @@ -696,6 +710,7 @@ describe('MongoshLoggingAndTelemetry', function () { session_id: logId, }, anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', }, ], [ @@ -707,6 +722,7 @@ describe('MongoshLoggingAndTelemetry', function () { session_id: logId, }, anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', }, ], [ @@ -719,12 +735,14 @@ describe('MongoshLoggingAndTelemetry', function () { shell: true, }, anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', }, ], [ 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Snippet Install', properties: { mongosh_version: '1.0.0', @@ -819,6 +837,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Deprecated Method', properties: { mongosh_version: '1.0.0', @@ -832,6 +851,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Deprecated Method', properties: { mongosh_version: '1.0.0', @@ -845,6 +865,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'Deprecated Method', properties: { mongosh_version: '1.0.0', @@ -858,6 +879,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'API Call', properties: { mongosh_version: '1.0.0', @@ -872,6 +894,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: '53defe995fa47e6c13102d9d', + deviceId: 'test-device', event: 'API Call', properties: { mongosh_version: '1.0.0', @@ -1023,6 +1046,7 @@ describe('MongoshLoggingAndTelemetry', function () { 'track', { anonymousId: undefined, + deviceId: 'test-device', event: 'New Connection', properties: { mongosh_version: '1.0.0', diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 69b77fa50..724b7aa37 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -52,10 +52,23 @@ import type { MongoshLoggingAndTelemetryArguments, MongoshTrackingProperties, } from './types'; +import { machineIdSync } from 'node-machine-id'; export function setupLoggingAndTelemetry( props: MongoshLoggingAndTelemetryArguments ): MongoshLoggingAndTelemetry { + if (!props.deviceId) { + try { + props.deviceId = machineIdSync(); + } catch (error) { + props.bus.emit( + 'mongosh:error', + new Error('Failed to get device ID'), + 'telemetry' + ); + } + } + const loggingAndTelemetry = new LoggingAndTelemetry(props); loggingAndTelemetry.setup(); @@ -80,6 +93,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { [key: string]: unknown; }; private readonly mongoshVersion: string; + private readonly deviceId: string | undefined; private log: MongoLogWriter; private pendingLogEvents: CallableFunction[] = []; @@ -91,12 +105,14 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { analytics, userTraits, mongoshVersion, + deviceId, }: MongoshLoggingAndTelemetryArguments) { this.bus = bus; this.analytics = analytics; this.log = LoggingAndTelemetry.dummyLogger; this.userTraits = userTraits; this.mongoshVersion = mongoshVersion; + this.deviceId = deviceId; } public setup(): void { @@ -196,6 +212,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { anonymousId: this.busEventState.telemetryAnonymousId ?? (this.busEventState.userId as string), + deviceId: this.deviceId, }; }; @@ -285,7 +302,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.busEventState.telemetryAnonymousId = newTelemetryUserIdentity.anonymousId; this.analytics.identify({ - anonymousId: newTelemetryUserIdentity.anonymousId, + ...getTelemetryUserIdentity(), traits: getUserTraits(), }); } diff --git a/packages/logging/src/types.ts b/packages/logging/src/types.ts index 797fd5990..b75b414a7 100644 --- a/packages/logging/src/types.ts +++ b/packages/logging/src/types.ts @@ -15,6 +15,8 @@ export type MongoshLoggingAndTelemetryArguments = { [key: string]: unknown; }; mongoshVersion: string; + /** Machine-specific ID; gets set automatically when omitted */ + deviceId?: string | undefined; }; export type MongoshTrackingProperties = { From b168ea2614c8a646b1c4b04927a41c54b1571611 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 24 Mar 2025 11:11:34 +0100 Subject: [PATCH 02/17] feat: align with Atlas CLI hashing and resolve device ID asynchronously --- .../logging/src/logging-and-telemetry.spec.ts | 120 +++++++++++++++++- packages/logging/src/logging-and-telemetry.ts | 78 ++++++++---- packages/logging/src/types.ts | 1 + 3 files changed, 170 insertions(+), 29 deletions(-) diff --git a/packages/logging/src/logging-and-telemetry.spec.ts b/packages/logging/src/logging-and-telemetry.spec.ts index 2eee4c122..bc4f40321 100644 --- a/packages/logging/src/logging-and-telemetry.spec.ts +++ b/packages/logging/src/logging-and-telemetry.spec.ts @@ -7,6 +7,10 @@ import type { MongoshBus } from '@mongosh/types'; import type { Writable } from 'stream'; import type { MongoshLoggingAndTelemetry } from '.'; import { setupLoggingAndTelemetry } from '.'; +import { getDeviceId } from './logging-and-telemetry'; +import sinon from 'sinon'; +import type { MongoshLoggingAndTelemetryArguments } from './types'; +import { eventually } from '../../../testing/eventually'; describe('MongoshLoggingAndTelemetry', function () { let logOutput: any[]; @@ -32,13 +36,8 @@ describe('MongoshLoggingAndTelemetry', function () { let loggingAndTelemetry: MongoshLoggingAndTelemetry; - beforeEach(function () { - logOutput = []; - analyticsOutput = []; - bus = new EventEmitter(); - - loggingAndTelemetry = setupLoggingAndTelemetry({ - bus, + const testLoggingArguments: Omit = + { analytics, deviceId: 'test-device', userTraits: { @@ -46,6 +45,16 @@ describe('MongoshLoggingAndTelemetry', function () { arch: process.arch, }, mongoshVersion: '1.0.0', + }; + + beforeEach(function () { + logOutput = []; + analyticsOutput = []; + bus = new EventEmitter(); + + loggingAndTelemetry = setupLoggingAndTelemetry({ + ...testLoggingArguments, + bus, }); logger = new MongoLogWriter(logId, `/tmp/${logId}_log`, { @@ -193,6 +202,94 @@ describe('MongoshLoggingAndTelemetry', function () { ]); }); + describe('device ID', function () { + let loggingAndTelemetry: MongoshLoggingAndTelemetry; + let bus: EventEmitter; + beforeEach(function () { + bus = new EventEmitter(); + loggingAndTelemetry = setupLoggingAndTelemetry({ + ...testLoggingArguments, + bus, + deviceId: undefined, + }); + }); + + it('uses device ID "unknown" if it fails to resolve it', async function () { + loggingAndTelemetry.attachLogger(logger); + // eslint-disable-next-line @typescript-eslint/no-var-requires + sinon.stub(require('child_process'), 'exec').throws(); + + bus.emit('mongosh:new-user', { userId, anonymousId: userId }); + + await eventually(() => { + expect(analyticsOutput).deep.equal([ + [ + 'identify', + { + deviceId: undefined, + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, + }, + }, + ], + [ + 'identify', + { + deviceId: 'unknown', + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, + }, + }, + ], + ]); + }); + sinon.restore(); + }); + + it('automatically sets up device ID for telemetry', async function () { + loggingAndTelemetry.attachLogger(logger); + + bus.emit('mongosh:new-user', { userId, anonymousId: userId }); + + const deviceId = await getDeviceId(); + + await eventually(() => { + expect(analyticsOutput).deep.equal([ + [ + 'identify', + { + deviceId: undefined, + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, + }, + }, + ], + [ + 'identify', + { + deviceId, + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, + }, + }, + ], + ]); + }); + }); + }); + it('detaching logger leads to no logging but persists analytics', function () { loggingAndTelemetry.attachLogger(logger); @@ -1060,4 +1157,13 @@ describe('MongoshLoggingAndTelemetry', function () { ], ]); }); + + describe('getDeviceId()', function () { + it('is consistent on the same machine', async function () { + const idA = await getDeviceId(); + const idB = await getDeviceId(); + + expect(idA).equals(idB); + }); + }); }); diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 724b7aa37..7f73f88c3 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -52,29 +52,35 @@ import type { MongoshLoggingAndTelemetryArguments, MongoshTrackingProperties, } from './types'; -import { machineIdSync } from 'node-machine-id'; +import { machineId } from 'node-machine-id'; +import { createHmac } from 'crypto'; export function setupLoggingAndTelemetry( props: MongoshLoggingAndTelemetryArguments ): MongoshLoggingAndTelemetry { - if (!props.deviceId) { - try { - props.deviceId = machineIdSync(); - } catch (error) { - props.bus.emit( - 'mongosh:error', - new Error('Failed to get device ID'), - 'telemetry' - ); - } - } - const loggingAndTelemetry = new LoggingAndTelemetry(props); loggingAndTelemetry.setup(); return loggingAndTelemetry; } +/** + * @returns A hashed, unique identifier for the running device. + * @throws If something goes wrong when getting the device ID. + */ +export async function getDeviceId(): Promise { + // 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 originalId = (await machineId(true)).toUpperCase(); + const hmac = createHmac('sha256', originalId); + + /** 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'); +} + class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { private static dummyLogger = new MongoLogWriter( '', @@ -170,6 +176,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { usesShellOption: false, telemetryAnonymousId: undefined, userId: undefined, + deviceId: undefined, }; private setupBusEventListeners(): void { @@ -207,15 +214,47 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { session_id: this.log.logId, }); + /** Eventually sets up the device ID and re-identifies the user. */ + const getCurrentDeviceId = async (): Promise => { + try { + this.busEventState.deviceId ??= this.deviceId ?? (await getDeviceId()); + } catch (error) { + this.bus.emit('mongosh:error', error as Error, 'telemetry'); + this.busEventState.deviceId = 'unknown'; + } + return this.busEventState.deviceId; + }; + const getTelemetryUserIdentity = (): MongoshAnalyticsIdentity => { return { + deviceId: this.busEventState.deviceId ?? this.deviceId, anonymousId: this.busEventState.telemetryAnonymousId ?? (this.busEventState.userId as string), - deviceId: this.deviceId, }; }; + const identifyUser = async (): Promise => { + // We first instantly identify the user with the + // current user information we have. + this.analytics.identify({ + ...getTelemetryUserIdentity(), + traits: getUserTraits(), + }); + + if (!this.busEventState.deviceId) { + // If the Device ID had not been resolved yet, + // we wait to resolve it and re-identify the user. + this.busEventState.deviceId ??= await getCurrentDeviceId(); + + this.analytics.identify({ + ...getTelemetryUserIdentity(), + deviceId: await getCurrentDeviceId(), + traits: getUserTraits(), + }); + } + }; + onBus('mongosh:start-mongosh-repl', (ev: StartMongoshReplEvent) => { this.log.info( 'MONGOSH', @@ -301,10 +340,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { } this.busEventState.telemetryAnonymousId = newTelemetryUserIdentity.anonymousId; - this.analytics.identify({ - ...getTelemetryUserIdentity(), - traits: getUserTraits(), - }); + + void identifyUser(); } ); @@ -320,10 +357,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { } else { this.busEventState.userId = updatedTelemetryUserIdentity.userId; } - this.analytics.identify({ - ...getTelemetryUserIdentity(), - traits: getUserTraits(), - }); + void identifyUser(); this.log.info( 'MONGOSH', mongoLogId(1_000_000_005), diff --git a/packages/logging/src/types.ts b/packages/logging/src/types.ts index b75b414a7..eb2250ff6 100644 --- a/packages/logging/src/types.ts +++ b/packages/logging/src/types.ts @@ -34,4 +34,5 @@ export type LoggingAndTelemetryBusEventState = { usesShellOption: boolean; telemetryAnonymousId: string | undefined; userId: string | undefined; + deviceId: string | undefined; }; From 79b643ce13180ac7662816d2e1a87cecb5ce3288 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 24 Mar 2025 12:41:43 +0100 Subject: [PATCH 03/17] fix: ignore node-fetch depalign --- .depalignrc.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.depalignrc.json b/.depalignrc.json index d42c9548c..4b9119aaa 100644 --- a/.depalignrc.json +++ b/.depalignrc.json @@ -18,6 +18,10 @@ "@typescript-eslint/parser": [ "^5.59.0", "^4.33.0" + ], + "node-fetch": [ + "^3.3.2", + "2.6.12" ] } } \ No newline at end of file From 52d19b7e17105a2698da1ec8b1b9ae4556b7d064 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 24 Mar 2025 13:08:40 +0100 Subject: [PATCH 04/17] fix: add sinon dependency --- package-lock.json | 112 +++++++++++++++++++++++++++++++++- packages/logging/package.json | 3 +- 2 files changed, 113 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8b71d48d9..71b8037cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29838,12 +29838,79 @@ "@mongodb-js/tsconfig-mongosh": "^1.0.0", "depcheck": "^1.4.7", "eslint": "^7.25.0", - "prettier": "^2.8.8" + "prettier": "^2.8.8", + "sinon": "^19.0.4" }, "engines": { "node": ">=14.15.1" } }, + "packages/logging/node_modules/@sinonjs/commons": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-3.0.1.tgz", + "integrity": "sha512-K3mCHKQ9sVh8o1C9cxkwxaOmXoAMlDxC1mYyHrjqOWEcBjYr76t96zL2zlj5dUGZ3HSw240X1qgH3Mjf1yJWpQ==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "type-detect": "4.0.8" + } + }, + "packages/logging/node_modules/@sinonjs/fake-timers": { + "version": "13.0.5", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-13.0.5.tgz", + "integrity": "sha512-36/hTbH2uaWuGVERyC6da9YwGWnzUZXuPro/F2LfsdOsLnCojz/iSH8MxUt/FD2S5XBSVPhmArFUXcpCQ2Hkiw==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1" + } + }, + "packages/logging/node_modules/@sinonjs/samsam": { + "version": "8.0.2", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-8.0.2.tgz", + "integrity": "sha512-v46t/fwnhejRSFTGqbpn9u+LQ9xJDse10gNnPgAcxgdoCDMXj/G2asWAC/8Qs+BAZDicX+MNZouXT1A7c83kVw==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1", + "lodash.get": "^4.4.2", + "type-detect": "^4.1.0" + } + }, + "packages/logging/node_modules/@sinonjs/samsam/node_modules/type-detect": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.1.0.tgz", + "integrity": "sha512-Acylog8/luQ8L7il+geoSxhEkazvkslg7PSNKOX59mbB9cOveP5aq9h74Y7YU8yDpJwetzQQrfIwtf4Wp4LKcw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=4" + } + }, + "packages/logging/node_modules/@sinonjs/text-encoding": { + "version": "0.7.3", + "resolved": "https://registry.npmjs.org/@sinonjs/text-encoding/-/text-encoding-0.7.3.tgz", + "integrity": "sha512-DE427ROAphMQzU4ENbliGYrBSYPXF+TtLg9S8vzeA+OF4ZKzoDdzfL8sxuMUGS/lgRhM6j1URSk9ghf7Xo1tyA==", + "dev": true, + "license": "(Unlicense OR Apache-2.0)" + }, + "packages/logging/node_modules/diff": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-7.0.0.tgz", + "integrity": "sha512-PJWHUb1RFevKCwaFA9RlG5tCd+FO5iRh9A8HEtkmBH2Li03iJriB6m6JIN4rGz3K3JLawI7/veA1xzRKP6ISBw==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, + "packages/logging/node_modules/just-extend": { + "version": "6.2.0", + "resolved": "https://registry.npmjs.org/just-extend/-/just-extend-6.2.0.tgz", + "integrity": "sha512-cYofQu2Xpom82S6qD778jBDpwvvy39s1l/hrYij2u9AMdQcGRpaBu6kY4mVhuno5kJVi1DAz4aiphA2WI1/OAw==", + "dev": true, + "license": "MIT" + }, "packages/logging/node_modules/mongodb-log-writer": { "version": "2.3.1", "resolved": "https://registry.npmjs.org/mongodb-log-writer/-/mongodb-log-writer-2.3.1.tgz", @@ -29862,6 +29929,49 @@ "integrity": "sha512-bLTHIHviJvTGJDvCECDBEDMk7beJQ4Fvoec50hgIax98ojzyTk9xIyrewFPM7yzlDVKTkkh864uxlkkTTLVsbg==", "license": "Apache-2.0" }, + "packages/logging/node_modules/nise": { + "version": "6.1.1", + "resolved": "https://registry.npmjs.org/nise/-/nise-6.1.1.tgz", + "integrity": "sha512-aMSAzLVY7LyeM60gvBS423nBmIPP+Wy7St7hsb+8/fc1HmeoHJfLO8CKse4u3BtOZvQLJghYPI2i/1WZrEj5/g==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1", + "@sinonjs/fake-timers": "^13.0.1", + "@sinonjs/text-encoding": "^0.7.3", + "just-extend": "^6.2.0", + "path-to-regexp": "^8.1.0" + } + }, + "packages/logging/node_modules/path-to-regexp": { + "version": "8.2.0", + "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-8.2.0.tgz", + "integrity": "sha512-TdrF7fW9Rphjq4RjrW0Kp2AW0Ahwu9sRGTkS6bvDi0SCwZlEZYmcfDbEsTz8RVk0EHIS/Vd1bv3JhG+1xZuAyQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=16" + } + }, + "packages/logging/node_modules/sinon": { + "version": "19.0.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-19.0.4.tgz", + "integrity": "sha512-myidFob7fjmYHJb+CHNLtAYScxn3sngGq4t75L2rCGGpE/k4OQVkN3KE5FsN+XkO2+fcDZ65PGvq3KHrlLAm7g==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1", + "@sinonjs/fake-timers": "^13.0.5", + "@sinonjs/samsam": "^8.0.1", + "diff": "^7.0.0", + "nise": "^6.1.1", + "supports-color": "^7.2.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, "packages/mongosh": { "version": "2.5.0", "license": "Apache-2.0", diff --git a/packages/logging/package.json b/packages/logging/package.json index dd543b7b1..c1b262ae3 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -31,7 +31,8 @@ "@mongodb-js/tsconfig-mongosh": "^1.0.0", "depcheck": "^1.4.7", "eslint": "^7.25.0", - "prettier": "^2.8.8" + "prettier": "^2.8.8", + "sinon": "^19.0.4" }, "scripts": { "test": "mocha -r \"../../scripts/import-expansions.js\" --timeout 15000 -r ts-node/register --reporter \"../../configs/mocha-config-mongosh/reporter.ts\" \"./src/**/*.spec.ts\"", From d84e466b1f6cdbe58655d2ca70ed9cddbfc3e0d7 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 24 Mar 2025 14:41:07 +0100 Subject: [PATCH 05/17] fix: allow multiple identify calls for device ID --- packages/logging/src/analytics-helpers.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/logging/src/analytics-helpers.ts b/packages/logging/src/analytics-helpers.ts index db529e90d..a1688b8b6 100644 --- a/packages/logging/src/analytics-helpers.ts +++ b/packages/logging/src/analytics-helpers.ts @@ -272,8 +272,11 @@ export class ThrottledAnalytics implements MongoshAnalytics { identify(message: AnalyticsIdentifyMessage): void { message = addTimestamp(message); - if (this.currentUserId) { - throw new Error('Identify can only be called once per user session'); + + if (this.currentUserId && !message.deviceId) { + throw new Error( + 'Identifying without device ID can only be called once per user session' + ); } this.currentUserId = message.userId ?? message.anonymousId; this.restorePromise = this.restoreThrottleState().then((enabled) => { From b112e1c48151978ec5a8bd2b37cbb321107f0127 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 25 Mar 2025 10:41:46 +0100 Subject: [PATCH 06/17] refactor: delay telemetry until device ID is resolved --- packages/logging/src/analytics-helpers.ts | 9 +- .../logging/src/logging-and-telemetry.spec.ts | 189 +++++++++++------- packages/logging/src/logging-and-telemetry.ts | 173 ++++++++-------- packages/logging/src/types.ts | 1 - 4 files changed, 209 insertions(+), 163 deletions(-) diff --git a/packages/logging/src/analytics-helpers.ts b/packages/logging/src/analytics-helpers.ts index a1688b8b6..3123111c0 100644 --- a/packages/logging/src/analytics-helpers.ts +++ b/packages/logging/src/analytics-helpers.ts @@ -18,7 +18,7 @@ export type AnalyticsIdentifyMessage = MongoshAnalyticsIdentity & { timestamp?: Date; }; -type AnalyticsTrackMessage = MongoshAnalyticsIdentity & { +export type AnalyticsTrackMessage = MongoshAnalyticsIdentity & { event: string; properties: { mongosh_version: string; @@ -272,11 +272,8 @@ export class ThrottledAnalytics implements MongoshAnalytics { identify(message: AnalyticsIdentifyMessage): void { message = addTimestamp(message); - - if (this.currentUserId && !message.deviceId) { - throw new Error( - 'Identifying without device ID can only be called once per user session' - ); + if (this.currentUserId) { + throw new Error('Identify can only be called once per user session'); } this.currentUserId = message.userId ?? message.anonymousId; this.restorePromise = this.restoreThrottleState().then((enabled) => { diff --git a/packages/logging/src/logging-and-telemetry.spec.ts b/packages/logging/src/logging-and-telemetry.spec.ts index bc4f40321..10864d70e 100644 --- a/packages/logging/src/logging-and-telemetry.spec.ts +++ b/packages/logging/src/logging-and-telemetry.spec.ts @@ -7,10 +7,10 @@ import type { MongoshBus } from '@mongosh/types'; import type { Writable } from 'stream'; import type { MongoshLoggingAndTelemetry } from '.'; import { setupLoggingAndTelemetry } from '.'; +import type { LoggingAndTelemetry } from './logging-and-telemetry'; import { getDeviceId } from './logging-and-telemetry'; import sinon from 'sinon'; import type { MongoshLoggingAndTelemetryArguments } from './types'; -import { eventually } from '../../../testing/eventually'; describe('MongoshLoggingAndTelemetry', function () { let logOutput: any[]; @@ -86,9 +86,11 @@ describe('MongoshLoggingAndTelemetry', function () { expect(() => loggingAndTelemetry.attachLogger(logger)).does.not.throw(); }); - it('tracks new local connection events', function () { + it('tracks new local connection events', async function () { loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; + expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.be.empty; @@ -142,8 +144,9 @@ describe('MongoshLoggingAndTelemetry', function () { ]); }); - it('tracks new atlas connection events', function () { + it('tracks new atlas connection events', async function () { loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.be.empty; @@ -203,95 +206,122 @@ describe('MongoshLoggingAndTelemetry', function () { }); describe('device ID', function () { - let loggingAndTelemetry: MongoshLoggingAndTelemetry; let bus: EventEmitter; beforeEach(function () { bus = new EventEmitter(); - loggingAndTelemetry = setupLoggingAndTelemetry({ + }); + + afterEach(function () { + sinon.restore(); + }); + + it('uses device ID "unknown" and logs error if it fails to resolve it', async function () { + // eslint-disable-next-line @typescript-eslint/no-var-requires + sinon.stub(require('child_process'), 'exec').throws(new Error('Test')); + const loggingAndTelemetry = setupLoggingAndTelemetry({ ...testLoggingArguments, bus, deviceId: undefined, }); - }); - - it('uses device ID "unknown" if it fails to resolve it', async function () { loggingAndTelemetry.attachLogger(logger); - // eslint-disable-next-line @typescript-eslint/no-var-requires - sinon.stub(require('child_process'), 'exec').throws(); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; bus.emit('mongosh:new-user', { userId, anonymousId: userId }); - await eventually(() => { - expect(analyticsOutput).deep.equal([ - [ - 'identify', - { - deviceId: undefined, - anonymousId: userId, - traits: { - platform: process.platform, - arch: process.arch, - session_id: logId, - }, - }, - ], - [ - 'identify', - { - deviceId: 'unknown', - anonymousId: userId, - traits: { - platform: process.platform, - arch: process.arch, - session_id: logId, - }, + expect(analyticsOutput).deep.equal([ + [ + 'identify', + { + deviceId: 'unknown', + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, }, - ], - ]); + }, + ], + ]); + expect(logOutput[0]).contains({ + c: 'MONGOSH', + id: 1000000006, + ctx: 'telemetry', + msg: 'Error: Test', }); - sinon.restore(); }); it('automatically sets up device ID for telemetry', async function () { + const loggingAndTelemetry = setupLoggingAndTelemetry({ + ...testLoggingArguments, + bus, + deviceId: undefined, + }); + loggingAndTelemetry.attachLogger(logger); bus.emit('mongosh:new-user', { userId, anonymousId: userId }); const deviceId = await getDeviceId(); - await eventually(() => { - expect(analyticsOutput).deep.equal([ - [ - 'identify', - { - deviceId: undefined, - anonymousId: userId, - traits: { - platform: process.platform, - arch: process.arch, - session_id: logId, - }, + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; + + expect(analyticsOutput).deep.equal([ + [ + 'identify', + { + deviceId, + anonymousId: userId, + traits: { + platform: process.platform, + arch: process.arch, + session_id: logId, }, - ], - [ - 'identify', - { - deviceId, - anonymousId: userId, - traits: { - platform: process.platform, - arch: process.arch, - session_id: logId, - }, - }, - ], - ]); + }, + ], + ]); + }); + + it('only delays analytic outputs, not logging', async function () { + // eslint-disable-next-line @typescript-eslint/no-empty-function + let resolveTelemetry: (value: unknown) => void = () => {}; + const delayedTelemetry = new Promise((resolve) => { + resolveTelemetry = (value) => resolve(value); }); + sinon + // eslint-disable-next-line @typescript-eslint/no-var-requires + .stub(require('node-machine-id'), 'machineId') + .resolves(delayedTelemetry); + + const loggingAndTelemetry = setupLoggingAndTelemetry({ + ...testLoggingArguments, + bus, + deviceId: undefined, + }); + + loggingAndTelemetry.attachLogger(logger); + + // This event has both analytics and logging + bus.emit('mongosh:use', { db: '' }); + + expect(logOutput).to.have.lengthOf(1); + expect(analyticsOutput).to.have.lengthOf(0); + + resolveTelemetry('1234'); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; + + expect(logOutput).to.have.lengthOf(1); + expect(analyticsOutput).to.have.lengthOf(1); + + // Hash created from machine ID 1234 + expect(analyticsOutput[0][1].deviceId).equals( + '8c9f929608f0ef13bfd5a290e0233f283e2cc25ffefc2ad8d9ef0650eb224a52' + ); }); }); - it('detaching logger leads to no logging but persists analytics', function () { + it('detaching logger leads to no logging but persists analytics', async function () { loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.have.lengthOf(0); @@ -328,7 +358,7 @@ describe('MongoshLoggingAndTelemetry', function () { expect(logOutput).to.have.lengthOf(4); }); - it('detaching logger mid-way leads to no logging but persists analytics', function () { + it('detaching logger mid-way leads to no logging but persists analytics', async function () { loggingAndTelemetry.attachLogger(logger); expect(logOutput).to.have.lengthOf(0); @@ -338,6 +368,8 @@ describe('MongoshLoggingAndTelemetry', function () { bus.emit('mongosh:use', { db: '' }); expect(logOutput).to.have.lengthOf(1); + + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(1); loggingAndTelemetry.detachLogger(); @@ -345,10 +377,12 @@ describe('MongoshLoggingAndTelemetry', function () { bus.emit('mongosh:use', { db: '' }); expect(logOutput).to.have.lengthOf(1); + + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(2); }); - it('detaching logger is recoverable', function () { + it('detaching logger is recoverable', async function () { loggingAndTelemetry.attachLogger(logger); expect(logOutput).to.have.lengthOf(0); @@ -358,6 +392,8 @@ describe('MongoshLoggingAndTelemetry', function () { bus.emit('mongosh:use', { db: '' }); expect(logOutput).to.have.lengthOf(1); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; + expect(analyticsOutput).to.have.lengthOf(1); loggingAndTelemetry.detachLogger(); @@ -365,6 +401,8 @@ describe('MongoshLoggingAndTelemetry', function () { bus.emit('mongosh:use', { db: '' }); expect(logOutput).to.have.lengthOf(1); + + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(2); loggingAndTelemetry.attachLogger(logger); @@ -372,11 +410,14 @@ describe('MongoshLoggingAndTelemetry', function () { bus.emit('mongosh:use', { db: '' }); expect(logOutput).to.have.lengthOf(2); + + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(3); }); - it('tracks a sequence of events', function () { + it('tracks a sequence of events', async function () { loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.be.empty; @@ -677,7 +718,7 @@ describe('MongoshLoggingAndTelemetry', function () { [ 'identify', { - anonymousId: '53defe995fa47e6c13102d9d', + anonymousId: userId, deviceId: 'test-device', traits: { platform: process.platform, @@ -689,7 +730,7 @@ describe('MongoshLoggingAndTelemetry', function () { [ 'identify', { - anonymousId: '53defe995fa47e6c13102d9d', + anonymousId: userId, deviceId: 'test-device', traits: { platform: process.platform, @@ -701,7 +742,7 @@ describe('MongoshLoggingAndTelemetry', function () { [ 'track', { - anonymousId: '53defe995fa47e6c13102d9d', + anonymousId: userId, deviceId: 'test-device', event: 'Startup Time', properties: { @@ -850,8 +891,9 @@ describe('MongoshLoggingAndTelemetry', function () { ]); }); - it('buffers deprecated API calls', function () { + it('buffers deprecated API calls', async function () { loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.be.empty; @@ -1005,6 +1047,7 @@ describe('MongoshLoggingAndTelemetry', function () { ]); bus.emit('mongosh:new-user', { userId, anonymousId: userId }); + logOutput = []; analyticsOutput = []; @@ -1027,6 +1070,7 @@ describe('MongoshLoggingAndTelemetry', function () { class: 'Database', method: 'cloneDatabase', }); + expect(analyticsOutput).to.have.lengthOf(2); }); @@ -1054,11 +1098,12 @@ describe('MongoshLoggingAndTelemetry', function () { expect(analyticsOutput).to.be.empty; }); - it('tracks custom logging events', function () { + it('tracks custom logging events', async function () { expect(logOutput).to.have.lengthOf(0); expect(analyticsOutput).to.be.empty; loggingAndTelemetry.attachLogger(logger); + await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; bus.emit('mongosh:connect', { uri: 'mongodb://localhost/', diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 7f73f88c3..9d0bd4a1e 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -39,6 +39,7 @@ import { MongoLogWriter } from 'mongodb-log-writer'; import { mongoLogId } from 'mongodb-log-writer'; import type { AnalyticsIdentifyMessage, + AnalyticsTrackMessage, MongoshAnalytics, MongoshAnalyticsIdentity, } from './analytics-helpers'; @@ -81,7 +82,8 @@ export async function getDeviceId(): Promise { return hmac.digest('hex'); } -class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { +/** @internal */ +export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { private static dummyLogger = new MongoLogWriter( '', null, @@ -99,12 +101,17 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { [key: string]: unknown; }; private readonly mongoshVersion: string; - private readonly deviceId: string | undefined; private log: MongoLogWriter; - private pendingLogEvents: CallableFunction[] = []; + private pendingBusEvents: CallableFunction[] = []; + private pendingTelemetryEvents: CallableFunction[] = []; private isSetup = false; - private isBufferingEvents = false; + private isBufferingBusEvents = false; + private isBufferingTelemetryEvents = false; + + private deviceId: string | undefined; + /** @internal */ + public setupTelemetryPromise: Promise = Promise.resolve(); constructor({ bus, @@ -125,9 +132,26 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { if (this.isSetup) { throw new Error('Setup can only be called once.'); } + this.isBufferingBusEvents = true; + this.isBufferingTelemetryEvents = true; + + this.setupTelemetryPromise = this.setupTelemetry(); this.setupBusEventListeners(); + this.isSetup = true; - this.isBufferingEvents = true; + } + + private async setupTelemetry(): Promise { + if (!this.deviceId) { + try { + this.deviceId ??= await getDeviceId(); + } catch (error) { + this.bus.emit('mongosh:error', error as Error, 'telemetry'); + this.deviceId = 'unknown'; + } + } + + this.runAndClearPendingTelemetryEvents(); } public attachLogger(logger: MongoLogWriter): void { @@ -141,9 +165,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { ); } this.log = logger; - this.isBufferingEvents = false; - this.runAndClearPendingEvents(); + this.runAndClearPendingBusEvents(); this.bus.emit('mongosh:log-initialized'); } @@ -151,14 +174,23 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { public detachLogger() { this.log = LoggingAndTelemetry.dummyLogger; // Still run any remaining pending events with the dummy log for telemetry purposes. - this.runAndClearPendingEvents(); + this.runAndClearPendingBusEvents(); + } + + private runAndClearPendingBusEvents() { + let pendingEvent: CallableFunction | undefined; + while ((pendingEvent = this.pendingBusEvents.shift())) { + pendingEvent(); + } + this.isBufferingBusEvents = false; } - private runAndClearPendingEvents() { + private runAndClearPendingTelemetryEvents() { let pendingEvent: CallableFunction | undefined; - while ((pendingEvent = this.pendingLogEvents.shift())) { + while ((pendingEvent = this.pendingTelemetryEvents.shift())) { pendingEvent(); } + this.isBufferingTelemetryEvents = false; } /** Information used and set by different bus events. */ @@ -176,7 +208,6 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { usesShellOption: false, telemetryAnonymousId: undefined, userId: undefined, - deviceId: undefined, }; private setupBusEventListeners(): void { @@ -194,11 +225,10 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { listener: (...args: Parameters) => void ) => { this.bus.on(event, ((...args: Parameters) => { - if (this.isBufferingEvents) { - this.pendingLogEvents.push(() => listener(...args)); + if (this.isBufferingBusEvents) { + this.pendingBusEvents.push(() => listener(...args)); return; } - listener(...args); }) as MongoshBusEventsMap[K]); return this.bus; @@ -214,44 +244,51 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { session_id: this.log.logId, }); - /** Eventually sets up the device ID and re-identifies the user. */ - const getCurrentDeviceId = async (): Promise => { - try { - this.busEventState.deviceId ??= this.deviceId ?? (await getDeviceId()); - } catch (error) { - this.bus.emit('mongosh:error', error as Error, 'telemetry'); - this.busEventState.deviceId = 'unknown'; - } - return this.busEventState.deviceId; - }; - const getTelemetryUserIdentity = (): MongoshAnalyticsIdentity => { return { - deviceId: this.busEventState.deviceId ?? this.deviceId, + deviceId: this.deviceId, anonymousId: this.busEventState.telemetryAnonymousId ?? (this.busEventState.userId as string), }; }; - const identifyUser = async (): Promise => { - // We first instantly identify the user with the - // current user information we have. - this.analytics.identify({ - ...getTelemetryUserIdentity(), - traits: getUserTraits(), - }); + const track = ( + message: Pick & { + properties?: Omit< + AnalyticsTrackMessage['properties'], + keyof MongoshTrackingProperties + >; + } + ): void => { + const callback = () => + this.analytics.track({ + ...getTelemetryUserIdentity(), + ...message, + properties: { + ...getTrackingProperties(), + ...message.properties, + }, + }); - if (!this.busEventState.deviceId) { - // If the Device ID had not been resolved yet, - // we wait to resolve it and re-identify the user. - this.busEventState.deviceId ??= await getCurrentDeviceId(); + if (this.isBufferingTelemetryEvents) { + this.pendingTelemetryEvents.push(callback); + } else { + callback(); + } + }; + const identify = (): void => { + const callback = () => this.analytics.identify({ ...getTelemetryUserIdentity(), - deviceId: await getCurrentDeviceId(), traits: getUserTraits(), }); + + if (this.isBufferingTelemetryEvents) { + this.pendingTelemetryEvents.push(callback); + } else { + callback(); } }; @@ -286,7 +323,6 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { atlas_hostname: args.is_atlas ? resolved_hostname : null, }; const properties = { - ...getTrackingProperties(), ...argsWithoutUriAndHostname, ...atlasHostname, }; @@ -304,8 +340,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { } ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'New Connection', properties, }); @@ -320,11 +355,9 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { ); const normalizedTimings = Object.fromEntries(normalizedTimingsArray); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Startup Time', properties: { - ...getTrackingProperties(), is_interactive: args.isInteractive, js_context: args.jsContext, ...normalizedTimings, @@ -341,7 +374,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.busEventState.telemetryAnonymousId = newTelemetryUserIdentity.anonymousId; - void identifyUser(); + void identify(); } ); @@ -357,7 +390,7 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { } else { this.busEventState.userId = updatedTelemetryUserIdentity.userId; } - void identifyUser(); + void identify(); this.log.info( 'MONGOSH', mongoLogId(1_000_000_005), @@ -385,11 +418,9 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { ); if (error.name.includes('Mongosh')) { - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Error', properties: { - ...getTrackingProperties(), name: mongoshError.name, code: mongoshError.code, scope: mongoshError.scope, @@ -439,12 +470,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { args ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Use', - properties: { - ...getTrackingProperties(), - }, }); }); @@ -457,11 +484,9 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { args ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Show', properties: { - ...getTrackingProperties(), method: args.method, }, }); @@ -503,13 +528,11 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { args ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: this.busEventState.hasStartedMongoshRepl ? 'Script Loaded' : 'Script Loaded CLI', properties: { - ...getTrackingProperties(), nested: args.nested, ...(this.busEventState.hasStartedMongoshRepl ? {} @@ -526,11 +549,9 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { 'Evaluating script passed on the command line' ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Script Evaluated', properties: { - ...getTrackingProperties(), shell: this.busEventState.usesShellOption, }, }); @@ -544,12 +565,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { 'Loading .mongoshrc.js' ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Mongoshrc Loaded', - properties: { - ...getTrackingProperties(), - }, }); }); @@ -561,12 +578,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { 'Warning about .mongorc.js/.mongoshrc.js mismatch' ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Mongorc Warning', - properties: { - ...getTrackingProperties(), - }, }); }); @@ -731,12 +744,8 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { ); if (ev.args[0] === 'install') { - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Snippet Install', - properties: { - ...getTrackingProperties(), - }, }); } }); @@ -789,21 +798,17 @@ class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { entry ); - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'Deprecated Method', properties: { - ...getTrackingProperties(), ...entry, }, }); } for (const [entry, count] of apiCalls) { - this.analytics.track({ - ...getTelemetryUserIdentity(), + track({ event: 'API Call', properties: { - ...getTrackingProperties(), ...entry, count, }, diff --git a/packages/logging/src/types.ts b/packages/logging/src/types.ts index eb2250ff6..b75b414a7 100644 --- a/packages/logging/src/types.ts +++ b/packages/logging/src/types.ts @@ -34,5 +34,4 @@ export type LoggingAndTelemetryBusEventState = { usesShellOption: boolean; telemetryAnonymousId: string | undefined; userId: string | undefined; - deviceId: string | undefined; }; From c8b7c9c5039021e3ee6d974cd5a167822d27a70d Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 26 Mar 2025 11:04:18 +0100 Subject: [PATCH 07/17] fix: flush events when exiting cli-repl --- packages/cli-repl/src/cli-repl.ts | 4 +++ .../logging/src/logging-and-telemetry.spec.ts | 3 --- packages/logging/src/logging-and-telemetry.ts | 26 ++++++++++++++++--- packages/logging/src/types.ts | 2 ++ 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index b67ca3135..b00c52e1c 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -1172,6 +1172,8 @@ export class CliRepl implements MongoshIOProvider { const analytics = this.toggleableAnalytics; let flushError: string | null = null; let flushDuration: number | null = null; + this.loggingAndTelemetry?.flush(); + if (analytics) { const flushStart = Date.now(); try { @@ -1194,6 +1196,7 @@ export class CliRepl implements MongoshIOProvider { } ); await this.logWriter?.flush(); + markTime(TimingCategories.Logging, 'flushed log writer'); this.bus.emit('mongosh:closed'); })()); @@ -1211,6 +1214,7 @@ export class CliRepl implements MongoshIOProvider { // onExit never returns. If it does, that's a bug. const error = new MongoshInternalError('onExit() unexpectedly returned'); this.bus.emit('mongosh:error', error, 'fatal'); + throw error; } diff --git a/packages/logging/src/logging-and-telemetry.spec.ts b/packages/logging/src/logging-and-telemetry.spec.ts index 10864d70e..742767bf0 100644 --- a/packages/logging/src/logging-and-telemetry.spec.ts +++ b/packages/logging/src/logging-and-telemetry.spec.ts @@ -378,7 +378,6 @@ describe('MongoshLoggingAndTelemetry', function () { expect(logOutput).to.have.lengthOf(1); - await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(2); }); @@ -402,7 +401,6 @@ describe('MongoshLoggingAndTelemetry', function () { expect(logOutput).to.have.lengthOf(1); - await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(2); loggingAndTelemetry.attachLogger(logger); @@ -411,7 +409,6 @@ describe('MongoshLoggingAndTelemetry', function () { expect(logOutput).to.have.lengthOf(2); - await (loggingAndTelemetry as LoggingAndTelemetry).setupTelemetryPromise; expect(analyticsOutput).to.have.lengthOf(3); }); diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 9d0bd4a1e..82cf24dc6 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -113,6 +113,9 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { /** @internal */ public setupTelemetryPromise: Promise = Promise.resolve(); + // eslint-disable-next-line @typescript-eslint/no-empty-function + private resolveDeviceId: (value: string) => void = () => {}; + constructor({ bus, analytics, @@ -132,8 +135,8 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { if (this.isSetup) { throw new Error('Setup can only be called once.'); } - this.isBufferingBusEvents = true; this.isBufferingTelemetryEvents = true; + this.isBufferingBusEvents = true; this.setupTelemetryPromise = this.setupTelemetry(); this.setupBusEventListeners(); @@ -141,10 +144,25 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.isSetup = true; } + public flush(): void { + // Run any telemetry events even if device ID hasn't been resolved yet + this.runAndClearPendingTelemetryEvents(); + + // Run any other pending events with the set or dummy log for telemetry purposes. + this.runAndClearPendingBusEvents(); + + this.resolveDeviceId('unknown'); + } + private async setupTelemetry(): Promise { if (!this.deviceId) { try { - this.deviceId ??= await getDeviceId(); + this.deviceId ??= await Promise.race([ + getDeviceId(), + new Promise((resolve) => { + this.resolveDeviceId = resolve; + }), + ]); } catch (error) { this.bus.emit('mongosh:error', error as Error, 'telemetry'); this.deviceId = 'unknown'; @@ -173,8 +191,8 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { public detachLogger() { this.log = LoggingAndTelemetry.dummyLogger; - // Still run any remaining pending events with the dummy log for telemetry purposes. - this.runAndClearPendingBusEvents(); + + this.flush(); } private runAndClearPendingBusEvents() { diff --git a/packages/logging/src/types.ts b/packages/logging/src/types.ts index b75b414a7..4f8f40eaa 100644 --- a/packages/logging/src/types.ts +++ b/packages/logging/src/types.ts @@ -6,6 +6,8 @@ import type { MultiSet } from './helpers'; export interface MongoshLoggingAndTelemetry { attachLogger(logger: MongoLogWriter): void; detachLogger(): void; + /** Flush any remaining log or telemetry events. */ + flush(): void; } export type MongoshLoggingAndTelemetryArguments = { From aa494a2d3ca590462fc60588a624e952c0c782d5 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 24 Apr 2025 23:40:48 +0200 Subject: [PATCH 08/17] fix: switch to native-machine-id --- package-lock.json | 19 +++++++++++++++++-- .../build/src/compile/signable-compiler.ts | 5 +++++ packages/cli-repl/webpack.config.js | 6 ++++++ packages/logging/package.json | 2 +- .../logging/src/logging-and-telemetry.spec.ts | 7 +++++-- packages/logging/src/logging-and-telemetry.ts | 12 +++++++----- 6 files changed, 41 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 71b8037cf..a9a20b0cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21748,6 +21748,20 @@ "license": "MIT", "optional": true }, + "node_modules/native-machine-id": { + "version": "0.0.11", + "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.0.11.tgz", + "integrity": "sha512-tbyYPSXyWZRM1vbSksf5YpsUQox2eI1DQMGrsbUPypkAkvLMBdYtR4vc1IHsyuYgBtxqycwPkrPyStLKbFJPPQ==", + "hasInstallScript": true, + "license": "Apache-2.0", + "dependencies": { + "bindings": "^1.5.0", + "node-addon-api": "^4.3.0" + }, + "bin": { + "native-machine-id": "dist/bin/machine-id.js" + } + }, "node_modules/natural-compare": { "version": "1.4.0", "license": "MIT" @@ -21996,7 +22010,8 @@ "version": "1.1.12", "resolved": "https://registry.npmjs.org/node-machine-id/-/node-machine-id-1.1.12.tgz", "integrity": "sha512-QNABxbrPa3qEIfrE6GOJ7BYIuignnJw7iQ2YPbc3Nla1HzRJjXzZOiikfF8m7eAMfichLt3M4VgLOetqgDmgGQ==", - "license": "MIT" + "license": "MIT", + "optional": true }, "node_modules/node-preload": { "version": "0.2.1", @@ -29830,7 +29845,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "node-machine-id": "^1.1.12" + "native-machine-id": "^0.0.11" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/build/src/compile/signable-compiler.ts b/packages/build/src/compile/signable-compiler.ts index c7cab80ff..ad2fd544c 100644 --- a/packages/build/src/compile/signable-compiler.ts +++ b/packages/build/src/compile/signable-compiler.ts @@ -132,6 +132,10 @@ export class SignableCompiler { path: await findModulePath('cli-repl', 'glibc-version'), requireRegexp: /\bglibc_version\.node$/, }; + const nativeMachineIdAddon = { + path: await findModulePath('logging', 'native-machine-id'), + requireRegexp: /\bnative_machine_id\.node$/, + }; // Warning! Until https://jira.mongodb.org/browse/MONGOSH-990, // packages/service-provider-node-driver *also* has a copy of these. // We use the versions included in packages/cli-repl here, so these @@ -186,6 +190,7 @@ export class SignableCompiler { kerberosAddon, cryptLibraryVersionAddon, glibcVersionAddon, + nativeMachineIdAddon, ] .concat(winCAAddon ? [winCAAddon] : []) .concat(winConsoleProcessListAddon ? [winConsoleProcessListAddon] : []) diff --git a/packages/cli-repl/webpack.config.js b/packages/cli-repl/webpack.config.js index 6fe7f5249..1c02b775e 100644 --- a/packages/cli-repl/webpack.config.js +++ b/packages/cli-repl/webpack.config.js @@ -87,6 +87,12 @@ const config = { 'commonjs2 ../build/Release/mongocrypt.node', '../build/Debug/mongocrypt.node': 'commonjs2 ../build/Debug/mongocrypt.node', + '../build/Release/native_machine_id.node': + 'commonjs2 ../build/Release/native_machine_id.node', + '../build/Debug/native_machine_id.node': + 'commonjs2 ../build/Debug/native_machine_id.node', + './build/Release/native_machine_id.node': + 'commonjs2 ./build/Release/native_machine_id.node', }, externalsPresets: { diff --git a/packages/logging/package.json b/packages/logging/package.json index c1b262ae3..21d098642 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -23,7 +23,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "node-machine-id": "^1.1.12" + "native-machine-id": "^0.0.11" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/src/logging-and-telemetry.spec.ts b/packages/logging/src/logging-and-telemetry.spec.ts index 742767bf0..7581f3a60 100644 --- a/packages/logging/src/logging-and-telemetry.spec.ts +++ b/packages/logging/src/logging-and-telemetry.spec.ts @@ -217,7 +217,10 @@ describe('MongoshLoggingAndTelemetry', function () { it('uses device ID "unknown" and logs error if it fails to resolve it', async function () { // eslint-disable-next-line @typescript-eslint/no-var-requires - sinon.stub(require('child_process'), 'exec').throws(new Error('Test')); + sinon + // eslint-disable-next-line @typescript-eslint/no-var-requires + .stub(require('native-machine-id'), 'getMachineId') + .rejects(new Error('Test')); const loggingAndTelemetry = setupLoggingAndTelemetry({ ...testLoggingArguments, bus, @@ -289,7 +292,7 @@ describe('MongoshLoggingAndTelemetry', function () { }); sinon // eslint-disable-next-line @typescript-eslint/no-var-requires - .stub(require('node-machine-id'), 'machineId') + .stub(require('native-machine-id'), 'getMachineId') .resolves(delayedTelemetry); const loggingAndTelemetry = setupLoggingAndTelemetry({ diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 82cf24dc6..57628d01b 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -53,7 +53,7 @@ import type { MongoshLoggingAndTelemetryArguments, MongoshTrackingProperties, } from './types'; -import { machineId } from 'node-machine-id'; +import { getMachineId } from 'native-machine-id'; import { createHmac } from 'crypto'; export function setupLoggingAndTelemetry( @@ -66,13 +66,15 @@ export function setupLoggingAndTelemetry( } /** - * @returns A hashed, unique identifier for the running device. - * @throws If something goes wrong when getting the device ID. + * @returns A hashed, unique identifier for the running device or `undefined` if not known. */ -export async function getDeviceId(): Promise { +export async function getDeviceId(): Promise { // 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 originalId = (await machineId(true)).toUpperCase(); + const originalId = (await getMachineId({ raw: true }))?.toUpperCase(); + if (!originalId) { + return 'unknown'; + } const hmac = createHmac('sha256', originalId); /** This matches the message used to create the hashes in Atlas CLI */ From 17612a8fd6099c97ac36c862c5c48e0ca9f5bc9a Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 24 Apr 2025 23:53:47 +0200 Subject: [PATCH 09/17] fix: use require instead of module import --- package-lock.json | 19 ++++++++++++++----- packages/logging/package.json | 2 +- packages/logging/src/logging-and-telemetry.ts | 6 ++++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index a9a20b0cb..0631cebdb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21749,19 +21749,28 @@ "optional": true }, "node_modules/native-machine-id": { - "version": "0.0.11", - "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.0.11.tgz", - "integrity": "sha512-tbyYPSXyWZRM1vbSksf5YpsUQox2eI1DQMGrsbUPypkAkvLMBdYtR4vc1IHsyuYgBtxqycwPkrPyStLKbFJPPQ==", + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.1.0.tgz", + "integrity": "sha512-Po7OPcXGsWZ/o+n93ZOhmF3G5RQsEUMTnVddX45u5GfoEnk803ba7lhztwMkDaPhUFHy5FpXLiytIFitVxMkTA==", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { "bindings": "^1.5.0", - "node-addon-api": "^4.3.0" + "node-addon-api": "^8.0.0" }, "bin": { "native-machine-id": "dist/bin/machine-id.js" } }, + "node_modules/native-machine-id/node_modules/node-addon-api": { + "version": "8.3.1", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.3.1.tgz", + "integrity": "sha512-lytcDEdxKjGJPTLEfW4mYMigRezMlyJY8W4wxJK8zE533Jlb8L8dRuObJFWg2P+AuOIxoCgKF+2Oq4d4Zd0OUA==", + "license": "MIT", + "engines": { + "node": "^18 || ^20 || >= 21" + } + }, "node_modules/natural-compare": { "version": "1.4.0", "license": "MIT" @@ -29845,7 +29854,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "native-machine-id": "^0.0.11" + "native-machine-id": "^0.1.0" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/package.json b/packages/logging/package.json index 21d098642..17c2a8c31 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -23,7 +23,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "native-machine-id": "^0.0.11" + "native-machine-id": "^0.1.0" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 57628d01b..181f846a2 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -53,7 +53,6 @@ import type { MongoshLoggingAndTelemetryArguments, MongoshTrackingProperties, } from './types'; -import { getMachineId } from 'native-machine-id'; import { createHmac } from 'crypto'; export function setupLoggingAndTelemetry( @@ -71,7 +70,10 @@ export function setupLoggingAndTelemetry( export async function getDeviceId(): Promise { // 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 originalId = (await getMachineId({ raw: true }))?.toUpperCase(); + const originalId = ( + await require('native-machine-id').getMachineId({ raw: true }) + )?.toUpperCase(); + if (!originalId) { return 'unknown'; } From 174ea25131f418f56e998849c4097c6121feb5f6 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 25 Apr 2025 00:00:53 +0200 Subject: [PATCH 10/17] chore: add type annotation --- packages/logging/src/logging-and-telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 181f846a2..f8a81a2a9 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -70,7 +70,7 @@ export function setupLoggingAndTelemetry( export async function getDeviceId(): Promise { // 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 originalId = ( + const originalId: string = ( await require('native-machine-id').getMachineId({ raw: true }) )?.toUpperCase(); From 4b68116f8e6c5a809f27648928bb95614aa1c561 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 25 Apr 2025 01:12:19 +0200 Subject: [PATCH 11/17] fix: add into category for e2e snapshot test --- packages/cli-repl/webpack.config.js | 6 ------ packages/e2e-tests/test/e2e-snapshot.spec.ts | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/cli-repl/webpack.config.js b/packages/cli-repl/webpack.config.js index 1c02b775e..6fe7f5249 100644 --- a/packages/cli-repl/webpack.config.js +++ b/packages/cli-repl/webpack.config.js @@ -87,12 +87,6 @@ const config = { 'commonjs2 ../build/Release/mongocrypt.node', '../build/Debug/mongocrypt.node': 'commonjs2 ../build/Debug/mongocrypt.node', - '../build/Release/native_machine_id.node': - 'commonjs2 ../build/Release/native_machine_id.node', - '../build/Debug/native_machine_id.node': - 'commonjs2 ../build/Debug/native_machine_id.node', - './build/Release/native_machine_id.node': - 'commonjs2 ./build/Release/native_machine_id.node', }, externalsPresets: { diff --git a/packages/e2e-tests/test/e2e-snapshot.spec.ts b/packages/e2e-tests/test/e2e-snapshot.spec.ts index f887fae94..4406ab4c4 100644 --- a/packages/e2e-tests/test/e2e-snapshot.spec.ts +++ b/packages/e2e-tests/test/e2e-snapshot.spec.ts @@ -144,7 +144,7 @@ describe('e2e snapshot support', function () { ); verifyAllInCategoryMatch( 'nodb-eval', - /^node_modules\/(kerberos|mongodb-client-encryption|glibc-version|@mongodb-js\/devtools-proxy-support|@mongodb-js\/socksv5|agent-base|(win|macos)-export-certificate-and-key|@tootallnate\/quickjs-emscripten)\// + /^node_modules\/(kerberos|native-machine-id|mongodb-client-encryption|glibc-version|@mongodb-js\/devtools-proxy-support|@mongodb-js\/socksv5|agent-base|(win|macos)-export-certificate-and-key|@tootallnate\/quickjs-emscripten)\// ); if (process.arch !== 's390x') { // quickjs is in the list above but should be exlucded anywhere but on s390x From 55e61d993b4960183dfda95567a61ba006f92e12 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 28 Apr 2025 09:55:58 +0200 Subject: [PATCH 12/17] fix: remove void --- packages/logging/src/logging-and-telemetry.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index f8a81a2a9..1ff9c1d66 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -396,7 +396,7 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.busEventState.telemetryAnonymousId = newTelemetryUserIdentity.anonymousId; - void identify(); + identify(); } ); @@ -412,7 +412,7 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { } else { this.busEventState.userId = updatedTelemetryUserIdentity.userId; } - void identify(); + identify(); this.log.info( 'MONGOSH', mongoLogId(1_000_000_005), From 7da490c75e0fd2df7048d16758ce5c88a2b6f4ae Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 29 Apr 2025 15:14:37 +0200 Subject: [PATCH 13/17] fix: update native-machine-id --- package-lock.json | 8 ++++---- packages/logging/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0631cebdb..979193e43 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21749,9 +21749,9 @@ "optional": true }, "node_modules/native-machine-id": { - "version": "0.1.0", - "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.1.0.tgz", - "integrity": "sha512-Po7OPcXGsWZ/o+n93ZOhmF3G5RQsEUMTnVddX45u5GfoEnk803ba7lhztwMkDaPhUFHy5FpXLiytIFitVxMkTA==", + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.1.1.tgz", + "integrity": "sha512-TNuaM2fiqnYER3XbfO0ki+ykXeWiWEeu2EYaeSjKu1L6NV1ZKetrLX1JY0dpGeOrNKLHOeVdfQDN38mX+h8WoA==", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { @@ -29854,7 +29854,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "native-machine-id": "^0.1.0" + "native-machine-id": "^0.1.1" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", diff --git a/packages/logging/package.json b/packages/logging/package.json index 17c2a8c31..eb3fc7267 100644 --- a/packages/logging/package.json +++ b/packages/logging/package.json @@ -23,7 +23,7 @@ "@mongosh/types": "3.6.0", "mongodb-log-writer": "^2.3.1", "mongodb-redact": "^1.1.5", - "native-machine-id": "^0.1.0" + "native-machine-id": "^0.1.1" }, "devDependencies": { "@mongodb-js/eslint-config-mongosh": "^1.0.0", From d97aa0c1eb738501ff311bd0ee6d1c65004bc58c Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Apr 2025 16:19:46 +0200 Subject: [PATCH 14/17] fix: add an e2e test and catch errors inside getDeviceId --- packages/e2e-tests/test/e2e.spec.ts | 14 ++++ packages/logging/src/logging-and-telemetry.ts | 68 ++++++++++--------- 2 files changed, 49 insertions(+), 33 deletions(-) diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index ff8004afd..7713a421a 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -771,6 +771,20 @@ describe('e2e', function () { ).to.include('string'); }); + it('sets device ID for telemetry', async function () { + const deviceId = ( + await shell.executeLine( + 'db._mongo._instanceState.evaluationListener.ioProvider.loggingAndTelemetry.deviceId' + ) + ) + .replace(/test>/g, '') + .trim(); + + expect(deviceId).not.to.equal('unknown'); + // Our hashed key is 64 hex chars + expect(deviceId).to.match(/^[a-f0-9]{64}$/); + }); + context('post-4.2', function () { skipIfServerVersion(testServer, '< 4.4'); it('allows calling convertShardKeyToHashed() as a global function', async function () { diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index 1ff9c1d66..cfcd492b3 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -64,28 +64,6 @@ export function setupLoggingAndTelemetry( return loggingAndTelemetry; } -/** - * @returns A hashed, unique identifier for the running device or `undefined` if not known. - */ -export async function getDeviceId(): Promise { - // 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 originalId: string = ( - await require('native-machine-id').getMachineId({ raw: true }) - )?.toUpperCase(); - - if (!originalId) { - return 'unknown'; - } - const hmac = createHmac('sha256', originalId); - - /** 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'); -} - /** @internal */ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { private static dummyLogger = new MongoLogWriter( @@ -158,19 +136,43 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.resolveDeviceId('unknown'); } + /** + * @returns A hashed, unique identifier for the running device or `"unknown"` if not known. + */ + private async getDeviceId(): Promise { + try { + // 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 originalId: string = + // eslint-disable-next-line @typescript-eslint/no-var-requires + await require('native-machine-id').getMachineId({ + raw: true, + }); + + if (!originalId) { + return 'unknown'; + } + const hmac = createHmac('sha256', originalId); + + /** 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) { + this.bus.emit('mongosh:error', error as Error, 'telemetry'); + return 'unknown'; + } + } + private async setupTelemetry(): Promise { if (!this.deviceId) { - try { - this.deviceId ??= await Promise.race([ - getDeviceId(), - new Promise((resolve) => { - this.resolveDeviceId = resolve; - }), - ]); - } catch (error) { - this.bus.emit('mongosh:error', error as Error, 'telemetry'); - this.deviceId = 'unknown'; - } + this.deviceId = await Promise.race([ + this.getDeviceId(), + new Promise((resolve) => { + this.resolveDeviceId = resolve; + }), + ]); } this.runAndClearPendingTelemetryEvents(); From 1704b73c2f1736ab58bb4fc51fb67c91a0f9d6c7 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 30 Apr 2025 16:41:27 +0200 Subject: [PATCH 15/17] fix: move getDeviceId to be independent from loggingAndTelemetry --- packages/logging/src/logging-and-telemetry.ts | 67 ++++++++++--------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/packages/logging/src/logging-and-telemetry.ts b/packages/logging/src/logging-and-telemetry.ts index cfcd492b3..6007ff8ee 100644 --- a/packages/logging/src/logging-and-telemetry.ts +++ b/packages/logging/src/logging-and-telemetry.ts @@ -55,6 +55,39 @@ import type { } from './types'; import { createHmac } from 'crypto'; +/** + * @returns A hashed, unique identifier for the running device or `"unknown"` if not known. + */ +export async function getDeviceId({ + onError, +}: { + onError?: (error: Error) => void; +} = {}): Promise { + try { + // 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 originalId: string = + // eslint-disable-next-line @typescript-eslint/no-var-requires + await require('native-machine-id').getMachineId({ + raw: true, + }); + + if (!originalId) { + return 'unknown'; + } + const hmac = createHmac('sha256', originalId); + + /** 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) { + onError?.(error as Error); + return 'unknown'; + } +} + export function setupLoggingAndTelemetry( props: MongoshLoggingAndTelemetryArguments ): MongoshLoggingAndTelemetry { @@ -136,39 +169,13 @@ export class LoggingAndTelemetry implements MongoshLoggingAndTelemetry { this.resolveDeviceId('unknown'); } - /** - * @returns A hashed, unique identifier for the running device or `"unknown"` if not known. - */ - private async getDeviceId(): Promise { - try { - // 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 originalId: string = - // eslint-disable-next-line @typescript-eslint/no-var-requires - await require('native-machine-id').getMachineId({ - raw: true, - }); - - if (!originalId) { - return 'unknown'; - } - const hmac = createHmac('sha256', originalId); - - /** 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) { - this.bus.emit('mongosh:error', error as Error, 'telemetry'); - return 'unknown'; - } - } - private async setupTelemetry(): Promise { if (!this.deviceId) { this.deviceId = await Promise.race([ - this.getDeviceId(), + getDeviceId({ + onError: (error) => + this.bus.emit('mongosh:error', error, 'telemetry'), + }), new Promise((resolve) => { this.resolveDeviceId = resolve; }), From 5a4c8e3a43a563bf37eb49e186a5fbd7749b107a Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 1 May 2025 12:42:44 +0200 Subject: [PATCH 16/17] fix: use first line of the output --- packages/e2e-tests/test/e2e.spec.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index 7713a421a..38fa95b9b 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -777,12 +777,17 @@ describe('e2e', function () { 'db._mongo._instanceState.evaluationListener.ioProvider.loggingAndTelemetry.deviceId' ) ) - .replace(/test>/g, '') - .trim(); + .split('\n')[0] + // Remove all whitespace + .replace(/\s+/g, ''); expect(deviceId).not.to.equal('unknown'); // Our hashed key is 64 hex chars - expect(deviceId).to.match(/^[a-f0-9]{64}$/); + + expect(deviceId).to.match( + /^[a-f0-9]{64}$/, + `deviceId did not match: |${deviceId}|` + ); }); context('post-4.2', function () { From caf256066523bc09c0c9e9f984d48be28600ea61 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 1 May 2025 13:06:58 +0200 Subject: [PATCH 17/17] fix: update connectivity commit --- packages/connectivity-tests/test/all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/connectivity-tests/test/all.sh b/packages/connectivity-tests/test/all.sh index 49b727043..a11dba694 100755 --- a/packages/connectivity-tests/test/all.sh +++ b/packages/connectivity-tests/test/all.sh @@ -20,7 +20,7 @@ fi git clone https://github.com/mongodb-js/devtools-docker-test-envs.git test-envs cd test-envs -git checkout ac8e6675fcd769f0bfc868c3ac73cdd8bcdcc792 +git checkout cbec3a65efe9def50638994ae6b36ffa3b4708ee "$CONNECTIVITY_TEST_SOURCE_DIR/ldap.sh" "$CONNECTIVITY_TEST_SOURCE_DIR/localhost.sh"