MM-57881 Add metric for the amount of time it takes for the RHS to load a thread (#26970)

* Assure most code dispatches selectPost to open RHS to a thread

* Add measurement of the time it takes to open the RHS

* Reduce unnecessary re-rendering of RhsThread

* Never use Jest's fake timers for the performance API

* Make constants for performance marks and measures

* Add missed constants
Этот коммит содержится в:
Harrison Healey
2024-05-09 13:30:36 -04:00
коммит произвёл GitHub
родитель de3c7ad544
Коммит 099f704d4f
11 изменённых файлов: 72 добавлений и 53 удалений

Просмотреть файл

@@ -19,6 +19,9 @@ const config = {
'utils/src/**/*.{js,jsx,ts,tsx}', 'utils/src/**/*.{js,jsx,ts,tsx}',
], ],
coverageReporters: ['lcov', 'text-summary'], coverageReporters: ['lcov', 'text-summary'],
fakeTimers: {
doNotFake: ['performance'],
},
moduleNameMapper: { moduleNameMapper: {
'^@mattermost/(components)$': '<rootDir>/../platform/$1/src', '^@mattermost/(components)$': '<rootDir>/../platform/$1/src',
'^@mattermost/(client)$': '<rootDir>/../platform/$1/src', '^@mattermost/(client)$': '<rootDir>/../platform/$1/src',

Просмотреть файл

@@ -31,23 +31,17 @@ import {getSearchTerms, getRhsState, getPluggableId, getFilesSearchExtFilter, ge
import {SidebarSize} from 'components/resizable_sidebar/constants'; import {SidebarSize} from 'components/resizable_sidebar/constants';
import {ActionTypes, RHSStates, Constants} from 'utils/constants'; import {ActionTypes, RHSStates, Constants} from 'utils/constants';
import {Mark, Measure, measureAndReport} from 'utils/performance_telemetry';
import {getBrowserUtcOffset, getUtcOffsetForTimeZone} from 'utils/timezone'; import {getBrowserUtcOffset, getUtcOffsetForTimeZone} from 'utils/timezone';
import type {GlobalState} from 'types/store'; import type {GlobalState} from 'types/store';
import type {RhsState} from 'types/store/rhs'; import type {RhsState} from 'types/store/rhs';
function selectPostFromRightHandSideSearchWithPreviousState(post: Post, previousRhsState?: RhsState): ActionFuncAsync<boolean, GlobalState> { function selectPostWithPreviousState(post: Post, previousRhsState?: RhsState): ActionFunc<boolean, GlobalState> {
return async (dispatch, getState) => { return (dispatch, getState) => {
const postRootId = post.root_id || post.id;
const state = getState(); const state = getState();
dispatch({ dispatch(selectPost(post, previousRhsState || getRhsState(state)));
type: ActionTypes.SELECT_POST,
postId: postRootId,
channelId: post.channel_id,
previousRhsState: previousRhsState || getRhsState(state),
timestamp: Date.now(),
});
return {data: true}; return {data: true};
}; };
@@ -118,7 +112,7 @@ export function goBack(): ActionFuncAsync<boolean, GlobalState> {
} }
export function selectPostFromRightHandSideSearch(post: Post) { export function selectPostFromRightHandSideSearch(post: Post) {
return selectPostFromRightHandSideSearchWithPreviousState(post); return selectPostWithPreviousState(post);
} }
export function selectPostFromRightHandSideSearchByPostId(postId: string): ActionFuncAsync<boolean, GlobalState> { export function selectPostFromRightHandSideSearchByPostId(postId: string): ActionFuncAsync<boolean, GlobalState> {
@@ -520,11 +514,14 @@ export function toggleRhsExpanded() {
}; };
} }
export function selectPost(post: Post) { export function selectPost(post: Post, previousRhsState?: RhsState) {
performance.mark(Mark.PostSelected);
return { return {
type: ActionTypes.SELECT_POST, type: ActionTypes.SELECT_POST,
postId: post.root_id || post.id, postId: post.root_id || post.id,
channelId: post.channel_id, channelId: post.channel_id,
previousRhsState,
timestamp: Date.now(), timestamp: Date.now(),
}; };
} }
@@ -614,7 +611,7 @@ export function openAtPrevious(previous: any): ThunkActionFunc<unknown, GlobalSt
} }
if (previous.selectedPostId) { if (previous.selectedPostId) {
const post = getPost(getState(), previous.selectedPostId); const post = getPost(getState(), previous.selectedPostId);
return post ? dispatch(selectPostFromRightHandSideSearchWithPreviousState(post, previous.previousRhsState)) : dispatch(openRHSSearch()); return post ? dispatch(selectPostWithPreviousState(post, previous.previousRhsState)) : dispatch(openRHSSearch());
} }
if (previous.selectedPostCardId) { if (previous.selectedPostCardId) {
const post = getPost(getState(), previous.selectedPostCardId); const post = getPost(getState(), previous.selectedPostCardId);
@@ -648,3 +645,11 @@ export function setEditChannelMembers(active: boolean) {
active, active,
}; };
} }
export function measureRhsOpened() {
return () => {
measureAndReport(Measure.RhsLoad, Mark.PostSelected, undefined, true);
performance.clearMarks(Mark.PostSelected);
};
}

