[real-time] record metrics on number of connected clients per project (#24727)

Note: "connected" here means across all real-time pods.

- editing_session_mode, counter
  - mode=connect - a client connected
  - mode=disconnect - a client disconnect
  - mode=update - continuous editing
  - status=empty - all clients disconnected
  - status=single - a single client is connected
  - status=multi - multiple clients are connected

- project_not_empty_since histogram with buckets [0,1h,2h,1d,2d,1w,30d]
  - status=empty/single/multi as described above

GitOrigin-RevId: 1cc42e72bbb5aae754399bdbc3f8771642f35c22

services/real-time/app/js/ConnectedUsersManager.js

@@ -3,6 +3,7 @@ const Settings = require('@overleaf/settings')
 const logger = require('@overleaf/logger')
 const redis = require('@overleaf/redis-wrapper')
 const OError = require('@overleaf/o-error')
+const Metrics = require('@overleaf/metrics')
 const rclient = redis.createClient(Settings.redis.realtime)
 const Keys = Settings.redis.realtime.key_schema
 
@@ -13,6 +14,20 @@ const FOUR_DAYS_IN_S = ONE_DAY_IN_S * 4
 const USER_TIMEOUT_IN_S = ONE_HOUR_IN_S / 4
 const REFRESH_TIMEOUT_IN_S = 10 // only show clients which have responded to a refresh request in the last 10 seconds
 
+function recordProjectNotEmptySinceMetric(res, status) {
+  const diff = Date.now() / 1000 - parseInt(res, 10)
+  const BUCKETS = [
+    0,
+    ONE_HOUR_IN_S,
+    2 * ONE_HOUR_IN_S,
+    ONE_DAY_IN_S,
+    2 * ONE_DAY_IN_S,
+    7 * ONE_DAY_IN_S,
+    30 * ONE_DAY_IN_S,
+  ]
+  Metrics.histogram('project_not_empty_since', diff, BUCKETS, { status })
+}
+
 module.exports = {
   // Use the same method for when a user connects, and when a user sends a cursor
   // update. This way we don't care if the connected_user key has expired when
@@ -23,6 +38,7 @@ module.exports = {
     const multi = rclient.multi()
 
     multi.sadd(Keys.clientsInProject({ project_id: projectId }), clientId)
+    multi.scard(Keys.clientsInProject({ project_id: projectId }))
     multi.expire(
       Keys.clientsInProject({ project_id: projectId }),
       FOUR_DAYS_IN_S
@@ -66,10 +82,15 @@ module.exports = {
       USER_TIMEOUT_IN_S
     )
 
-    multi.exec(function (err) {
+    multi.exec(function (err, res) {
       if (err) {
         err = new OError('problem marking user as connected').withCause(err)
       }
+      const [, nConnectedClients] = res
+      Metrics.inc('editing_session_mode', 1, {
+        mode: cursorData ? 'update' : 'connect',
+        status: nConnectedClients === 1 ? 'single' : 'multi',
+      })
       callback(err)
     })
   },
@@ -100,6 +121,7 @@ module.exports = {
     logger.debug({ projectId, clientId }, 'marking user as disconnected')
     const multi = rclient.multi()
     multi.srem(Keys.clientsInProject({ project_id: projectId }), clientId)
+    multi.scard(Keys.clientsInProject({ project_id: projectId }))
     multi.expire(
       Keys.clientsInProject({ project_id: projectId }),
       FOUR_DAYS_IN_S
@@ -107,10 +129,54 @@ module.exports = {
     multi.del(
       Keys.connectedUser({ project_id: projectId, client_id: clientId })
     )
-    multi.exec(function (err) {
+    multi.exec(function (err, res) {
       if (err) {
         err = new OError('problem marking user as disconnected').withCause(err)
       }
+      const [, nConnectedClients] = res
+      const status =
+        nConnectedClients === 0
+          ? 'empty'
+          : nConnectedClients === 1
+            ? 'single'
+            : 'multi'
+      Metrics.inc('editing_session_mode', 1, {
+        mode: 'disconnect',
+        status,
+      })
+      if (status === 'empty') {
+        rclient.getdel(Keys.projectNotEmptySince({ projectId }), (err, res) => {
+          if (err) {
+            logger.warn(
+              { err, projectId },
+              'could not collect projectNotEmptySince'
+            )
+          } else if (res) {
+            recordProjectNotEmptySinceMetric(res, status)
+          }
+        })
+      } else {
+        // Only populate projectNotEmptySince when more clients remain connected.
+        const nowInSeconds = Math.ceil(Date.now() / 1000).toString()
+        rclient.set(
+          Keys.projectNotEmptySince({ projectId }),
+          nowInSeconds,
+          'NX',
+          'GET',
+          'EX',
+          31 * ONE_DAY_IN_S,
+          (err, res) => {
+            if (err) {
+              logger.warn(
+                { err, projectId },
+                'could not set/collect projectNotEmptySince'
+              )
+            } else if (res) {
+              recordProjectNotEmptySinceMetric(res, status)
+            }
+          }
+        )
+      }
       callback(err)
     })
   },

services/real-time/config/settings.defaults.js

@@ -38,6 +38,9 @@ const settings = {
         connectedUser({ project_id, client_id }) {
           return `connected_user:{${project_id}}:${client_id}`
         },
+        projectNotEmptySince({ projectId }) {
+          return `projectNotEmptySince:{${projectId}}`
+        },
       },
       maxRetriesPerRequest: parseInt(
         process.env.REAL_TIME_REDIS_MAX_RETRIES_PER_REQUEST ||

services/real-time/test/unit/js/ConnectedUsersManagerTests.js

@@ -30,12 +30,18 @@ describe('ConnectedUsersManager', function () {
             connectedUser({ project_id: projectId, client_id: clientId }) {
               return `connected_user:${projectId}:${clientId}`
             },
+            projectNotEmptySince({ projectId }) {
+              return `projectNotEmptySince:{${projectId}}`
+            },
           },
         },
       },
     }
     this.rClient = {
       auth() {},
+      getdel: sinon.stub(),
+      scard: sinon.stub(),
+      set: sinon.stub(),
       setex: sinon.stub(),
       sadd: sinon.stub(),
       get: sinon.stub(),
@@ -51,10 +57,15 @@ describe('ConnectedUsersManager', function () {
       },
     }
     tk.freeze(new Date())
+    this.Metrics = {
+      inc: sinon.stub(),
+      histogram: sinon.stub(),
+    }
 
     this.ConnectedUsersManager = SandboxedModule.require(modulePath, {
       requires: {
         '@overleaf/settings': this.settings,
+        '@overleaf/metrics': this.Metrics,
         '@overleaf/redis-wrapper': {
           createClient: () => {
             return this.rClient
@@ -83,7 +94,7 @@ describe('ConnectedUsersManager', function () {
 
   describe('updateUserPosition', function () {
     beforeEach(function () {
-      return this.rClient.exec.callsArgWith(0)
+      this.rClient.exec.yields(null, [1, 1])
     })
 
     it('should set a key with the date and give it a ttl', function (done) {
@@ -240,7 +251,7 @@ describe('ConnectedUsersManager', function () {
       )
     })
 
-    return it('should set the cursor position when provided', function (done) {
+    it('should set the cursor position when provided', function (done) {
       return this.ConnectedUsersManager.updateUserPosition(
         this.project_id,
         this.client_id,
@@ -259,11 +270,72 @@ describe('ConnectedUsersManager', function () {
         }
       )
     })
+
+    describe('editing_session_mode', function () {
+      const cases = {
+        'should bump the metric when connecting to empty room': {
+          nConnectedClients: 1,
+          cursorData: null,
+          labels: {
+            mode: 'connect',
+            status: 'single',
+          },
+        },
+        'should bump the metric when connecting to non-empty room': {
+          nConnectedClients: 2,
+          cursorData: null,
+          labels: {
+            mode: 'connect',
+            status: 'multi',
+          },
+        },
+        'should bump the metric when updating in empty room': {
+          nConnectedClients: 1,
+          cursorData: { row: 42 },
+          labels: {
+            mode: 'update',
+            status: 'single',
+          },
+        },
+        'should bump the metric when updating in non-empty room': {
+          nConnectedClients: 2,
+          cursorData: { row: 42 },
+          labels: {
+            mode: 'update',
+            status: 'multi',
+          },
+        },
+      }
+
+      for (const [
+        name,
+        { nConnectedClients, cursorData, labels },
+      ] of Object.entries(cases)) {
+        it(name, function (done) {
+          this.rClient.exec.yields(null, [1, nConnectedClients])
+          this.ConnectedUsersManager.updateUserPosition(
+            this.project_id,
+            this.client_id,
+            this.user,
+            cursorData,
+            err => {
+              if (err) return done(err)
+              expect(this.Metrics.inc).to.have.been.calledWith(
+                'editing_session_mode',
+                1,
+                labels
+              )
+              done()
+            }
+          )
+        })
+      }
+    })
   })
 
   describe('markUserAsDisconnected', function () {
     beforeEach(function () {
-      return this.rClient.exec.callsArgWith(0)
+      this.rClient.exec.yields(null, [1, 0])
     })
 
     it('should remove the user from the set', function (done) {
@@ -294,7 +366,7 @@ describe('ConnectedUsersManager', function () {
       )
     })
 
-    return it('should add a ttl to the connected user set so it stays clean', function (done) {
+    it('should add a ttl to the connected user set so it stays clean', function (done) {
       return this.ConnectedUsersManager.markUserAsDisconnected(
         this.project_id,
         this.client_id,
@@ -310,6 +382,163 @@ describe('ConnectedUsersManager', function () {
         }
       )
     })
+
+    describe('editing_session_mode', function () {
+      const cases = {
+        'should bump the metric when disconnecting from now empty room': {
+          nConnectedClients: 0,
+          labels: {
+            mode: 'disconnect',
+            status: 'empty',
+          },
+        },
+        'should bump the metric when disconnecting from now single room': {
+          nConnectedClients: 1,
+          labels: {
+            mode: 'disconnect',
+            status: 'single',
+          },
+        },
+        'should bump the metric when disconnecting from now multi room': {
+          nConnectedClients: 2,
+          labels: {
+            mode: 'disconnect',
+            status: 'multi',
+          },
+        },
+      }
+
+      for (const [name, { nConnectedClients, labels }] of Object.entries(
+        cases
+      )) {
+        it(name, function (done) {
+          this.rClient.exec.yields(null, [1, nConnectedClients])
+          this.ConnectedUsersManager.markUserAsDisconnected(
+            this.project_id,
+            this.client_id,
+            err => {
+              if (err) return done(err)
+              expect(this.Metrics.inc).to.have.been.calledWith(
+                'editing_session_mode',
+                1,
+                labels
+              )
+              done()
+            }
+          )
+        })
+      }
+    })
+
+    describe('projectNotEmptySince', function () {
+      it('should clear the projectNotEmptySince key when empty and skip metric if not set', function (done) {
+        this.rClient.exec.yields(null, [1, 0])
+        this.rClient.getdel.yields(null, '')
+        this.ConnectedUsersManager.markUserAsDisconnected(
+          this.project_id,
+          this.client_id,
+          err => {
+            if (err) return done(err)
+            expect(this.rClient.getdel).to.have.been.calledWith(
+              `projectNotEmptySince:{${this.project_id}}`
+            )
+            expect(this.Metrics.histogram).to.not.have.been.called
+            done()
+          }
+        )
+      })
+      it('should clear the projectNotEmptySince key when empty and record metric if set', function (done) {
+        this.rClient.exec.yields(null, [1, 0])
+        tk.freeze(1_234_000)
+        this.rClient.getdel.yields(null, '1230')
+        this.ConnectedUsersManager.markUserAsDisconnected(
+          this.project_id,
+          this.client_id,
+          err => {
+            if (err) return done(err)
+            expect(this.rClient.getdel).to.have.been.calledWith(
+              `projectNotEmptySince:{${this.project_id}}`
+            )
+            expect(this.Metrics.histogram).to.have.been.calledWith(
+              'project_not_empty_since',
+              4,
+              sinon.match.any,
+              { status: 'empty' }
+            )
+            done()
+          }
+        )
+      })
+      it('should set projectNotEmptySince key when single and skip metric if not set before', function (done) {
+        this.rClient.exec.yields(null, [1, 1])
+        tk.freeze(1_233_001) // should ceil up
+        this.rClient.set.yields(null, '')
+        this.ConnectedUsersManager.markUserAsDisconnected(
+          this.project_id,
+          this.client_id,
+          err => {
+            if (err) return done(err)
+            expect(this.rClient.set).to.have.been.calledWith(
+              `projectNotEmptySince:{${this.project_id}}`,
+              '1234',
+              'NX',
+              'GET',
+              'EX',
+              31 * 24 * 60 * 60
+            )
+            expect(this.Metrics.histogram).to.not.have.been.called
+            done()
+          }
+        )
+      })
+      const cases = {
+        'should set projectNotEmptySince key when single and record metric if set before':
+          {
+            nConnectedClients: 1,
+            labels: {
+              status: 'single',
+            },
+          },
+        'should set projectNotEmptySince key when multi and record metric if set before':
+          {
+            nConnectedClients: 2,
+            labels: {
+              status: 'multi',
+            },
+          },
+      }
+      for (const [name, { nConnectedClients, labels }] of Object.entries(
+        cases
+      )) {
+        it(name, function (done) {
+          this.rClient.exec.yields(null, [1, nConnectedClients])
+          tk.freeze(1_235_000)
+          this.rClient.set.yields(null, '1230')
+          this.ConnectedUsersManager.markUserAsDisconnected(
+            this.project_id,
+            this.client_id,
+            err => {
+              if (err) return done(err)
+              expect(this.rClient.set).to.have.been.calledWith(
+                `projectNotEmptySince:{${this.project_id}}`,
+                '1235',
+                'NX',
+                'GET',
+                'EX',
+                31 * 24 * 60 * 60
+              )
+              expect(this.Metrics.histogram).to.have.been.calledWith(
+                'project_not_empty_since',
+                5,
+                sinon.match.any,
+                labels
+              )
+              done()
+            }
+          )
+        })
+      }
+    })
   })
 
   describe('_getConnectedUser', function () {