Skip to content

Commit

Permalink
Add performance metrics to the app (#7953) (#8008)
Browse files Browse the repository at this point in the history
* Add performance metrics to the app

* add batcher and improve handling

* Add tests

* Fix test

* Address feedback

* Address feedback

* Address feedback

* update podfile

(cherry picked from commit 5f01f9e)

Co-authored-by: Daniel Espino García <[email protected]>
  • Loading branch information
mattermost-build and larkox authored Jun 12, 2024
1 parent 1594d5e commit 78c01a7
Show file tree
Hide file tree
Showing 32 changed files with 1,383 additions and 28 deletions.
201 changes: 201 additions & 0 deletions app/actions/remote/entry/notification.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import {ActionType} from '@constants';
import DatabaseManager from '@database/manager';
import NetworkManager from '@managers/network_manager';
import PerformanceMetricsManager from '@managers/performance_metrics_manager';
import {prepareThreadsFromReceivedPosts} from '@queries/servers/thread';
import NavigationStore from '@store/navigation_store';
import {mockApiClient} from '@test/mock_api_client';
import TestHelper from '@test/test_helper';

import {pushNotificationEntry} from './notification';

import type ServerDataOperator from '@database/operator/server_data_operator';

jest.mock('@managers/performance_metrics_manager');
jest.mock('@store/navigation_store');

const mockedNavigationStore = jest.mocked(NavigationStore);

describe('Performance metrics are set correctly', () => {
const serverUrl = 'http://www.someserverurl.com';
let operator: ServerDataOperator;
let post: Post;
beforeAll(() => {
mockApiClient.get.mockImplementation((url: string) => {
if (url.match(/\/api\/v4\/channels\/[a-z1-90-]*\/posts/)) {
return {status: 200, ok: true, data: {order: [], posts: {}}};
}
if (url.match(/\/api\/v4\/channels\/[a-z1-90-]*\/stats/)) {
return {status: 200, ok: true, data: {}};
}
if (url.match(/\/api\/v4\/posts\/[a-z1-90-]*\/thread/)) {
return {status: 200, ok: true, data: {order: [], posts: {}}};
}
console.log(`GET ${url} not registered in the mock`);
return {status: 404, ok: false};
});

mockApiClient.post.mockImplementation((url: string) => {
if (url.match(/\/api\/v4\/channels\/members\/me\/view/)) {
return {status: 200, ok: true, data: {}};
}

console.log(`POST ${url} not registered in the mock`);
return {status: 404, ok: false};
});
mockedNavigationStore.waitUntilScreenIsTop.mockImplementation(() => Promise.resolve());

// There are no problems when running the tests for this file alone without this line
// but for some reason, when running several tests together, it fails if we don't add this.
mockedNavigationStore.getScreensInStack.mockImplementation(() => []);
});
afterAll(() => {
mockApiClient.get.mockReset();
mockApiClient.post.mockReset();
});

beforeEach(async () => {
const client = await NetworkManager.createClient(serverUrl);
expect(client).toBeTruthy();
operator = (await TestHelper.setupServerDatabase(serverUrl)).operator;
await DatabaseManager.setActiveServerDatabase(serverUrl);
post = TestHelper.fakePost(TestHelper.basicChannel!.id);
await operator.handlePosts({
actionType: ActionType.POSTS.RECEIVED_NEW,
order: [post.id],
posts: [post],
prepareRecordsOnly: false,
});
const threadModels = await prepareThreadsFromReceivedPosts(operator, [post], true);
await operator.batchRecords(threadModels, 'test');
});

afterEach(async () => {
await TestHelper.tearDown();
NetworkManager.invalidateClient(serverUrl);
});

it('channel notification', async () => {
await operator.handleConfigs({
configs: [
{id: 'CollapsedThreads', value: 'default_on'},
{id: 'FeatureFlagCollapsedThreads', value: 'true'},
],
configsToDelete: [],
prepareRecordsOnly: false,
});

await pushNotificationEntry(serverUrl, {
channel_id: TestHelper.basicChannel!.id,
team_id: TestHelper.basicTeam!.id,
isCRTEnabled: false, // isCRTEnabled is not checked at this level
post_id: '', // Post ID is not checked at this level
type: '', // Type is not checked at this level
version: '', // Version is not checked at this level
});

expect(PerformanceMetricsManager.setLoadTarget).toHaveBeenCalledWith('CHANNEL');
});

it('thread notification', async () => {
const commentPost = TestHelper.fakePost(TestHelper.basicChannel!.id);
commentPost.root_id = post.id;
await operator.handlePosts({
actionType: ActionType.POSTS.RECEIVED_NEW,
order: [commentPost.id],
posts: [commentPost],
prepareRecordsOnly: false,
});
const threadModels = await prepareThreadsFromReceivedPosts(operator, [commentPost], true);
await operator.batchRecords(threadModels, 'test');
await operator.handleConfigs({
configs: [
{id: 'CollapsedThreads', value: 'default_on'},
{id: 'FeatureFlagCollapsedThreads', value: 'true'},
],
configsToDelete: [],
prepareRecordsOnly: false,
});

await pushNotificationEntry(serverUrl, {
root_id: post.id,
channel_id: TestHelper.basicChannel!.id,
team_id: TestHelper.basicTeam!.id,
isCRTEnabled: false, // isCRTEnabled is not checked at this level
post_id: '', // Post ID is not checked at this level
type: '', // Type is not checked at this level
version: '', // Version is not checked at this level
});

expect(PerformanceMetricsManager.setLoadTarget).toHaveBeenCalledWith('THREAD');
});

it('thread notification with wrong root id', async () => {
const commentPost = TestHelper.fakePost(TestHelper.basicChannel!.id);
commentPost.root_id = post.id;
await operator.handlePosts({
actionType: ActionType.POSTS.RECEIVED_NEW,
order: [commentPost.id],
posts: [commentPost],
prepareRecordsOnly: false,
});
const threadModels = await prepareThreadsFromReceivedPosts(operator, [commentPost], true);
await operator.batchRecords(threadModels, 'test');
await operator.handleConfigs({
configs: [
{id: 'CollapsedThreads', value: 'default_on'},
{id: 'FeatureFlagCollapsedThreads', value: 'true'},
],
configsToDelete: [],
prepareRecordsOnly: false,
});

await pushNotificationEntry(serverUrl, {
root_id: commentPost.id,
channel_id: TestHelper.basicChannel!.id,
team_id: TestHelper.basicTeam!.id,
isCRTEnabled: false, // isCRTEnabled is not checked at this level
post_id: '', // Post ID is not checked at this level
type: '', // Type is not checked at this level
version: '', // Version is not checked at this level
});

expect(PerformanceMetricsManager.setLoadTarget).toHaveBeenCalledWith('THREAD');
});

it('thread notification with non crt', async () => {
const commentPost = TestHelper.fakePost(TestHelper.basicChannel!.id);
commentPost.root_id = post.id;
await operator.handlePosts({
actionType: ActionType.POSTS.RECEIVED_NEW,
order: [commentPost.id],
posts: [commentPost],
prepareRecordsOnly: false,
});
const threadModels = await prepareThreadsFromReceivedPosts(operator, [commentPost], true);
await operator.batchRecords(threadModels, 'test');
await operator.handleConfigs({
configs: [
{id: 'CollapsedThreads', value: 'disabled'},
{id: 'FeatureFlagCollapsedThreads', value: 'false'},
],
configsToDelete: [],
prepareRecordsOnly: false,
});

await pushNotificationEntry(serverUrl, {
root_id: post.id,
channel_id: TestHelper.basicChannel!.id,
team_id: TestHelper.basicTeam!.id,
isCRTEnabled: false, // isCRTEnabled is not checked at this level
post_id: '', // Post ID is not checked at this level
type: '', // Type is not checked at this level
version: '', // Version is not checked at this level
});

expect(PerformanceMetricsManager.setLoadTarget).toHaveBeenCalledWith('CHANNEL');
});
});
4 changes: 4 additions & 0 deletions app/actions/remote/entry/notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {fetchMyTeam} from '@actions/remote/team';
import {fetchAndSwitchToThread} from '@actions/remote/thread';
import {getDefaultThemeByAppearance} from '@context/theme';
import DatabaseManager from '@database/manager';
import PerformanceMetricsManager from '@managers/performance_metrics_manager';
import WebsocketManager from '@managers/websocket_manager';
import {getMyChannel} from '@queries/servers/channel';
import {getPostById} from '@queries/servers/post';
Expand Down Expand Up @@ -103,13 +104,16 @@ export async function pushNotificationEntry(serverUrl: string, notification: Not
const actualRootId = post && ('root_id' in post ? post.root_id : post.rootId);

if (actualRootId) {
PerformanceMetricsManager.setLoadTarget('THREAD');
await fetchAndSwitchToThread(serverUrl, actualRootId, true);
} else if (post) {
PerformanceMetricsManager.setLoadTarget('THREAD');
await fetchAndSwitchToThread(serverUrl, rootId, true);
} else {
emitNotificationError('Post');
}
} else {
PerformanceMetricsManager.setLoadTarget('CHANNEL');
await switchToChannelById(serverUrl, channelId, teamId);
}
}
Expand Down
67 changes: 67 additions & 0 deletions app/actions/remote/performance.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import NetworkManager from '@managers/network_manager';
import {mockApiClient} from '@test/mock_api_client';

