Merge pull request #4123 from overleaf/jpa-pdf-caching-one-split-test

[misc] rework roll-out of pdf caching

GitOrigin-RevId: 98ff50918050fe8e9fb5bfecb862657d48cd2726

services/web/app/src/Features/Project/ProjectController.js

@@ -710,6 +710,21 @@ const ProjectController = {
         flushToTpds: cb => {
           TpdsProjectFlusher.flushProjectToTpdsIfNeeded(projectId, cb)
         },
+        pdfCachingFeatureFlag(cb) {
+          if (!Settings.enablePdfCaching) return cb(null, '')
+          if (!userId) return cb(null, '')
+          SplitTestHandler.getTestSegmentation(
+            userId,
+            'pdf_caching_beta',
+            (err, segmentation) => {
+              if (err) {
+                // Do not fail loading the editor.
+                return cb(null, '')
+              }
+              cb(null, (segmentation && segmentation.variant) || '')
+            }
+          )
+        },
       },
       (err, results) => {
         if (err != null) {
@@ -720,6 +735,7 @@ const ProjectController = {
         const { user } = results
         const { subscription } = results
         const { brandVariation } = results
+        const { pdfCachingFeatureFlag } = results
 
         const anonRequestToken = TokenAccessHandler.getRequestToken(
           req,
@@ -791,142 +807,105 @@ const ProjectController = {
               }
             }
 
-            let trackPdfDownload = false
-            let enablePdfCaching = false
-
-            const render = () => {
-              res.render('project/editor', {
-                title: project.name,
-                priority_title: true,
-                bodyClasses: ['editor'],
-                project_id: project._id,
-                user: {
-                  id: userId,
-                  email: user.email,
-                  first_name: user.first_name,
-                  last_name: user.last_name,
-                  referal_id: user.referal_id,
-                  signUpDate: user.signUpDate,
-                  allowedFreeTrial: allowedFreeTrial,
-                  featureSwitches: user.featureSwitches,
-                  features: user.features,
-                  refProviders: _.mapValues(user.refProviders, Boolean),
-                  alphaProgram: user.alphaProgram,
-                  betaProgram: user.betaProgram,
-                  isAdmin: user.isAdmin,
-                },
-                userSettings: {
-                  mode: user.ace.mode,
-                  editorTheme: user.ace.theme,
-                  fontSize: user.ace.fontSize,
-                  autoComplete: user.ace.autoComplete,
-                  autoPairDelimiters: user.ace.autoPairDelimiters,
-                  pdfViewer: user.ace.pdfViewer,
-                  syntaxValidation: user.ace.syntaxValidation,
-                  fontFamily: user.ace.fontFamily || 'lucida',
-                  lineHeight: user.ace.lineHeight || 'normal',
-                  overallTheme: user.ace.overallTheme,
-                },
-                privilegeLevel,
-                chatUrl: Settings.apis.chat.url,
-                anonymous,
-                anonymousAccessToken: anonymous ? anonRequestToken : null,
-                isTokenMember,
-                isRestrictedTokenMember: AuthorizationManager.isRestrictedUser(
-                  userId,
-                  privilegeLevel,
-                  isTokenMember
-                ),
-                languages: Settings.languages,
-                editorThemes: THEME_LIST,
-                maxDocLength: Settings.max_doc_length,
-                useV2History:
-                  project.overleaf &&
-                  project.overleaf.history &&
-                  Boolean(project.overleaf.history.display),
-                brandVariation,
-                allowedImageNames,
-                gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl,
-                wsUrl,
-                showSupport: Features.hasFeature('support'),
-                showNewLogsUI: shouldDisplayFeature(
-                  'new_logs_ui',
-                  logsUIVariant.newLogsUI
-                ),
-                logsUISubvariant: logsUIVariant.subvariant,
-                showNewNavigationUI: shouldDisplayFeature(
-                  'new_navigation_ui',
-                  user.alphaProgram
-                ),
-                showReactShareModal: shouldDisplayFeature(
-                  'new_share_modal_ui',
-                  true
-                ),
-                showReactDropboxModal: shouldDisplayFeature(
-                  'new_dropbox_modal_ui',
-                  user.betaProgram
-                ),
-                showReactGithubSync: shouldDisplayFeature(
-                  'new_github_sync_ui',
-                  user.betaProgram || user.alphaProgram
-                ),
-                showNewBinaryFileUI: shouldDisplayFeature('new_binary_file'),
-                showSymbolPalette: shouldDisplayFeature('symbol_palette'),
-                trackPdfDownload,
-                enablePdfCaching,
-              })
-              timer.done()
+            function partOfPdfCachingRollout(flag) {
+              if (!Settings.enablePdfCaching) {
+                // The feature is disabled globally.
+                return false
+              }
+              const canSeeFeaturePreview =
+                user.alphaProgram ||
+                (user.betaProgram && pdfCachingFeatureFlag.includes(flag))
+              if (!canSeeFeaturePreview) {
+                // The user is not in the target group.
+                return false
+              }
+              // Optionally let the user opt-out.
+              // The will opt-out of both caching and metrics collection,
+              //  as if this editing session never happened.
+              return shouldDisplayFeature('enable_pdf_caching', true)
             }
 
-            Promise.all([
-              (async () => {
-                if (Settings.enablePdfCaching) {
-                  if (user.alphaProgram) {
-                    trackPdfDownload = true
-                  } else if (user.betaProgram) {
-                    const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
-                      userId,
-                      'track_pdf_download'
-                    )
-                    trackPdfDownload =
-                      testSegmentation.enabled &&
-                      testSegmentation.variant === 'enabled'
-                  }
-                }
-              })(),
-              (async () => {
-                if (Settings.enablePdfCaching) {
-                  if (user.alphaProgram) {
-                    enablePdfCaching = shouldDisplayFeature(
-                      'enable_pdf_caching',
-                      true
-                    )
-                  } else if (user.betaProgram) {
-                    const testSegmentation = await SplitTestHandler.promises.getTestSegmentation(
-                      userId,
-                      'enable_pdf_caching'
-                    )
-                    enablePdfCaching = shouldDisplayFeature(
-                      'enable_pdf_caching',
-                      testSegmentation.enabled &&
-                        testSegmentation.variant === 'enabled'
-                    )
-                  } else {
-                    enablePdfCaching = shouldDisplayFeature(
-                      'enable_pdf_caching',
-                      false
-                    )
-                  }
-                }
-              })(),
-            ])
-              .then(() => {
-                render()
-              })
-              .catch(error => {
-                logger.error({ err: error }, 'Failed to get test segmentation')
-                render()
-              })
+            res.render('project/editor', {
+              title: project.name,
+              priority_title: true,
+              bodyClasses: ['editor'],
+              project_id: project._id,
+              user: {
+                id: userId,
+                email: user.email,
+                first_name: user.first_name,
+                last_name: user.last_name,
+                referal_id: user.referal_id,
+                signUpDate: user.signUpDate,
+                allowedFreeTrial: allowedFreeTrial,
+                featureSwitches: user.featureSwitches,
+                features: user.features,
+                refProviders: _.mapValues(user.refProviders, Boolean),
+                alphaProgram: user.alphaProgram,
+                betaProgram: user.betaProgram,
+                isAdmin: user.isAdmin,
+              },
+              userSettings: {
+                mode: user.ace.mode,
+                editorTheme: user.ace.theme,
+                fontSize: user.ace.fontSize,
+                autoComplete: user.ace.autoComplete,
+                autoPairDelimiters: user.ace.autoPairDelimiters,
+                pdfViewer: user.ace.pdfViewer,
+                syntaxValidation: user.ace.syntaxValidation,
+                fontFamily: user.ace.fontFamily || 'lucida',
+                lineHeight: user.ace.lineHeight || 'normal',
+                overallTheme: user.ace.overallTheme,
+              },
+              privilegeLevel,
+              chatUrl: Settings.apis.chat.url,
+              anonymous,
+              anonymousAccessToken: anonymous ? anonRequestToken : null,
+              isTokenMember,
+              isRestrictedTokenMember: AuthorizationManager.isRestrictedUser(
+                userId,
+                privilegeLevel,
+                isTokenMember
+              ),
+              languages: Settings.languages,
+              editorThemes: THEME_LIST,
+              maxDocLength: Settings.max_doc_length,
+              useV2History:
+                project.overleaf &&
+                project.overleaf.history &&
+                Boolean(project.overleaf.history.display),
+              brandVariation,
+              allowedImageNames,
+              gitBridgePublicBaseUrl: Settings.gitBridgePublicBaseUrl,
+              wsUrl,
+              showSupport: Features.hasFeature('support'),
+              showNewLogsUI: shouldDisplayFeature(
+                'new_logs_ui',
+                logsUIVariant.newLogsUI
+              ),
+              logsUISubvariant: logsUIVariant.subvariant,
+              showNewNavigationUI: shouldDisplayFeature(
+                'new_navigation_ui',
+                user.alphaProgram
+              ),
+              showReactShareModal: shouldDisplayFeature(
+                'new_share_modal_ui',
+                true
+              ),
+              showReactDropboxModal: shouldDisplayFeature(
+                'new_dropbox_modal_ui',
+                user.betaProgram
+              ),
+              showReactGithubSync: shouldDisplayFeature(
+                'new_github_sync_ui',
+                user.betaProgram || user.alphaProgram
+              ),
+              showNewBinaryFileUI: shouldDisplayFeature('new_binary_file'),
+              showSymbolPalette: shouldDisplayFeature('symbol_palette'),
+              trackPdfDownload: partOfPdfCachingRollout('collect-metrics'),
+              enablePdfCaching: partOfPdfCachingRollout('enable-caching'),
+            })
+            timer.done()
           }
         )
       }

