Merge pull request #7347 from overleaf/bg-record-snapshot-metrics

record metrics for docupdater range operations

GitOrigin-RevId: 41c500630dd81670d2798d388f2961a19df76b63

services/document-updater/app/js/RangesManager.js

@@ -13,8 +13,11 @@
 let RangesManager
 const RangesTracker = require('./RangesTracker')
 const logger = require('@overleaf/logger')
+const Metrics = require('./Metrics')
 const _ = require('lodash')
 
+const RANGE_DELTA_BUCKETS = [0, 1, 2, 3, 4, 5, 10, 20, 50]
+
 module.exports = RangesManager = {
   MAX_COMMENTS: 500,
   MAX_CHANGES: 2000,
@@ -32,7 +35,8 @@ module.exports = RangesManager = {
     }
     const { changes, comments } = _.cloneDeep(entries)
     const rangesTracker = new RangesTracker(changes, comments)
-    const emptyRangeCountBefore = RangesManager._emptyRangesCount(rangesTracker)
+    const [emptyRangeCountBefore, totalRangeCountBefore] =
+      RangesManager._emptyRangesCount(rangesTracker)
     for (const update of Array.from(updates)) {
       rangesTracker.track_changes = !!update.meta.tc
       if (update.meta.tc) {
@@ -74,8 +78,18 @@ module.exports = RangesManager = {
       return callback(error)
     }
 
-    const emptyRangeCountAfter = RangesManager._emptyRangesCount(rangesTracker)
+    const [emptyRangeCountAfter, totalRangeCountAfter] =
+      RangesManager._emptyRangesCount(rangesTracker)
     const rangesWereCollapsed = emptyRangeCountAfter > emptyRangeCountBefore
+    // monitor the change in range count, we may want to snapshot before large decreases
+    if (totalRangeCountAfter < totalRangeCountBefore) {
+      Metrics.histogram(
+        'range-delta',
+        totalRangeCountBefore - totalRangeCountAfter,
+        RANGE_DELTA_BUCKETS,
+        { status_code: rangesWereCollapsed ? 'saved' : 'unsaved' }
+      )
+    }
     const response = RangesManager._getRanges(rangesTracker)
     logger.debug(
       {
@@ -144,19 +158,22 @@ module.exports = RangesManager = {
   },
 
   _emptyRangesCount(ranges) {
-    let count = 0
+    let emptyCount = 0
+    let totalCount = 0
     for (const comment of Array.from(ranges.comments || [])) {
+      totalCount++
       if (comment.op.c === '') {
-        count++
+        emptyCount++
       }
     }
     for (const change of Array.from(ranges.changes || [])) {
+      totalCount++
       if (change.op.i != null) {
         if (change.op.i === '') {
-          count++
+          emptyCount++
         }
       }
     }
-    return count
+    return [emptyCount, totalCount]
   },
 }

services/document-updater/app/js/UpdateManager.js

@@ -239,6 +239,7 @@ module.exports = UpdateManager = {
                           return callback(error)
                         }
                         if (ranges_were_collapsed) {
+                          Metrics.inc('doc-snapshot')
                           logger.debug(
                             {
                               project_id,

services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js

@@ -18,7 +18,9 @@ const SandboxedModule = require('sandboxed-module')
 
 describe('RangesManager', function () {
   beforeEach(function () {
-    this.RangesManager = SandboxedModule.require(modulePath)
+    this.RangesManager = SandboxedModule.require(modulePath, {
+      requires: { './Metrics': (this.Metrics = { histogram: sinon.stub() }) },
+    })
 
     this.doc_id = 'doc-id-123'
     this.project_id = 'project-id-123'
@@ -302,7 +304,7 @@ describe('RangesManager', function () {
       })
     })
 
-    return describe('with an update that collapses a range', function () {
+    describe('with an update that collapses a range', function () {
       beforeEach(function () {
         this.updates = [
           {
@@ -351,6 +353,70 @@ describe('RangesManager', function () {
         return expect(ranges_were_collapsed).to.equal(true)
       })
     })
+
+    describe('with an update that deletes ranges', function () {
+      beforeEach(function () {
+        this.updates = [
+          {
+            meta: {
+              user_id: this.user_id,
+            },
+            op: [
+              {
+                d: 'one two three four five',
+                p: 0,
+              },
+            ],
+          },
+        ]
+        this.entries = {
+          comments: [
+            {
+              op: {
+                c: 'n',
+                p: 1,
+                t: 'thread-id-2',
+              },
+              metadata: {
+                user_id: this.user_id,
+              },
+            },
+          ],
+          changes: [
+            {
+              op: {
+                i: 'hello',
+                p: 1,
+                t: 'thread-id-2',
+              },
+              metadata: {
+                user_id: this.user_id,
+              },
+            },
+          ],
+        }
+        return this.RangesManager.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.entries,
+          this.updates,
+          this.newDocLines,
+          this.callback
+        )
+      })
+
+      it('should increment the range-delta histogram', function () {
+        this.Metrics.histogram.called.should.equal(true)
+      })
+
+      return it('should return ranges_were_collapsed == true', function () {
+        this.callback.called.should.equal(true)
+        const [error, entries, ranges_were_collapsed] = Array.from(
+          this.callback.args[0]
+        )
+        return expect(ranges_were_collapsed).to.equal(true)
+      })
+    })
   })
 
   return describe('acceptChanges', function () {

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -30,6 +30,7 @@ describe('UpdateManager', function () {
         './ShareJsUpdateManager': (this.ShareJsUpdateManager = {}),
         './HistoryManager': (this.HistoryManager = {}),
         './Metrics': (this.Metrics = {
+          inc: sinon.stub(),
           Timer: (Timer = (function () {
             Timer = class Timer {
               static initClass() {
@@ -489,6 +490,10 @@ describe('UpdateManager', function () {
         )
       })
 
+      it('should increment the doc-snapshot metric', function () {
+        this.Metrics.inc.calledWith('doc-snapshot').should.equal(true)
+      })
+
       return it('should call SnapshotManager.recordSnapshot', function () {
         return this.SnapshotManager.recordSnapshot
           .calledWith(