import {sendPerformanceReport} from './performance';

describe('sendPerformanceReport', () => {
const serverUrl = 'http://www.someserverurl.com';
const report: PerformanceReport = {
counters: [],
start: 1234,
end: 1235,
histograms: [
{
metric: 'metric1',
timestamp: 1234,
value: 123,
},
{
metric: 'metric1',
timestamp: 1234,
value: 124,
},
{
metric: 'metric2',
timestamp: 1234,
value: 125,
},
],
labels: {
agent: 'rnapp',
platform: 'ios',
},
version: '0.1.0',
};

beforeAll(() => {
mockApiClient.post.mockImplementation(() => ({status: 200, ok: true}));
});

afterAll(() => {
mockApiClient.post.mockReset();
});

beforeEach(async () => {
const client = await NetworkManager.createClient(serverUrl);
expect(client).toBeTruthy();
});

afterEach(async () => {
NetworkManager.invalidateClient(serverUrl);
});

it('happy path', async () => {
const {error} = await sendPerformanceReport(serverUrl, report);
expect(error).toBeFalsy();
expect(mockApiClient.post).toHaveBeenCalledWith(`${serverUrl}/api/v4/client_perf`, {body: report, headers: {}});
});

it('properly returns error', async () => {
mockApiClient.post.mockImplementationOnce(() => ({status: 404, ok: false}));
const {error} = await sendPerformanceReport(serverUrl, report);
expect(error).toBeTruthy();
});
});
20 changes: 20 additions & 0 deletions app/actions/remote/performance.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import NetworkManager from '@managers/network_manager';
import {getFullErrorMessage} from '@utils/errors';
import {logDebug} from '@utils/log';

