[monorail] Add issue detail SPA ts_mon metrics.
We want to measure two metrics:
* Time from Navigation Start to Comment List, for when the user
is navigating into the SPA from outside. ("from outside app")
* Time from Click to Comment List, for when the user
is navigating within the SPA. ("within app")
These metrics do not include the non-JS work: style, layout, paint, composite,
which may be ~10-100 ms based on the number of comments.
For marking the "Click", it's hard to keep track of all the links that navigate
to other issues, so instead I've used the start of the issue detail page load.
This is ~1-10 ms after the click.
Marking when the comment list is rendered is surprisingly difficult. I've used
MrIssueDetails.updated() because there is only one on the issue detail page.
The number and order of updates vary based on the order of network responses, so
I've added some filters to pinpoint only the cases we care about. These filters
may or may not prove to be brittle in the future. lit-element updates as also
asynchronous, so I recursively collect all the child nodes and wait for all of
them to finish updating.
Bug: monorail:5565
Change-Id: Id6a77833974563f9cd6b65034a92af590a64746e
Reviewed-on: https://chromium-review.googlesource.com/c/infra/infra/+/1650135
Reviewed-by: Tiffany Zhang <zhangtiff@chromium.org>
Reviewed-by: Jeff Carpenter <jeffcarp@chromium.org>
Commit-Queue: Dave Tu <dtu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#23694}
diff --git a/appengine/monorail/framework/ts_mon_js.py b/appengine/monorail/framework/ts_mon_js.py
index 0435c7e..7188c91 100644
--- a/appengine/monorail/framework/ts_mon_js.py
+++ b/appengine/monorail/framework/ts_mon_js.py
@@ -50,8 +50,17 @@
), field_spec=STANDARD_FIELDS + [ts_mon.IntegerField('date_range')])
# Page load metrics.
+ISSUE_COMMENTS_LOAD_EXTRA_FIELDS = [
+ ts_mon.StringField('template_name'),
+ ts_mon.BooleanField('full_app_load'),
+]
+ISSUE_COMMENTS_LOAD_LATENCY_METRIC = ts_mon.CumulativeDistributionMetric(
+ 'monorail/frontend/issue_comments_load_latency', (
+ 'Time from navigation or click to issue comments loaded.'
+ ), field_spec=STANDARD_FIELDS + ISSUE_COMMENTS_LOAD_EXTRA_FIELDS,
+ units=ts_mon.MetricsDataUnits.MILLISECONDS)
DOM_CONTENT_LOADED_EXTRA_FIELDS = [
- ts_mon.StringField('template_name')]
+ ts_mon.StringField('template_name')]
DOM_CONTENT_LOADED_METRIC = ts_mon.CumulativeDistributionMetric(
'frontend/dom_content_loaded', (
'domContentLoaded performance timing.'
@@ -68,6 +77,7 @@
ISSUE_UPDATE_LATENCY_METRIC,
AUTOCOMPLETE_POPULATE_LATENCY_METRIC,
CHARTS_SWITCH_DATE_RANGE_METRIC,
+ ISSUE_COMMENTS_LOAD_LATENCY_METRIC,
DOM_CONTENT_LOADED_METRIC])
def xsrf_is_valid(self, body):
diff --git a/appengine/monorail/static_src/elements/issue-detail/mr-issue-details/mr-issue-details.js b/appengine/monorail/static_src/elements/issue-detail/mr-issue-details/mr-issue-details.js
index 8b65675..3dab8b3 100644
--- a/appengine/monorail/static_src/elements/issue-detail/mr-issue-details/mr-issue-details.js
+++ b/appengine/monorail/static_src/elements/issue-detail/mr-issue-details/mr-issue-details.js
@@ -4,8 +4,9 @@
import {LitElement, html, css} from 'lit-element';
-import {connectStore} from 'elements/reducers/base.js';
+import {store, connectStore} from 'elements/reducers/base.js';
import * as issue from 'elements/reducers/issue.js';
+import * as ui from 'elements/reducers/ui.js';
import 'elements/framework/mr-comment-content/mr-description.js';
import '../mr-comment-list/mr-comment-list.js';
import '../metadata/mr-edit-metadata/mr-edit-issue.js';
@@ -43,11 +44,21 @@
}
render() {
+ let comments = [];
+ let descriptions = [];
+
+ if (this.commentsByApproval && this.commentsByApproval.has('')) {
+ // Comments without an approval go into the main view.
+ const mainComments = this.commentsByApproval.get('');
+ comments = mainComments.slice(1);
+ descriptions = commentListToDescriptionList(mainComments);
+ }
+
return html`
- <mr-description .descriptionList=${this._descriptions}></mr-description>
+ <mr-description .descriptionList=${descriptions}></mr-description>
<mr-comment-list
headingLevel="2"
- .comments=${this.comments}
+ .comments=${comments}
.commentsShownCount=${this.commentsShownCount}
>
<mr-edit-issue></mr-edit-issue>
@@ -57,26 +68,83 @@
static get properties() {
return {
- comments: {type: Array},
+ commentsByApproval: {type: Object},
commentsShownCount: {type: Number},
- _descriptions: {type: Array},
};
}
constructor() {
super();
- this.comments = [];
- this._descriptions = [];
+ this.commentsByApproval = new Map();
}
stateChanged(state) {
- const commentsByApproval = issue.commentsByApprovalName(state);
- if (commentsByApproval && commentsByApproval.has('')) {
- // Comments without an approval go into the main view.
- const comments = commentsByApproval.get('');
- this.comments = comments.slice(1);
- this._descriptions = commentListToDescriptionList(comments);
+ this.commentsByApproval = issue.commentsByApprovalName(state);
+ }
+
+ updated(changedProperties) {
+ super.updated(changedProperties);
+ this._measureCommentLoadTime(changedProperties);
+ }
+
+ async _measureCommentLoadTime(changedProperties) {
+ if (!changedProperties.has('commentsByApproval')) {
+ return;
}
+ if (!this.commentsByApproval || this.commentsByApproval.size === 0) {
+ // For cold loads, if the GetIssue call returns before ListComments,
+ // commentsByApproval is initially set to an empty Map. Filter that out.
+ return;
+ }
+ const fullAppLoad = ui.navigationCount(store.getState()) === 1;
+ if (!(fullAppLoad || changedProperties.get('commentsByApproval'))) {
+ // For hot loads, the previous issue data is still in the Redux store, so
+ // the first update sets the comments to the previous issue's comments.
+ // We need to wait for the following update.
+ return;
+ }
+ const startMark = fullAppLoad ? undefined : 'start load issue detail page';
+ if (startMark && !performance.getEntriesByName(startMark).length) {
+ // Modifying the issue template, description, comments, or attachments
+ // triggers a comment update. We only want to include full issue loads.
+ return;
+ }
+
+ await Promise.all(_subtreeUpdateComplete(this));
+
+ const endMark = 'finish load issue detail comments';
+ performance.mark(endMark);
+
+ const measurementType = fullAppLoad ? 'from outside app' : 'within app';
+ const measurementName = `load issue detail page (${measurementType})`;
+ performance.measure(measurementName, startMark, endMark);
+
+ const measurement =
+ performance.getEntriesByName(measurementName)[0].duration;
+ window.getTSMonClient().recordIssueCommentsLoadTiming(
+ measurement, fullAppLoad);
+
+ // Be sure to clear this mark even on full page navigations.
+ performance.clearMarks('start load issue detail page');
+ performance.clearMarks(endMark);
+ performance.clearMeasures(measurementName);
}
}
+
+/**
+ * Recursively traverses all shadow DOMs in an element subtree and returns an
+ * Array containing the updateComplete Promises for all lit-element nodes.
+ * @param {!LitElement} element
+ * @return {!Array<Promise<Boolean>>}
+ */
+function _subtreeUpdateComplete(element) {
+ if (!(element.shadowRoot && element.updateComplete)) {
+ return [];
+ }
+
+ const children = element.shadowRoot.querySelectorAll('*');
+ const childPromises = Array.from(children, (e) => _subtreeUpdateComplete(e));
+ return [element.updateComplete].concat(...childPromises);
+}
+
customElements.define('mr-issue-details', MrIssueDetails);
diff --git a/appengine/monorail/static_src/elements/mr-app/mr-app.js b/appengine/monorail/static_src/elements/mr-app/mr-app.js
index d57bc20..d153ba6 100644
--- a/appengine/monorail/static_src/elements/mr-app/mr-app.js
+++ b/appengine/monorail/static_src/elements/mr-app/mr-app.js
@@ -124,6 +124,9 @@
this.queryParams = ctx.query;
this._currentContext = ctx;
+ // Increment the count of navigations in the Redux store.
+ store.dispatch(ui.incrementNavigationCount());
+
next();
});
page('/p/:project/issues/detail', this._loadIssuePage.bind(this));
@@ -150,6 +153,9 @@
}
async _loadIssuePage(ctx, next) {
+ performance.clearMarks('start load issue detail page');
+ performance.mark('start load issue detail page');
+
store.dispatch(issue.setIssueRef(
Number.parseInt(ctx.query.id), ctx.params.project));
diff --git a/appengine/monorail/static_src/elements/reducers/ui.js b/appengine/monorail/static_src/elements/reducers/ui.js
index 71c7a0d..2ed0b9d 100644
--- a/appengine/monorail/static_src/elements/reducers/ui.js
+++ b/appengine/monorail/static_src/elements/reducers/ui.js
@@ -6,18 +6,24 @@
import {createReducer} from './redux-helpers.js';
// Actions
+const INCREMENT_NAVIGATION_COUNT = 'INCREMENT_NAVIGATION_COUNT';
const REPORT_DIRTY_FORM = 'REPORT_DIRTY_FORM';
const CLEAR_DIRTY_FORMS = 'CLEAR_DIRTY_FORMS';
const SET_FOCUS_ID = 'SET_FOCUS_ID';
/* State Shape
{
+ navigationCount: Number,
dirtyForms: Array,
focusId: String,
}
*/
// Reducers
+const navigationCountReducer = createReducer(0, {
+ [INCREMENT_NAVIGATION_COUNT]: (state) => state + 1,
+});
+
const dirtyFormsReducer = createReducer([], {
[REPORT_DIRTY_FORM]: (state, action) => {
const newState = [...state];
@@ -37,6 +43,8 @@
});
export const reducer = combineReducers({
+ // Count of "page" navigations.
+ navigationCount: navigationCountReducer,
// Forms to be checked for user changes before leaving the page.
dirtyForms: dirtyFormsReducer,
// The ID of the element to be focused, as given by the hash part of the URL.
@@ -44,10 +52,15 @@
});
// Selectors
+export const navigationCount = (state) => state.ui.navigationCount;
export const dirtyForms = (state) => state.ui.dirtyForms;
export const focusId = (state) => state.ui.focusId;
// Action Creators
+export const incrementNavigationCount = () => {
+ return {type: INCREMENT_NAVIGATION_COUNT};
+};
+
export const reportDirtyForm = (name, isDirty) => {
return {type: REPORT_DIRTY_FORM, name, isDirty};
};
diff --git a/appengine/monorail/static_src/monitoring/monorail-ts-mon.js b/appengine/monorail/static_src/monitoring/monorail-ts-mon.js
index de1203f..17c32cb 100644
--- a/appengine/monorail/static_src/monitoring/monorail-ts-mon.js
+++ b/appengine/monorail/static_src/monitoring/monorail-ts-mon.js
@@ -77,6 +77,18 @@
]))
);
+ this.issueCommentsLoadMetric = this.cumulativeDistribution(
+ 'monorail/frontend/issue_comments_load_latency',
+ 'Time from navigation or click to issue comments loaded.',
+ null, (new Map([
+ ['client_id', TSMonClient.stringField('client_id')],
+ ['host_name', TSMonClient.stringField('host_name')],
+ ['template_name', TSMonClient.stringField('template_name')],
+ ['document_visible', TSMonClient.boolField('document_visible')],
+ ['full_app_load', TSMonClient.boolField('full_app_load')],
+ ]))
+ );
+
this.pageLoadMetric = this.cumulativeDistribution(
'frontend/dom_content_loaded',
'domContentLoaded performance timing.',
@@ -155,6 +167,17 @@
}
}
+ recordIssueCommentsLoadTiming(value, fullAppLoad) {
+ const metricFields = new Map([
+ ['client_id', this.clientId],
+ ['host_name', window.CS_env.app_version],
+ ['template_name', PAGE_TYPES.ISSUE_DETAIL_SPA],
+ ['document_visible', MonorailTSMon.isPageVisible()],
+ ['full_app_load', fullAppLoad],
+ ]);
+ this.issueCommentsLoadMetric.add(value, metricFields);
+ }
+
recordIssueDetailTiming(maxThresholdMs=PAGE_LOAD_MAX_THRESHOLD) {
this.recordPageLoadTiming(PAGE_TYPES.ISSUE_DETAIL, maxThresholdMs);
}