services/web/config/settings.defaults.js

@@ -344,27 +344,32 @@ module.exports = {
       ],
     },
     {
-      id: 'enable_pdf_caching',
-      active: process.env.SPLIT_TEST_ENABLE_PDF_CACHING_ACTIVE === 'true',
+      id: 'pdf_caching_beta',
+      active: process.env.SPLIT_TEST_PDF_CACHING_BETA_ACTIVE === 'true',
       variants: [
         {
-          id: 'enabled',
+          id: 'collect-metrics-only',
           rolloutPercent: parseInt(
-            process.env.SPLIT_TEST_ENABLE_PDF_CACHING_ENABLE_ROLLOUT_PERCENT ||
+            process.env
+              .SPLIT_TEST_PDF_CACHING_BETA_COLLECT_METRICS_ONLY_ROLLOUT_PERCENT ||
               '0',
             10
           ),
         },
-      ],
-    },
-    {
-      id: 'track_pdf_download',
-      active: process.env.SPLIT_TEST_TRACK_PDF_DOWNLOAD_ACTIVE === 'true',
-      variants: [
         {
-          id: 'enabled',
+          id: 'collect-metrics-and-enable-caching',
           rolloutPercent: parseInt(
-            process.env.SPLIT_TEST_TRACK_PDF_DOWNLOAD_ENABLE_ROLLOUT_PERCENT ||
+            process.env
+              .SPLIT_TEST_PDF_CACHING_BETA_COLLECT_METRICS_AND_ENABLE_CACHING_ROLLOUT_PERCENT ||
+              '0',
+            10
+          ),
+        },
+        {
+          id: 'enable-caching-only',
+          rolloutPercent: parseInt(
+            process.env
+              .SPLIT_TEST_PDF_CACHING_BETA_ENABLE_CACHING_ONLY_ROLLOUT_PERCENT ||
               '0',
             10
           ),

services/web/frontend/js/ide/pdf/controllers/PdfJsMetrics.js

@@ -1,5 +1,5 @@
 import { v4 as uuid } from 'uuid'
-import { sendMBSampled } from '../../../infrastructure/event-tracking'
+import { sendMB } from '../../../infrastructure/event-tracking'
 
 const pdfJsMetrics = {
   id: uuid(),
@@ -72,7 +72,7 @@ function submitCompileMetrics(metrics) {
     compileTimeServerE2E: timings.compileE2E,
   }
   sl_console.log('/event/compile-metrics', JSON.stringify(metrics))
-  sendMBSampled('compile-metrics', leanMetrics, SAMPLING_RATE)
+  sendMB('compile-metrics-v2', leanMetrics, SAMPLING_RATE)
 }
 
 function submitPDFBandwidth(metrics) {
@@ -83,5 +83,5 @@ function submitPDFBandwidth(metrics) {
     })
   })
   sl_console.log('/event/pdf-bandwidth', JSON.stringify(metrics))
-  sendMBSampled('pdf-bandwidth', metricsFlat, SAMPLING_RATE)
+  sendMB('pdf-bandwidth-v2', metricsFlat, SAMPLING_RATE)
 }

