From 3b12b40936de3077cd795e80032b2fb45efedba2 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 25 Feb 2025 15:31:52 +0100 Subject: [PATCH 1/3] feat(connection-form): add connection and anonymous id to app name --- .../src/stores/connections-store-redux.ts | 3 +++ .../src/hooks/use-connect-form.ts | 9 ++++++++- .../src/utils/set-app-name-if-missing.spec.ts | 19 +++++++++++++++---- .../src/utils/set-app-name-if-missing.ts | 18 ++++++++++++++---- 4 files changed, 40 insertions(+), 9 deletions(-) diff --git a/packages/compass-connections/src/stores/connections-store-redux.ts b/packages/compass-connections/src/stores/connections-store-redux.ts index f9a4e9abacf..87bbad60d98 100644 --- a/packages/compass-connections/src/stores/connections-store-redux.ts +++ b/packages/compass-connections/src/stores/connections-store-redux.ts @@ -1492,6 +1492,7 @@ const connectWithOptions = ( forceConnectionOptions, browserCommandForOIDCAuth, maximumNumberOfActiveConnections, + telemetryAnonymousId, } = preferences.getPreferences(); const connectionProgress = getNotificationTriggers(); @@ -1553,10 +1554,12 @@ const connectWithOptions = ( cloneDeep(connectionOptions), SecretsForConnection.get(connectionInfo.id) ?? {} ), + connectionId: connectionInfo.id, defaultAppName: appName, preferences: { forceConnectionOptions: forceConnectionOptions ?? [], browserCommandForOIDCAuth, + telemetryAnonymousId, }, notifyDeviceFlow: (deviceFlowInfo) => { connectionProgress.openNotifyDeviceAuthModal( diff --git a/packages/connection-form/src/hooks/use-connect-form.ts b/packages/connection-form/src/hooks/use-connect-form.ts index 117e546265b..137512a1e4d 100644 --- a/packages/connection-form/src/hooks/use-connect-form.ts +++ b/packages/connection-form/src/hooks/use-connect-form.ts @@ -843,11 +843,13 @@ function setInitialState({ export function adjustConnectionOptionsBeforeConnect({ connectionOptions, + connectionId, defaultAppName, notifyDeviceFlow, preferences, }: { connectionOptions: Readonly; + connectionId: string; defaultAppName?: string; notifyDeviceFlow?: (deviceFlowInformation: { verificationUrl: string; @@ -855,6 +857,7 @@ export function adjustConnectionOptionsBeforeConnect({ }) => void; preferences: { browserCommandForOIDCAuth?: string; + telemetryAnonymousId?: string; forceConnectionOptions: [string, string][]; }; }): ConnectionOptions { @@ -863,7 +866,11 @@ export function adjustConnectionOptionsBeforeConnect({ ) => ConnectionOptions)[] = [ adjustCSFLEParams, unsetFleOptionsIfEmptyAutoEncryption, - setAppNameParamIfMissing(defaultAppName), + setAppNameParamIfMissing({ + defaultAppName, + connectionId, + telemetryAnonymousId: preferences.telemetryAnonymousId, + }), adjustOIDCConnectionOptionsBeforeConnect({ browserCommandForOIDCAuth: preferences.browserCommandForOIDCAuth, notifyDeviceFlow, diff --git a/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts b/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts index e867fd2ebd8..c03e9059e13 100644 --- a/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts +++ b/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts @@ -4,7 +4,10 @@ import { setAppNameParamIfMissing } from './set-app-name-if-missing'; describe('setAppNameParamIfMissing', function () { it('leaves options unchanged if no default appName was specified', function () { expect( - setAppNameParamIfMissing()({ + setAppNameParamIfMissing({ + connectionId: '123', + telemetryAnonymousId: '789', + })({ connectionString: 'mongodb://localhost/', }) ).to.deep.equal({ @@ -14,7 +17,11 @@ describe('setAppNameParamIfMissing', function () { it('leaves options unchanged if appName was already part of the connection string', function () { expect( - setAppNameParamIfMissing('defaultAppName')({ + setAppNameParamIfMissing({ + defaultAppName: 'defaultAppName', + connectionId: '123', + telemetryAnonymousId: '789', + })({ connectionString: 'mongodb://localhost/?appName=foobar', }) ).to.deep.equal({ @@ -24,11 +31,15 @@ describe('setAppNameParamIfMissing', function () { it('sets appName to a default value if not already set', function () { expect( - setAppNameParamIfMissing('defaultAppName')({ + setAppNameParamIfMissing({ + defaultAppName: 'defaultAppName', + connectionId: '123', + telemetryAnonymousId: '789', + })({ connectionString: 'mongodb://localhost/', }) ).to.deep.equal({ - connectionString: 'mongodb://localhost/?appName=defaultAppName', + connectionString: 'mongodb://localhost/?appName=defaultAppName-789-123', }); }); }); diff --git a/packages/connection-form/src/utils/set-app-name-if-missing.ts b/packages/connection-form/src/utils/set-app-name-if-missing.ts index 015c1918683..449cbaa2355 100644 --- a/packages/connection-form/src/utils/set-app-name-if-missing.ts +++ b/packages/connection-form/src/utils/set-app-name-if-missing.ts @@ -3,9 +3,15 @@ import type { MongoClientOptions } from 'mongodb'; import { ConnectionString } from 'mongodb-connection-string-url'; import type { ConnectionOptions } from 'mongodb-data-service'; -export function setAppNameParamIfMissing( - defaultAppName?: string -): (connectionOptions: Readonly) => ConnectionOptions { +export function setAppNameParamIfMissing({ + defaultAppName, + telemetryAnonymousId, + connectionId, +}: { + defaultAppName?: string; + telemetryAnonymousId?: string; + connectionId: string; +}): (connectionOptions: Readonly) => ConnectionOptions { return (connectionOptions) => { const connectionStringUrl = new ConnectionString( connectionOptions.connectionString @@ -14,7 +20,11 @@ export function setAppNameParamIfMissing( const searchParams = connectionStringUrl.typedSearchParams(); if (!searchParams.has('appName') && defaultAppName !== undefined) { - searchParams.set('appName', defaultAppName); + const appName = `${defaultAppName}${ + telemetryAnonymousId ? `-${telemetryAnonymousId}` : '' + }-${connectionId}`; + + searchParams.set('appName', appName); connectionOptions = { ...cloneDeep(connectionOptions), connectionString: connectionStringUrl.toString(), From b23fcebb04e4859f9edbd321e0a59787e1f6f3be Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 27 Feb 2025 14:56:48 +0100 Subject: [PATCH 2/3] test: update connectionString assertion --- packages/compass-web/src/entrypoint.spec.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/compass-web/src/entrypoint.spec.tsx b/packages/compass-web/src/entrypoint.spec.tsx index e574f38003f..7a454794780 100644 --- a/packages/compass-web/src/entrypoint.spec.tsx +++ b/packages/compass-web/src/entrypoint.spec.tsx @@ -57,6 +57,8 @@ describe('CompassWeb', function () { Sinon.resetHistory(); }); + const testAnonymousId = 'test-anonymous-id'; + async function renderCompassWebAndConnect( props: Partial> = {}, connectFn = mockConnectFn @@ -72,6 +74,7 @@ describe('CompassWeb', function () { {...props} initialPreferences={{ enableCreatingNewConnections: true, + telemetryAnonymousId: testAnonymousId, ...props.initialPreferences, }} onFailToLoadConnections={() => {}} @@ -97,16 +100,20 @@ describe('CompassWeb', function () { screen.getByText('Connecting to localhost:27017'); }); - expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property( - 'connectionString', - 'mongodb://localhost:27017/?appName=Compass+Web' - ); - await waitFor(() => { screen.getByText('Connected to localhost:27017'); }); expect(onTrackSpy).to.have.been.calledWith('New Connection'); + + const connectionId = onTrackSpy.firstCall.args[1][ + 'connection_id' + ] as string; + + expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property( + 'connectionString', + `mongodb://localhost:27017/?appName=Compass+Web-${testAnonymousId}-${connectionId}` + ); }); it('should render error state if connection fails', async function () { From 0d21b46e8b4bc119dbcccf3af761206444b99878 Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 3 Mar 2025 10:41:09 +0100 Subject: [PATCH 3/3] feat: append connection and telemetry id only for Atlas connections --- .../src/stores/connections-store-redux.ts | 2 +- packages/compass-web/src/entrypoint.spec.tsx | 17 +++++--------- .../src/hooks/use-connect-form.ts | 7 +++--- .../src/utils/set-app-name-if-missing.spec.ts | 22 +++++++++++++++++-- .../src/utils/set-app-name-if-missing.ts | 10 ++++++--- 5 files changed, 37 insertions(+), 21 deletions(-) diff --git a/packages/compass-connections/src/stores/connections-store-redux.ts b/packages/compass-connections/src/stores/connections-store-redux.ts index 87bbad60d98..fbe6b14a83f 100644 --- a/packages/compass-connections/src/stores/connections-store-redux.ts +++ b/packages/compass-connections/src/stores/connections-store-redux.ts @@ -1554,7 +1554,7 @@ const connectWithOptions = ( cloneDeep(connectionOptions), SecretsForConnection.get(connectionInfo.id) ?? {} ), - connectionId: connectionInfo.id, + connectionInfo, defaultAppName: appName, preferences: { forceConnectionOptions: forceConnectionOptions ?? [], diff --git a/packages/compass-web/src/entrypoint.spec.tsx b/packages/compass-web/src/entrypoint.spec.tsx index 7a454794780..e574f38003f 100644 --- a/packages/compass-web/src/entrypoint.spec.tsx +++ b/packages/compass-web/src/entrypoint.spec.tsx @@ -57,8 +57,6 @@ describe('CompassWeb', function () { Sinon.resetHistory(); }); - const testAnonymousId = 'test-anonymous-id'; - async function renderCompassWebAndConnect( props: Partial> = {}, connectFn = mockConnectFn @@ -74,7 +72,6 @@ describe('CompassWeb', function () { {...props} initialPreferences={{ enableCreatingNewConnections: true, - telemetryAnonymousId: testAnonymousId, ...props.initialPreferences, }} onFailToLoadConnections={() => {}} @@ -100,20 +97,16 @@ describe('CompassWeb', function () { screen.getByText('Connecting to localhost:27017'); }); + expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property( + 'connectionString', + 'mongodb://localhost:27017/?appName=Compass+Web' + ); + await waitFor(() => { screen.getByText('Connected to localhost:27017'); }); expect(onTrackSpy).to.have.been.calledWith('New Connection'); - - const connectionId = onTrackSpy.firstCall.args[1][ - 'connection_id' - ] as string; - - expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property( - 'connectionString', - `mongodb://localhost:27017/?appName=Compass+Web-${testAnonymousId}-${connectionId}` - ); }); it('should render error state if connection fails', async function () { diff --git a/packages/connection-form/src/hooks/use-connect-form.ts b/packages/connection-form/src/hooks/use-connect-form.ts index 137512a1e4d..bfad34f1073 100644 --- a/packages/connection-form/src/hooks/use-connect-form.ts +++ b/packages/connection-form/src/hooks/use-connect-form.ts @@ -843,13 +843,13 @@ function setInitialState({ export function adjustConnectionOptionsBeforeConnect({ connectionOptions, - connectionId, + connectionInfo, defaultAppName, notifyDeviceFlow, preferences, }: { connectionOptions: Readonly; - connectionId: string; + connectionInfo: Readonly>; defaultAppName?: string; notifyDeviceFlow?: (deviceFlowInformation: { verificationUrl: string; @@ -868,7 +868,8 @@ export function adjustConnectionOptionsBeforeConnect({ unsetFleOptionsIfEmptyAutoEncryption, setAppNameParamIfMissing({ defaultAppName, - connectionId, + connectionId: connectionInfo.id, + isAtlas: !!connectionInfo.atlasMetadata, telemetryAnonymousId: preferences.telemetryAnonymousId, }), adjustOIDCConnectionOptionsBeforeConnect({ diff --git a/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts b/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts index c03e9059e13..3495cd8df4c 100644 --- a/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts +++ b/packages/connection-form/src/utils/set-app-name-if-missing.spec.ts @@ -7,6 +7,7 @@ describe('setAppNameParamIfMissing', function () { setAppNameParamIfMissing({ connectionId: '123', telemetryAnonymousId: '789', + isAtlas: false, })({ connectionString: 'mongodb://localhost/', }) @@ -21,6 +22,7 @@ describe('setAppNameParamIfMissing', function () { defaultAppName: 'defaultAppName', connectionId: '123', telemetryAnonymousId: '789', + isAtlas: false, })({ connectionString: 'mongodb://localhost/?appName=foobar', }) @@ -29,17 +31,33 @@ describe('setAppNameParamIfMissing', function () { }); }); - it('sets appName to a default value if not already set', function () { + it('sets appName to a default app name if not atlas and not already set', function () { expect( setAppNameParamIfMissing({ defaultAppName: 'defaultAppName', connectionId: '123', telemetryAnonymousId: '789', + isAtlas: false, })({ connectionString: 'mongodb://localhost/', }) ).to.deep.equal({ - connectionString: 'mongodb://localhost/?appName=defaultAppName-789-123', + connectionString: 'mongodb://localhost/?appName=defaultAppName', + }); + }); + + it('sets appName to a default app name, anonymous id, and connection id if it is atlas and not already set', function () { + expect( + setAppNameParamIfMissing({ + defaultAppName: 'defaultAppName', + connectionId: '123', + telemetryAnonymousId: '789', + isAtlas: true, + })({ + connectionString: 'mongodb://atlas/', + }) + ).to.deep.equal({ + connectionString: 'mongodb://atlas/?appName=defaultAppName-789-123', }); }); }); diff --git a/packages/connection-form/src/utils/set-app-name-if-missing.ts b/packages/connection-form/src/utils/set-app-name-if-missing.ts index 449cbaa2355..d52dab7c5a8 100644 --- a/packages/connection-form/src/utils/set-app-name-if-missing.ts +++ b/packages/connection-form/src/utils/set-app-name-if-missing.ts @@ -7,10 +7,12 @@ export function setAppNameParamIfMissing({ defaultAppName, telemetryAnonymousId, connectionId, + isAtlas, }: { defaultAppName?: string; telemetryAnonymousId?: string; connectionId: string; + isAtlas: boolean; }): (connectionOptions: Readonly) => ConnectionOptions { return (connectionOptions) => { const connectionStringUrl = new ConnectionString( @@ -20,9 +22,11 @@ export function setAppNameParamIfMissing({ const searchParams = connectionStringUrl.typedSearchParams(); if (!searchParams.has('appName') && defaultAppName !== undefined) { - const appName = `${defaultAppName}${ - telemetryAnonymousId ? `-${telemetryAnonymousId}` : '' - }-${connectionId}`; + const appName = isAtlas + ? `${defaultAppName}${ + telemetryAnonymousId ? `-${telemetryAnonymousId}` : '' + }-${connectionId}` + : defaultAppName; searchParams.set('appName', appName); connectionOptions = {