From f0b3d8a26a5a806d7cb21768b3d1ff62f27d62fa Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Fri, 21 May 2021 13:32:07 +0200 Subject: [PATCH] Merge pull request #4076 from overleaf/jpa-events-split-test [misc] submit events and prepare roll-out for pdf caching w/ split test GitOrigin-RevId: a7b7af65e1adf5bf003b65d96f1641a343b4b09c --- .../src/Features/Project/ProjectController.js | 22 ++++++- services/web/app/views/project/editor.pug | 1 + .../js/ide/pdf/controllers/PdfController.js | 52 +++------------- .../js/ide/pdf/controllers/PdfJsMetrics.js | 60 +++++++++++++++++++ .../js/infrastructure/event-tracking.js | 6 ++ services/web/frontend/js/serviceWorker.js | 2 + .../src/Project/ProjectControllerTests.js | 4 ++ 7 files changed, 99 insertions(+), 48 deletions(-) create mode 100644 services/web/frontend/js/ide/pdf/controllers/PdfJsMetrics.js diff --git a/services/web/app/src/Features/Project/ProjectController.js b/services/web/app/src/Features/Project/ProjectController.js index dfb466ab2a..858c2c2c0d 100644 --- a/services/web/app/src/Features/Project/ProjectController.js +++ b/services/web/app/src/Features/Project/ProjectController.js @@ -37,6 +37,7 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle const UserController = require('../User/UserController') const AnalyticsManager = require('../Analytics/AnalyticsManager') const Modules = require('../../infrastructure/Modules') +const { getTestSegmentation } = require('../SplitTests/SplitTestHandler') const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI') const _ssoAvailable = (affiliation, session, linkedInstitutionIds) => { @@ -801,6 +802,22 @@ const ProjectController = { } } + const trackPdfDownload = + Settings.enablePdfCaching && + (user.alphaProgram || + (user.betaProgram && + getTestSegmentation(userId, 'track_pdf_download').variant === + 'enabled')) + const enablePdfCaching = + Settings.enablePdfCaching && + shouldDisplayFeature( + 'enable_pdf_caching', + user.alphaProgram || + (user.betaProgram && + getTestSegmentation(userId, 'enable_pdf_caching') + .variant === 'enabled') + ) + res.render('project/editor', { title: project.name, priority_title: true, @@ -878,9 +895,8 @@ const ProjectController = { ), showNewBinaryFileUI: shouldDisplayFeature('new_binary_file'), showSymbolPalette: shouldDisplayFeature('symbol_palette'), - enablePdfCaching: - Settings.enablePdfCaching && - shouldDisplayFeature('enable_pdf_caching', user.alphaProgram), + trackPdfDownload, + enablePdfCaching, }) timer.done() } diff --git a/services/web/app/views/project/editor.pug b/services/web/app/views/project/editor.pug index 1a9b3b3d78..fc05f46c5d 100644 --- a/services/web/app/views/project/editor.pug +++ b/services/web/app/views/project/editor.pug @@ -188,6 +188,7 @@ block append meta meta(name="ol-logsUISubvariant" content=logsUISubvariant) meta(name="ol-showSymbolPalette" data-type="boolean" content=showSymbolPalette) meta(name="ol-enablePdfCaching" data-type="boolean" content=enablePdfCaching) + meta(name="ol-trackPdfDownload" data-type="boolean" content=trackPdfDownload) - var fileActionI18n = ['edited', 'renamed', 'created', 'deleted'].reduce((acc, i) => {acc[i] = translate('file_action_' + i); return acc}, {}) meta(name="ol-fileActionI18n" data-type="json" content=fileActionI18n) diff --git a/services/web/frontend/js/ide/pdf/controllers/PdfController.js b/services/web/frontend/js/ide/pdf/controllers/PdfController.js index 889666ceed..996bda9994 100644 --- a/services/web/frontend/js/ide/pdf/controllers/PdfController.js +++ b/services/web/frontend/js/ide/pdf/controllers/PdfController.js @@ -7,6 +7,7 @@ import { rootContext } from '../../../shared/context/root-context' import 'ace/ace' import getMeta from '../../../utils/meta' import { waitForServiceWorker } from '../../pdfng/directives/serviceWorkerManager' +import { trackPdfDownload } from './PdfJsMetrics' const AUTO_COMPILE_MAX_WAIT = 5000 // We add a 1 second debounce to sending user changes to server if they aren't @@ -405,25 +406,13 @@ App.controller( // convert the qs hash into a query string and append it $scope.pdf.url += createQueryString(qs) - const { - onFirstRenderDone, - firstRenderDone, - updateConsumedBandwidth, - } = trackPDFDownload() - $scope.pdf.firstRenderDone = firstRenderDone - $scope.pdf.updateConsumedBandwidth = updateConsumedBandwidth - const { serviceWorkerMetrics } = response - - onFirstRenderDone.then(({ latencyFetch, latencyRender }) => { - sl_console.log( - JSON.stringify({ - latencyFetch, - latencyRender, - totalBandwidthReportedByPdfJs, - serviceWorkerMetrics, - }) + if (getMeta('ol-trackPdfDownload')) { + const { firstRenderDone, updateConsumedBandwidth } = trackPdfDownload( + response ) - }) + $scope.pdf.firstRenderDone = firstRenderDone + $scope.pdf.updateConsumedBandwidth = updateConsumedBandwidth + } // Save all downloads as files qs.popupDownload = true @@ -572,33 +561,6 @@ App.controller( ) } - let totalBandwidthReportedByPdfJs = 0 - function trackPDFDownload() { - const t0 = performance.now() - let bandwidth = 0 - function firstRenderDone({ timePDFFetched, timePDFRendered }) { - const latencyFetch = timePDFFetched - t0 - // The renderer does not yield in case the browser tab is hidden. - // It will yield when the browser tab is visible again. - // This will skew our performance metrics for rendering! - const latencyRender = timePDFRendered - t0 - done({ latencyFetch, latencyRender }) - } - function updateConsumedBandwidth(bytes) { - totalBandwidthReportedByPdfJs += bytes - bandwidth - bandwidth = bytes - } - let done - const onFirstRenderDone = new Promise(resolve => { - done = resolve - }) - return { - onFirstRenderDone, - firstRenderDone, - updateConsumedBandwidth, - } - } - function fetchLogs(fileByPath, options) { let blgFile, chktexFile, logFile diff --git a/services/web/frontend/js/ide/pdf/controllers/PdfJsMetrics.js b/services/web/frontend/js/ide/pdf/controllers/PdfJsMetrics.js new file mode 100644 index 0000000000..c06904e714 --- /dev/null +++ b/services/web/frontend/js/ide/pdf/controllers/PdfJsMetrics.js @@ -0,0 +1,60 @@ +import { v4 as uuid } from 'uuid' +import { sendMBSampled } from '../../../infrastructure/event-tracking' + +const pdfJsMetrics = { + id: uuid(), + epoch: Date.now(), + totalBandwidth: 0, +} + +const SAMPLING_RATE = 0.01 + +export function trackPdfDownload(response) { + const { serviceWorkerMetrics, stats, timings } = response + + const t0 = performance.now() + let bandwidth = 0 + function firstRenderDone({ timePDFFetched, timePDFRendered }) { + const latencyFetch = timePDFFetched - t0 + // The renderer does not yield in case the browser tab is hidden. + // It will yield when the browser tab is visible again. + // This will skew our performance metrics for rendering! + const latencyRender = timePDFRendered - timePDFFetched + done({ latencyFetch, latencyRender }) + } + function updateConsumedBandwidth(bytes) { + pdfJsMetrics.totalBandwidth += bytes - bandwidth + bandwidth = bytes + } + let done + const onFirstRenderDone = new Promise(resolve => { + done = resolve + }) + + // Submit latency along with compile context. + onFirstRenderDone.then(({ latencyFetch, latencyRender }) => { + submitCompileMetrics({ + latencyFetch, + latencyRender, + stats, + timings, + }) + }) + // Submit bandwidth counter separate from compile context. + submitPDFBandwidth({ pdfJsMetrics, serviceWorkerMetrics }) + + return { + firstRenderDone, + updateConsumedBandwidth, + } +} + +function submitCompileMetrics(metrics) { + sl_console.log('/event/compile-metrics', JSON.stringify(metrics)) + sendMBSampled('compile-metrics', metrics, SAMPLING_RATE) +} + +function submitPDFBandwidth(metrics) { + sl_console.log('/event/pdf-bandwidth', JSON.stringify(metrics)) + sendMBSampled('pdf-bandwidth', metrics, SAMPLING_RATE) +} diff --git a/services/web/frontend/js/infrastructure/event-tracking.js b/services/web/frontend/js/infrastructure/event-tracking.js index 81efb218d0..fc9096519d 100644 --- a/services/web/frontend/js/infrastructure/event-tracking.js +++ b/services/web/frontend/js/infrastructure/event-tracking.js @@ -11,3 +11,9 @@ export function sendMB(key, body = {}) { // ignore errors }) } + +export function sendMBSampled(key, body = {}, rate = 0.01) { + if (Math.random() < rate) { + sendMB(key, body) + } +} diff --git a/services/web/frontend/js/serviceWorker.js b/services/web/frontend/js/serviceWorker.js index db3313ec2e..26dd06cc72 100644 --- a/services/web/frontend/js/serviceWorker.js +++ b/services/web/frontend/js/serviceWorker.js @@ -1,9 +1,11 @@ +import { v4 as uuid } from 'uuid' const COMPILE_REQUEST_MATCHER = /^\/project\/[0-9a-f]{24}\/compile$/ const MIN_CHUNK_SIZE = 128 * 1024 const PDF_FILES = new Map() const METRICS = { + id: uuid(), epoch: Date.now(), cachedBytes: 0, fetchedBytes: 0, diff --git a/services/web/test/unit/src/Project/ProjectControllerTests.js b/services/web/test/unit/src/Project/ProjectControllerTests.js index e243c9197a..792a2959ca 100644 --- a/services/web/test/unit/src/Project/ProjectControllerTests.js +++ b/services/web/test/unit/src/Project/ProjectControllerTests.js @@ -124,12 +124,16 @@ describe('ProjectController', function () { .stub() .returns({ newLogsUI: false, subvariant: null }), } + this.SplitTestHandler = { + getTestSegmentation: sinon.stub().returns({ enabled: false }), + } this.ProjectController = SandboxedModule.require(MODULE_PATH, { requires: { mongodb: { ObjectId }, 'settings-sharelatex': this.settings, '@overleaf/metrics': this.Metrics, + '../SplitTests/SplitTestHandler': this.SplitTestHandler, './ProjectDeleter': this.ProjectDeleter, './ProjectDuplicator': this.ProjectDuplicator, './ProjectCreationHandler': this.ProjectCreationHandler,