Просмотреть файл

@@ -13,7 +13,7 @@ import LoadingScreen from 'components/loading_screen';
import VirtPostList from 'components/post_view/post_list_virtualized/post_list_virtualized'; import VirtPostList from 'components/post_view/post_list_virtualized/post_list_virtualized';
import {PostRequestTypes} from 'utils/constants'; import {PostRequestTypes} from 'utils/constants';
import {measureAndReport} from 'utils/performance_telemetry'; import {Mark, Measure, measureAndReport} from 'utils/performance_telemetry';
import {getOldestPostId, getLatestPostId} from 'utils/post_utils'; import {getOldestPostId, getLatestPostId} from 'utils/post_utils';
const MAX_NUMBER_OF_AUTO_RETRIES = 3; const MAX_NUMBER_OF_AUTO_RETRIES = 3;
@@ -22,18 +22,18 @@ export const MAX_EXTRA_PAGES_LOADED = 10;
// Measures the time between channel or team switch started and the post list component rendering posts. // Measures the time between channel or team switch started and the post list component rendering posts.
// Set "fresh" to true when the posts have not been loaded before. // Set "fresh" to true when the posts have not been loaded before.
function markAndMeasureChannelSwitchEnd(fresh = false) { function markAndMeasureChannelSwitchEnd(fresh = false) {
mark('PostList#component'); mark(Mark.PostListLoaded);
// Send new performance metrics to server // Send new performance metrics to server
const channelSwitch = measureAndReport('channel_switch', 'SidebarChannelLink#click', 'PostList#component', true); const channelSwitch = measureAndReport(Measure.ChannelSwitch, Mark.ChannelLinkClicked, Mark.PostListLoaded, true);
const teamSwitch = measureAndReport('team_switch', 'TeamLink#click', 'PostList#component', true); const teamSwitch = measureAndReport(Measure.TeamSwitch, Mark.TeamLinkClicked, Mark.PostListLoaded, true);
// Send old performance metrics to Rudder // Send old performance metrics to Rudder
if (shouldTrackPerformance()) { if (shouldTrackPerformance()) {
if (channelSwitch) { if (channelSwitch) {
const requestCount1 = countRequestsBetween('SidebarChannelLink#click', 'PostList#component'); const requestCount1 = countRequestsBetween(Mark.ChannelLinkClicked, Mark.PostListLoaded);
trackEvent('performance', 'channel_switch', { trackEvent('performance', Measure.ChannelSwitch, {
duration: Math.round(channelSwitch.duration), duration: Math.round(channelSwitch.duration),
fresh, fresh,
requestCount: requestCount1, requestCount: requestCount1,
@@ -41,9 +41,9 @@ function markAndMeasureChannelSwitchEnd(fresh = false) {
} }
if (teamSwitch) { if (teamSwitch) {
const requestCount2 = countRequestsBetween('TeamLink#click', 'PostList#component'); const requestCount2 = countRequestsBetween(Mark.TeamLinkClicked, Mark.PostListLoaded);
trackEvent('performance', 'team_switch', { trackEvent('performance', Measure.TeamSwitch, {
duration: Math.round(teamSwitch.duration), duration: Math.round(teamSwitch.duration),
fresh, fresh,
requestCount: requestCount2, requestCount: requestCount2,
@@ -53,9 +53,9 @@ function markAndMeasureChannelSwitchEnd(fresh = false) {
// Clear all the metrics so that we can differentiate between a channel and team switch next time this is called // Clear all the metrics so that we can differentiate between a channel and team switch next time this is called
clearMarks([ clearMarks([
'SidebarChannelLink#click', Mark.ChannelLinkClicked,
'TeamLink#click', Mark.TeamLinkClicked,
'PostList#component', Mark.PostListLoaded,
]); ]);
} }

Просмотреть файл

@@ -3,9 +3,6 @@
import {connect} from 'react-redux'; import {connect} from 'react-redux';
import type {Post} from '@mattermost/types/posts';
import {makeGetPostsForThread} from 'mattermost-redux/selectors/entities/posts';
import {getCurrentTeam} from 'mattermost-redux/selectors/entities/teams'; import {getCurrentTeam} from 'mattermost-redux/selectors/entities/teams';
import {getSelectedChannel, getSelectedPost} from 'selectors/rhs'; import {getSelectedChannel, getSelectedPost} from 'selectors/rhs';
@@ -14,24 +11,15 @@ import type {GlobalState} from 'types/store';
import RhsThread from './rhs_thread'; import RhsThread from './rhs_thread';
function makeMapStateToProps() { function mapStateToProps(state: GlobalState) {
const getPostsForThread = makeGetPostsForThread(); const selected = getSelectedPost(state);
const channel = getSelectedChannel(state);
const currentTeam = getCurrentTeam(state);
return function mapStateToProps(state: GlobalState) { return {
const selected = getSelectedPost(state); selected,
const channel = getSelectedChannel(state); channel,
const currentTeam = getCurrentTeam(state); currentTeam,
let posts: Post[] = [];
if (selected) {
posts = getPostsForThread(state, selected.id);
}
return {
selected,
channel,
posts,
currentTeam,
};
}; };
} }
export default connect(makeMapStateToProps)(RhsThread); export default connect(mapStateToProps)(RhsThread);

Просмотреть файл

@@ -17,7 +17,6 @@ import type {FakePost, RhsState} from 'types/store/rhs';
type Props = { type Props = {
currentTeam?: Team; currentTeam?: Team;
posts: Post[];
channel?: Channel; channel?: Channel;
selected: Post | FakePost; selected: Post | FakePost;
previousRhsState?: RhsState; previousRhsState?: RhsState;
@@ -26,7 +25,6 @@ type Props = {
const RhsThread = ({ const RhsThread = ({
currentTeam, currentTeam,
channel, channel,
posts,
selected, selected,
previousRhsState, previousRhsState,
}: Props) => { }: Props) => {
@@ -39,7 +37,7 @@ const RhsThread = ({
} }
}, [currentTeam, channel]); }, [currentTeam, channel]);
if (posts == null || selected == null || !channel) { if (selected == null || !channel) {
return ( return (
<div/> <div/>
); );

Просмотреть файл

@@ -18,6 +18,7 @@ import Pluggable from 'plugins/pluggable';
import Constants, {RHSStates} from 'utils/constants'; import Constants, {RHSStates} from 'utils/constants';
import {wrapEmojis} from 'utils/emoji_utils'; import {wrapEmojis} from 'utils/emoji_utils';
import {cmdOrCtrlPressed} from 'utils/keyboard'; import {cmdOrCtrlPressed} from 'utils/keyboard';
import {Mark} from 'utils/performance_telemetry';
import {localizeMessage} from 'utils/utils'; import {localizeMessage} from 'utils/utils';
import type {RhsState} from 'types/store/rhs'; import type {RhsState} from 'types/store/rhs';
@@ -135,7 +136,7 @@ export default class SidebarChannelLink extends React.PureComponent<Props, State
removeTooltipLink = (): void => this.gmItemRef.current?.removeAttribute?.('aria-describedby'); removeTooltipLink = (): void => this.gmItemRef.current?.removeAttribute?.('aria-describedby');
handleChannelClick = (event: React.MouseEvent<HTMLAnchorElement>): void => { handleChannelClick = (event: React.MouseEvent<HTMLAnchorElement>): void => {
mark('SidebarChannelLink#click'); mark(Mark.ChannelLinkClicked);
this.handleSelectChannel(event); this.handleSelectChannel(event);
if (this.props.rhsOpen && this.props.rhsState === RHSStates.EDIT_HISTORY) { if (this.props.rhsOpen && this.props.rhsState === RHSStates.EDIT_HISTORY) {

Просмотреть файл

@@ -13,6 +13,8 @@ import TeamIcon from 'components/widgets/team_icon/team_icon';
import WithTooltip from 'components/with_tooltip'; import WithTooltip from 'components/with_tooltip';
import {ShortcutKeys} from 'components/with_tooltip/shortcut'; import {ShortcutKeys} from 'components/with_tooltip/shortcut';
import {Mark} from 'utils/performance_telemetry';
const messages = defineMessages({ const messages = defineMessages({
nameUndefined: { nameUndefined: {
id: 'team.button.name_undefined', id: 'team.button.name_undefined',
@@ -59,7 +61,7 @@ export default function TeamButton({
const {formatMessage} = useIntl(); const {formatMessage} = useIntl();
const handleSwitch = useCallback((e: React.MouseEvent) => { const handleSwitch = useCallback((e: React.MouseEvent) => {
mark('TeamLink#click'); mark(Mark.TeamLinkClicked);
e.preventDefault(); e.preventDefault();
switchTeam(url); switchTeam(url);

Просмотреть файл

@@ -10,6 +10,7 @@ import {getCurrentUserId} from 'mattermost-redux/selectors/entities/common';
import {getPost} from 'mattermost-redux/selectors/entities/posts'; import {getPost} from 'mattermost-redux/selectors/entities/posts';
import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences'; import {isCollapsedThreadsEnabled} from 'mattermost-redux/selectors/entities/preferences';
import {measureRhsOpened} from 'actions/views/rhs';
import {getIsMobileView} from 'selectors/views/browser'; import {getIsMobileView} from 'selectors/views/browser';
import {makePrepareReplyIdsForThreadViewer, makeGetThreadLastViewedAt} from 'selectors/views/threads'; import {makePrepareReplyIdsForThreadViewer, makeGetThreadLastViewedAt} from 'selectors/views/threads';
@@ -58,4 +59,8 @@ function makeMapStateToProps() {
}; };
} }
export default connect(makeMapStateToProps)(ThreadViewerVirtualized); const mapDispatchToProps = {
measureRhsOpened,
};
export default connect(makeMapStateToProps, mapDispatchToProps)(ThreadViewerVirtualized);

Просмотреть файл

@@ -39,6 +39,7 @@ function getBasePropsAndState(): [Props, DeepPartial<GlobalState>] {
isMobileView: false, isMobileView: false,
isThreadView: false, isThreadView: false,
newMessagesSeparatorActions: [], newMessagesSeparatorActions: [],
measureRhsOpened: jest.fn(),
}; };
const state: DeepPartial<GlobalState> = { const state: DeepPartial<GlobalState> = {

Просмотреть файл

@@ -41,6 +41,7 @@ type Props = {
isThreadView: boolean; isThreadView: boolean;
newMessagesSeparatorActions: PluginComponent[]; newMessagesSeparatorActions: PluginComponent[];
inputPlaceholder?: string; inputPlaceholder?: string;
measureRhsOpened: () => void;
} }
type State = { type State = {
@@ -122,6 +123,8 @@ class ThreadViewerVirtualized extends PureComponent<Props, State> {
componentDidMount() { componentDidMount() {
this.mounted = true; this.mounted = true;
this.props.measureRhsOpened();
} }
componentWillUnmount() { componentWillUnmount() {

Просмотреть файл

@@ -1,6 +1,19 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information. // See LICENSE.txt for license information.
export const enum Mark {
ChannelLinkClicked = 'SidebarChannelLink#click',
PostListLoaded = 'PostList#component',
PostSelected = 'PostList#postSelected',
TeamLinkClicked = 'TeamLink#click',
}
export const enum Measure {
ChannelSwitch = 'channel_switch',
RhsLoad = 'rhs_load',
TeamSwitch = 'team_switch',
}
export function markAndReport(name: string): PerformanceMark { export function markAndReport(name: string): PerformanceMark {
return performance.mark(name, { return performance.mark(name, {
detail: { detail: {
@@ -11,12 +24,12 @@ export function markAndReport(name: string): PerformanceMark {
/** /**
* Measures the duration between two performance marks, schedules it to be reported to the server, and returns the * Measures the duration between two performance marks, schedules it to be reported to the server, and returns the
* PerformanceMeasure created by doing this. * PerformanceMeasure created by doing this. If endMark is omitted, the measure will measure the duration until now.
* *
* If either the start or end mark does not exist, undefined will be returned and, if canFail is false, an error * If either the start or end mark does not exist, undefined will be returned and, if canFail is false, an error
* will be logged. * will be logged.
*/ */
export function measureAndReport(measureName: string, startMark: string, endMark: string, canFail = false): PerformanceMeasure | undefined { export function measureAndReport(measureName: string, startMark: string, endMark: string | undefined, canFail = false): PerformanceMeasure | undefined {
const options: PerformanceMeasureOptions = { const options: PerformanceMeasureOptions = {
start: startMark, start: startMark,
end: endMark, end: endMark,