Skip to content

Commit

Permalink
Chore: Add preload metric to Document viewers (#907)
Browse files Browse the repository at this point in the history
  • Loading branch information
JustinHoldstock committed Feb 4, 2019
1 parent a18002b commit f4956aa
Show file tree
Hide file tree
Showing 8 changed files with 186 additions and 21 deletions.
4 changes: 3 additions & 1 deletion src/lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ export const PREVIEW_METRIC = 'preview_metric';
// Milestone events for loading performance
export const LOAD_METRIC = {
previewLoadEvent: 'load', // Event name for preview_metric events related to loading times.
previewPreloadEvent: 'preload', // Event name for preview_metrics based on preload times.
fileInfoTime: 'file_info_time', // Round trip time from file info request to received file info.
convertTime: 'convert_time', // Time it took from receiving file info to being able to request the rep.
downloadResponseTime: 'download_response_time', // Time it took for TTFB when requesting a rep.
fullDocumentLoadTime: 'full_document_load_time' // How long it took to load the document so it could be previewed.
fullDocumentLoadTime: 'full_document_load_time', // How long it took to load the document so it could be previewed.
preloadTime: 'preload_time' // How long it takes to preload the document.
};

export const DURATION_METRIC = 'preview_duration_metric';
Expand Down
3 changes: 2 additions & 1 deletion src/lib/viewers/BaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ class BaseViewer extends EventEmitter {
* @return {void}
*/
startLoadTimer() {
const tag = Timer.createTag(this.options.file.id, LOAD_METRIC.fullDocumentLoadTime);
const { file } = this.options;
const tag = Timer.createTag(file.id, LOAD_METRIC.fullDocumentLoadTime);
Timer.start(tag);
}

Expand Down
52 changes: 51 additions & 1 deletion src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import {
} from '../../util';
import { ICON_PRINT_CHECKMARK } from '../../icons/icons';
import { JS, PRELOAD_JS, CSS } from './docAssets';
import { ERROR_CODE, VIEWER_EVENT } from '../../events';
import { ERROR_CODE, VIEWER_EVENT, LOAD_METRIC } from '../../events';
import Timer from '../../Timer';

const CURRENT_PAGE_MAP_KEY = 'doc-current-page-map';
const DEFAULT_SCALE_DELTA = 1.1;
Expand Down Expand Up @@ -254,6 +255,7 @@ class DocBaseViewer extends BaseViewer {

const { url_template: template } = preloadRep.content;
const preloadUrlWithAuth = this.createContentUrlWithAuthParams(template);
this.startPreloadTimer();
this.preloader.showPreload(preloadUrlWithAuth, this.containerEl);
}

Expand Down Expand Up @@ -669,6 +671,54 @@ class DocBaseViewer extends BaseViewer {
super.resize();
}

/**
* Starts timer for preload event
*
* @protected
* @return {void}
*/
startPreloadTimer() {
const { file } = this.options;
const tag = Timer.createTag(file.id, LOAD_METRIC.preloadTime);
Timer.start(tag);
}

/**
* Stop and report time to preload document
*
* @protected
* @return {void}
*/
stopPreloadTimer() {
const { file } = this.options;
const tag = Timer.createTag(file.id, LOAD_METRIC.preloadTime);
const time = Timer.get(tag);

if (!time || !time.start) {
return;
}

Timer.stop(tag);
this.emitMetric({
name: LOAD_METRIC.previewPreloadEvent,
data: time.elapsed
});
Timer.reset(tag);
}

/**
* Callback for preload event, from preloader.
*
* @protected
* @return {void}
*/
onPreload() {
const { logger } = this.options;
logger.setPreloaded();
this.stopPreloadTimer();
this.resetLoadTimeout(); // Some content is visible - reset load timeout
}

//--------------------------------------------------------------------------
// Private
//--------------------------------------------------------------------------
Expand Down
5 changes: 1 addition & 4 deletions src/lib/viewers/doc/DocumentViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ class DocumentViewer extends DocBaseViewer {

// Set up preloader
this.preloader = new DocPreloader(this.previewUI);
this.preloader.addListener('preload', () => {
this.options.logger.setPreloaded();
this.resetLoadTimeout(); // Some content is visible - reset load timeout
});
this.preloader.addListener('preload', this.onPreload.bind(this));
}

/**
Expand Down
5 changes: 1 addition & 4 deletions src/lib/viewers/doc/PresentationViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ class PresentationViewer extends DocBaseViewer {

// Set up preloader
this.preloader = new PresentationPreloader(this.previewUI);
this.preloader.addListener('preload', () => {
this.options.logger.setPreloaded();
this.resetLoadTimeout(); // Some content is visible - reset load timeout
});
this.preloader.addListener('preload', this.onPreload.bind(this));
}

/**
Expand Down
128 changes: 126 additions & 2 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ import {
QUERY_PARAM_ENCODING,
ENCODING_TYPES
} from '../../../constants';

import { ICON_PRINT_CHECKMARK } from '../../../icons/icons';
import { VIEWER_EVENT } from '../../../events';
import { VIEWER_EVENT, LOAD_METRIC } from '../../../events';
import Timer from '../../../Timer';

const LOAD_TIMEOUT_MS = 180000; // 3 min timeout
const PRINT_TIMEOUT_MS = 1000; // Wait 1s before trying to print
Expand Down Expand Up @@ -397,6 +397,31 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {

docBase.showPreload();
});

it('should start preload timer for metrics', () => {
const preloadUrl = 'someUrl';
docBase.options.file = {};
sandbox.stub(docBase, 'getCachedPage').returns(1);
sandbox.stub(file, 'getRepresentation').returns({
content: {
url_template: ''
},
status: {
state: STATUS_SUCCESS
}
});
sandbox
.stub(docBase, 'getViewerOption')
.withArgs('preload')
.returns(true);
sandbox.stub(docBase, 'createContentUrlWithAuthParams').returns(preloadUrl);

const startPreloadTimerStub = sandbox.stub(docBase, 'startPreloadTimer');

docBase.showPreload();

expect(startPreloadTimerStub).to.be.called;
});
});

describe('hidePreload', () => {
Expand Down Expand Up @@ -1089,6 +1114,105 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
});
});

describe('startPreloadTimer()', () => {
afterEach(() => {
Timer.reset();
});

it('should create a tag and start a timer related to preload for the file being loaded', () => {
const id = '12345';
const tag = Timer.createTag(id, LOAD_METRIC.preloadTime);
docBase.options.file = {
id
};

const startStub = sandbox.stub(Timer, 'start');
docBase.startPreloadTimer();

expect(startStub).to.be.calledWith(tag);
});
});

describe('stopPreloadTimer()', () => {
const id = '123456';
const tag = Timer.createTag(id, LOAD_METRIC.preloadTime);
beforeEach(() => {
docBase.options.file = {
id
};
});

afterEach(() => {
Timer.reset();
});

it('should do nothing if preload timer was not started for that file', () => {
const stopStub = sandbox.stub(Timer, 'stop');
docBase.stopPreloadTimer();

expect(stopStub).to.not.be.called;
});

it('should stop and reset the timer for the file preload event', () => {
const stopStub = sandbox.stub(Timer, 'stop');
const resetStub = sandbox.stub(Timer, 'reset');
Timer.start(tag);

docBase.stopPreloadTimer();

expect(stopStub).to.be.calledWith(tag);
expect(resetStub).to.be.calledWith(tag);
});

it('should emit a preload event for metrics logging', () => {
const elapsed = 100;
const preloadTime = {
start: 1,
end: 101,
elapsed
};
sandbox.stub(Timer, 'get').returns(preloadTime);
const metricStub = sandbox.stub(docBase, 'emitMetric');

docBase.stopPreloadTimer();

expect(metricStub).to.be.calledWith({
name: LOAD_METRIC.previewPreloadEvent,
data: elapsed
});
});
});

describe('onPreload()', () => {
let logger;
beforeEach(() => {
logger = {
setPreloaded: sandbox.stub()
};
docBase.options.logger = logger;
});

it('should invoke "setPreloaded" on logger for legacy metrics preload calculation', () => {
docBase.onPreload();

expect(logger.setPreloaded).to.be.called;
});

it('should stop preload timer for that file', () => {
const stopStub = sandbox.stub(docBase, 'stopPreloadTimer');
docBase.onPreload();

expect(stopStub).to.be.called;
});

it('should reset load timeout to prevent preview timeout', () => {
const resetStub = sandbox.stub(docBase, 'resetLoadTimeout');
docBase.onPreload();

expect(resetStub).to.be.called;
});
});

describe('setupPdfjs()', () => {
beforeEach(() => {
stubs.urlCreator = sandbox.stub(util, 'createAssetUrlCreator').returns(() => {
Expand Down
5 changes: 1 addition & 4 deletions src/lib/viewers/doc/__tests__/DocumentViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,14 @@ describe('lib/viewers/doc/DocumentViewer', () => {
expect(doc.preloader).to.be.instanceof(DocPreloader);
});

it('should set logger to be preloaded and reset load timeout when preload event is received', () => {
it('should invoke onPreload callback', () => {
doc.options.logger = {
setPreloaded: sandbox.stub()
};
stubs.setPreloaded = doc.options.logger.setPreloaded;
stubs.resetLoadTimeout = sandbox.stub(doc, 'resetLoadTimeout');

doc.preloader.emit('preload');

expect(stubs.setPreloaded).to.be.called;
expect(stubs.resetLoadTimeout).to.be.called;
});
});

Expand Down
5 changes: 1 addition & 4 deletions src/lib/viewers/doc/__tests__/PresentationViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,14 @@ describe('lib/viewers/doc/PresentationViewer', () => {
expect(presentation.preloader).to.be.instanceof(PresentationPreloader);
});

it('should set logger to be preloaded and reset load timeout when preload event is received', () => {
it('should invoke onPreload callback', () => {
presentation.options.logger = {
setPreloaded: sandbox.stub()
};
stubs.setPreloaded = presentation.options.logger.setPreloaded;
stubs.resetLoadTimeout = sandbox.stub(presentation, 'resetLoadTimeout');

presentation.preloader.emit('preload');

expect(stubs.setPreloaded).to.be.called;
expect(stubs.resetLoadTimeout).to.be.called;
});
});

Expand Down

0 comments on commit f4956aa

Please sign in to comment.