Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore(metrics): add new timer for full preview load time tracking #922

Merged
merged 5 commits into from
Feb 12, 2019
Merged

Chore(metrics): add new timer for full preview load time tracking #922

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f442cf8
Chore(metrics): add new timer for full preview load time tracking
Feb 12, 2019
31d60b8
Chore(metrics): fullDocumentLoadTime -> contentLoadTime
Feb 12, 2019
56c2730
Chore(metrics): preview_load metrics tag to LOAD_METRICS event object
Feb 12, 2019
6a3881d
Chore(metrics): update tests
Feb 12, 2019
1461cfa
Merge branch 'master' into update-total-preview-load-time
JustinHoldstock Feb 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 27 additions & 20 deletions src/lib/Preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const LOG_RETRY_TIMEOUT_MS = 500; // retry interval for logging preview event
const LOG_RETRY_COUNT = 3; // number of times to retry logging preview event
const MS_IN_S = 1000; // ms in a sec
const SUPPORT_URL = 'https://support.box.com';
const PREVIEW_LOAD_TIME_TAG = 'preview_loading'; // Tag for timing how long it takes to load preview

// All preview assets are relative to preview.js. Here we create a location
// object that mimics the window location object and points to where
Expand Down Expand Up @@ -753,6 +754,10 @@ class Preview extends EventEmitter {
);
}

// Start the preview duration timer when the user starts to perceive preview's load
const tag = Timer.createTag(this.file.id, PREVIEW_LOAD_TIME_TAG);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about this metric if reload is called with skipServerUpdate = true? That code path seems to bypass load

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For us, our most important metrics are going to be related to our network response times, and served content times. If it's coming from a "reload" most of the content will already be in the browser cache, which we don't care so much about.

Timer.start(tag);

// If file version ID is specified, increment retry count if it matches current file version ID
if (fileVersionId) {
if (fileVersionId === currentFileVersionId) {
Expand Down Expand Up @@ -1245,8 +1250,10 @@ class Preview extends EventEmitter {
*/
finishLoading(data = {}) {
if (this.file && this.file.id) {
const tag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime);
Timer.stop(tag);
const contentLoadTag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime);
JustinHoldstock marked this conversation as resolved.
Show resolved Hide resolved
Timer.stop(contentLoadTag);
const previewLoadTag = Timer.createTag(this.file.id, PREVIEW_LOAD_TIME_TAG);
Timer.stop(previewLoadTag);
}

// Log now that loading is finished
Expand Down Expand Up @@ -1530,34 +1537,34 @@ class Preview extends EventEmitter {
return;
}

const infoTag = Timer.createTag(this.file.id, LOAD_METRIC.fileInfoTime);
const convertTag = Timer.createTag(this.file.id, LOAD_METRIC.convertTime);
const downloadTag = Timer.createTag(this.file.id, LOAD_METRIC.downloadResponseTime);
const fullLoadTag = Timer.createTag(this.file.id, LOAD_METRIC.fullDocumentLoadTime);
const { id } = this.file;

const infoTag = Timer.createTag(id, LOAD_METRIC.fileInfoTime);
const convertTag = Timer.createTag(id, LOAD_METRIC.convertTime);
const downloadTag = Timer.createTag(id, LOAD_METRIC.downloadResponseTime);
const contentLoadTag = Timer.createTag(id, LOAD_METRIC.fullDocumentLoadTime);
const previewLoadTag = Timer.createTag(id, PREVIEW_LOAD_TIME_TAG);
JustinHoldstock marked this conversation as resolved.
Show resolved Hide resolved

const timerList = [
Timer.get(infoTag) || {},
Timer.get(convertTag) || {},
Timer.get(downloadTag) || {},
Timer.get(fullLoadTag) || {}
];
const times = timerList.map((timer) => parseInt(timer.elapsed, 10) || 0);
const total = times.reduce((acc, current) => acc + current);
const infoTime = Timer.get(infoTag) || {};
const convertTime = Timer.get(convertTag) || {};
const downloadTime = Timer.get(downloadTag) || {};
const contentLoadTime = Timer.get(contentLoadTag) || {};
const previewLoadTime = Timer.get(previewLoadTag) || {};

const event = {
encoding,
event_name: LOAD_METRIC.previewLoadEvent,
value: total, // Sum of all available load times.
[LOAD_METRIC.fileInfoTime]: times[0],
[LOAD_METRIC.convertTime]: times[1],
[LOAD_METRIC.downloadResponseTime]: times[2],
[LOAD_METRIC.fullDocumentLoadTime]: times[3],
value: previewLoadTime.elapsed || 0,
[LOAD_METRIC.fileInfoTime]: infoTime.elapsed || 0,
[LOAD_METRIC.convertTime]: convertTime.elapsed || 0,
[LOAD_METRIC.downloadResponseTime]: downloadTime.elapsed || 0,
[LOAD_METRIC.fullDocumentLoadTime]: contentLoadTime.elapsed || 0,
...this.createLogEvent()
};

this.emit(PREVIEW_METRIC, event);

Timer.reset([infoTag, convertTag, downloadTag, fullLoadTag]);
Timer.reset([infoTag, convertTag, downloadTag, contentLoadTag, previewLoadTag]);
JustinHoldstock marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down