Skip to content

Commit

Permalink
[MM-60086][MM-60610] Implement performanceMonitor, collect CPU/memory…
Browse files Browse the repository at this point in the history
… usage data and send via API (#3165)

* [MM-60086][MM-60610] Implement performanceMonitor, collect CPU/memory usage data and send via API

* Translations

* PR feedback

* Update api-types package
  • Loading branch information
devinbinnie authored Oct 18, 2024
1 parent 9d52ee7 commit 1029516
Show file tree
Hide file tree
Showing 38 changed files with 590 additions and 6 deletions.
2 changes: 2 additions & 0 deletions api-types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ export type DesktopAPI = {
openCallsUserSettings: () => void;
onOpenCallsUserSettings: (listener: () => void) => () => void;

onSendMetrics: (listener: (metricsMap: Map<string, {cpu?: number; memory?: number}>) => void) => () => void;

// Utility
unregister: (channel: string) => void;
}
4 changes: 4 additions & 0 deletions api-types/lib/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,9 @@ export type DesktopAPI = {
onOpenStopRecordingModal: (listener: (channelID: string) => void) => () => void;
openCallsUserSettings: () => void;
onOpenCallsUserSettings: (listener: () => void) => () => void;
onSendMetrics: (listener: (metricsMap: Map<string, {
cpu?: number;
memory?: number;
}>) => void) => () => void;
unregister: (channel: string) => void;
};
4 changes: 2 additions & 2 deletions api-types/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion api-types/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@mattermost/desktop-api",
"version": "5.10.0-1",
"version": "5.10.0-2",
"description": "Shared types for the Desktop App API provided to the Web App",
"keywords": [
"mattermost"
Expand Down
2 changes: 2 additions & 0 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@
"renderer.components.settingsPage.downloadLocation.description": "Specify the folder where files will download.",
"renderer.components.settingsPage.enableHardwareAcceleration": "Use GPU hardware acceleration",
"renderer.components.settingsPage.enableHardwareAcceleration.description": "If enabled, {appName} UI is rendered more efficiently but can lead to decreased stability for some systems.",
"renderer.components.settingsPage.enableMetrics": "Send anonymous usage data to your configured servers",
"renderer.components.settingsPage.enableMetrics.description": "Sends usage data about the application and its performance to your configured servers that accept it.",
"renderer.components.settingsPage.flashWindow": "Flash taskbar icon when a new message is received",
"renderer.components.settingsPage.flashWindow.description": "If enabled, the taskbar icon will flash for a few seconds when a new message is received.",
"renderer.components.settingsPage.flashWindow.description.linuxFunctionality": "This functionality may not work with all Linux window managers.",
Expand Down
1 change: 1 addition & 0 deletions src/common/Validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ const configDataSchemaV3 = Joi.object<ConfigV3>({
alwaysClose: Joi.boolean(),
logLevel: Joi.string().default('info'),
appLanguage: Joi.string().allow(''),
enableMetrics: Joi.boolean(),
});

// eg. data['community.mattermost.com'] = { data: 'certificate data', issuerName: 'COMODO RSA Domain Validation Secure Server CA'};
Expand Down
4 changes: 4 additions & 0 deletions src/common/communication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,7 @@ export const GET_NONCE = 'get-nonce';
export const DEVELOPER_MODE_UPDATED = 'developer-mode-updated';
export const IS_DEVELOPER_MODE_ENABLED = 'is-developer-mode-enabled';
export const GET_DEVELOPER_MODE_SETTING = 'get-developer-mode-setting';

export const METRICS_SEND = 'metrics-send';
export const METRICS_RECEIVE = 'metrics-receive';
export const METRICS_REQUEST = 'metrics-request';
1 change: 1 addition & 0 deletions src/common/config/defaultPreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const defaultPreferences: ConfigV3 = {
downloadLocation: getDefaultDownloadLocation(),
startInFullscreen: false,
logLevel: 'info',
enableMetrics: true,
};

export default defaultPreferences;
4 changes: 4 additions & 0 deletions src/common/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ export class Config extends EventEmitter {
return this.combinedData?.appLanguage;
}

get enableMetrics() {
return this.combinedData?.enableMetrics;
}

/**
* Gets the servers from registry into the config object and reload
*
Expand Down
6 changes: 6 additions & 0 deletions src/common/config/migrationPreferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,11 @@ export default function migrateConfigItems(config: Config) {
didMigrate = true;
}

if (!migrationPrefs.getValue('enableMetrics')) {
config.enableMetrics = true;
migrationPrefs.setValue('enableMetrics', true);
didMigrate = true;
}

return didMigrate;
}
4 changes: 3 additions & 1 deletion src/main/app/initialize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ jest.mock('electron', () => ({
handle: jest.fn(),
},
}));

jest.mock('main/performanceMonitor', () => ({
init: jest.fn(),
}));
jest.mock('main/i18nManager', () => ({
localizeMessage: jest.fn(),
setLocale: jest.fn(),
Expand Down
5 changes: 5 additions & 0 deletions src/main/app/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import i18nManager from 'main/i18nManager';
import NonceManager from 'main/nonceManager';
import {getDoNotDisturb} from 'main/notifications';
import parseArgs from 'main/ParseArgs';
import PerformanceMonitor from 'main/performanceMonitor';
import PermissionsManager from 'main/permissionsManager';
import Tray from 'main/tray/tray';
import TrustedOriginsStore from 'main/trustedOrigins';
Expand Down Expand Up @@ -448,6 +449,10 @@ async function initializeAfterAppReady() {
AppVersionManager.lastAppVersion = app.getVersion();

handleMainWindowIsShown();

// The metrics won't start collecting for another minute
// so we can assume if we start now everything should be loaded by the time we're done
PerformanceMonitor.init();
}

function onUserActivityStatus(status: {
Expand Down
255 changes: 255 additions & 0 deletions src/main/performanceMonitor.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,255 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {app, ipcMain, powerMonitor} from 'electron';

import {EMIT_CONFIGURATION, METRICS_RECEIVE, METRICS_REQUEST, METRICS_SEND} from 'common/communication';
import Config from 'common/config';

import {PerformanceMonitor} from './performanceMonitor';

jest.mock('electron', () => ({
app: {
getAppMetrics: jest.fn(),
},
ipcMain: {
on: jest.fn(),
off: jest.fn(),
},
powerMonitor: {
on: jest.fn(),
},
}));

jest.mock('common/config', () => ({
enableMetrics: true,
}));

describe('main/performanceMonitor', () => {
let makeWebContents;
beforeAll(() => {
jest.useFakeTimers();
jest.spyOn(global, 'setInterval');
jest.spyOn(global, 'clearInterval');
});

beforeEach(() => {
app.getAppMetrics.mockReturnValue([]);

let cb;
ipcMain.on.mockImplementation((channel, listener) => {
if (channel === METRICS_RECEIVE) {
cb = listener;
}
});

makeWebContents = (id, resolve) => ({
send: jest.fn().mockImplementation((channel, arg1, arg2) => {
if (channel === METRICS_REQUEST) {
cb({sender: {id}}, arg1, {serverId: arg2, cpu: id, memory: id * 100});
}
if (channel === METRICS_SEND) {
resolve(arg1);
}
}),
on: (_, listener) => listener(),
id,
});
});

afterEach(() => {
Config.enableMetrics = true;
});

it('should start and stop with config changes', () => {
let emitConfigCb;
ipcMain.on.mockImplementation((channel, listener) => {
if (channel === EMIT_CONFIGURATION) {
emitConfigCb = listener;
}
});

const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();
expect(setInterval).toHaveBeenCalled();

Config.enableMetrics = false;
emitConfigCb();
expect(clearInterval).toHaveBeenCalled();

Config.enableMetrics = true;
emitConfigCb();
expect(setInterval).toHaveBeenCalledTimes(2);
});

it('should start and stop with power monitor changes', () => {
const listeners = new Map();
powerMonitor.on.mockImplementation((channel, listener) => {
listeners.set(channel, listener);
});

const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();
expect(setInterval).toHaveBeenCalled();

listeners.get('suspend')();
expect(clearInterval).toHaveBeenCalled();

setInterval.mockClear();
clearInterval.mockClear();

listeners.get('resume')();
expect(setInterval).toHaveBeenCalled();

listeners.get('lock-screen')();
expect(clearInterval).toHaveBeenCalled();

setInterval.mockClear();
clearInterval.mockClear();

listeners.get('unlock-screen')();
expect(setInterval).toHaveBeenCalled();

listeners.get('speed-limit-change')(50);
expect(clearInterval).toHaveBeenCalled();

setInterval.mockClear();
clearInterval.mockClear();

listeners.get('speed-limit-change')(100);
expect(setInterval).toHaveBeenCalled();
});

describe('init', () => {
it('should not start until init', () => {
const performanceMonitor = new PerformanceMonitor();
expect(setInterval).not.toHaveBeenCalled();

performanceMonitor.init();
expect(setInterval).toHaveBeenCalled();
});

it('should run app metrics for node on init', () => {
const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();
expect(app.getAppMetrics).toHaveBeenCalled();
});

it('should not start if disabled by config', () => {
Config.enableMetrics = false;

const performanceMonitor = new PerformanceMonitor();
expect(setInterval).not.toHaveBeenCalled();

performanceMonitor.init();
expect(setInterval).not.toHaveBeenCalled();
});
});

describe('registerView', () => {
it('should send metrics to registered server views', async () => {
const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
});

jest.runOnlyPendingTimers();

expect(await sendValue).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
});

it('should send metrics for other tabs to registered server views', async () => {
const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
performanceMonitor.registerView('view-2', makeWebContents(2, resolve), 'server-1');
});

jest.runOnlyPendingTimers();

expect(await sendValue).toEqual(new Map([['view-2', {cpu: 2, memory: 200, serverId: 'server-1'}], ['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
});

it('should not send metrics for tabs of other servers to registered server views', async () => {
const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
performanceMonitor.registerView('view-2', makeWebContents(2, resolve), 'server-2');
});

jest.runOnlyPendingTimers();

expect(await sendValue).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
});

it('should always include node metrics', async () => {
app.getAppMetrics.mockReturnValue([{
name: 'main',
type: 'Browser',
cpu: {percentCPUUsage: 50},
memory: {privateBytes: 1000},
}]);

const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
});

jest.runOnlyPendingTimers();

expect(await sendValue).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}], ['main', {cpu: 50, memory: 1000}]]));
});

it('should never include tabs from getAppMetrics', async () => {
app.getAppMetrics.mockReturnValue([{
name: 'other-server',
type: 'Tab',
cpu: {percentCPUUsage: 50},
memory: {privateBytes: 1000},
}]);

const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
});

jest.runOnlyPendingTimers();

expect(await sendValue).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
});
});

describe('unregisterView', () => {
it('should not send after the view is removed', async () => {
const performanceMonitor = new PerformanceMonitor();
performanceMonitor.init();

const sendValue = new Promise((resolve) => {
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
performanceMonitor.registerServerView('view-2', makeWebContents(2, resolve), 'server-1');
});

jest.runOnlyPendingTimers();
expect(await sendValue).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}], ['view-2', {cpu: 2, memory: 200, serverId: 'server-1'}]]));

// Have to re-register to make sure the promise resolves
const sendValue2 = new Promise((resolve) => {
performanceMonitor.unregisterView(2);
performanceMonitor.registerServerView('view-1', makeWebContents(1, resolve), 'server-1');
});

jest.runOnlyPendingTimers();
expect(await sendValue2).toEqual(new Map([['view-1', {cpu: 1, memory: 100, serverId: 'server-1'}]]));
});
});
});
Loading

0 comments on commit 1029516

Please sign in to comment.