Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -49,6 +49,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
Loading