Skip to content

Commit d1e5d27

Browse files
authored
fix(compass): move telemetry setup to main process entirely COMPASS-6113 COMPASS-6092 (#3534)
Set up telemetry entirely as part of the main process initialization, and use that to ensure that telemetry events from CLI connection export/import are sent.
1 parent 38f0a54 commit d1e5d27

File tree

8 files changed

+81
-103
lines changed

8 files changed

+81
-103
lines changed

packages/compass-e2e-tests/helpers/commands/set-feature.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,13 @@ export async function setFeature(
66
value: boolean | string
77
): Promise<void> {
88
await browser.execute(
9-
(_name, _value) => {
9+
async (_name, _value) => {
1010
// eslint-disable-next-line @typescript-eslint/no-var-requires
11-
require('electron').ipcRenderer.invoke('compass:save-preferences', {
11+
await require('electron').ipcRenderer.invoke('compass:save-preferences', {
1212
[_name]: _value,
1313
});
1414
},
1515
name,
1616
value
1717
);
18-
19-
// Enable telemetry (CompassTelemetry.state).
20-
// Setting the feature above, just updates it in the global hadron.
21-
// Since the app has bootrapped already, we force update.
22-
if (name === 'trackUsageStatistics') {
23-
const event = value ? 'compass:usage:enabled' : 'compass:usage:disabled';
24-
await browser.execute((_event) => {
25-
// eslint-disable-next-line @typescript-eslint/no-var-requires
26-
require('electron').ipcRenderer.call(_event);
27-
}, event);
28-
}
2918
}

packages/compass-e2e-tests/tests/import-export-connections.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@ import os from 'os';
44
import path from 'path';
55
import { promises as fs } from 'fs';
66
import * as Selectors from '../helpers/selectors';
7+
import type { Telemetry } from '../helpers/telemetry';
8+
import { startTelemetryServer } from '../helpers/telemetry';
79

810
describe('Connection Import / Export', function () {
911
let tmpdir: string;
1012
let i = 0;
1113
let originalDisableKeychainUsage: string | undefined;
14+
let telemetry: Telemetry;
1215

1316
beforeEach(async function () {
17+
telemetry = await startTelemetryServer();
18+
1419
tmpdir = path.join(
1520
os.tmpdir(),
1621
`compass-import-export-${Date.now().toString(32)}-${++i}`
@@ -33,6 +38,8 @@ describe('Connection Import / Export', function () {
3338
else delete process.env.COMPASS_E2E_DISABLE_KEYCHAIN_USAGE;
3439

3540
await fs.rmdir(tmpdir, { recursive: true });
41+
42+
await telemetry.stop();
3643
});
3744

3845
for (const variant of ['plaintext', 'encrypted'] as const) {
@@ -59,6 +66,7 @@ describe('Connection Import / Export', function () {
5966
}
6067

6168
{
69+
const existingEventsCount = telemetry.events().length;
6270
// Export favorite, roughly verify file contents
6371
await runCompassOnce([
6472
`--export-connections=${file}`,
@@ -79,12 +87,18 @@ describe('Connection Import / Export', function () {
7987
);
8088
expect(conn.connectionSecrets).to.not.exist;
8189
} else {
82-
console.log({ conn });
8390
expect(conn.connectionOptions.connectionString).to.equal(
8491
connectionStringWithoutCredentials
8592
);
8693
expect(conn.connectionSecrets).to.be.a('string');
8794
}
95+
96+
const newEvents = telemetry.events().slice(existingEventsCount);
97+
expect(newEvents).to.have.lengthOf(1);
98+
expect(newEvents[0].event).to.equal('Connection Exported');
99+
expect(newEvents[0].properties.context).to.equal('CLI');
100+
// inequality since we may have lingering connections from other test runs
101+
expect(newEvents[0].properties.count).to.be.greaterThanOrEqual(1);
88102
}
89103

90104
{
@@ -100,11 +114,19 @@ describe('Connection Import / Export', function () {
100114
}
101115

102116
{
117+
const existingEventsCount = telemetry.events().length;
103118
// Import favorite
104119
await runCompassOnce([
105120
`--import-connections=${file}`,
106121
...passphraseArgs,
107122
]);
123+
124+
const newEvents = telemetry.events().slice(existingEventsCount);
125+
expect(newEvents).to.have.lengthOf(1);
126+
expect(newEvents[0].event).to.equal('Connection Imported');
127+
expect(newEvents[0].properties.context).to.equal('CLI');
128+
// inequality since we may have lingering connections from other test runs
129+
expect(newEvents[0].properties.count).to.be.greaterThanOrEqual(1);
108130
}
109131

110132
{

packages/compass-e2e-tests/tests/logging.test.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -139,27 +139,6 @@ describe('Logging and Telemetry integration', function () {
139139
expect(actual.cryptSharedLibPath).to.include('mongo_crypt_v1');
140140
},
141141
},
142-
{
143-
s: 'I',
144-
c: 'COMPASS-TELEMETRY',
145-
id: 1_001_000_095,
146-
ctx: 'Telemetry',
147-
msg: 'Disabling Telemetry reporting',
148-
},
149-
{
150-
s: 'I',
151-
c: 'COMPASS-TELEMETRY',
152-
id: 1_001_000_093,
153-
ctx: 'Telemetry',
154-
msg: 'Loading telemetry config',
155-
attr: (actual: any) => {
156-
expect(actual.telemetryCapableEnvironment).to.equal(true);
157-
expect(actual.hasAnalytics).to.equal(true);
158-
expect(actual.currentUserId).to.not.exist;
159-
expect(actual.telemetryAnonymousId).to.be.a('string');
160-
expect(actual.state).to.equal('waiting-for-user-config');
161-
},
162-
},
163142
{
164143
s: 'I',
165144
c: 'COMPASS-APP',

packages/compass-preferences-model/src/preferences.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ export class Preferences {
676676
changedPreferencesValues: Partial<AllPreferences>
677677
): void {
678678
for (const callback of this._onPreferencesChangedCallbacks) {
679-
return callback(changedPreferencesValues);
679+
callback(changedPreferencesValues);
680680
}
681681
}
682682

packages/compass/src/app/index.js

Lines changed: 4 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const ipc = require('hadron-ipc');
22
const remote = require('@electron/remote');
3-
const semver = require('semver');
43

54
const { preferencesAccess: preferences } = require('compass-preferences-model');
65

@@ -195,39 +194,19 @@ const Application = View.extend({
195194
fetchUser: async function () {
196195
debug('getting user preferences');
197196
const {
198-
currentUserId,
199197
telemetryAnonymousId,
200-
trackUsageStatistics,
201-
lastKnownVersion,
198+
lastKnownVersion
202199
} = preferences.getPreferences();
203200

204-
// Check if uuid was stored as currentUserId, if not pass telemetryAnonymousId to fetch a user.
205-
const user = await User.getOrCreate(currentUserId || telemetryAnonymousId);
206-
207-
const changedPreferences = { telemetryAnonymousId: user.id };
201+
// The main process ensured that `telemetryAnonymousId` contains the id of the User model
202+
const user = await User.getOrCreate(telemetryAnonymousId);
208203

209204
this.user.set(user.serialize());
210205
this.user.trigger('sync');
211206
debug('user fetch successful', user.serialize());
212207

213208
this.previousVersion = lastKnownVersion || '0.0.0';
214-
215-
if (semver.neq(this.previousVersion, APP_VERSION)) {
216-
changedPreferences.lastKnownVersion = APP_VERSION;
217-
}
218-
219-
const savedPreferences = await preferences.savePreferences(
220-
changedPreferences
221-
);
222-
223-
ipc.call('compass:usage:identify', {
224-
currentUserId: savedPreferences.currentUserId,
225-
telemetryAnonymousId: user.id,
226-
});
227-
ipc.call(
228-
trackUsageStatistics ? 'compass:usage:enabled' : 'compass:usage:disabled'
229-
);
230-
209+
await preferences.savePreferences({ lastKnownVersion: APP_VERSION });
231210
return user;
232211
},
233212
});

packages/compass/src/main/application.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { CompassTelemetry } from './telemetry';
1010
import { CompassWindowManager } from './window-manager';
1111
import { CompassMenu } from './menu';
1212
import { setupCSFLELibrary } from './setup-csfle-library';
13-
import { setupPreferences } from 'compass-preferences-model';
13+
import { setupPreferencesAndUserModel } from './setup-preferences-and-user-model';
1414
import type { ParsedGlobalPreferencesResult } from 'compass-preferences-model';
1515

1616
const debug = createDebug('mongodb-compass:main:application');
@@ -40,8 +40,9 @@ class CompassApplication {
4040
}
4141

4242
this.setupUserDirectory();
43-
await setupPreferences(globalPreferences);
44-
await Promise.all([this.setupLogging(), this.setupTelemetry()]);
43+
await setupPreferencesAndUserModel(globalPreferences);
44+
await this.setupLogging();
45+
await this.setupTelemetry();
4546

4647
if (mode === 'CLI') {
4748
return;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import type { ParsedGlobalPreferencesResult} from 'compass-preferences-model';
2+
import preferences, { setupPreferences } from 'compass-preferences-model';
3+
// @ts-expect-error no TS definitions available
4+
import User from 'compass-user-model';
5+
6+
export async function setupPreferencesAndUserModel(globalPreferences: ParsedGlobalPreferencesResult): Promise<void> {
7+
await setupPreferences(globalPreferences);
8+
9+
const {
10+
currentUserId,
11+
telemetryAnonymousId
12+
} = preferences.getPreferences();
13+
14+
// Check if uuid was stored as currentUserId, if not pass telemetryAnonymousId to fetch a user.
15+
const user = await User.getOrCreate(currentUserId || telemetryAnonymousId);
16+
const changedPreferences = { telemetryAnonymousId: user.id };
17+
await preferences.savePreferences(changedPreferences);
18+
}

packages/compass/src/main/telemetry.ts

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createLoggerAndTelemetry } from '@mongodb-js/compass-logging';
55
import type { CompassApplication } from './application';
66
import type { EventEmitter } from 'events';
77
import { getOsInfo } from './get-os-info';
8+
import preferences from 'compass-preferences-model';
89

910
const { log, mongoLogId } = createLoggerAndTelemetry('COMPASS-TELEMETRY');
1011

@@ -28,8 +29,7 @@ const telemetryCapableEnvironment = !!(
2829

2930
class CompassTelemetry {
3031
private static analytics: Analytics | null = null;
31-
private static state: 'enabled' | 'disabled' | 'waiting-for-user-config' =
32-
'waiting-for-user-config';
32+
private static state: 'enabled' | 'disabled' = 'disabled';
3333
private static queuedEvents: EventInfo[] = []; // Events that happen before we fetch user preferences
3434
private static currentUserId?: string; // Deprecated field. Should be used only for old users to keep their analytics in Segment.
3535
private static telemetryAnonymousId = ''; // The randomly generated anonymous user id.
@@ -56,7 +56,6 @@ class CompassTelemetry {
5656
const commonProperties = this._getCommonProperties();
5757

5858
if (
59-
this.state === 'waiting-for-user-config' ||
6059
!this.telemetryAnonymousId
6160
) {
6261
this.queuedEvents.push(info);
@@ -126,50 +125,41 @@ class CompassTelemetry {
126125
}
127126

128127
private static _init(app: typeof CompassApplication) {
129-
process.on('compass:track', (meta: EventInfo) => {
130-
this._track(meta);
131-
});
132-
133-
ipcMain.respondTo('compass:track', (evt, meta: EventInfo) => {
134-
(process as EventEmitter).emit('compass:track', meta);
135-
});
136-
137-
ipcMain.respondTo(
138-
'compass:usage:identify',
139-
(evt, { currentUserId, telemetryAnonymousId }: { currentUserId?: string; telemetryAnonymousId: string }) => {
140-
// This always happens after the first enable/disable call.
141-
this.currentUserId = currentUserId;
142-
this.telemetryAnonymousId = telemetryAnonymousId;
143-
void this.identify();
144-
}
145-
);
146-
147-
ipcMain.respondTo('compass:usage:enabled', () => {
148-
log.info(
149-
mongoLogId(1_001_000_094),
150-
'Telemetry',
151-
'Enabling Telemetry reporting'
152-
);
153-
if (this.state !== 'enabled') {
128+
const { trackUsageStatistics, currentUserId, telemetryAnonymousId } = preferences.getPreferences();
129+
this.currentUserId = currentUserId;
130+
this.telemetryAnonymousId = telemetryAnonymousId;
131+
const onTrackUsageStatisticsChanged = (value: boolean) => {
132+
if (value && this.state !== 'enabled') {
133+
log.info(
134+
mongoLogId(1_001_000_094),
135+
'Telemetry',
136+
'Enabling Telemetry reporting'
137+
);
154138
this.state = 'enabled';
155139
void this.identify();
156-
}
157-
});
158-
159-
ipcMain.respondTo('compass:usage:disabled', () => {
160-
log.info(
161-
mongoLogId(1_001_000_095),
162-
'Telemetry',
163-
'Disabling Telemetry reporting'
164-
);
165-
if (this.state === 'enabled') {
140+
} else if (!value && this.state !== 'disabled') {
141+
log.info(
142+
mongoLogId(1_001_000_095),
143+
'Telemetry',
144+
'Disabling Telemetry reporting'
145+
);
166146
this._track({
167147
event: 'Telemetry Disabled',
168148
properties: {},
169149
});
170150
this.analytics?.flush();
151+
this.state = 'disabled';
171152
}
172-
this.state = 'disabled';
153+
};
154+
onTrackUsageStatisticsChanged(trackUsageStatistics); // initial setup with current value
155+
preferences.onPreferenceValueChanged('trackUsageStatistics', onTrackUsageStatisticsChanged);
156+
157+
process.on('compass:track', (meta: EventInfo) => {
158+
this._track(meta);
159+
});
160+
161+
ipcMain.respondTo('compass:track', (evt, meta: EventInfo) => {
162+
(process as EventEmitter).emit('compass:track', meta);
173163
});
174164

175165
// only used in tests

0 commit comments

Comments
 (0)