From 0409676c417a47e49351c3eaecff7e47b6bdc47a Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 20 Jul 2022 09:15:41 +0100 Subject: [PATCH] Merge pull request #8911 from overleaf/jpa-pdf-detach-pdf-caching-metrics [web] fix forwarding of pdf caching metrics in pdf-detach mode GitOrigin-RevId: c4a65190102ca4abfb7970186bc5ee785bff14c6 --- .../pdf-preview/components/pdf-js-viewer.js | 8 +++++++- .../js/features/pdf-preview/util/metrics.js | 17 ++++++++++------- .../pdf-preview/util/pdf-caching-transport.js | 7 +++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js b/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js index fc6fc8c09b..d7b99cf974 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js @@ -10,6 +10,7 @@ import withErrorBoundary from '../../../infrastructure/error-boundary' import ErrorBoundaryFallback from './error-boundary-fallback' import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context' import { captureException } from '../../../infrastructure/error-reporter' +import { getPdfCachingMetrics } from '../util/metrics' function PdfJsViewer({ url, pdfFile }) { const { _id: projectId } = useProjectContext() @@ -81,7 +82,12 @@ function PdfJsViewer({ url, pdfFile }) { // We are omitting the render time in case we detect this state. latencyRender = Math.ceil(timePDFRendered - timePDFFetched) } - firstRenderDone({ latencyFetch, latencyRender }) + firstRenderDone({ + latencyFetch, + latencyRender, + // Let the pdfCachingMetrics round trip to account for pdf-detach. + pdfCachingMetrics: getPdfCachingMetrics(), + }) } const handlePagesinit = () => { diff --git a/services/web/frontend/js/features/pdf-preview/util/metrics.js b/services/web/frontend/js/features/pdf-preview/util/metrics.js index 6068c04fc6..962b6ea01b 100644 --- a/services/web/frontend/js/features/pdf-preview/util/metrics.js +++ b/services/web/frontend/js/features/pdf-preview/util/metrics.js @@ -4,15 +4,17 @@ import getMeta from '../../../utils/meta' // VERSION should get incremented when making changes to caching behavior or // adjusting metrics collection. -const VERSION = 3 +const VERSION = 4 // editing session id const EDITOR_SESSION_ID = uuid() -let pdfCachingMetrics +const pdfCachingMetrics = { + viewerId: EDITOR_SESSION_ID, +} -export function setCachingMetrics(metrics) { - pdfCachingMetrics = metrics +export function getPdfCachingMetrics() { + return pdfCachingMetrics } export function trackPdfDownload(response, compileTimeClientE2E, t0) { @@ -25,8 +27,9 @@ export function trackPdfDownload(response, compileTimeClientE2E, t0) { // There can be multiple "first" renderings with two pdf viewers. // E.g. two pdf detach tabs or pdf detacher plus pdf detach. + // Let the pdfCachingMetrics round trip to account for pdf-detach. let isFirstRender = true - function firstRenderDone({ latencyFetch, latencyRender }) { + function firstRenderDone({ latencyFetch, latencyRender, pdfCachingMetrics }) { if (!isFirstRender) return isFirstRender = false @@ -43,6 +46,7 @@ export function trackPdfDownload(response, compileTimeClientE2E, t0) { latencyFetch, latencyRender, compileTimeClientE2E, + ...pdfCachingMetrics, }) } } @@ -58,8 +62,7 @@ function submitCompileMetrics(metrics) { version: VERSION, ...metrics, id: EDITOR_SESSION_ID, - ...(pdfCachingMetrics || {}), } - sl_console.log('/event/compile-metrics', JSON.stringify(metrics)) + sl_console.log('/event/compile-metrics', JSON.stringify(leanMetrics)) sendMB('compile-metrics-v6', leanMetrics) } diff --git a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js index 0c147cfacb..695cc6b806 100644 --- a/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js +++ b/services/web/frontend/js/features/pdf-preview/util/pdf-caching-transport.js @@ -1,14 +1,14 @@ import { fallbackRequest, fetchRange } from './pdf-caching' import getMeta from '../../../utils/meta' import { captureException } from '../../../infrastructure/error-reporter' -import { setCachingMetrics } from './metrics' +import { getPdfCachingMetrics } from './metrics' export function generatePdfCachingTransportFactory(PDFJS) { if (getMeta('ol-pdfCachingMode') !== 'enabled') { return () => null } const cached = new Set() - const metrics = { + const metrics = Object.assign(getPdfCachingMetrics(), { failedCount: 0, tooLargeOverheadCount: 0, tooManyRequestsCount: 0, @@ -18,10 +18,9 @@ export function generatePdfCachingTransportFactory(PDFJS) { fetchedBytes: 0, requestedCount: 0, requestedBytes: 0, - } + }) const verifyChunks = new URLSearchParams(window.location.search).get('verify_chunks') === 'true' - setCachingMetrics(metrics) class PDFDataRangeTransport extends PDFJS.PDFDataRangeTransport { constructor(url, pdfFile, reject) {