[clsi] consolidate metrics for clsi-perf (#30746)

* [clsi] remove all clsi-perf/health-check metrics

* [clsi] always emit E2E compile time metric

* [clsi] do not collect metrics for clsi-cache-template compiles

* [clsi] fix unit tests: request.metricsOpts always exists

* [clsi] use a gauge for the e2e compile time metric of clsi-perf

Co-authored-by: Eric Mc Sween <eric.mcsween@overleaf.com>

* [clsi] remove metrics for binary file downloads from clsi-perf

---------

Co-authored-by: Eric Mc Sween <eric.mcsween@overleaf.com>
GitOrigin-RevId: 7995512e57c802086350e3d1a0ec5213ecdf0a05

services/clsi/test/acceptance/js/SimpleLatexFileTests.js

@@ -1,7 +1,8 @@
 const Client = require('./helpers/Client')
-const { fetchNothing } = require('@overleaf/fetch-utils')
+const { fetchNothing, fetchString } = require('@overleaf/fetch-utils')
 const ClsiApp = require('./helpers/ClsiApp')
 const { expect } = require('chai')
+const Settings = require('@overleaf/settings')
 
 describe('Simple LaTeX file', function () {
   const content = `\
@@ -15,6 +16,9 @@ Hello world
       description: 'simple file',
       request: {
         resources: [{ path: 'main.tex', content }],
+        options: {
+          compileGroup: 'simple-latex-file',
+        },
       },
     },
     {
@@ -22,8 +26,10 @@ Hello world
       request: {
         resources: [{ path: 'main.tex', content }],
         options: {
+          enablePdfCaching: false,
           metricsPath: 'clsi-perf',
-          metricsMethod: 'priority',
+          metricsMethod: 'memoir-manual',
+          compileGroup: 'clsi-perf', // only used by tests, not by the service
         },
       },
     },
@@ -65,8 +71,46 @@ Hello world
         response.status.should.equal(200)
       })
 
+      if (scenario.description === 'clsi-perf request') {
+        it('should only emit e2e compile time metric', async function () {
+          const metrics = await fetchString(`${Settings.apis.clsi.url}/metrics`)
+          const byPath = `path="${scenario.request.options.metricsPath}"`
+          const byMethod = `method="${scenario.request.options.metricsMethod}"`
+          const byVariant = `variant="${scenario.request.options.metricsMethod}"`
+          const byGroup = `group="${scenario.request.options.compileGroup}"`
+          expect(metrics).to.not.include(byPath)
+          expect(metrics).to.not.include(byMethod)
+          expect(metrics).to.not.include(byGroup)
+          expect(metrics).to.include(byVariant)
+          expect(metrics.match(new RegExp(byVariant, 'g'))).to.have.lengthOf(1)
+        })
+      } else {
+        it('should shard metrics by compileGroup', async function () {
+          const metrics = await fetchString(`${Settings.apis.clsi.url}/metrics`)
+          const byGroup = `group="${scenario.request.options.compileGroup}"`
+          expect(metrics).to.include(byGroup)
+          expect(metrics.match(new RegExp(byGroup, 'g'))).to.have.lengthOf(134)
+        })
+      }
+
       it('should return only the expected keys for stats and timings', function () {
         const { stats, timings } = this.body.compile
+        let pdfCachingStats = []
+        let pdfCachingTimings = []
+        if (scenario.request.options.enablePdfCaching !== false) {
+          pdfCachingStats = [
+            'pdf-caching-total-ranges-size',
+            'pdf-caching-reclaimed-space',
+            'pdf-caching-new-ranges-size',
+            'pdf-caching-n-ranges',
+            'pdf-caching-n-new-ranges',
+          ]
+          pdfCachingTimings = [
+            'compute-pdf-caching',
+            'pdf-caching-overhead-delete-stale-hashes',
+          ]
+        }
+
         // Note: chai's all.keys assertion rejects extra keys
         stats.should.have.all.keys(
           'isInitialCompile',
@@ -75,20 +119,15 @@ Hello world
           'latex-runs-with-errors',
           'latex-runs-1',
           'latex-runs-with-errors-1',
-          'pdf-caching-total-ranges-size',
-          'pdf-caching-reclaimed-space',
-          'pdf-caching-new-ranges-size',
-          'pdf-caching-n-ranges',
-          'pdf-caching-n-new-ranges',
-          'pdf-size'
+          'pdf-size',
+          ...pdfCachingStats
         )
         timings.should.have.all.keys(
           'sync',
           'compile',
           'output',
           'compileE2E',
-          'compute-pdf-caching',
-          'pdf-caching-overhead-delete-stale-hashes'
+          ...pdfCachingTimings
         )
       })
     })

services/clsi/test/unit/js/ResourceWriterTests.js

@@ -246,6 +246,7 @@ describe('ResourceWriter', function () {
         syncType: 'incremental',
         syncState: (this.syncState = '1234567890abcdef'),
         resources: this.resources,
+        metricsOpts: { path: 'foo' },
       }
       this.OutputFileFinder.findOutputFiles = sinon
         .stub()

services/clsi/test/unit/js/StatsManagerTests.js

@@ -85,7 +85,7 @@ describe('StatsManager', function () {
 
   describe('sampleRequest', function () {
     it('should return undefined if there is no user_id', function () {
-      const request = {}
+      const request = { metricsOpts: {} }
       const percentage = 50
       expect(sampleRequest(request, percentage)).to.be.undefined
     })
@@ -127,23 +127,17 @@ describe('StatsManager', function () {
     })
 
     it('should return undefined if the sampling percentage is 0', function () {
-      const request = { user_id: 'some-user' }
+      const request = { user_id: 'some-user', metricsOpts: {} }
       const percentage = 0
       expect(sampleRequest(request, percentage)).to.be.undefined
     })
 
     it('should return undefined if the sampling percentage is negative', function () {
-      const request = { user_id: 'some-user' }
+      const request = { user_id: 'some-user', metricsOpts: {} }
       const percentage = -10
       expect(sampleRequest(request, percentage)).to.be.undefined
     })
 
-    it('should sample if there are no metricsOpts', function () {
-      const request = { user_id: 'test-key-in' } // percentile 13
-      const percentage = 40
-      expect(sampleRequest(request, percentage)).to.be.true
-    })
-
     it('should sample if metricsOpts has no path', function () {
       const request = { user_id: 'test-key-in', metricsOpts: {} } // percentile 13
       const percentage = 40
@@ -151,13 +145,13 @@ describe('StatsManager', function () {
     })
 
     it('should return true for a request that should be sampled', function () {
-      const request = { user_id: 'test-key-in' } // percentile 13
+      const request = { user_id: 'test-key-in', metricsOpts: {} } // percentile 13
       const percentage = 40
       expect(sampleRequest(request, percentage)).to.be.true
     })
 
     it('should return false for a request that should not be sampled', function () {
-      const request = { user_id: 'test-key-outer' } // percentile 47
+      const request = { user_id: 'test-key-outer', metricsOpts: {} } // percentile 47
       const percentage = 40
       expect(sampleRequest(request, percentage)).to.be.false
     })