services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -128,7 +128,7 @@ describe('ProjectController', function () {
       promises: {
         getTestSegmentation: sinon.stub().resolves({ enabled: false }),
       },
-      getTestSegmentation: sinon.stub().returns({ enabled: false }),
+      getTestSegmentation: sinon.stub().yields(null, { enabled: false }),
     }
 
     this.ProjectController = SandboxedModule.require(MODULE_PATH, {
@@ -1076,6 +1076,21 @@ describe('ProjectController', function () {
     })
 
     describe('pdf caching feature flags', function () {
+      /* eslint-disable mocha/no-identical-title */
+      function showNoVariant() {
+        beforeEach(function () {
+          this.SplitTestHandler.getTestSegmentation = sinon
+            .stub()
+            .yields(null, { enabled: false })
+        })
+      }
+      function showVariant(variant) {
+        beforeEach(function () {
+          this.SplitTestHandler.getTestSegmentation = sinon
+            .stub()
+            .yields(null, { enabled: true, variant })
+        })
+      }
       function expectBandwidthTrackingEnabled() {
         it('should track pdf bandwidth', function (done) {
           this.res.render = (pageName, opts) => {
@@ -1112,15 +1127,10 @@ describe('ProjectController', function () {
           this.ProjectController.loadEditor(this.req, this.res)
         })
       }
-
-      beforeEach(function () {
-        this.settings.enablePdfCaching = true
-      })
-
-      describe('regular user', function () {
+      function expectToCollectMetricsAndCachePDF() {
         describe('with no query', function () {
-          expectBandwidthTrackingDisabled()
-          expectPDFCachingDisabled()
+          expectBandwidthTrackingEnabled()
+          expectPDFCachingEnabled()
         })
 
         describe('with enable_pdf_caching=false', function () {
@@ -1135,25 +1145,21 @@ describe('ProjectController', function () {
           beforeEach(function () {
             this.req.query.enable_pdf_caching = 'true'
           })
-          expectBandwidthTrackingDisabled()
+          expectBandwidthTrackingEnabled()
           expectPDFCachingEnabled()
         })
-      })
-
-      describe('alpha user', function () {
-        beforeEach(function () {
-          this.user.alphaProgram = true
-        })
+      }
+      function expectToCollectMetricsOnly() {
         describe('with no query', function () {
           expectBandwidthTrackingEnabled()
-          expectPDFCachingEnabled()
+          expectPDFCachingDisabled()
         })
 
         describe('with enable_pdf_caching=false', function () {
           beforeEach(function () {
             this.req.query.enable_pdf_caching = 'false'
           })
-          expectBandwidthTrackingEnabled()
+          expectBandwidthTrackingDisabled()
           expectPDFCachingDisabled()
         })
 
@@ -1162,15 +1168,34 @@ describe('ProjectController', function () {
             this.req.query.enable_pdf_caching = 'true'
           })
           expectBandwidthTrackingEnabled()
+          expectPDFCachingDisabled()
+        })
+      }
+
+      function expectToCachePDFOnly() {
+        describe('with no query', function () {
+          expectBandwidthTrackingDisabled()
           expectPDFCachingEnabled()
         })
-      })
 
-      describe('beta user', function () {
-        beforeEach(function () {
-          this.user.betaProgram = true
+        describe('with enable_pdf_caching=false', function () {
+          beforeEach(function () {
+            this.req.query.enable_pdf_caching = 'false'
+          })
+          expectBandwidthTrackingDisabled()
+          expectPDFCachingDisabled()
         })
 
+        describe('with enable_pdf_caching=true', function () {
+          beforeEach(function () {
+            this.req.query.enable_pdf_caching = 'true'
+          })
+          expectBandwidthTrackingDisabled()
+          expectPDFCachingEnabled()
+        })
+      }
+
+      function expectToNotBeEnrolledAtAll() {
         describe('with no query', function () {
           expectBandwidthTrackingDisabled()
           expectPDFCachingDisabled()
@@ -1189,7 +1214,129 @@ describe('ProjectController', function () {
             this.req.query.enable_pdf_caching = 'true'
           })
           expectBandwidthTrackingDisabled()
-          expectPDFCachingEnabled()
+          expectPDFCachingDisabled()
+        })
+      }
+
+      function tagAnonymous() {
+        beforeEach(function () {
+          this.AuthenticationController.isUserLoggedIn = sinon
+            .stub()
+            .returns(false)
+        })
+      }
+      function tagAlpha() {
+        beforeEach(function () {
+          this.user.alphaProgram = true
+        })
+      }
+      function tagBeta() {
+        beforeEach(function () {
+          this.user.betaProgram = true
+        })
+      }
+
+      beforeEach(function () {
+        this.settings.enablePdfCaching = true
+      })
+
+      describe('alpha rollout', function () {
+        describe('regular user', function () {
+          expectToNotBeEnrolledAtAll()
+        })
+        describe('anonymous user', function () {
+          tagAnonymous()
+          expectToNotBeEnrolledAtAll()
+        })
+        describe('alpha user', function () {
+          tagAlpha()
+          expectToCollectMetricsAndCachePDF()
+        })
+        describe('beta user', function () {
+          tagBeta()
+          expectToNotBeEnrolledAtAll()
+        })
+      })
+
+      describe('during beta roll-out', function () {
+        describe('disabled', function () {
+          showNoVariant()
+
+          describe('regular user', function () {
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('anonymous user', function () {
+            tagAnonymous()
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('alpha user', function () {
+            tagAlpha()
+            expectToCollectMetricsAndCachePDF()
+          })
+          describe('beta user', function () {
+            tagBeta()
+            expectToNotBeEnrolledAtAll()
+          })
+        })
+
+        describe('variant=collect-metrics', function () {
+          showVariant('collect-metrics')
+
+          describe('regular user', function () {
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('anonymous user', function () {
+            tagAnonymous()
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('alpha user', function () {
+            tagAlpha()
+            expectToCollectMetricsAndCachePDF()
+          })
+          describe('beta user', function () {
+            tagBeta()
+            expectToCollectMetricsOnly()
+          })
+        })
+
+        describe('variant=collect-metrics-and-enable-caching', function () {
+          showVariant('collect-metrics-and-enable-caching')
+
+          describe('regular user', function () {
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('anonymous user', function () {
+            tagAnonymous()
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('alpha user', function () {
+            tagAlpha()
+            expectToCollectMetricsAndCachePDF()
+          })
+          describe('beta user', function () {
+            tagBeta()
+            expectToCollectMetricsAndCachePDF()
+          })
+        })
+
+        describe('variant=enable-caching-only', function () {
+          showVariant('enable-caching-only')
+
+          describe('regular user', function () {
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('anonymous user', function () {
+            tagAnonymous()
+            expectToNotBeEnrolledAtAll()
+          })
+          describe('alpha user', function () {
+            tagAlpha()
+            expectToCollectMetricsAndCachePDF()
+          })
+          describe('beta user', function () {
+            tagBeta()
+            expectToCachePDFOnly()
+          })
         })
       })
     })