import {forceLogoutIfNecessary} from './session';

export const sendPerformanceReport = async (serverUrl: string, report: PerformanceReport) => {
try {
const client = NetworkManager.getClient(serverUrl);
await client.sendPerformanceReport(report);
return {};
} catch (error) {
logDebug('error on sendPerformanceReport', getFullErrorMessage(error));
forceLogoutIfNecessary(serverUrl, error);
return {error};
}
};
4 changes: 2 additions & 2 deletions app/actions/remote/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type AuthorsRequest = {
error?: unknown;
}

export async function createPost(serverUrl: string, post: Partial<Post>, files: FileInfo[] = []): Promise<{data?: boolean; error?: any}> {
export async function createPost(serverUrl: string, post: Partial<Post>, files: FileInfo[] = []): Promise<{data?: boolean; error?: unknown}> {
const operator = DatabaseManager.serverDatabases[serverUrl]?.operator;
if (!operator) {
return {error: `${serverUrl} database not found`};
Expand Down Expand Up @@ -571,7 +571,7 @@ export async function fetchPostThread(serverUrl: string, postId: string, options
});
const result = processPostsFetched(data);
let posts: Model[] = [];
if (!fetchOnly) {
if (result.posts.length && !fetchOnly) {
const models: Model[] = [];
posts = await operator.handlePosts({
...result,
Expand Down
4 changes: 4 additions & 0 deletions app/client/rest/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ export default class ClientBase {
return this.getPluginRoute(Calls.PluginId);
}

getPerformanceRoute() {
return `${this.urlVersion}/client_perf`;
}

doFetch = async (url: string, options: ClientOptions, returnDataOnly = true) => {
let request;
const method = options.method?.toLowerCase();
Expand Down
8 changes: 8 additions & 0 deletions app/client/rest/general.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface ClientGeneralMix {
getChannelDataRetentionPolicies: (userId: string, page?: number, perPage?: number) => Promise<PoliciesResponse<ChannelDataRetentionPolicy>>;
getRolesByNames: (rolesNames: string[]) => Promise<Role[]>;
getRedirectLocation: (urlParam: string) => Promise<Record<string, string>>;
sendPerformanceReport: (batch: PerformanceReport) => Promise<{}>;
}

const ClientGeneral = <TBase extends Constructor<ClientBase>>(superclass: TBase) => class extends superclass {
Expand Down Expand Up @@ -111,6 +112,13 @@ const ClientGeneral = <TBase extends Constructor<ClientBase>>(superclass: TBase)
const url = `${this.getRedirectLocationRoute()}${buildQueryString({url: urlParam})}`;
return this.doFetch(url, {method: 'get'});
};

sendPerformanceReport = async (report: PerformanceReport) => {
return this.doFetch(
this.getPerformanceRoute(),
{method: 'post', body: report},
);
};
};

export default ClientGeneral;
7 changes: 6 additions & 1 deletion app/components/loading/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type LoadingProps = {
themeColor?: keyof Theme;
footerText?: string;
footerTextStyles?: TextStyle;
testID?: string;
}

const Loading = ({
Expand All @@ -22,12 +23,16 @@ const Loading = ({
themeColor,
footerText,
footerTextStyles,
testID,
}: LoadingProps) => {
const theme = useTheme();
const indicatorColor = themeColor ? theme[themeColor] : color;

return (
<View style={containerStyle}>
<View
style={containerStyle}
testID={testID}
>
<ActivityIndicator
color={indicatorColor}
size={size}
Expand Down
Loading

0 comments on commit 78c01a7

Please sign in to comment.