From 857819ccb92036727e3da49a765cfd189e575c17 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Fri, 9 Feb 2024 16:19:16 +0100 Subject: [PATCH 1/8] fix: send atlas uid as userId to compass telemetry COMPASS-7610 --- packages/atlas-service/src/main.ts | 6 +++++- .../src/preferences-schema.ts | 12 ++++++++++++ packages/compass/src/main/telemetry.ts | 14 +++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/atlas-service/src/main.ts b/packages/atlas-service/src/main.ts index 6b0fb073104..dcfbaa3656d 100644 --- a/packages/atlas-service/src/main.ts +++ b/packages/atlas-service/src/main.ts @@ -374,7 +374,11 @@ export class AtlasService { 'AtlasService', 'Signed in successfully' ); - track('Atlas Sign In Success', getTrackingUserInfo(userInfo)); + const { auid } = getTrackingUserInfo(userInfo); + track('Atlas Sign In Success', { auid }); + await this.preferences.savePreferences({ + telemetryAtlasUserId: auid, + }); return userInfo; } catch (err) { track('Atlas Sign In Error', { diff --git a/packages/compass-preferences-model/src/preferences-schema.ts b/packages/compass-preferences-model/src/preferences-schema.ts index 0cfc28b6a37..a186b489e2a 100644 --- a/packages/compass-preferences-model/src/preferences-schema.ts +++ b/packages/compass-preferences-model/src/preferences-schema.ts @@ -67,6 +67,7 @@ export type InternalUserPreferences = { lastKnownVersion: string; currentUserId?: string; telemetryAnonymousId?: string; + telemetryAtlasUserId?: string; userCreatedAt: number; }; @@ -313,6 +314,17 @@ export const storedUserPreferencesProps: Required<{ validator: z.string().uuid().optional(), type: 'string', }, + /** + * Stores a unique telemetry atlas ID for the current user. + */ + telemetryAtlasUserId: { + ui: false, + cli: false, + global: false, + description: null, + validator: z.string().optional(), + type: 'string', + }, /** * Stores the timestamp for when the user was created */ diff --git a/packages/compass/src/main/telemetry.ts b/packages/compass/src/main/telemetry.ts index e5276aa85a5..f475712332c 100644 --- a/packages/compass/src/main/telemetry.ts +++ b/packages/compass/src/main/telemetry.ts @@ -36,6 +36,7 @@ class CompassTelemetry { private static state: 'enabled' | 'disabled' = 'disabled'; private static queuedEvents: EventInfo[] = []; // Events that happen before we fetch user preferences private static telemetryAnonymousId = ''; // The randomly generated anonymous user id. + private static telemetryAtlasUserId?: string; private static lastReportedScreen = ''; private static osInfo: ReturnType extends Promise ? Partial @@ -79,6 +80,7 @@ class CompassTelemetry { } this.analytics.track({ + userId: this.telemetryAtlasUserId, anonymousId: this.telemetryAnonymousId, event: info.event, properties: { ...info.properties, ...commonProperties }, @@ -105,6 +107,7 @@ class CompassTelemetry { this.telemetryAnonymousId ) { this.analytics.identify({ + userId: this.telemetryAtlasUserId, anonymousId: this.telemetryAnonymousId, traits: { ...this._getCommonProperties(), @@ -127,9 +130,10 @@ class CompassTelemetry { private static async _init(app: typeof CompassApplication) { const { preferences } = app; - const { trackUsageStatistics, telemetryAnonymousId } = + const { trackUsageStatistics, telemetryAnonymousId, telemetryAtlasUserId } = preferences.getPreferences(); this.telemetryAnonymousId = telemetryAnonymousId ?? ''; + this.telemetryAtlasUserId = telemetryAtlasUserId; try { this.osInfo = await getOsInfo(); @@ -176,11 +180,19 @@ class CompassTelemetry { this.state = 'disabled'; } }; + const onAtlasUserIdChanged = (value?: string) => { + if (value) this.identify(); + }; + onTrackUsageStatisticsChanged(trackUsageStatistics); // initial setup with current value preferences.onPreferenceValueChanged( 'trackUsageStatistics', onTrackUsageStatisticsChanged ); + preferences.onPreferenceValueChanged( + 'telemetryAtlasUserId', + onAtlasUserIdChanged + ); process.on('compass:track', (meta: EventInfo) => { this._track(meta); From 7127816ba1b14f05938203ccf4b90c8b8f75c7a2 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 12 Feb 2024 11:22:24 +0100 Subject: [PATCH 2/8] test: update atlas-service tests --- packages/atlas-service/src/main.spec.ts | 33 ++++++++++++++----------- packages/atlas-service/src/main.ts | 11 +-------- packages/atlas-service/src/util.spec.ts | 16 ++++++++++++ packages/atlas-service/src/util.ts | 9 +++++++ 4 files changed, 44 insertions(+), 25 deletions(-) create mode 100644 packages/atlas-service/src/util.spec.ts diff --git a/packages/atlas-service/src/main.spec.ts b/packages/atlas-service/src/main.spec.ts index eb8ab2dd166..b4552d52da5 100644 --- a/packages/atlas-service/src/main.spec.ts +++ b/packages/atlas-service/src/main.spec.ts @@ -1,11 +1,11 @@ import Sinon from 'sinon'; import { expect } from 'chai'; -import { AtlasService, getTrackingUserInfo, throwIfNotOk } from './main'; +import { AtlasService, throwIfNotOk } from './main'; +import * as util from './util'; import { EventEmitter } from 'events'; import { createSandboxFromDefaultPreferences } from 'compass-preferences-model'; import type { PreferencesAccess } from 'compass-preferences-model'; import type { AtlasUserConfigStore } from './user-config-store'; -import type { AtlasUserInfo } from './util'; function getListenerCount(emitter: EventEmitter) { return emitter.eventNames().reduce((acc, name) => { @@ -74,6 +74,12 @@ describe('AtlasServiceMain', function () { let preferences: PreferencesAccess; + let getTrackingUserInfoStub; + + before(function () { + getTrackingUserInfoStub = sandbox.stub(util, 'getTrackingUserInfo'); + }); + beforeEach(async function () { AtlasService['ipcMain'] = { handle: sandbox.stub(), @@ -114,8 +120,15 @@ describe('AtlasServiceMain', function () { sandbox.resetHistory(); }); + after(function () { + sandbox.restore(); + }); + describe('signIn', function () { it('should sign in using oidc plugin', async function () { + const atlasUid = 'abcdefgh'; + getTrackingUserInfoStub.returns({ auid: atlasUid }); + const userInfo = await AtlasService.signIn(); expect( mockOidcPlugin.mongoClientOptions.authMechanismProperties @@ -124,6 +137,9 @@ describe('AtlasServiceMain', function () { // proper error message from oidc plugin in case of failed sign in ).to.have.been.calledTwice; expect(userInfo).to.have.property('sub', '1234'); + expect(preferences.getPreferences().telemetryAtlasUserId).to.equal( + atlasUid + ); }); it('should debounce inflight sign in requests', async function () { @@ -522,19 +538,6 @@ describe('AtlasServiceMain', function () { }); }); - describe('getTrackingUserInfo', function () { - it('should return required tracking info from user info', function () { - expect( - getTrackingUserInfo({ - sub: '1234', - primaryEmail: 'test@example.com', - } as AtlasUserInfo) - ).to.deep.eq({ - auid: '03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4', - }); - }); - }); - describe('setupAIAccess', function () { beforeEach(async function () { await preferences.savePreferences({ diff --git a/packages/atlas-service/src/main.ts b/packages/atlas-service/src/main.ts index dcfbaa3656d..eccb414ab78 100644 --- a/packages/atlas-service/src/main.ts +++ b/packages/atlas-service/src/main.ts @@ -1,8 +1,7 @@ import { shell, app } from 'electron'; import { URL, URLSearchParams } from 'url'; -import { createHash } from 'crypto'; import type { AuthFlowType, MongoDBOIDCPlugin } from '@mongodb-js/oidc-plugin'; -import { AtlasServiceError } from './util'; +import { AtlasServiceError, getTrackingUserInfo } from './util'; import { createMongoDBOIDCPlugin, hookLoggerToMongoLogWriter as oidcPluginHookLoggerToMongoLogWriter, @@ -114,14 +113,6 @@ const TOKEN_TYPE_TO_HINT = { refreshToken: 'refresh_token', } as const; -export function getTrackingUserInfo(userInfo: AtlasUserInfo) { - return { - // AUID is shared Cloud user identificator that can be tracked through - // various MongoDB properties - auid: createHash('sha256').update(userInfo.sub, 'utf8').digest('hex'), - }; -} - export type AtlasServiceConfig = { atlasApiBaseUrl: string; atlasApiUnauthBaseUrl: string; diff --git a/packages/atlas-service/src/util.spec.ts b/packages/atlas-service/src/util.spec.ts new file mode 100644 index 00000000000..65ae6a9667f --- /dev/null +++ b/packages/atlas-service/src/util.spec.ts @@ -0,0 +1,16 @@ +import { getTrackingUserInfo } from './util'; +import type { AtlasUserInfo } from './util'; +import { expect } from 'chai'; + +describe('getTrackingUserInfo', function () { + it('should return required tracking info from user info', function () { + expect( + getTrackingUserInfo({ + sub: '1234', + primaryEmail: 'test@example.com', + } as AtlasUserInfo) + ).to.deep.eq({ + auid: '03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4', + }); + }); +}); diff --git a/packages/atlas-service/src/util.ts b/packages/atlas-service/src/util.ts index 83802330ef4..6adfd3b3c41 100644 --- a/packages/atlas-service/src/util.ts +++ b/packages/atlas-service/src/util.ts @@ -1,6 +1,7 @@ import type * as plugin from '@mongodb-js/oidc-plugin'; import util from 'util'; import type { AtlasUserConfig } from './user-config-store'; +import { createHash } from 'crypto'; export type AtlasUserInfo = { sub: string; @@ -160,3 +161,11 @@ export class AtlasServiceError extends Error { this.detail = detail; } } + +export function getTrackingUserInfo(userInfo: AtlasUserInfo) { + return { + // AUID is shared Cloud user identificator that can be tracked through + // various MongoDB properties + auid: createHash('sha256').update(userInfo.sub, 'utf8').digest('hex'), + }; +} From 59b58b0f9562f866887ddc9e046ae00bcfcdf737 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 12 Feb 2024 13:46:23 +0100 Subject: [PATCH 3/8] fix: only identify when userId exists --- packages/compass/src/main/telemetry.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/compass/src/main/telemetry.ts b/packages/compass/src/main/telemetry.ts index f475712332c..30b2d577406 100644 --- a/packages/compass/src/main/telemetry.ts +++ b/packages/compass/src/main/telemetry.ts @@ -104,7 +104,8 @@ class CompassTelemetry { if ( this.state === 'enabled' && this.analytics && - this.telemetryAnonymousId + this.telemetryAnonymousId && + this.telemetryAtlasUserId ) { this.analytics.identify({ userId: this.telemetryAtlasUserId, From ba72113440543627bc1903e14a7ed329fbdfec58 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Mon, 12 Feb 2024 13:58:33 +0100 Subject: [PATCH 4/8] add type --- packages/atlas-service/src/main.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/atlas-service/src/main.spec.ts b/packages/atlas-service/src/main.spec.ts index b4552d52da5..d763dd0073a 100644 --- a/packages/atlas-service/src/main.spec.ts +++ b/packages/atlas-service/src/main.spec.ts @@ -74,7 +74,9 @@ describe('AtlasServiceMain', function () { let preferences: PreferencesAccess; - let getTrackingUserInfoStub; + let getTrackingUserInfoStub: Sinon.SinonStubbedMember< + typeof util.getTrackingUserInfo + >; before(function () { getTrackingUserInfoStub = sandbox.stub(util, 'getTrackingUserInfo'); From 6f8d06401c4a66b45b3e5572fb947c4e612b4d60 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Tue, 13 Feb 2024 15:54:42 +0100 Subject: [PATCH 5/8] test: update e2e tests --- .../tests/atlas-login.test.ts | 49 +++++++++++++++++++ .../compass-e2e-tests/tests/logging.test.ts | 15 ++---- packages/compass/src/main/telemetry.ts | 5 +- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/packages/compass-e2e-tests/tests/atlas-login.test.ts b/packages/compass-e2e-tests/tests/atlas-login.test.ts index e733a0b3494..c51cf12528e 100644 --- a/packages/compass-e2e-tests/tests/atlas-login.test.ts +++ b/packages/compass-e2e-tests/tests/atlas-login.test.ts @@ -13,6 +13,8 @@ import { expect } from 'chai'; import { createNumbersCollection } from '../helpers/insert-data'; import { AcceptTOSToggle } from '../helpers/selectors'; import { startMockAtlasServiceServer } from '../helpers/atlas-service'; +import type { Telemetry } from '../helpers/telemetry'; +import { startTelemetryServer } from '../helpers/telemetry'; const DEFAULT_TOKEN_PAYLOAD = { expires_in: 3600, @@ -145,6 +147,53 @@ describe('Atlas Login', function () { }); }); + describe('telemetry', () => { + let telemetry: Telemetry; + + before(async function () { + telemetry = await startTelemetryServer(); + }); + + it('should send identify after the user has logged in', async function () { + const atlasUserIdBefore = await browser.getFeature( + 'telemetryAtlasUserId' + ); + expect(atlasUserIdBefore).to.not.exist; + + try { + await browser.openSettingsModal('Feature Preview'); + + await browser.clickVisible(Selectors.LogInWithAtlasButton); + + const loginStatus = browser.$(Selectors.AtlasLoginStatus); + await browser.waitUntil(async () => { + return ( + (await loginStatus.getText()).trim() === + 'Logged in with Atlas account test@example.com' + ); + }); + } finally { + await telemetry.stop(); + } + const atlasUserIdAfter = await browser.getFeature( + 'telemetryAtlasUserId' + ); + expect(atlasUserIdAfter).to.be.a('string'); + + console.log({ + telemetry: telemetry + .events() + .map(({ type, event }) => ({ type, event })), + }); + + const identify = telemetry + .events() + .find((entry) => entry.type === 'identify'); + expect(identify.traits.platform).to.equal(process.platform); + expect(identify.traits.arch).to.match(/^(x64|arm64)$/); + }); + }); + it('should allow to accept TOS when signed in', async function () { await browser.openSettingsModal('Feature Preview'); diff --git a/packages/compass-e2e-tests/tests/logging.test.ts b/packages/compass-e2e-tests/tests/logging.test.ts index 395c6bdb51f..f52bc1b393a 100644 --- a/packages/compass-e2e-tests/tests/logging.test.ts +++ b/packages/compass-e2e-tests/tests/logging.test.ts @@ -42,14 +42,6 @@ describe('Logging and Telemetry integration', function () { ); }); - it('tracks an event for identify call', function () { - const identify = telemetry - .events() - .find((entry) => entry.type === 'identify'); - expect(identify.traits.platform).to.equal(process.platform); - expect(identify.traits.arch).to.match(/^(x64|arm64)$/); - }); - it('tracks an event for shell use', function () { const shellUse = telemetry .events() @@ -379,13 +371,16 @@ describe('Logging and Telemetry integration', function () { }); }); - describe('on subsequent run', function () { + describe('on subsequent run - with atlas user id', function () { let compass: Compass; let telemetry: Telemetry; + const auid = 'abcdef'; before(async function () { telemetry = await startTelemetryServer(); compass = await init(this.test?.fullTitle()); + + await compass.browser.setFeature('telemetryAtlasUserId', auid); }); afterEach(async function () { @@ -398,8 +393,6 @@ describe('Logging and Telemetry integration', function () { }); it('tracks an event for identify call', function () { - console.log(telemetry.events()); - const identify = telemetry .events() .find((entry) => entry.type === 'identify'); diff --git a/packages/compass/src/main/telemetry.ts b/packages/compass/src/main/telemetry.ts index 30b2d577406..48d2a024398 100644 --- a/packages/compass/src/main/telemetry.ts +++ b/packages/compass/src/main/telemetry.ts @@ -182,7 +182,10 @@ class CompassTelemetry { } }; const onAtlasUserIdChanged = (value?: string) => { - if (value) this.identify(); + if (value) { + this.telemetryAtlasUserId = value; + this.identify(); + } }; onTrackUsageStatisticsChanged(trackUsageStatistics); // initial setup with current value From b8fb7c3537b82d0d9c7bc28e1f07564c85556a12 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Tue, 13 Feb 2024 18:07:34 +0100 Subject: [PATCH 6/8] cleanup --- packages/compass-e2e-tests/tests/atlas-login.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/compass-e2e-tests/tests/atlas-login.test.ts b/packages/compass-e2e-tests/tests/atlas-login.test.ts index c51cf12528e..1c451de964a 100644 --- a/packages/compass-e2e-tests/tests/atlas-login.test.ts +++ b/packages/compass-e2e-tests/tests/atlas-login.test.ts @@ -180,12 +180,6 @@ describe('Atlas Login', function () { ); expect(atlasUserIdAfter).to.be.a('string'); - console.log({ - telemetry: telemetry - .events() - .map(({ type, event }) => ({ type, event })), - }); - const identify = telemetry .events() .find((entry) => entry.type === 'identify'); From eb91d5627ff41df6a34fd088960d9ab61b39b601 Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 14 Feb 2024 09:20:10 +0100 Subject: [PATCH 7/8] update test --- .../tests/atlas-login.test.ts | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/compass-e2e-tests/tests/atlas-login.test.ts b/packages/compass-e2e-tests/tests/atlas-login.test.ts index 1c451de964a..7989d1ef817 100644 --- a/packages/compass-e2e-tests/tests/atlas-login.test.ts +++ b/packages/compass-e2e-tests/tests/atlas-login.test.ts @@ -154,27 +154,28 @@ describe('Atlas Login', function () { telemetry = await startTelemetryServer(); }); + after(async function () { + await telemetry.stop(); + }); + it('should send identify after the user has logged in', async function () { const atlasUserIdBefore = await browser.getFeature( 'telemetryAtlasUserId' ); expect(atlasUserIdBefore).to.not.exist; - try { - await browser.openSettingsModal('Feature Preview'); + await browser.openSettingsModal('Feature Preview'); - await browser.clickVisible(Selectors.LogInWithAtlasButton); + await browser.clickVisible(Selectors.LogInWithAtlasButton); + + const loginStatus = browser.$(Selectors.AtlasLoginStatus); + await browser.waitUntil(async () => { + return ( + (await loginStatus.getText()).trim() === + 'Logged in with Atlas account test@example.com' + ); + }); - const loginStatus = browser.$(Selectors.AtlasLoginStatus); - await browser.waitUntil(async () => { - return ( - (await loginStatus.getText()).trim() === - 'Logged in with Atlas account test@example.com' - ); - }); - } finally { - await telemetry.stop(); - } const atlasUserIdAfter = await browser.getFeature( 'telemetryAtlasUserId' ); From ff1203d87e0b7829ac86f5e060ea2b5989b0d7ff Mon Sep 17 00:00:00 2001 From: Paula Stachova Date: Wed, 14 Feb 2024 13:13:10 +0100 Subject: [PATCH 8/8] revert: bring back the anonymous identify --- packages/compass-e2e-tests/tests/logging.test.ts | 8 ++++++++ packages/compass/src/main/telemetry.ts | 3 +-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/compass-e2e-tests/tests/logging.test.ts b/packages/compass-e2e-tests/tests/logging.test.ts index f52bc1b393a..1df5547242a 100644 --- a/packages/compass-e2e-tests/tests/logging.test.ts +++ b/packages/compass-e2e-tests/tests/logging.test.ts @@ -42,6 +42,14 @@ describe('Logging and Telemetry integration', function () { ); }); + it('tracks an event for identify call', function () { + const identify = telemetry + .events() + .find((entry) => entry.type === 'identify'); + expect(identify.traits.platform).to.equal(process.platform); + expect(identify.traits.arch).to.match(/^(x64|arm64)$/); + }); + it('tracks an event for shell use', function () { const shellUse = telemetry .events() diff --git a/packages/compass/src/main/telemetry.ts b/packages/compass/src/main/telemetry.ts index 48d2a024398..d2422f73139 100644 --- a/packages/compass/src/main/telemetry.ts +++ b/packages/compass/src/main/telemetry.ts @@ -104,8 +104,7 @@ class CompassTelemetry { if ( this.state === 'enabled' && this.analytics && - this.telemetryAnonymousId && - this.telemetryAtlasUserId + this.telemetryAnonymousId ) { this.analytics.identify({ userId: this.telemetryAtlasUserId,