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

Add performance metrics to the app #7953

Merged
merged 10 commits into from
Jun 12, 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
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
Loading