From b2b9d4ef66c66ad2613ed1cf5e578ee2caf5b71d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 14 May 2024 18:25:59 +0200 Subject: [PATCH 1/8] Add performance metrics to the app --- app/actions/remote/entry/notification.ts | 4 ++ app/components/post_draft/post_draft.tsx | 6 +++ .../team_list/team_item/team_item.tsx | 13 +++++- app/init/launch.ts | 5 +++ app/managers/performance_metrics_manager.ts | 41 +++++++++++++++++++ .../categories_list/categories/categories.tsx | 10 +++++ .../home/channel_list/channel_list.tsx | 5 +++ package-lock.json | 10 +++++ package.json | 4 ++ 9 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 app/managers/performance_metrics_manager.ts diff --git a/app/actions/remote/entry/notification.ts b/app/actions/remote/entry/notification.ts index 18063756477..480eb7e2374 100644 --- a/app/actions/remote/entry/notification.ts +++ b/app/actions/remote/entry/notification.ts @@ -5,6 +5,7 @@ import {fetchMyChannel, switchToChannelById} from '@actions/remote/channel'; import {fetchPostById} from '@actions/remote/post'; import {fetchMyTeam} from '@actions/remote/team'; import {fetchAndSwitchToThread} from '@actions/remote/thread'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import {getDefaultThemeByAppearance} from '@context/theme'; import DatabaseManager from '@database/manager'; import WebsocketManager from '@managers/websocket_manager'; @@ -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) { + performance_metrics_manager.setLoadTarget('THREAD'); await fetchAndSwitchToThread(serverUrl, actualRootId, true); } else if (post) { + performance_metrics_manager.setLoadTarget('THREAD'); await fetchAndSwitchToThread(serverUrl, rootId, true); } else { emitNotificationError('Post'); } } else { + performance_metrics_manager.setLoadTarget('CHANNEL'); await switchToChannelById(serverUrl, channelId, teamId); } } diff --git a/app/components/post_draft/post_draft.tsx b/app/components/post_draft/post_draft.tsx index b73d6bba04b..d60488d1499 100644 --- a/app/components/post_draft/post_draft.tsx +++ b/app/components/post_draft/post_draft.tsx @@ -6,6 +6,7 @@ import {Platform} from 'react-native'; import {KeyboardTrackingView, type KeyboardTrackingViewRef} from 'react-native-keyboard-tracking-view'; import {useSafeAreaInsets} from 'react-native-safe-area-context'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Autocomplete from '@components/autocomplete'; import {View as ViewConstants} from '@constants'; import {useServerUrl} from '@context/server'; @@ -73,6 +74,11 @@ function PostDraft({ setCursorPosition(message.length); }, [channelId, rootId]); + useEffect(() => { + performance_metrics_manager.finishLoad(rootId ? 'THREAD' : 'CHANNEL'); + performance_metrics_manager.endMetric('channelSwitch'); + }, []); + const keyboardAdjustment = (isTablet && isChannelScreen) ? KEYBOARD_TRACKING_OFFSET : 0; const insetsAdjustment = (isTablet && isChannelScreen) ? 0 : insets.bottom; const autocompletePosition = AUTOCOMPLETE_ADJUST + Platform.select({ diff --git a/app/components/team_sidebar/team_list/team_item/team_item.tsx b/app/components/team_sidebar/team_list/team_item/team_item.tsx index c3a204b26be..6cf45b1d29f 100644 --- a/app/components/team_sidebar/team_list/team_item/team_item.tsx +++ b/app/components/team_sidebar/team_list/team_item/team_item.tsx @@ -1,10 +1,11 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {useCallback} from 'react'; import {View} from 'react-native'; import {handleTeamChange} from '@actions/remote/team'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Badge from '@components/badge'; import TouchableWithFeedback from '@components/touchable_with_feedback'; import {useServerUrl} from '@context/server'; @@ -62,6 +63,14 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro const styles = getStyleSheet(theme); const serverUrl = useServerUrl(); + const onPress = useCallback(() => { + if (!team) { + return; + } + performance_metrics_manager.startMetric('teamSwitch'); + handleTeamChange(serverUrl, team.id); + }, []); + if (!team) { return null; } @@ -92,7 +101,7 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro <> handleTeamChange(serverUrl, team.id)} + onPress={onPress} type='opacity' testID={teamItemTestId} > diff --git a/app/init/launch.ts b/app/init/launch.ts index f03c6426c2e..772bcdf34a7 100644 --- a/app/init/launch.ts +++ b/app/init/launch.ts @@ -8,6 +8,7 @@ import {Notifications} from 'react-native-notifications'; import {switchToChannelById} from '@actions/remote/channel'; import {appEntry, pushNotificationEntry, upgradeEntry} from '@actions/remote/entry'; import {fetchAndSwitchToThread} from '@actions/remote/thread'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import LocalConfig from '@assets/config.json'; import {DeepLink, Events, Launch, PushNotification} from '@constants'; import DatabaseManager from '@database/manager'; @@ -178,9 +179,13 @@ const launchToHome = async (props: LaunchProps) => { const lastViewedThread = await getLastViewedThreadIdAndServer(); if (lastViewedThread && lastViewedThread.server_url === props.serverUrl && lastViewedThread.thread_id) { + performance_metrics_manager.setLoadTarget('THREAD'); fetchAndSwitchToThread(props.serverUrl!, lastViewedThread.thread_id); } else if (lastViewedChannel && lastViewedChannel.server_url === props.serverUrl && lastViewedChannel.channel_id) { + performance_metrics_manager.setLoadTarget('CHANNEL'); switchToChannelById(props.serverUrl!, lastViewedChannel.channel_id); + } else { + performance_metrics_manager.setLoadTarget('HOME'); } appEntry(props.serverUrl!); diff --git a/app/managers/performance_metrics_manager.ts b/app/managers/performance_metrics_manager.ts new file mode 100644 index 00000000000..3d7b367ac75 --- /dev/null +++ b/app/managers/performance_metrics_manager.ts @@ -0,0 +1,41 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {getTimeSinceStartup} from 'react-native-startup-time'; + +type Target = 'HOME' | 'CHANNEL' | 'THREAD' | undefined; +type MetricName = 'channelSwitch' | + 'teamSwitch'; + +class PerformanceMetricsManager { + private startTimes: {[metricName: string]: undefined | number} = {}; + private target: Target; + + public setLoadTarget(target: Target) { + this.target = target; + } + + public finishLoad(location: Target) { + if (this.target !== location) { + return; + } + + getTimeSinceStartup().then((v) => console.log(`Took ${v}`)); + } + + public startMetric(metricName: MetricName) { + this.startTimes[metricName] = Date.now(); + } + + public endMetric(metricName: MetricName) { + const startTime = this.startTimes[metricName]; + if (startTime === undefined) { + return; + } + + console.log(`CS took ${Date.now() - startTime}`); + delete this.startTimes[metricName]; + } +} + +export default new PerformanceMetricsManager(); diff --git a/app/screens/home/channel_list/categories_list/categories/categories.tsx b/app/screens/home/channel_list/categories_list/categories/categories.tsx index d7e42b9781c..b5e699e3a1a 100644 --- a/app/screens/home/channel_list/categories_list/categories/categories.tsx +++ b/app/screens/home/channel_list/categories_list/categories/categories.tsx @@ -6,6 +6,7 @@ import {useIntl} from 'react-intl'; import {FlatList, StyleSheet, View} from 'react-native'; import {switchToChannelById} from '@actions/remote/channel'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Loading from '@components/loading'; import {useServerUrl} from '@context/server'; import {useIsTablet} from '@hooks/device'; @@ -68,6 +69,7 @@ const Categories = ({ const [initiaLoad, setInitialLoad] = useState(!categoriesToShow.length); const onChannelSwitch = useCallback(async (c: Channel | ChannelModel) => { + performance_metrics_manager.startMetric('channelSwitch'); switchToChannelById(serverUrl, c.id); }, [serverUrl]); @@ -103,6 +105,14 @@ const Categories = ({ return () => clearTimeout(t); }, []); + useEffect(() => { + if (switchingTeam) { + return; + } + + performance_metrics_manager.endMetric('teamSwitch'); + }, [switchingTeam]); + if (!categories.length) { return ; } diff --git a/app/screens/home/channel_list/channel_list.tsx b/app/screens/home/channel_list/channel_list.tsx index 9baec4ae11c..a71da69cb65 100644 --- a/app/screens/home/channel_list/channel_list.tsx +++ b/app/screens/home/channel_list/channel_list.tsx @@ -10,6 +10,7 @@ import Animated, {useAnimatedStyle, withTiming} from 'react-native-reanimated'; import {type Edge, SafeAreaView, useSafeAreaInsets} from 'react-native-safe-area-context'; import {refetchCurrentUser} from '@actions/remote/user'; +import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import FloatingCallContainer from '@calls/components/floating_call_container'; import AnnouncementBanner from '@components/announcement_banner'; import ConnectionBanner from '@components/connection_banner'; @@ -169,6 +170,10 @@ const ChannelListScreen = (props: ChannelProps) => { } }, []); + useEffect(() => { + performance_metrics_manager.finishLoad('HOME'); + }, []); + return ( <> diff --git a/package-lock.json b/package-lock.json index d03c18e3f64..de2790be6f3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -86,6 +86,7 @@ "react-native-section-list-get-item-layout": "2.2.3", "react-native-shadow-2": "7.0.8", "react-native-share": "10.1.0", + "react-native-startup-time": "2.1.0", "react-native-svg": "15.1.0", "react-native-vector-icons": "10.0.3", "react-native-video": "5.2.1", @@ -17360,6 +17361,15 @@ "react-native": "*" } }, + "node_modules/react-native-startup-time": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/react-native-startup-time/-/react-native-startup-time-2.1.0.tgz", + "integrity": "sha512-cm014/ig/ddHEEiAKoxfTDxUwrs5gaoxKpAOnpMIuhW5yozLxhouDtkozD26TLQ2UcIgftowgaScPMeZnI1YYg==", + "peerDependencies": { + "react": "^16.8.3", + "react-native": ">=0.59.0" + } + }, "node_modules/react-native-svg": { "version": "15.1.0", "resolved": "https://registry.npmjs.org/react-native-svg/-/react-native-svg-15.1.0.tgz", diff --git a/package.json b/package.json index 6d476867430..3cbc167bea2 100644 --- a/package.json +++ b/package.json @@ -87,6 +87,7 @@ "react-native-section-list-get-item-layout": "2.2.3", "react-native-shadow-2": "7.0.8", "react-native-share": "10.1.0", + "react-native-startup-time": "2.1.0", "react-native-svg": "15.1.0", "react-native-vector-icons": "10.0.3", "react-native-video": "5.2.1", @@ -199,6 +200,9 @@ "updatesnapshot": "jest --updateSnapshot" }, "overrides": { + "react-native-startup-time": { + "react": "^18.2.0" + }, "@testing-library/react-hooks": { "@types/react": "^18.2.37", "react": "^18.2.0", From 0cfc907f250204418b785d2dc6d6f2d9fe1c2a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Fri, 17 May 2024 16:50:37 +0200 Subject: [PATCH 2/8] add batcher and improve handling --- app/actions/remote/entry/notification.ts | 8 +- app/actions/remote/performance.ts | 20 ++++ app/client/rest/base.ts | 4 + app/client/rest/general.ts | 8 ++ app/components/post_draft/post_draft.tsx | 6 +- .../team_list/team_item/team_item.tsx | 4 +- app/init/launch.ts | 8 +- app/managers/performance_metrics_manager.ts | 41 ------- .../performance_metrics_manager/index.ts | 68 ++++++++++++ .../performance_metrics_batcher.ts | 103 ++++++++++++++++++ .../performance_metrics_manager/types.d.ts | 23 ++++ .../categories_list/categories/categories.tsx | 6 +- .../home/channel_list/channel_list.tsx | 4 +- types/api/config.d.ts | 1 + 14 files changed, 245 insertions(+), 59 deletions(-) create mode 100644 app/actions/remote/performance.ts delete mode 100644 app/managers/performance_metrics_manager.ts create mode 100644 app/managers/performance_metrics_manager/index.ts create mode 100644 app/managers/performance_metrics_manager/performance_metrics_batcher.ts create mode 100644 app/managers/performance_metrics_manager/types.d.ts diff --git a/app/actions/remote/entry/notification.ts b/app/actions/remote/entry/notification.ts index 480eb7e2374..2876c801526 100644 --- a/app/actions/remote/entry/notification.ts +++ b/app/actions/remote/entry/notification.ts @@ -5,9 +5,9 @@ import {fetchMyChannel, switchToChannelById} from '@actions/remote/channel'; import {fetchPostById} from '@actions/remote/post'; import {fetchMyTeam} from '@actions/remote/team'; import {fetchAndSwitchToThread} from '@actions/remote/thread'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; 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'; @@ -104,16 +104,16 @@ export async function pushNotificationEntry(serverUrl: string, notification: Not const actualRootId = post && ('root_id' in post ? post.root_id : post.rootId); if (actualRootId) { - performance_metrics_manager.setLoadTarget('THREAD'); + PerformanceMetricsManager.setLoadTarget('THREAD'); await fetchAndSwitchToThread(serverUrl, actualRootId, true); } else if (post) { - performance_metrics_manager.setLoadTarget('THREAD'); + PerformanceMetricsManager.setLoadTarget('THREAD'); await fetchAndSwitchToThread(serverUrl, rootId, true); } else { emitNotificationError('Post'); } } else { - performance_metrics_manager.setLoadTarget('CHANNEL'); + PerformanceMetricsManager.setLoadTarget('CHANNEL'); await switchToChannelById(serverUrl, channelId, teamId); } } diff --git a/app/actions/remote/performance.ts b/app/actions/remote/performance.ts new file mode 100644 index 00000000000..4a68935d2d5 --- /dev/null +++ b/app/actions/remote/performance.ts @@ -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}; + } +}; diff --git a/app/client/rest/base.ts b/app/client/rest/base.ts index 343cf8f3d36..a8f803ab3b0 100644 --- a/app/client/rest/base.ts +++ b/app/client/rest/base.ts @@ -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(); diff --git a/app/client/rest/general.ts b/app/client/rest/general.ts index c84f470864c..76d85a9cf93 100644 --- a/app/client/rest/general.ts +++ b/app/client/rest/general.ts @@ -24,6 +24,7 @@ export interface ClientGeneralMix { getChannelDataRetentionPolicies: (userId: string, page?: number, perPage?: number) => Promise>; getRolesByNames: (rolesNames: string[]) => Promise; getRedirectLocation: (urlParam: string) => Promise>; + sendPerformanceReport: (batch: PerformanceReport) => Promise<{}>; } const ClientGeneral = >(superclass: TBase) => class extends superclass { @@ -111,6 +112,13 @@ const ClientGeneral = >(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; diff --git a/app/components/post_draft/post_draft.tsx b/app/components/post_draft/post_draft.tsx index d60488d1499..7c4cc1cd2c3 100644 --- a/app/components/post_draft/post_draft.tsx +++ b/app/components/post_draft/post_draft.tsx @@ -6,13 +6,13 @@ import {Platform} from 'react-native'; import {KeyboardTrackingView, type KeyboardTrackingViewRef} from 'react-native-keyboard-tracking-view'; import {useSafeAreaInsets} from 'react-native-safe-area-context'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Autocomplete from '@components/autocomplete'; import {View as ViewConstants} from '@constants'; import {useServerUrl} from '@context/server'; import {useAutocompleteDefaultAnimatedValues} from '@hooks/autocomplete'; import {useIsTablet, useKeyboardHeight} from '@hooks/device'; import {useDefaultHeaderHeight} from '@hooks/header'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import Archived from './archived'; import DraftHandler from './draft_handler'; @@ -75,8 +75,8 @@ function PostDraft({ }, [channelId, rootId]); useEffect(() => { - performance_metrics_manager.finishLoad(rootId ? 'THREAD' : 'CHANNEL'); - performance_metrics_manager.endMetric('channelSwitch'); + PerformanceMetricsManager.finishLoad(rootId ? 'THREAD' : 'CHANNEL', serverUrl); + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl); }, []); const keyboardAdjustment = (isTablet && isChannelScreen) ? KEYBOARD_TRACKING_OFFSET : 0; diff --git a/app/components/team_sidebar/team_list/team_item/team_item.tsx b/app/components/team_sidebar/team_list/team_item/team_item.tsx index 6cf45b1d29f..8e5565a30db 100644 --- a/app/components/team_sidebar/team_list/team_item/team_item.tsx +++ b/app/components/team_sidebar/team_list/team_item/team_item.tsx @@ -5,11 +5,11 @@ import React, {useCallback} from 'react'; import {View} from 'react-native'; import {handleTeamChange} from '@actions/remote/team'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Badge from '@components/badge'; import TouchableWithFeedback from '@components/touchable_with_feedback'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import {makeStyleSheetFromTheme} from '@utils/theme'; import TeamIcon from './team_icon'; @@ -67,7 +67,7 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro if (!team) { return; } - performance_metrics_manager.startMetric('teamSwitch'); + PerformanceMetricsManager.startMetric('mobile_team_switch'); handleTeamChange(serverUrl, team.id); }, []); diff --git a/app/init/launch.ts b/app/init/launch.ts index 772bcdf34a7..c9cfb365c75 100644 --- a/app/init/launch.ts +++ b/app/init/launch.ts @@ -8,11 +8,11 @@ import {Notifications} from 'react-native-notifications'; import {switchToChannelById} from '@actions/remote/channel'; import {appEntry, pushNotificationEntry, upgradeEntry} from '@actions/remote/entry'; import {fetchAndSwitchToThread} from '@actions/remote/thread'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import LocalConfig from '@assets/config.json'; import {DeepLink, Events, Launch, PushNotification} from '@constants'; import DatabaseManager from '@database/manager'; import {getActiveServerUrl, getServerCredentials, removeServerCredentials} from '@init/credentials'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import {getLastViewedChannelIdAndServer, getOnboardingViewed, getLastViewedThreadIdAndServer} from '@queries/app/global'; import {getThemeForCurrentTeam} from '@queries/servers/preference'; import {getCurrentUserId} from '@queries/servers/system'; @@ -179,13 +179,13 @@ const launchToHome = async (props: LaunchProps) => { const lastViewedThread = await getLastViewedThreadIdAndServer(); if (lastViewedThread && lastViewedThread.server_url === props.serverUrl && lastViewedThread.thread_id) { - performance_metrics_manager.setLoadTarget('THREAD'); + PerformanceMetricsManager.setLoadTarget('THREAD'); fetchAndSwitchToThread(props.serverUrl!, lastViewedThread.thread_id); } else if (lastViewedChannel && lastViewedChannel.server_url === props.serverUrl && lastViewedChannel.channel_id) { - performance_metrics_manager.setLoadTarget('CHANNEL'); + PerformanceMetricsManager.setLoadTarget('CHANNEL'); switchToChannelById(props.serverUrl!, lastViewedChannel.channel_id); } else { - performance_metrics_manager.setLoadTarget('HOME'); + PerformanceMetricsManager.setLoadTarget('HOME'); } appEntry(props.serverUrl!); diff --git a/app/managers/performance_metrics_manager.ts b/app/managers/performance_metrics_manager.ts deleted file mode 100644 index 3d7b367ac75..00000000000 --- a/app/managers/performance_metrics_manager.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {getTimeSinceStartup} from 'react-native-startup-time'; - -type Target = 'HOME' | 'CHANNEL' | 'THREAD' | undefined; -type MetricName = 'channelSwitch' | - 'teamSwitch'; - -class PerformanceMetricsManager { - private startTimes: {[metricName: string]: undefined | number} = {}; - private target: Target; - - public setLoadTarget(target: Target) { - this.target = target; - } - - public finishLoad(location: Target) { - if (this.target !== location) { - return; - } - - getTimeSinceStartup().then((v) => console.log(`Took ${v}`)); - } - - public startMetric(metricName: MetricName) { - this.startTimes[metricName] = Date.now(); - } - - public endMetric(metricName: MetricName) { - const startTime = this.startTimes[metricName]; - if (startTime === undefined) { - return; - } - - console.log(`CS took ${Date.now() - startTime}`); - delete this.startTimes[metricName]; - } -} - -export default new PerformanceMetricsManager(); diff --git a/app/managers/performance_metrics_manager/index.ts b/app/managers/performance_metrics_manager/index.ts new file mode 100644 index 00000000000..d081478534f --- /dev/null +++ b/app/managers/performance_metrics_manager/index.ts @@ -0,0 +1,68 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {getTimeSinceStartup} from 'react-native-startup-time'; + +import Batcher from './performance_metrics_batcher'; + +type Target = 'HOME' | 'CHANNEL' | 'THREAD' | undefined; +type MetricName = 'mobile_channel_switch' | + 'mobile_team_switch'; + +class PerformanceMetricsManager { + private startTimes: {[metricName: string]: undefined | number} = {}; + private target: Target; + private batchers: {[serverUrl: string]: Batcher} = {}; + + private ensureBatcher(serverUrl: string) { + if (this.batchers[serverUrl]) { + return this.batchers[serverUrl]; + } + + this.batchers[serverUrl] = new Batcher(serverUrl); + return this.batchers[serverUrl]; + } + + public setLoadTarget(target: Target) { + this.target = target; + } + + public finishLoad(location: Target, serverUrl: string) { + if (this.target !== location) { + return; + } + + getTimeSinceStartup().then((measure) => { + this.ensureBatcher(serverUrl).addToBatch({ + metric: 'mobile_load', + value: measure / 1000, + timestamp: Date.now(), + }); + }); + } + + public startMetric(metricName: MetricName) { + this.startTimes[metricName] = global.performance.now(); + } + + public endMetric(metricName: MetricName, serverUrl: string) { + const startTime = this.startTimes[metricName]; + if (startTime === undefined) { + return; + } + + const dateNow = Date.now(); + const performanceNow = global.performance.now(); + const performanceNowSkew = dateNow - performanceNow; + + this.ensureBatcher(serverUrl).addToBatch({ + metric: metricName, + value: (performanceNow - startTime) / 1000, + timestamp: Math.round(startTime + performanceNowSkew), + }); + + delete this.startTimes[metricName]; + } +} + +export default new PerformanceMetricsManager(); diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts new file mode 100644 index 00000000000..bf04060571f --- /dev/null +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts @@ -0,0 +1,103 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {AppState, Platform, type AppStateStatus} from 'react-native'; + +import {sendPerformanceReport} from '@actions/remote/performance'; +import DatabaseManager from '@database/manager'; +import {getConfigValue} from '@queries/servers/system'; +import {toMilliseconds} from '@utils/datetime'; +import {logDebug} from '@utils/log'; + +const MAX_BATCH_SIZE = 100; +const INTERVAL_TIME = toMilliseconds({seconds: 60}); + +class Batcher { + private started = false; + private batch: PerformanceReportMeasure[] = []; + private serverUrl: string; + private sendTimeout: NodeJS.Timeout | undefined; + private lastAppStateIsActive = AppState.currentState === 'active'; + + constructor(serverUrl: string) { + this.serverUrl = serverUrl; + AppState.addEventListener('change', (appState) => this.onAppStateChange(appState)); + } + + private start() { + this.started = true; + clearTimeout(this.sendTimeout); + this.sendTimeout = setTimeout(() => this.sendBatch(), INTERVAL_TIME); + } + + private async sendBatch() { + clearTimeout(this.sendTimeout); + if (this.batch.length === 0) { + return; + } + + const toSend = this.getReport(); + + // Empty the batch as soon as possible to avoid race conditions + this.batch = []; + this.started = false; + + const database = DatabaseManager.serverDatabases[this.serverUrl]?.database; + if (!database) { + return; + } + + const clientPerformanceSetting = getConfigValue(database, 'EnableClientMetrics'); + if (!clientPerformanceSetting) { + return; + } + + await sendPerformanceReport(this.serverUrl, toSend); + } + + private getReport(): PerformanceReport { + let start = this.batch[0].timestamp; + let end = this.batch[0].timestamp; + for (const measure of this.batch) { + start = Math.min(start, measure.timestamp); + end = Math.max(end, measure.timestamp); + } + if (start === end) { + end += 1; + } + + return { + version: '0.1.0', + labels: { + platform: Platform.select({ios: 'ios', default: 'android'}), + agent: 'rnapp', + }, + start, + end, + counters: [], + histograms: this.batch, + }; + } + + private onAppStateChange(appState: AppStateStatus) { + const isAppStateActive = appState === 'active'; + if (this.lastAppStateIsActive !== isAppStateActive && !isAppStateActive) { + this.sendBatch(); + } + this.lastAppStateIsActive = isAppStateActive; + } + + public addToBatch(measure: PerformanceReportMeasure) { + if (!this.started) { + this.start(); + } + + logDebug('Performance metric:', measure); + this.batch.push(measure); + if (this.batch.length >= MAX_BATCH_SIZE) { + this.sendBatch(); + } + } +} + +export default Batcher; diff --git a/app/managers/performance_metrics_manager/types.d.ts b/app/managers/performance_metrics_manager/types.d.ts new file mode 100644 index 00000000000..fabff0a3b8e --- /dev/null +++ b/app/managers/performance_metrics_manager/types.d.ts @@ -0,0 +1,23 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +type PerformanceReportMeasure = { + metric: string; + value: number; + timestamp: number; +} + +type PerformanceReport = { + version: '0.1.0'; + + labels: { + platform: PlatformLabel; + agent: 'rnapp'; + }; + + start: number; + end: number; + + counters: PerformanceReportMeasure[]; + histograms: PerformanceReportMeasure[]; +} diff --git a/app/screens/home/channel_list/categories_list/categories/categories.tsx b/app/screens/home/channel_list/categories_list/categories/categories.tsx index b5e699e3a1a..194279bb8b7 100644 --- a/app/screens/home/channel_list/categories_list/categories/categories.tsx +++ b/app/screens/home/channel_list/categories_list/categories/categories.tsx @@ -6,11 +6,11 @@ import {useIntl} from 'react-intl'; import {FlatList, StyleSheet, View} from 'react-native'; import {switchToChannelById} from '@actions/remote/channel'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import Loading from '@components/loading'; import {useServerUrl} from '@context/server'; import {useIsTablet} from '@hooks/device'; import {useTeamSwitch} from '@hooks/team_switch'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import CategoryBody from './body'; import LoadCategoriesError from './error'; @@ -69,7 +69,7 @@ const Categories = ({ const [initiaLoad, setInitialLoad] = useState(!categoriesToShow.length); const onChannelSwitch = useCallback(async (c: Channel | ChannelModel) => { - performance_metrics_manager.startMetric('channelSwitch'); + PerformanceMetricsManager.startMetric('mobile_channel_switch'); switchToChannelById(serverUrl, c.id); }, [serverUrl]); @@ -110,7 +110,7 @@ const Categories = ({ return; } - performance_metrics_manager.endMetric('teamSwitch'); + PerformanceMetricsManager.endMetric('mobile_team_switch', serverUrl); }, [switchingTeam]); if (!categories.length) { diff --git a/app/screens/home/channel_list/channel_list.tsx b/app/screens/home/channel_list/channel_list.tsx index a71da69cb65..5617908af74 100644 --- a/app/screens/home/channel_list/channel_list.tsx +++ b/app/screens/home/channel_list/channel_list.tsx @@ -10,7 +10,6 @@ import Animated, {useAnimatedStyle, withTiming} from 'react-native-reanimated'; import {type Edge, SafeAreaView, useSafeAreaInsets} from 'react-native-safe-area-context'; import {refetchCurrentUser} from '@actions/remote/user'; -import performance_metrics_manager from '@app/managers/performance_metrics_manager'; import FloatingCallContainer from '@calls/components/floating_call_container'; import AnnouncementBanner from '@components/announcement_banner'; import ConnectionBanner from '@components/connection_banner'; @@ -19,6 +18,7 @@ import {Navigation as NavigationConstants, Screens} from '@constants'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; import {useIsTablet} from '@hooks/device'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import {resetToTeams, openToS} from '@screens/navigation'; import NavigationStore from '@store/navigation_store'; import {isMainActivity} from '@utils/helpers'; @@ -171,7 +171,7 @@ const ChannelListScreen = (props: ChannelProps) => { }, []); useEffect(() => { - performance_metrics_manager.finishLoad('HOME'); + PerformanceMetricsManager.finishLoad('HOME', serverUrl); }, []); return ( diff --git a/types/api/config.d.ts b/types/api/config.d.ts index 567c53d85e6..8712e8537e8 100644 --- a/types/api/config.d.ts +++ b/types/api/config.d.ts @@ -46,6 +46,7 @@ interface ClientConfig { EnableBanner: string; EnableBotAccountCreation: string; EnableChannelViewedMessages: string; + EnableClientMetrics: string; EnableCluster: string; EnableCommands: string; EnableCompliance: string; From b3c5c1875fc8c8b3cac0df0f0882fb22ad5afd25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 22 May 2024 12:28:17 +0200 Subject: [PATCH 3/8] Add tests --- app/actions/remote/entry/notification.test.ts | 197 +++++++++++++++++ app/actions/remote/performance.test.ts | 67 ++++++ app/actions/remote/post.ts | 4 +- app/components/post_draft/post_draft.test.tsx | 59 +++++ .../team_list/team_item/team_item.test.tsx | 64 ++++++ .../team_list/team_item/team_item.tsx | 6 +- .../performance_metrics_manager/index.test.ts | 208 ++++++++++++++++++ .../performance_metrics_batcher.test.ts | 158 +++++++++++++ .../performance_metrics_batcher.ts | 4 +- .../performance_metrics_manager/test_utils.ts | 16 ++ .../categories/categories.test.tsx | 45 +++- .../home/channel_list/channel_list.test.tsx | 51 +++++ test/intl-test-helper.tsx | 17 +- test/mock_api_client.ts | 11 + test/setup.ts | 31 ++- test/test_helper.ts | 50 ++++- 16 files changed, 964 insertions(+), 24 deletions(-) create mode 100644 app/actions/remote/entry/notification.test.ts create mode 100644 app/actions/remote/performance.test.ts create mode 100644 app/components/post_draft/post_draft.test.tsx create mode 100644 app/components/team_sidebar/team_list/team_item/team_item.test.tsx create mode 100644 app/managers/performance_metrics_manager/index.test.ts create mode 100644 app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts create mode 100644 app/managers/performance_metrics_manager/test_utils.ts create mode 100644 app/screens/home/channel_list/channel_list.test.tsx create mode 100644 test/mock_api_client.ts diff --git a/app/actions/remote/entry/notification.test.ts b/app/actions/remote/entry/notification.test.ts new file mode 100644 index 00000000000..220ccaa3a33 --- /dev/null +++ b/app/actions/remote/entry/notification.test.ts @@ -0,0 +1,197 @@ +// 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()); + }); + 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'); + }); +}); diff --git a/app/actions/remote/performance.test.ts b/app/actions/remote/performance.test.ts new file mode 100644 index 00000000000..bf64faf789e --- /dev/null +++ b/app/actions/remote/performance.test.ts @@ -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(); + }); +}); diff --git a/app/actions/remote/post.ts b/app/actions/remote/post.ts index d557a7d12aa..8f80cf53e3e 100644 --- a/app/actions/remote/post.ts +++ b/app/actions/remote/post.ts @@ -52,7 +52,7 @@ type AuthorsRequest = { error?: unknown; } -export async function createPost(serverUrl: string, post: Partial, files: FileInfo[] = []): Promise<{data?: boolean; error?: any}> { +export async function createPost(serverUrl: string, post: Partial, files: FileInfo[] = []): Promise<{data?: boolean; error?: unknown}> { const operator = DatabaseManager.serverDatabases[serverUrl]?.operator; if (!operator) { return {error: `${serverUrl} database not found`}; @@ -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, diff --git a/app/components/post_draft/post_draft.test.tsx b/app/components/post_draft/post_draft.test.tsx new file mode 100644 index 00000000000..2caa3ab95ef --- /dev/null +++ b/app/components/post_draft/post_draft.test.tsx @@ -0,0 +1,59 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React, {type ComponentProps} from 'react'; + +import NetworkManager from '@managers/network_manager'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; +import {renderWithEverything} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import PostDraft from './post_draft'; + +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('@managers/performance_metrics_manager'); + +function getBaseProps(): ComponentProps { + return { + canPost: true, + channelId: '', + channelIsReadOnly: false, + containerHeight: 0, + deactivatedChannel: false, + isChannelScreen: true, + keyboardTracker: {current: null}, + accessoriesContainerID: '', + canShowPostPriority: false, + channelIsArchived: false, + }; +} + +describe('performance metrics', () => { + let database: Database; + const serverUrl = 'http://www.someserverurl.com'; + beforeEach(async () => { + const client = await NetworkManager.createClient(serverUrl); + expect(client).toBeTruthy(); + database = (await TestHelper.setupServerDatabase(serverUrl)).database; + }); + + afterEach(async () => { + await TestHelper.tearDown(); + NetworkManager.invalidateClient(serverUrl); + }); + + it('on channel', () => { + const props = getBaseProps(); + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.finishLoad).toHaveBeenCalledWith('CHANNEL', serverUrl); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledWith('mobile_channel_switch', serverUrl); + }); + it('on thread', () => { + const props = getBaseProps(); + props.rootId = 'someId'; + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.finishLoad).toHaveBeenCalledWith('THREAD', serverUrl); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledWith('mobile_channel_switch', serverUrl); + }); +}); diff --git a/app/components/team_sidebar/team_list/team_item/team_item.test.tsx b/app/components/team_sidebar/team_list/team_item/team_item.test.tsx new file mode 100644 index 00000000000..ff4195d9783 --- /dev/null +++ b/app/components/team_sidebar/team_list/team_item/team_item.test.tsx @@ -0,0 +1,64 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {render, screen, userEvent} from '@testing-library/react-native'; +import React, {type ComponentProps} from 'react'; + +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; +import {getTeamById} from '@queries/servers/team'; +import TestHelper from '@test/test_helper'; + +import TeamItem from './team_item'; + +import type TeamModel from '@typings/database/models/servers/team'; + +jest.mock('@managers/performance_metrics_manager'); + +function getBaseProps(): ComponentProps { + return { + hasUnreads: false, + mentionCount: 0, + selected: false, + }; +} + +describe('performance metrics', () => { + const serverUrl = 'http://www.someserverurl.com'; + let team: TeamModel | undefined; + beforeAll(() => { + jest.useFakeTimers({legacyFakeTimers: true}); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + beforeEach(async () => { + userEvent.setup({advanceTimers: jest.advanceTimersByTime}); + const {database} = await TestHelper.setupServerDatabase(serverUrl); + team = await getTeamById(database, TestHelper.basicTeam!.id); + }); + + afterEach(async () => { + await TestHelper.tearDown(); + }); + + it('happy path', async () => { + const baseProps = getBaseProps(); + baseProps.team = team; + render(); + const button = await screen.findByTestId(`team_sidebar.team_list.team_item.${team!.id}.not_selected`); + await userEvent.press(button); + expect(PerformanceMetricsManager.startMetric).toHaveBeenCalledWith('mobile_team_switch'); + }); + + it('do not start when the team is already selected', async () => { + const baseProps = getBaseProps(); + baseProps.team = team; + baseProps.selected = true; + render(); + const button = await screen.findByTestId(`team_sidebar.team_list.team_item.${team!.id}.selected`); + await userEvent.press(button); + expect(PerformanceMetricsManager.startMetric).not.toHaveBeenCalled(); + }); +}); diff --git a/app/components/team_sidebar/team_list/team_item/team_item.tsx b/app/components/team_sidebar/team_list/team_item/team_item.tsx index 8e5565a30db..36b3e3d74a4 100644 --- a/app/components/team_sidebar/team_list/team_item/team_item.tsx +++ b/app/components/team_sidebar/team_list/team_item/team_item.tsx @@ -67,9 +67,13 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro if (!team) { return; } + if (selected) { + return; + } + PerformanceMetricsManager.startMetric('mobile_team_switch'); handleTeamChange(serverUrl, team.id); - }, []); + }, [selected]); if (!team) { return null; diff --git a/app/managers/performance_metrics_manager/index.test.ts b/app/managers/performance_metrics_manager/index.test.ts new file mode 100644 index 00000000000..900951b5e5b --- /dev/null +++ b/app/managers/performance_metrics_manager/index.test.ts @@ -0,0 +1,208 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {mockApiClient} from '@test/mock_api_client'; +import TestHelper from '@test/test_helper'; + +import NetworkManager from '../network_manager'; + +import {getBaseReportRequest} from './test_utils'; + +import PerformanceMetricsManager from '.'; + +jest.mock('@utils/log', () => ({ + logDebug: () => '', +})); + +jest.mock('react-native-startup-time', () => ({ + getTimeSinceStartup: () => Promise.resolve(2000), +})); + +describe('load metrics', () => { + const serverUrl = 'http://www.someserverurl.com/'; + const expectedUrl = `${serverUrl}/api/v4/client_perf`; + + const measure: PerformanceReportMeasure = { + metric: 'mobile_load', + timestamp: 1577836861000, + value: 2, + }; + + beforeEach(async () => { + NetworkManager.createClient(serverUrl); + const {operator} = await TestHelper.setupServerDatabase(serverUrl); + await operator.handleConfigs({configs: [{id: 'EnableClientMetrics', value: 'true'}], configsToDelete: [], prepareRecordsOnly: false}); + jest.useFakeTimers({doNotFake: [ + 'cancelAnimationFrame', + 'cancelIdleCallback', + 'clearImmediate', + 'clearInterval', + 'clearTimeout', + 'hrtime', + 'nextTick', + 'performance', + 'queueMicrotask', + 'requestAnimationFrame', + 'requestIdleCallback', + 'setImmediate', + 'setInterval', + ]}).setSystemTime(new Date('2020-01-01')); + }); + afterEach(async () => { + jest.useRealTimers(); + NetworkManager.invalidateClient(serverUrl); + await TestHelper.tearDown(); + }); + + it('only load on target', async () => { + const expectedRequest = getBaseReportRequest(measure.timestamp, measure.timestamp + 1); + expectedRequest.body.histograms = [measure]; + + PerformanceMetricsManager.setLoadTarget('CHANNEL'); + PerformanceMetricsManager.finishLoad('HOME', serverUrl); + await TestHelper.tick(); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + PerformanceMetricsManager.finishLoad('CHANNEL', serverUrl); + await TestHelper.tick(); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl, expectedRequest); + }); +}); + +describe('other metrics', () => { + const serverUrl1 = 'http://www.someserverurl.com/'; + const expectedUrl1 = `${serverUrl1}/api/v4/client_perf`; + + const serverUrl2 = 'http://www.otherserverurl.com/'; + const expectedUrl2 = `${serverUrl2}/api/v4/client_perf`; + + const measure1: PerformanceReportMeasure = { + metric: 'mobile_channel_switch', + timestamp: 1577836800000, + value: 0.1, + }; + + const measure2: PerformanceReportMeasure = { + metric: 'mobile_team_switch', + timestamp: 1577836800050, + value: 0.15, + }; + + beforeEach(async () => { + NetworkManager.createClient(serverUrl1); + NetworkManager.createClient(serverUrl2); + const {operator: operator1} = await TestHelper.setupServerDatabase(serverUrl1); + const {operator: operator2} = await TestHelper.setupServerDatabase(serverUrl2); + await operator1.handleConfigs({configs: [{id: 'EnableClientMetrics', value: 'true'}], configsToDelete: [], prepareRecordsOnly: false}); + await operator2.handleConfigs({configs: [{id: 'EnableClientMetrics', value: 'true'}], configsToDelete: [], prepareRecordsOnly: false}); + jest.useFakeTimers({doNotFake: [ + 'cancelAnimationFrame', + 'cancelIdleCallback', + 'clearImmediate', + 'clearInterval', + 'clearTimeout', + 'hrtime', + 'nextTick', + 'queueMicrotask', + 'requestAnimationFrame', + 'requestIdleCallback', + 'setImmediate', + 'setInterval', + ]}).setSystemTime(new Date('2020-01-01')); + }); + afterEach(async () => { + jest.useRealTimers(); + NetworkManager.invalidateClient(serverUrl1); + NetworkManager.invalidateClient(serverUrl2); + await TestHelper.tearDown(); + }); + + it('do not send metrics when we dont start them', async () => { + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + }); + + it('send metric after it has been started', async () => { + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); + expectedRequest.body.histograms = [measure1]; + + PerformanceMetricsManager.startMetric('mobile_channel_switch'); + jest.advanceTimersByTime(100); + + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + await TestHelper.tick(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl1, expectedRequest); + }); + + it('a second end metric does not generate a second measure', async () => { + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); + expectedRequest.body.histograms = [measure1]; + + PerformanceMetricsManager.startMetric('mobile_channel_switch'); + jest.advanceTimersByTime(100); + + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + await TestHelper.tick(); + jest.advanceTimersByTime(100); + + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + await TestHelper.tick(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl1, expectedRequest); + }); + + it('different metrics do not interfere', async () => { + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure2.timestamp); + expectedRequest.body.histograms = [measure1, measure2]; + + PerformanceMetricsManager.startMetric('mobile_channel_switch'); + jest.advanceTimersByTime(50); + PerformanceMetricsManager.startMetric('mobile_team_switch'); + jest.advanceTimersByTime(50); + + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + await TestHelper.tick(); + jest.advanceTimersByTime(100); + PerformanceMetricsManager.endMetric('mobile_team_switch', serverUrl1); + await TestHelper.tick(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl1, expectedRequest); + }); + + it('metrics to different servers do not interfere', async () => { + const expectedRequest1 = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); + expectedRequest1.body.histograms = [measure1]; + + const expectedRequest2 = getBaseReportRequest(measure2.timestamp, measure2.timestamp + 1); + expectedRequest2.body.histograms = [measure2]; + + PerformanceMetricsManager.startMetric('mobile_channel_switch'); + jest.advanceTimersByTime(50); + PerformanceMetricsManager.startMetric('mobile_team_switch'); + jest.advanceTimersByTime(50); + + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); + await TestHelper.tick(); + jest.advanceTimersByTime(100); + PerformanceMetricsManager.endMetric('mobile_team_switch', serverUrl2); + await TestHelper.tick(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl1, expectedRequest1); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl2, expectedRequest2); + }); +}); diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts new file mode 100644 index 00000000000..b25c3357504 --- /dev/null +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts @@ -0,0 +1,158 @@ +// 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 TestHelper from '@test/test_helper'; + +import Batcher from './performance_metrics_batcher'; +import {getBaseReportRequest} from './test_utils'; + +import type ServerDataOperator from '@database/operator/server_data_operator'; + +jest.mock('@utils/log', () => ({ + logDebug: () => '', +})); + +describe('perfromance metrics batcher', () => { + const serverUrl = 'http://www.someserverurl.com'; + const expectedUrl = `${serverUrl}/api/v4/client_perf`; + + const measure1: PerformanceReportMeasure = { + metric: 'someMetric', + timestamp: 1234, + value: 1.5, + }; + let operator: ServerDataOperator; + + const measure2: PerformanceReportMeasure = { + metric: 'someOtherMetric', + timestamp: 1235, + value: 2.5, + }; + + const measure3: PerformanceReportMeasure = { + metric: 'yetAnother', + timestamp: 1236, + value: 0.5, + }; + + async function setMetricsConfig(value: string) { + await operator.handleConfigs({configs: [{id: 'EnableClientMetrics', value}], configsToDelete: [], prepareRecordsOnly: false}); + } + + beforeEach(async () => { + NetworkManager.createClient(serverUrl); + operator = (await TestHelper.setupServerDatabase(serverUrl)).operator; + await setMetricsConfig('true'); + jest.useFakeTimers({doNotFake: [ + 'Date', + 'cancelAnimationFrame', + 'cancelIdleCallback', + 'clearImmediate', + 'clearInterval', + 'clearTimeout', + 'hrtime', + 'nextTick', + 'performance', + 'queueMicrotask', + 'requestAnimationFrame', + 'requestIdleCallback', + 'setImmediate', + 'setInterval', + ]}); + }); + afterEach(async () => { + jest.useRealTimers(); + NetworkManager.invalidateClient(serverUrl); + await TestHelper.tearDown(); + }); + + it('properly send batches only after timeout', async () => { + const batcher = new Batcher(serverUrl); + + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure2.timestamp); + expectedRequest.body.histograms = [measure1, measure2]; + + batcher.addToBatch(measure1); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + + batcher.addToBatch(measure2); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl, expectedRequest); + }); + + it('properly set end after start when only one element', async () => { + const batcher = new Batcher(serverUrl); + + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); + expectedRequest.body.histograms = [measure1]; + + batcher.addToBatch(measure1); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl, expectedRequest); + }); + + it('send the batch directly after maximum batch size is reached', async () => { + const batcher = new Batcher(serverUrl); + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure2.timestamp); + for (let i = 0; i < 99; i++) { + batcher.addToBatch(measure1); + expectedRequest.body.histograms.push(measure1); + } + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + + batcher.addToBatch(measure2); + expectedRequest.body.histograms.push(measure2); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl, expectedRequest); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledTimes(1); + }); + + it('do not send batches when the config is set to false', async () => { + await setMetricsConfig('false'); + const batcher = new Batcher(serverUrl); + batcher.addToBatch(measure2); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + }); + + it('old elements do not drip into the next batch', async () => { + const batcher = new Batcher(serverUrl); + let expectedRequest = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); + expectedRequest.body.histograms = [measure1]; + + batcher.addToBatch(measure1); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenLastCalledWith(expectedUrl, expectedRequest); + + expectedRequest = getBaseReportRequest(measure2.timestamp, measure2.timestamp + 1); + expectedRequest.body.histograms = []; + for (let i = 0; i < 100; i++) { + batcher.addToBatch(measure2); + expectedRequest.body.histograms.push(measure2); + } + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenLastCalledWith(expectedUrl, expectedRequest); + + expectedRequest = getBaseReportRequest(measure3.timestamp, measure3.timestamp + 1); + expectedRequest.body.histograms = [measure3]; + + batcher.addToBatch(measure3); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenLastCalledWith(expectedUrl, expectedRequest); + }); +}); diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts index bf04060571f..900ecb332f9 100644 --- a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts @@ -47,8 +47,8 @@ class Batcher { return; } - const clientPerformanceSetting = getConfigValue(database, 'EnableClientMetrics'); - if (!clientPerformanceSetting) { + const clientPerformanceSetting = await getConfigValue(database, 'EnableClientMetrics'); + if (clientPerformanceSetting !== 'true') { return; } diff --git a/app/managers/performance_metrics_manager/test_utils.ts b/app/managers/performance_metrics_manager/test_utils.ts new file mode 100644 index 00000000000..232261704c5 --- /dev/null +++ b/app/managers/performance_metrics_manager/test_utils.ts @@ -0,0 +1,16 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +export function getBaseReportRequest(start: number, end: number): {body: PerformanceReport; headers: {}} { + return { + body: { + version: '0.1.0', + start, + end, + labels: {agent: 'rnapp', platform: 'ios'}, + histograms: [], + counters: [], + }, + headers: {}, + }; +} diff --git a/app/screens/home/channel_list/categories_list/categories/categories.test.tsx b/app/screens/home/channel_list/categories_list/categories/categories.test.tsx index 851539b13ef..3a0fb9f7a6a 100644 --- a/app/screens/home/channel_list/categories_list/categories/categories.test.tsx +++ b/app/screens/home/channel_list/categories_list/categories/categories.test.tsx @@ -2,14 +2,19 @@ // See LICENSE.txt for license information. import React from 'react'; +import {DeviceEventEmitter} from 'react-native'; -import {renderWithEverything} from '@test/intl-test-helper'; +import {Events} from '@constants'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; +import {renderWithEverything, act} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; import Categories from '.'; import type Database from '@nozbe/watermelondb/Database'; +jest.mock('@managers/performance_metrics_manager'); + describe('components/channel_list/categories', () => { let database: Database; beforeAll(async () => { @@ -17,6 +22,10 @@ describe('components/channel_list/categories', () => { database = server.database; }); + afterAll(async () => { + await TestHelper.tearDown(); + }); + it('render without error', () => { const wrapper = renderWithEverything( , @@ -26,3 +35,37 @@ describe('components/channel_list/categories', () => { expect(wrapper.toJSON()).toBeTruthy(); }); }); + +describe('performance metrics', () => { + let database: Database; + const serverUrl = 'http://www.someserverurl.com'; + beforeAll(async () => { + const server = await TestHelper.setupServerDatabase(serverUrl); + database = server.database; + }); + + afterAll(async () => { + await TestHelper.tearDown(); + }); + + it('properly send metric on load', () => { + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledWith('mobile_team_switch', serverUrl); + }); + + it('properly call again after switching teams', async () => { + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(1); + await act(async () => { + DeviceEventEmitter.emit(Events.TEAM_SWITCH, true); + await TestHelper.wait(100); + }); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(1); + await act(async () => { + DeviceEventEmitter.emit(Events.TEAM_SWITCH, false); + await TestHelper.wait(100); + }); + expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(2); + expect(PerformanceMetricsManager.endMetric).toHaveBeenLastCalledWith('mobile_team_switch', serverUrl); + }); +}); diff --git a/app/screens/home/channel_list/channel_list.test.tsx b/app/screens/home/channel_list/channel_list.test.tsx new file mode 100644 index 00000000000..b6ca4a39f81 --- /dev/null +++ b/app/screens/home/channel_list/channel_list.test.tsx @@ -0,0 +1,51 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React, {type ComponentProps} from 'react'; + +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; +import {renderWithEverything} from '@test/intl-test-helper'; +import TestHelper from '@test/test_helper'; + +import ChannelListScreen from './channel_list'; + +import type {Database} from '@nozbe/watermelondb'; + +jest.mock('@managers/performance_metrics_manager'); +jest.mock('@react-navigation/native', () => ({ + useIsFocused: () => true, + useNavigation: () => ({isFocused: () => true}), + useRoute: () => ({}), +})); + +function getBaseProps(): ComponentProps { + return { + hasChannels: true, + hasCurrentUser: true, + hasMoreThanOneTeam: true, + hasTeams: true, + isCRTEnabled: true, + isLicensed: true, + launchType: 'normal', + showIncomingCalls: true, + showToS: false, + currentUserId: 'someId', + }; +} + +describe('performance metrics', () => { + let database: Database; + const serverUrl = 'http://www.someserverurl.com'; + beforeAll(async () => { + const server = await TestHelper.setupServerDatabase(serverUrl); + database = server.database; + }); + + it('finish load on load', () => { + jest.useFakeTimers(); + const props = getBaseProps(); + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.finishLoad).toHaveBeenCalledWith('HOME', serverUrl); + jest.useRealTimers(); + }); +}); diff --git a/test/intl-test-helper.tsx b/test/intl-test-helper.tsx index b424f7a1787..159295c14ca 100644 --- a/test/intl-test-helper.tsx +++ b/test/intl-test-helper.tsx @@ -2,11 +2,12 @@ // See LICENSE.txt for license information. import {DatabaseProvider} from '@nozbe/watermelondb/react'; -import {render} from '@testing-library/react-native'; +import {render, type RenderOptions} from '@testing-library/react-native'; import React, {type ReactElement} from 'react'; import {IntlProvider} from 'react-intl'; import {SafeAreaProvider} from 'react-native-safe-area-context'; +import ServerUrlProvider from '@context/server'; import {ThemeContext, getDefaultThemeByAppearance} from '@context/theme'; import {getTranslations} from '@i18n'; @@ -48,13 +49,13 @@ export function renderWithIntlAndTheme(ui: ReactElement, {locale = 'en', ...rend return render(ui, {wrapper: Wrapper, ...renderOptions}); } -export function renderWithEverything(ui: ReactElement, {locale = 'en', database, ...renderOptions}: {locale?: string; database?: Database; renderOptions?: any} = {}) { +export function renderWithEverything(ui: ReactElement, {locale = 'en', database, serverUrl, ...renderOptions}: {locale?: string; database?: Database; serverUrl?: string; renderOptions?: RenderOptions} = {}) { function Wrapper({children}: {children: ReactElement}) { if (!database) { return null; } - return ( + const wrapper = ( ); + + if (serverUrl) { + return ( + + {wrapper} + + ); + } + + return wrapper; } return render(ui, {wrapper: Wrapper, ...renderOptions}); diff --git a/test/mock_api_client.ts b/test/mock_api_client.ts new file mode 100644 index 00000000000..4e470f42f32 --- /dev/null +++ b/test/mock_api_client.ts @@ -0,0 +1,11 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {RequestOptions} from '@mattermost/react-native-network-client'; + +export const mockApiClient = { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + get: jest.fn((url: string, options?: RequestOptions) => ({status: 200, ok: true})), + // eslint-disable-next-line @typescript-eslint/no-unused-vars + post: jest.fn((url: string, options?: RequestOptions) => ({status: 200, ok: true})), +}; diff --git a/test/setup.ts b/test/setup.ts index eb39d8ba687..4b916220125 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -9,6 +9,9 @@ import 'react-native-gesture-handler/jestSetup'; import mockSafeAreaContext from 'react-native-safe-area-context/jest/mock'; import {v4 as uuidv4} from 'uuid'; +import {mockApiClient} from './mock_api_client'; + +import type {RequestOptions} from '@mattermost/react-native-network-client'; import type {ReadDirItem, StatResult} from 'react-native-fs'; // @ts-expect-error Promise does not exists in global @@ -184,17 +187,11 @@ jest.doMock('react-native', () => { jest.mock('react-native-vector-icons', () => { const React = jest.requireActual('react'); - const PropTypes = jest.requireActual('prop-types'); class CompassIcon extends React.PureComponent { render() { return React.createElement('Icon', this.props); } } - CompassIcon.propTypes = { - name: PropTypes.string, - size: PropTypes.number, - style: PropTypes.oneOfType([PropTypes.array, PropTypes.number, PropTypes.object]), - }; CompassIcon.getImageSource = jest.fn().mockResolvedValue({}); return { createIconSet: () => CompassIcon, @@ -254,6 +251,8 @@ jest.mock('react-native-device-info', () => { hasNotch: () => true, isTablet: () => false, getApplicationName: () => 'Mattermost', + getSystemName: () => 'ios', + getSystemVersion: () => '0.0.0', }; }); @@ -365,6 +364,8 @@ jest.mock('@screens/navigation', () => ({ popToRoot: jest.fn(() => Promise.resolve()), dismissModal: jest.fn(() => Promise.resolve()), dismissAllModals: jest.fn(() => Promise.resolve()), + dismissAllModalsAndPopToScreen: jest.fn(), + dismissAllModalsAndPopToRoot: jest.fn(), dismissOverlay: jest.fn(() => Promise.resolve()), })); @@ -384,12 +385,28 @@ jest.mock('@mattermost/react-native-emm', () => ({ useManagedConfig: () => ({}), })); +jest.mock('@react-native-clipboard/clipboard', () => ({})); + +jest.mock('react-native-document-picker', () => ({})); + +jest.mock('@mattermost/react-native-network-client', () => ({ + getOrCreateAPIClient: (serverUrl: string) => ({client: { + baseUrl: serverUrl, + get: (url: string, options?: RequestOptions) => mockApiClient.get(`${serverUrl}${url}`, options), + post: (url: string, options?: RequestOptions) => mockApiClient.post(`${serverUrl}${url}`, options), + invalidate: jest.fn(), + }}), + RetryTypes: { + EXPONENTIAL_RETRY: 'exponential', + }, +})); + jest.mock('react-native-safe-area-context', () => mockSafeAreaContext); jest.mock('react-native-reanimated', () => require('react-native-reanimated/mock')); jest.mock('react-native-permissions', () => require('react-native-permissions/mock')); -declare const global: {requestAnimationFrame: (callback: any) => void}; +declare const global: {requestAnimationFrame: (callback: () => void) => void}; global.requestAnimationFrame = (callback) => { setTimeout(callback, 0); }; diff --git a/test/test_helper.ts b/test/test_helper.ts index 0962b1e7dfd..0217332632c 100644 --- a/test/test_helper.ts +++ b/test/test_helper.ts @@ -18,7 +18,6 @@ import {generateId} from '@utils/general'; import type {APIClientInterface} from '@mattermost/react-native-network-client'; -const PASSWORD = 'password1'; const DEFAULT_LOCALE = 'en'; class TestHelper { @@ -51,8 +50,8 @@ class TestHelper { this.basicRoles = null; } - setupServerDatabase = async () => { - const serverUrl = 'https://appv1.mattermost.com'; + setupServerDatabase = async (url?: string) => { + const serverUrl = url || 'https://appv1.mattermost.com'; await DatabaseManager.init([serverUrl]); const {database, operator} = DatabaseManager.serverDatabases[serverUrl]!; @@ -291,7 +290,7 @@ class TestHelper { return 'success' + this.generateId() + '@simulator.amazonses.com'; }; - fakePost = (channelId: string) => { + fakePost = (channelId: string, userId?: string): Post => { const time = Date.now(); return { @@ -301,6 +300,17 @@ class TestHelper { update_at: time, message: `Unit Test ${this.generateId()}`, type: '', + delete_at: 0, + edit_at: 0, + hashtags: '', + is_pinned: false, + metadata: {}, + original_id: '', + pending_post_id: '', + props: {}, + reply_count: 0, + root_id: '', + user_id: userId || this.generateId(), }; }; @@ -314,7 +324,7 @@ class TestHelper { }; }; - fakeTeam = () => { + fakeTeam = (): Team => { const name = this.generateId(); let inviteId = this.generateId(); if (inviteId.length > 32) { @@ -322,6 +332,7 @@ class TestHelper { } return { + id: this.generateId(), name, display_name: `Unit Test ${name}`, type: 'O' as const, @@ -334,6 +345,9 @@ class TestHelper { allow_open_invite: true, group_constrained: false, last_team_icon_update: 0, + create_at: 0, + delete_at: 0, + update_at: 0, }; }; @@ -361,11 +375,9 @@ class TestHelper { }; }; - fakeUser = () => { + fakeUser = (): UserProfile => { return { email: this.fakeEmail(), - allow_marketing: true, - password: PASSWORD, locale: DEFAULT_LOCALE, username: this.generateId(), first_name: this.generateId(), @@ -373,6 +385,27 @@ class TestHelper { create_at: Date.now(), delete_at: 0, roles: 'system_user', + auth_service: '', + id: this.generateId(), + nickname: '', + notify_props: this.fakeNotifyProps(), + position: '', + update_at: 0, + }; + }; + + fakeNotifyProps = (): UserNotifyProps => { + return { + channel: 'false', + comments: 'root', + desktop: 'default', + desktop_sound: 'false', + email: 'false', + first_name: 'false', + highlight_keys: '', + mention_keys: '', + push: 'default', + push_status: 'away', }; }; @@ -665,6 +698,7 @@ class TestHelper { }; wait = (time: number) => new Promise((resolve) => setTimeout(resolve, time)); + tick = () => new Promise((r) => setImmediate(r)); } export default new TestHelper(); From 4b93df589dd96c8fdb6520851e210fc6619c1d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 22 May 2024 12:54:01 +0200 Subject: [PATCH 4/8] Fix test --- app/actions/remote/entry/notification.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/actions/remote/entry/notification.test.ts b/app/actions/remote/entry/notification.test.ts index 220ccaa3a33..7b25eb41e46 100644 --- a/app/actions/remote/entry/notification.test.ts +++ b/app/actions/remote/entry/notification.test.ts @@ -47,6 +47,10 @@ describe('Performance metrics are set correctly', () => { 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(); From 54f9bb86506605240e951aab63970973b30451b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Mon, 27 May 2024 11:44:42 +0200 Subject: [PATCH 5/8] Address feedback --- app/components/loading/index.tsx | 7 ++++++- app/managers/performance_metrics_manager/index.test.ts | 6 +++--- app/managers/performance_metrics_manager/index.ts | 4 ++-- .../performance_metrics_batcher.ts | 2 +- .../categories_list/categories/categories.test.tsx | 10 +++++----- .../categories_list/categories/categories.tsx | 1 + test/setup.ts | 2 ++ 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/components/loading/index.tsx b/app/components/loading/index.tsx index 21b2f3169d6..f85d28bb231 100644 --- a/app/components/loading/index.tsx +++ b/app/components/loading/index.tsx @@ -13,6 +13,7 @@ type LoadingProps = { themeColor?: keyof Theme; footerText?: string; footerTextStyles?: TextStyle; + testID?: string; } const Loading = ({ @@ -22,12 +23,16 @@ const Loading = ({ themeColor, footerText, footerTextStyles, + testID, }: LoadingProps) => { const theme = useTheme(); const indicatorColor = themeColor ? theme[themeColor] : color; return ( - + { const measure: PerformanceReportMeasure = { metric: 'mobile_load', timestamp: 1577836861000, - value: 2, + value: 2000, }; beforeEach(async () => { @@ -82,13 +82,13 @@ describe('other metrics', () => { const measure1: PerformanceReportMeasure = { metric: 'mobile_channel_switch', timestamp: 1577836800000, - value: 0.1, + value: 100, }; const measure2: PerformanceReportMeasure = { metric: 'mobile_team_switch', timestamp: 1577836800050, - value: 0.15, + value: 150, }; beforeEach(async () => { diff --git a/app/managers/performance_metrics_manager/index.ts b/app/managers/performance_metrics_manager/index.ts index d081478534f..5e557960587 100644 --- a/app/managers/performance_metrics_manager/index.ts +++ b/app/managers/performance_metrics_manager/index.ts @@ -35,7 +35,7 @@ class PerformanceMetricsManager { getTimeSinceStartup().then((measure) => { this.ensureBatcher(serverUrl).addToBatch({ metric: 'mobile_load', - value: measure / 1000, + value: measure, timestamp: Date.now(), }); }); @@ -57,7 +57,7 @@ class PerformanceMetricsManager { this.ensureBatcher(serverUrl).addToBatch({ metric: metricName, - value: (performanceNow - startTime) / 1000, + value: performanceNow - startTime, timestamp: Math.round(startTime + performanceNowSkew), }); diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts index 900ecb332f9..7fccb269e6c 100644 --- a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts @@ -32,6 +32,7 @@ class Batcher { private async sendBatch() { clearTimeout(this.sendTimeout); + this.started = false; if (this.batch.length === 0) { return; } @@ -40,7 +41,6 @@ class Batcher { // Empty the batch as soon as possible to avoid race conditions this.batch = []; - this.started = false; const database = DatabaseManager.serverDatabases[this.serverUrl]?.database; if (!database) { diff --git a/app/screens/home/channel_list/categories_list/categories/categories.test.tsx b/app/screens/home/channel_list/categories_list/categories/categories.test.tsx index 3a0fb9f7a6a..bfacb314bc6 100644 --- a/app/screens/home/channel_list/categories_list/categories/categories.test.tsx +++ b/app/screens/home/channel_list/categories_list/categories/categories.test.tsx @@ -6,7 +6,7 @@ import {DeviceEventEmitter} from 'react-native'; import {Events} from '@constants'; import PerformanceMetricsManager from '@managers/performance_metrics_manager'; -import {renderWithEverything, act} from '@test/intl-test-helper'; +import {renderWithEverything, act, waitFor, screen, waitForElementToBeRemoved} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; import Categories from '.'; @@ -56,15 +56,15 @@ describe('performance metrics', () => { it('properly call again after switching teams', async () => { renderWithEverything(, {database, serverUrl}); expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(1); - await act(async () => { + act(() => { DeviceEventEmitter.emit(Events.TEAM_SWITCH, true); - await TestHelper.wait(100); }); + await waitFor(() => expect(screen.queryByTestId('categories.loading')).toBeVisible()); expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(1); - await act(async () => { + act(() => { DeviceEventEmitter.emit(Events.TEAM_SWITCH, false); - await TestHelper.wait(100); }); + await waitForElementToBeRemoved(() => screen.queryByTestId('categories.loading')); expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledTimes(2); expect(PerformanceMetricsManager.endMetric).toHaveBeenLastCalledWith('mobile_team_switch', serverUrl); }); diff --git a/app/screens/home/channel_list/categories_list/categories/categories.tsx b/app/screens/home/channel_list/categories_list/categories/categories.tsx index 194279bb8b7..6e2e19fe32b 100644 --- a/app/screens/home/channel_list/categories_list/categories/categories.tsx +++ b/app/screens/home/channel_list/categories_list/categories/categories.tsx @@ -149,6 +149,7 @@ const Categories = ({ )} diff --git a/test/setup.ts b/test/setup.ts index 4b916220125..4134ddcdefd 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -14,6 +14,8 @@ import {mockApiClient} from './mock_api_client'; import type {RequestOptions} from '@mattermost/react-native-network-client'; import type {ReadDirItem, StatResult} from 'react-native-fs'; +import '@testing-library/react-native/extend-expect'; + // @ts-expect-error Promise does not exists in global global.Promise = jest.requireActual('promise'); From a2f771ff47c91d849b9e35a12e2348204c70a0a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Thu, 30 May 2024 12:16:18 +0200 Subject: [PATCH 6/8] Address feedback --- app/components/post_draft/post_draft.tsx | 6 --- .../post/post.test.tsx} | 53 +++++++++++------- app/components/post_list/post/post.tsx | 54 +++++++++++++++++-- .../team_list/team_item/team_item.tsx | 5 +- .../performance_metrics_manager/index.test.ts | 23 ++++---- .../performance_metrics_manager/index.ts | 53 +++++++++++------- .../performance_metrics_batcher.test.ts | 37 +++++++++++++ .../performance_metrics_batcher.ts | 36 ++++++------- package-lock.json | 21 ++++---- package.json | 2 +- test/setup.ts | 9 +++- test/test_helper.ts | 8 +++ types/api/config.d.ts | 2 +- 13 files changed, 215 insertions(+), 94 deletions(-) rename app/components/{post_draft/post_draft.test.tsx => post_list/post/post.test.tsx} (53%) diff --git a/app/components/post_draft/post_draft.tsx b/app/components/post_draft/post_draft.tsx index 7c4cc1cd2c3..b73d6bba04b 100644 --- a/app/components/post_draft/post_draft.tsx +++ b/app/components/post_draft/post_draft.tsx @@ -12,7 +12,6 @@ import {useServerUrl} from '@context/server'; import {useAutocompleteDefaultAnimatedValues} from '@hooks/autocomplete'; import {useIsTablet, useKeyboardHeight} from '@hooks/device'; import {useDefaultHeaderHeight} from '@hooks/header'; -import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import Archived from './archived'; import DraftHandler from './draft_handler'; @@ -74,11 +73,6 @@ function PostDraft({ setCursorPosition(message.length); }, [channelId, rootId]); - useEffect(() => { - PerformanceMetricsManager.finishLoad(rootId ? 'THREAD' : 'CHANNEL', serverUrl); - PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl); - }, []); - const keyboardAdjustment = (isTablet && isChannelScreen) ? KEYBOARD_TRACKING_OFFSET : 0; const insetsAdjustment = (isTablet && isChannelScreen) ? 0 : insets.bottom; const autocompletePosition = AUTOCOMPLETE_ADJUST + Platform.select({ diff --git a/app/components/post_draft/post_draft.test.tsx b/app/components/post_list/post/post.test.tsx similarity index 53% rename from app/components/post_draft/post_draft.test.tsx rename to app/components/post_list/post/post.test.tsx index 2caa3ab95ef..a48637b4c72 100644 --- a/app/components/post_draft/post_draft.test.tsx +++ b/app/components/post_list/post/post.test.tsx @@ -5,37 +5,45 @@ import React, {type ComponentProps} from 'react'; import NetworkManager from '@managers/network_manager'; import PerformanceMetricsManager from '@managers/performance_metrics_manager'; +import {getPostById} from '@queries/servers/post'; import {renderWithEverything} from '@test/intl-test-helper'; import TestHelper from '@test/test_helper'; -import PostDraft from './post_draft'; +import Post from './post'; import type {Database} from '@nozbe/watermelondb'; +import type PostModel from '@typings/database/models/servers/post'; jest.mock('@managers/performance_metrics_manager'); -function getBaseProps(): ComponentProps { - return { - canPost: true, - channelId: '', - channelIsReadOnly: false, - containerHeight: 0, - deactivatedChannel: false, - isChannelScreen: true, - keyboardTracker: {current: null}, - accessoriesContainerID: '', - canShowPostPriority: false, - channelIsArchived: false, - }; -} - describe('performance metrics', () => { let database: Database; + let post: PostModel; + + function getBaseProps(): ComponentProps { + return { + appsEnabled: false, + canDelete: false, + customEmojiNames: [], + differentThreadSequence: false, + hasFiles: false, + hasReactions: false, + hasReplies: false, + highlightReplyBar: false, + isEphemeral: false, + isPostAddChannelMember: false, + isPostPriorityEnabled: false, + location: 'Channel', + post, + }; + } + const serverUrl = 'http://www.someserverurl.com'; beforeEach(async () => { const client = await NetworkManager.createClient(serverUrl); expect(client).toBeTruthy(); database = (await TestHelper.setupServerDatabase(serverUrl)).database; + post = (await getPostById(database, TestHelper.basicPost!.id))!; }); afterEach(async () => { @@ -43,16 +51,23 @@ describe('performance metrics', () => { NetworkManager.invalidateClient(serverUrl); }); + it('do not call the performance metrics if it is not the last post', () => { + const props = getBaseProps(); + props.nextPost = {} as PostModel; + renderWithEverything(, {database, serverUrl}); + expect(PerformanceMetricsManager.finishLoad).not.toHaveBeenCalled(); + expect(PerformanceMetricsManager.endMetric).not.toHaveBeenCalled(); + }); it('on channel', () => { const props = getBaseProps(); - renderWithEverything(, {database, serverUrl}); + renderWithEverything(, {database, serverUrl}); expect(PerformanceMetricsManager.finishLoad).toHaveBeenCalledWith('CHANNEL', serverUrl); expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledWith('mobile_channel_switch', serverUrl); }); it('on thread', () => { const props = getBaseProps(); - props.rootId = 'someId'; - renderWithEverything(, {database, serverUrl}); + props.location = 'Thread'; + renderWithEverything(, {database, serverUrl}); expect(PerformanceMetricsManager.finishLoad).toHaveBeenCalledWith('THREAD', serverUrl); expect(PerformanceMetricsManager.endMetric).toHaveBeenCalledWith('mobile_channel_switch', serverUrl); }); diff --git a/app/components/post_list/post/post.tsx b/app/components/post_list/post/post.tsx index 476e12b87b2..d11097f602e 100644 --- a/app/components/post_list/post/post.tsx +++ b/app/components/post_list/post/post.tsx @@ -17,6 +17,7 @@ import * as Screens from '@constants/screens'; import {useServerUrl} from '@context/server'; import {useTheme} from '@context/theme'; import {useIsTablet} from '@hooks/device'; +import PerformanceMetricsManager from '@managers/performance_metrics_manager'; import {openAsBottomSheet} from '@screens/navigation'; import {hasJumboEmojiOnly} from '@utils/emoji/helpers'; import {fromAutoResponder, isFromWebhook, isPostFailed, isPostPendingOrFailed, isSystemMessage} from '@utils/post'; @@ -60,6 +61,7 @@ type PostProps = { post: PostModel; rootId?: string; previousPost?: PostModel; + nextPost?: PostModel; hasReactions: boolean; searchPatterns?: SearchPattern[]; shouldRenderReplyButton?: boolean; @@ -109,10 +111,39 @@ const getStyleSheet = makeStyleSheetFromTheme((theme: Theme) => { }); const Post = ({ - appsEnabled, canDelete, currentUser, customEmojiNames, differentThreadSequence, hasFiles, hasReplies, highlight, highlightPinnedOrSaved = true, highlightReplyBar, - isCRTEnabled, isConsecutivePost, isEphemeral, isFirstReply, isSaved, isLastReply, isPostAcknowledgementEnabled, isPostAddChannelMember, isPostPriorityEnabled, - location, post, rootId, hasReactions, searchPatterns, shouldRenderReplyButton, skipSavedHeader, skipPinnedHeader, showAddReaction = true, style, - testID, thread, previousPost, + appsEnabled, + canDelete, + currentUser, + customEmojiNames, + differentThreadSequence, + hasFiles, + hasReplies, + highlight, + highlightPinnedOrSaved = true, + highlightReplyBar, + isCRTEnabled, + isConsecutivePost, + isEphemeral, + isFirstReply, + isSaved, + isLastReply, + isPostAcknowledgementEnabled, + isPostAddChannelMember, + isPostPriorityEnabled, + location, + post, + rootId, + hasReactions, + searchPatterns, + shouldRenderReplyButton, + skipSavedHeader, + skipPinnedHeader, + showAddReaction = true, + style, + testID, + thread, + previousPost, + nextPost, }: PostProps) => { const pressDetected = useRef(false); const intl = useIntl(); @@ -145,6 +176,8 @@ const Post = ({ return false; }, [customEmojiNames, post.message]); + const isLastPost = !nextPost; + const handlePostPress = () => { if ([Screens.SAVED_MESSAGES, Screens.MENTIONS, Screens.SEARCH, Screens.PINNED_MESSAGES].includes(location)) { showPermalink(serverUrl, '', post.id); @@ -216,6 +249,19 @@ const Post = ({ }; }, [post.id]); + useEffect(() => { + if (!isLastPost) { + return; + } + + if (location !== 'Channel' && location !== 'Thread') { + return; + } + + PerformanceMetricsManager.finishLoad(location === 'Thread' ? 'THREAD' : 'CHANNEL', serverUrl); + PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl); + }, []); + const highlightSaved = isSaved && !skipSavedHeader; const hightlightPinned = post.isPinned && !skipPinnedHeader; const itemTestID = `${testID}.${post.id}`; diff --git a/app/components/team_sidebar/team_list/team_item/team_item.tsx b/app/components/team_sidebar/team_list/team_item/team_item.tsx index 36b3e3d74a4..32e4cbc1bef 100644 --- a/app/components/team_sidebar/team_list/team_item/team_item.tsx +++ b/app/components/team_sidebar/team_list/team_item/team_item.tsx @@ -64,10 +64,7 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro const serverUrl = useServerUrl(); const onPress = useCallback(() => { - if (!team) { - return; - } - if (selected) { + if (!team || selected) { return; } diff --git a/app/managers/performance_metrics_manager/index.test.ts b/app/managers/performance_metrics_manager/index.test.ts index 64ba8866dbf..165c650649a 100644 --- a/app/managers/performance_metrics_manager/index.test.ts +++ b/app/managers/performance_metrics_manager/index.test.ts @@ -1,6 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import performance from 'react-native-performance'; + import {mockApiClient} from '@test/mock_api_client'; import TestHelper from '@test/test_helper'; @@ -10,13 +12,12 @@ import {getBaseReportRequest} from './test_utils'; import PerformanceMetricsManager from '.'; +const TEST_EPOCH = 1577836800000; jest.mock('@utils/log', () => ({ logDebug: () => '', })); -jest.mock('react-native-startup-time', () => ({ - getTimeSinceStartup: () => Promise.resolve(2000), -})); +performance.timeOrigin = TEST_EPOCH; describe('load metrics', () => { const serverUrl = 'http://www.someserverurl.com/'; @@ -24,8 +25,8 @@ describe('load metrics', () => { const measure: PerformanceReportMeasure = { metric: 'mobile_load', - timestamp: 1577836861000, - value: 2000, + timestamp: TEST_EPOCH + 61000, + value: 61000, }; beforeEach(async () => { @@ -40,13 +41,12 @@ describe('load metrics', () => { 'clearTimeout', 'hrtime', 'nextTick', - 'performance', 'queueMicrotask', 'requestAnimationFrame', 'requestIdleCallback', 'setImmediate', 'setInterval', - ]}).setSystemTime(new Date('2020-01-01')); + ]}).setSystemTime(new Date(TEST_EPOCH)); }); afterEach(async () => { jest.useRealTimers(); @@ -55,6 +55,7 @@ describe('load metrics', () => { }); it('only load on target', async () => { + performance.mark('nativeLaunchStart'); const expectedRequest = getBaseReportRequest(measure.timestamp, measure.timestamp + 1); expectedRequest.body.histograms = [measure]; @@ -81,13 +82,13 @@ describe('other metrics', () => { const measure1: PerformanceReportMeasure = { metric: 'mobile_channel_switch', - timestamp: 1577836800000, + timestamp: TEST_EPOCH + 100, value: 100, }; const measure2: PerformanceReportMeasure = { metric: 'mobile_team_switch', - timestamp: 1577836800050, + timestamp: TEST_EPOCH + 150 + 50, value: 150, }; @@ -111,7 +112,7 @@ describe('other metrics', () => { 'requestIdleCallback', 'setImmediate', 'setInterval', - ]}).setSystemTime(new Date('2020-01-01')); + ]}).setSystemTime(new Date(TEST_EPOCH)); }); afterEach(async () => { jest.useRealTimers(); @@ -120,7 +121,7 @@ describe('other metrics', () => { await TestHelper.tearDown(); }); - it('do not send metrics when we dont start them', async () => { + it('do not send metrics when we do not start them', async () => { PerformanceMetricsManager.endMetric('mobile_channel_switch', serverUrl1); jest.advanceTimersByTime(61000); diff --git a/app/managers/performance_metrics_manager/index.ts b/app/managers/performance_metrics_manager/index.ts index 5e557960587..ab69c996c04 100644 --- a/app/managers/performance_metrics_manager/index.ts +++ b/app/managers/performance_metrics_manager/index.ts @@ -1,7 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {getTimeSinceStartup} from 'react-native-startup-time'; +import {AppState, type AppStateStatus} from 'react-native'; +import performance from 'react-native-performance'; import Batcher from './performance_metrics_batcher'; @@ -10,9 +11,24 @@ type MetricName = 'mobile_channel_switch' | 'mobile_team_switch'; class PerformanceMetricsManager { - private startTimes: {[metricName: string]: undefined | number} = {}; private target: Target; private batchers: {[serverUrl: string]: Batcher} = {}; + private hasRegisteredLoad = false; + private lastAppStateIsActive = AppState.currentState === 'active'; + + constructor() { + AppState.addEventListener('change', (appState) => this.onAppStateChange(appState)); + } + + private onAppStateChange(appState: AppStateStatus) { + const isAppStateActive = appState === 'active'; + if (this.lastAppStateIsActive !== isAppStateActive && !isAppStateActive) { + for (const batcher of Object.values(this.batchers)) { + batcher.forceSend(); + } + } + this.lastAppStateIsActive = isAppStateActive; + } private ensureBatcher(serverUrl: string) { if (this.batchers[serverUrl]) { @@ -28,40 +44,41 @@ class PerformanceMetricsManager { } public finishLoad(location: Target, serverUrl: string) { - if (this.target !== location) { + if (this.target !== location || this.hasRegisteredLoad) { return; } - getTimeSinceStartup().then((measure) => { - this.ensureBatcher(serverUrl).addToBatch({ - metric: 'mobile_load', - value: measure, - timestamp: Date.now(), - }); + const measure = performance.measure('mobile_load', 'nativeLaunchStart'); + this.ensureBatcher(serverUrl).addToBatch({ + metric: 'mobile_load', + value: measure.duration, + timestamp: Date.now(), }); + performance.clearMeasures('mobile_load'); + this.hasRegisteredLoad = true; } public startMetric(metricName: MetricName) { - this.startTimes[metricName] = global.performance.now(); + performance.mark(metricName); } public endMetric(metricName: MetricName, serverUrl: string) { - const startTime = this.startTimes[metricName]; - if (startTime === undefined) { + const marks = performance.getEntriesByName(metricName, 'mark'); + if (!marks.length) { return; } - const dateNow = Date.now(); - const performanceNow = global.performance.now(); - const performanceNowSkew = dateNow - performanceNow; + const measureName = `${metricName}_measure`; + const measure = performance.measure(measureName, metricName); this.ensureBatcher(serverUrl).addToBatch({ metric: metricName, - value: performanceNow - startTime, - timestamp: Math.round(startTime + performanceNowSkew), + value: measure.duration, + timestamp: Date.now(), }); - delete this.startTimes[metricName]; + performance.clearMarks(metricName); + performance.clearMeasures(metricName); } } diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts index b25c3357504..4092909cdcb 100644 --- a/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.test.ts @@ -128,6 +128,18 @@ describe('perfromance metrics batcher', () => { expect(mockApiClient.post).not.toHaveBeenCalled(); }); + it('do not send batches when the config is set to false even on force send', async () => { + await setMetricsConfig('false'); + const batcher = new Batcher(serverUrl); + batcher.addToBatch(measure2); + batcher.forceSend(); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + }); + it('old elements do not drip into the next batch', async () => { const batcher = new Batcher(serverUrl); let expectedRequest = getBaseReportRequest(measure1.timestamp, measure1.timestamp + 1); @@ -155,4 +167,29 @@ describe('perfromance metrics batcher', () => { await TestHelper.tick(); expect(mockApiClient.post).toHaveBeenLastCalledWith(expectedUrl, expectedRequest); }); + + it('force send sends the batch, and does not get resent after the timeout', async () => { + const batcher = new Batcher(serverUrl); + + const expectedRequest = getBaseReportRequest(measure1.timestamp, measure2.timestamp); + expectedRequest.body.histograms = [measure1, measure2]; + + batcher.addToBatch(measure1); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + + batcher.addToBatch(measure2); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).toHaveBeenCalledWith(expectedUrl, expectedRequest); + + mockApiClient.post.mockClear(); + + jest.advanceTimersByTime(61000); + await TestHelper.tick(); + expect(mockApiClient.post).not.toHaveBeenCalled(); + }); }); diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts index 7fccb269e6c..ea9c30c1f11 100644 --- a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts @@ -1,7 +1,7 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import {AppState, Platform, type AppStateStatus} from 'react-native'; +import {Platform} from 'react-native'; import {sendPerformanceReport} from '@actions/remote/performance'; import DatabaseManager from '@database/manager'; @@ -13,26 +13,30 @@ const MAX_BATCH_SIZE = 100; const INTERVAL_TIME = toMilliseconds({seconds: 60}); class Batcher { - private started = false; private batch: PerformanceReportMeasure[] = []; private serverUrl: string; private sendTimeout: NodeJS.Timeout | undefined; - private lastAppStateIsActive = AppState.currentState === 'active'; constructor(serverUrl: string) { this.serverUrl = serverUrl; - AppState.addEventListener('change', (appState) => this.onAppStateChange(appState)); } - private start() { - this.started = true; + private started() { + return Boolean(this.sendTimeout); + } + + private clearTimeout() { clearTimeout(this.sendTimeout); + this.sendTimeout = undefined; + } + + private start() { + this.clearTimeout(); this.sendTimeout = setTimeout(() => this.sendBatch(), INTERVAL_TIME); } private async sendBatch() { - clearTimeout(this.sendTimeout); - this.started = false; + this.clearTimeout(); if (this.batch.length === 0) { return; } @@ -79,25 +83,21 @@ class Batcher { }; } - private onAppStateChange(appState: AppStateStatus) { - const isAppStateActive = appState === 'active'; - if (this.lastAppStateIsActive !== isAppStateActive && !isAppStateActive) { - this.sendBatch(); - } - this.lastAppStateIsActive = isAppStateActive; - } - public addToBatch(measure: PerformanceReportMeasure) { - if (!this.started) { + if (!this.started()) { this.start(); } - logDebug('Performance metric:', measure); + logDebug('Performance metric:', measure, Date.now()); this.batch.push(measure); if (this.batch.length >= MAX_BATCH_SIZE) { this.sendBatch(); } } + + public forceSend() { + this.sendBatch(); + } } export default Batcher; diff --git a/package-lock.json b/package-lock.json index de2790be6f3..47fb9eea340 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,7 +6,7 @@ "packages": { "": { "name": "mattermost-mobile", - "version": "2.16.0", + "version": "2.17.0", "hasInstallScript": true, "license": "Apache 2.0", "dependencies": { @@ -79,6 +79,7 @@ "react-native-math-view": "3.9.5", "react-native-navigation": "7.39.1", "react-native-notifications": "5.1.0", + "react-native-performance": "5.1.2", "react-native-permissions": "4.1.5", "react-native-reanimated": "3.8.1", "react-native-safe-area-context": "4.9.0", @@ -86,7 +87,6 @@ "react-native-section-list-get-item-layout": "2.2.3", "react-native-shadow-2": "7.0.8", "react-native-share": "10.1.0", - "react-native-startup-time": "2.1.0", "react-native-svg": "15.1.0", "react-native-vector-icons": "10.0.3", "react-native-video": "5.2.1", @@ -17259,6 +17259,14 @@ "react-native": "*" } }, + "node_modules/react-native-performance": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/react-native-performance/-/react-native-performance-5.1.2.tgz", + "integrity": "sha512-l5JOJphNzox9a9icL3T6O/gEqZuqWqcbejW04WPa10m0UanBdIYrNkPFl48B3ivWw3MabpjB6GiDYv7old9/fw==", + "peerDependencies": { + "react-native": "*" + } + }, "node_modules/react-native-permissions": { "version": "4.1.5", "resolved": "https://registry.npmjs.org/react-native-permissions/-/react-native-permissions-4.1.5.tgz", @@ -17361,15 +17369,6 @@ "react-native": "*" } }, - "node_modules/react-native-startup-time": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/react-native-startup-time/-/react-native-startup-time-2.1.0.tgz", - "integrity": "sha512-cm014/ig/ddHEEiAKoxfTDxUwrs5gaoxKpAOnpMIuhW5yozLxhouDtkozD26TLQ2UcIgftowgaScPMeZnI1YYg==", - "peerDependencies": { - "react": "^16.8.3", - "react-native": ">=0.59.0" - } - }, "node_modules/react-native-svg": { "version": "15.1.0", "resolved": "https://registry.npmjs.org/react-native-svg/-/react-native-svg-15.1.0.tgz", diff --git a/package.json b/package.json index 3cbc167bea2..a3e1798e146 100644 --- a/package.json +++ b/package.json @@ -80,6 +80,7 @@ "react-native-math-view": "3.9.5", "react-native-navigation": "7.39.1", "react-native-notifications": "5.1.0", + "react-native-performance": "5.1.2", "react-native-permissions": "4.1.5", "react-native-reanimated": "3.8.1", "react-native-safe-area-context": "4.9.0", @@ -87,7 +88,6 @@ "react-native-section-list-get-item-layout": "2.2.3", "react-native-shadow-2": "7.0.8", "react-native-share": "10.1.0", - "react-native-startup-time": "2.1.0", "react-native-svg": "15.1.0", "react-native-vector-icons": "10.0.3", "react-native-video": "5.2.1", diff --git a/test/setup.ts b/test/setup.ts index 4134ddcdefd..790cc2b461b 100644 --- a/test/setup.ts +++ b/test/setup.ts @@ -408,7 +408,14 @@ jest.mock('react-native-safe-area-context', () => mockSafeAreaContext); jest.mock('react-native-reanimated', () => require('react-native-reanimated/mock')); jest.mock('react-native-permissions', () => require('react-native-permissions/mock')); -declare const global: {requestAnimationFrame: (callback: () => void) => void}; +declare const global: { + requestAnimationFrame: (callback: () => void) => void; + performance: { + now: () => number; + }; +}; global.requestAnimationFrame = (callback) => { setTimeout(callback, 0); }; + +global.performance.now = () => Date.now(); diff --git a/test/test_helper.ts b/test/test_helper.ts index 0217332632c..0439a1177ab 100644 --- a/test/test_helper.ts +++ b/test/test_helper.ts @@ -10,6 +10,7 @@ import nock from 'nock'; import Config from '@assets/config.json'; import {Client} from '@client/rest'; +import {ActionType} from '@constants'; import {SYSTEM_IDENTIFIERS} from '@constants/database'; import {PUSH_PROXY_STATUS_VERIFIED} from '@constants/push_proxy'; import DatabaseManager from '@database/manager'; @@ -111,6 +112,13 @@ class TestHelper { systems: [{id: SYSTEM_IDENTIFIERS.PUSH_VERIFICATION_STATUS, value: PUSH_PROXY_STATUS_VERIFIED}], }); + await operator.handlePosts({ + actionType: ActionType.POSTS.RECEIVED_NEW, + order: [this.basicPost!.id], + posts: [this.basicPost!], + prepareRecordsOnly: false, + }); + return {database, operator}; }; diff --git a/types/api/config.d.ts b/types/api/config.d.ts index 8712e8537e8..195415e52e0 100644 --- a/types/api/config.d.ts +++ b/types/api/config.d.ts @@ -46,7 +46,7 @@ interface ClientConfig { EnableBanner: string; EnableBotAccountCreation: string; EnableChannelViewedMessages: string; - EnableClientMetrics: string; + EnableClientMetrics?: string; EnableCluster: string; EnableCommands: string; EnableCompliance: string; From 2dcf85e84601299c4cd0ec18e816ad504f20e1f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 11 Jun 2024 10:58:41 +0200 Subject: [PATCH 7/8] Address feedback --- app/components/post_list/post/index.ts | 1 + app/components/post_list/post/post.test.tsx | 3 ++- app/components/post_list/post/post.tsx | 6 ++---- .../team_sidebar/team_list/team_item/team_item.tsx | 2 +- app/managers/performance_metrics_manager/index.ts | 2 +- .../performance_metrics_batcher.ts | 2 +- package.json | 3 --- 7 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/components/post_list/post/index.ts b/app/components/post_list/post/index.ts index 02e25d45275..38d49b8a3ef 100644 --- a/app/components/post_list/post/index.ts +++ b/app/components/post_list/post/index.ts @@ -154,6 +154,7 @@ const withPost = withObservables( post: post.observe(), thread: isCRTEnabled ? observeThreadById(database, post.id) : of$(undefined), hasReactions, + isLastPost: of$(!nextPost), }; }); diff --git a/app/components/post_list/post/post.test.tsx b/app/components/post_list/post/post.test.tsx index a48637b4c72..4a8d981e998 100644 --- a/app/components/post_list/post/post.test.tsx +++ b/app/components/post_list/post/post.test.tsx @@ -35,6 +35,7 @@ describe('performance metrics', () => { isPostPriorityEnabled: false, location: 'Channel', post, + isLastPost: true, }; } @@ -53,7 +54,7 @@ describe('performance metrics', () => { it('do not call the performance metrics if it is not the last post', () => { const props = getBaseProps(); - props.nextPost = {} as PostModel; + props.isLastPost = false; renderWithEverything(, {database, serverUrl}); expect(PerformanceMetricsManager.finishLoad).not.toHaveBeenCalled(); expect(PerformanceMetricsManager.endMetric).not.toHaveBeenCalled(); diff --git a/app/components/post_list/post/post.tsx b/app/components/post_list/post/post.tsx index d11097f602e..bca30018a7c 100644 --- a/app/components/post_list/post/post.tsx +++ b/app/components/post_list/post/post.tsx @@ -61,7 +61,7 @@ type PostProps = { post: PostModel; rootId?: string; previousPost?: PostModel; - nextPost?: PostModel; + isLastPost: boolean; hasReactions: boolean; searchPatterns?: SearchPattern[]; shouldRenderReplyButton?: boolean; @@ -143,7 +143,7 @@ const Post = ({ testID, thread, previousPost, - nextPost, + isLastPost, }: PostProps) => { const pressDetected = useRef(false); const intl = useIntl(); @@ -176,8 +176,6 @@ const Post = ({ return false; }, [customEmojiNames, post.message]); - const isLastPost = !nextPost; - const handlePostPress = () => { if ([Screens.SAVED_MESSAGES, Screens.MENTIONS, Screens.SEARCH, Screens.PINNED_MESSAGES].includes(location)) { showPermalink(serverUrl, '', post.id); diff --git a/app/components/team_sidebar/team_list/team_item/team_item.tsx b/app/components/team_sidebar/team_list/team_item/team_item.tsx index 32e4cbc1bef..89d9b73fb8e 100644 --- a/app/components/team_sidebar/team_list/team_item/team_item.tsx +++ b/app/components/team_sidebar/team_list/team_item/team_item.tsx @@ -70,7 +70,7 @@ export default function TeamItem({team, hasUnreads, mentionCount, selected}: Pro PerformanceMetricsManager.startMetric('mobile_team_switch'); handleTeamChange(serverUrl, team.id); - }, [selected]); + }, [selected, team?.id, serverUrl]); if (!team) { return null; diff --git a/app/managers/performance_metrics_manager/index.ts b/app/managers/performance_metrics_manager/index.ts index ab69c996c04..db97b4ee4cd 100644 --- a/app/managers/performance_metrics_manager/index.ts +++ b/app/managers/performance_metrics_manager/index.ts @@ -78,7 +78,7 @@ class PerformanceMetricsManager { }); performance.clearMarks(metricName); - performance.clearMeasures(metricName); + performance.clearMeasures(measureName); } } diff --git a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts index ea9c30c1f11..aad2210c2cc 100644 --- a/app/managers/performance_metrics_manager/performance_metrics_batcher.ts +++ b/app/managers/performance_metrics_manager/performance_metrics_batcher.ts @@ -88,7 +88,7 @@ class Batcher { this.start(); } - logDebug('Performance metric:', measure, Date.now()); + logDebug('Performance metric:', measure); this.batch.push(measure); if (this.batch.length >= MAX_BATCH_SIZE) { this.sendBatch(); diff --git a/package.json b/package.json index 7ea99a494fd..fdfbaff6a9c 100644 --- a/package.json +++ b/package.json @@ -200,9 +200,6 @@ "updatesnapshot": "jest --updateSnapshot" }, "overrides": { - "react-native-startup-time": { - "react": "^18.2.0" - }, "@testing-library/react-hooks": { "@types/react": "^18.2.37", "react": "^18.2.0", From 2f9b7fed31a62815557f15b01f43bdd8baca9b3b Mon Sep 17 00:00:00 2001 From: Daniel Espino Date: Tue, 11 Jun 2024 11:45:59 +0200 Subject: [PATCH 8/8] update podfile --- ios/Podfile.lock | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 5f2a709a5aa..9ab4c7c5c22 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -952,6 +952,8 @@ PODS: - react-native-paste-input (0.7.1): - React-Core - Swime (= 3.0.6) + - react-native-performance (5.1.2): + - React-Core - react-native-safe-area-context (4.9.0): - React-Core - react-native-video (5.2.1): @@ -1267,6 +1269,7 @@ DEPENDENCIES: - "react-native-network-client (from `../node_modules/@mattermost/react-native-network-client`)" - react-native-notifications (from `../node_modules/react-native-notifications`) - "react-native-paste-input (from `../node_modules/@mattermost/react-native-paste-input`)" + - react-native-performance (from `../node_modules/react-native-performance`) - react-native-safe-area-context (from `../node_modules/react-native-safe-area-context`) - react-native-video (from `../node_modules/react-native-video`) - react-native-webrtc (from `../node_modules/react-native-webrtc`) @@ -1424,6 +1427,8 @@ EXTERNAL SOURCES: :path: "../node_modules/react-native-notifications" react-native-paste-input: :path: "../node_modules/@mattermost/react-native-paste-input" + react-native-performance: + :path: "../node_modules/react-native-performance" react-native-safe-area-context: :path: "../node_modules/react-native-safe-area-context" react-native-video: @@ -1581,6 +1586,7 @@ SPEC CHECKSUMS: react-native-network-client: a7e0e465f0de5ea75cef5c557df0d9dc0adbf6a9 react-native-notifications: 4601a5a8db4ced6ae7cfc43b44d35fe437ac50c4 react-native-paste-input: d2136a8269eb8ad57d81407ee2b8a646f738a694 + react-native-performance: ff93f8af3b2ee9519fd7879896aa9b8b8272691d react-native-safe-area-context: b97eb6f9e3b7f437806c2ce5983f479f8eb5de4b react-native-video: c26780b224543c62d5e1b2a7244a5cd1b50e8253 react-native-webrtc: 255a1172fd31525b952b36aef7b8e9a41de325e5