Merge pull request #17268 from overleaf/dp-remove-old-mongo-metrics

Remove timeAsyncMethod mongo metrics

GitOrigin-RevId: 1ba3a1fd51b9d0766355c31791ae9836d832afe8

libraries/metrics/index.js

@@ -180,4 +180,3 @@ module.exports.leaked_sockets = require('./leaked_sockets')
 module.exports.event_loop = require('./event_loop')
 module.exports.memory = require('./memory')
 module.exports.mongodb = require('./mongodb')
-module.exports.timeAsyncMethod = require('./timeAsyncMethod')

libraries/metrics/test/unit/js/timeAsyncMethodTests.js

@@ -1,201 +0,0 @@
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const chai = require('chai')
-const { expect } = chai
-const path = require('path')
-const modulePath = path.join(__dirname, '../../../timeAsyncMethod.js')
-const SandboxedModule = require('sandboxed-module')
-const sinon = require('sinon')
-
-describe('timeAsyncMethod', function () {
-  beforeEach(function () {
-    this.Timer = { done: sinon.stub() }
-    this.TimerConstructor = sinon.stub().returns(this.Timer)
-    this.metrics = {
-      Timer: this.TimerConstructor,
-      inc: sinon.stub(),
-    }
-    this.timeAsyncMethod = SandboxedModule.require(modulePath, {
-      requires: {
-        './index': this.metrics,
-      },
-    })
-
-    return (this.testObject = {
-      nextNumber(n, callback) {
-        return setTimeout(() => callback(null, n + 1), 100)
-      },
-    })
-  })
-
-  it('should have the testObject behave correctly before wrapping', function (done) {
-    return this.testObject.nextNumber(2, (err, result) => {
-      expect(err).to.not.exist
-      expect(result).to.equal(3)
-      return done()
-    })
-  })
-
-  it('should wrap method without error', function (done) {
-    this.timeAsyncMethod(
-      this.testObject,
-      'nextNumber',
-      'someContext.TestObject'
-    )
-    return done()
-  })
-
-  it('should transparently wrap method invocation in timer', function (done) {
-    this.timeAsyncMethod(
-      this.testObject,
-      'nextNumber',
-      'someContext.TestObject'
-    )
-    return this.testObject.nextNumber(2, (err, result) => {
-      expect(err).to.not.exist
-      expect(result).to.equal(3)
-      expect(this.TimerConstructor.callCount).to.equal(1)
-      expect(this.Timer.done.callCount).to.equal(1)
-      return done()
-    })
-  })
-
-  it('should increment success count', function (done) {
-    this.metrics.inc = sinon.stub()
-    this.timeAsyncMethod(
-      this.testObject,
-      'nextNumber',
-      'someContext.TestObject'
-    )
-    return this.testObject.nextNumber(2, (err, result) => {
-      if (err) {
-        return done(err)
-      }
-      expect(this.metrics.inc.callCount).to.equal(1)
-      expect(
-        this.metrics.inc.calledWith('someContext_result', 1, {
-          method: 'TestObject_nextNumber',
-          status: 'success',
-        })
-      ).to.equal(true)
-      return done()
-    })
-  })
-
-  describe('when base method produces an error', function () {
-    beforeEach(function () {
-      this.metrics.inc = sinon.stub()
-      return (this.testObject.nextNumber = function (n, callback) {
-        return setTimeout(() => callback(new Error('woops')), 100)
-      })
-    })
-
-    it('should propagate the error transparently', function (done) {
-      this.timeAsyncMethod(
-        this.testObject,
-        'nextNumber',
-        'someContext.TestObject'
-      )
-      return this.testObject.nextNumber(2, (err, result) => {
-        expect(err).to.exist
-        expect(err).to.be.instanceof(Error)
-        expect(result).to.not.exist
-        return done()
-      })
-    })
-
-    return it('should increment failure count', function (done) {
-      this.timeAsyncMethod(
-        this.testObject,
-        'nextNumber',
-        'someContext.TestObject'
-      )
-      return this.testObject.nextNumber(2, (err, result) => {
-        expect(err).to.exist
-        expect(this.metrics.inc.callCount).to.equal(1)
-        expect(
-          this.metrics.inc.calledWith('someContext_result', 1, {
-            method: 'TestObject_nextNumber',
-            status: 'failed',
-          })
-        ).to.equal(true)
-        return done()
-      })
-    })
-  })
-
-  describe('when a logger is supplied', function () {
-    beforeEach(function () {
-      return (this.logger = { debug: sinon.stub(), log: sinon.stub() })
-    })
-
-    return it('should also call logger.debug', function (done) {
-      this.timeAsyncMethod(
-        this.testObject,
-        'nextNumber',
-        'someContext.TestObject',
-        this.logger
-      )
-      return this.testObject.nextNumber(2, (err, result) => {
-        expect(err).to.not.exist
-        expect(result).to.equal(3)
-        expect(this.TimerConstructor.callCount).to.equal(1)
-        expect(this.Timer.done.callCount).to.equal(1)
-        expect(this.logger.debug.callCount).to.equal(1)
-        return done()
-      })
-    })
-  })
-
-  describe('when the wrapper cannot be applied', function () {
-    beforeEach(function () {})
-
-    return it('should raise an error', function () {
-      const badWrap = () => {
-        return this.timeAsyncMethod(
-          this.testObject,
-          'DEFINITELY_NOT_A_REAL_METHOD',
-          'someContext.TestObject'
-        )
-      }
-      return expect(badWrap).to.throw(
-        /^.*expected object property 'DEFINITELY_NOT_A_REAL_METHOD' to be a function.*$/
-      )
-    })
-  })
-
-  return describe('when the wrapped function is not using a callback', function () {
-    beforeEach(function () {
-      this.realMethod = sinon.stub().returns(42)
-      return (this.testObject.nextNumber = this.realMethod)
-    })
-
-    it('should not throw an error', function () {
-      this.timeAsyncMethod(
-        this.testObject,
-        'nextNumber',
-        'someContext.TestObject'
-      )
-      const badCall = () => {
-        return this.testObject.nextNumber(2)
-      }
-      return expect(badCall).to.not.throw(Error)
-    })
-
-    return it('should call the underlying method', function () {
-      this.timeAsyncMethod(
-        this.testObject,
-        'nextNumber',
-        'someContext.TestObject'
-      )
-      const result = this.testObject.nextNumber(12)
-      expect(this.realMethod.callCount).to.equal(1)
-      expect(this.realMethod.calledWith(12)).to.equal(true)
-      return expect(result).to.equal(42)
-    })
-  })
-})

libraries/metrics/timeAsyncMethod.js

@@ -1,86 +0,0 @@
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS201: Simplify complex destructure assignments
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-
-module.exports = function (obj, methodName, prefix, logger) {
-  let modifedMethodName
-  const metrics = require('./index')
-
-  if (typeof obj[methodName] !== 'function') {
-    throw new Error(
-      `[Metrics] expected object property '${methodName}' to be a function`
-    )
-  }
-
-  const key = `${prefix}.${methodName}`
-
-  const realMethod = obj[methodName]
-
-  const splitPrefix = prefix.split('.')
-  const startPrefix = splitPrefix[0]
-
-  if (splitPrefix[1] != null) {
-    modifedMethodName = `${splitPrefix[1]}_${methodName}`
-  } else {
-    modifedMethodName = methodName
-  }
-  return (obj[methodName] = function (...originalArgs) {
-    const adjustedLength = Math.max(originalArgs.length, 1)
-    const firstArgs = originalArgs.slice(0, adjustedLength - 1)
-    const callback = originalArgs[adjustedLength - 1]
-
-    if (callback == null || typeof callback !== 'function') {
-      if (logger != null) {
-        logger.debug(
-          `[Metrics] expected wrapped method '${methodName}' to be invoked with a callback`
-        )
-      }
-      return realMethod.apply(this, originalArgs)
-    }
-
-    const timer = new metrics.Timer(startPrefix, 1, {
-      method: modifedMethodName,
-    })
-
-    return realMethod.call(
-      this,
-      ...Array.from(firstArgs),
-      function (...callbackArgs) {
-        const elapsedTime = timer.done()
-        const possibleError = callbackArgs[0]
-        if (possibleError != null) {
-          metrics.inc(`${startPrefix}_result`, 1, {
-            status: 'failed',
-            method: modifedMethodName,
-          })
-        } else {
-          metrics.inc(`${startPrefix}_result`, 1, {
-            status: 'success',
-            method: modifedMethodName,
-          })
-        }
-        if (logger != null) {
-          const loggableArgs = {}
-          try {
-            for (let idx = 0; idx < firstArgs.length; idx++) {
-              const arg = firstArgs[idx]
-              if (arg.toString().match(/^[0-9a-f]{24}$/)) {
-                loggableArgs[`${idx}`] = arg
-              }
-            }
-          } catch (error) {}
-          logger.debug(
-            { key, args: loggableArgs, elapsedTime },
-            '[Metrics] timed async method call'
-          )
-        }
-        return callback.apply(this, callbackArgs)
-      }
-    )
-  })
-}

services/docstore/test/unit/js/MongoManagerTests.js

@@ -22,7 +22,6 @@ describe('MongoManager', function () {
           db: this.db,
           ObjectId,
         },
-        '@overleaf/metrics': { timeAsyncMethod: sinon.stub() },
         '@overleaf/settings': {
           max_deleted_docs: 42,
           docstore: { archivingLockDurationMs: 5000 },

services/notifications/app/js/Notifications.js

@@ -9,12 +9,10 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let Notifications
 const logger = require('@overleaf/logger')
 const { db, ObjectId } = require('./mongodb')
-const metrics = require('@overleaf/metrics')
 
-module.exports = Notifications = {
+module.exports = {
   getUserNotifications(userId, callback) {
     if (callback == null) {
       callback = function () {}
@@ -132,6 +130,3 @@ module.exports = Notifications = {
     db.notifications.deleteOne(searchOps, callback)
   },
 }
-;['getUserNotifications', 'addNotification'].map(method =>
-  metrics.timeAsyncMethod(Notifications, method, 'mongo.Notifications', logger)
-)

services/notifications/test/unit/js/NotificationsTests.js

@@ -41,7 +41,6 @@ describe('Notifications Tests', function () {
       requires: {
         '@overleaf/settings': {},
         './mongodb': { db: this.db, ObjectId },
-        '@overleaf/metrics': { timeAsyncMethod: sinon.stub() },
       },
     })
 

services/web/app/src/Features/Institutions/InstitutionsAPI.js

@@ -1,7 +1,6 @@
 const { callbackify } = require('util')
 const OError = require('@overleaf/o-error')
 const logger = require('@overleaf/logger')
-const metrics = require('@overleaf/metrics')
 const settings = require('@overleaf/settings')
 const request = require('requestretry')
 const { promisifyAll } = require('@overleaf/promise-utils')
@@ -373,18 +372,6 @@ function makeAffiliationRequest(options, callback) {
     callback(null, body)
   })
 }
-;[
-  'getInstitutionAffiliations',
-  'getConfirmedInstitutionAffiliations',
-  'getUserAffiliations',
-].map(method =>
-  metrics.timeAsyncMethod(
-    InstitutionsAPI,
-    method,
-    'mongo.InstitutionsAPI',
-    logger
-  )
-)
 
 InstitutionsAPI.promises = promisifyAll(InstitutionsAPI, {
   without: [

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

@@ -1,4 +1,3 @@
-const logger = require('@overleaf/logger')
 const OError = require('@overleaf/o-error')
 const metrics = require('@overleaf/metrics')
 const Settings = require('@overleaf/settings')
@@ -242,10 +241,3 @@ module.exports = {
     createExampleProject,
   },
 }
-
-metrics.timeAsyncMethod(
-  module.exports,
-  'createBlankProject',
-  'mongo.ProjectCreationHandler',
-  logger
-)

services/web/app/src/Features/Spelling/LearnedWordsManager.js

@@ -1,6 +1,4 @@
 const { db } = require('../../infrastructure/mongodb')
-const logger = require('@overleaf/logger')
-const metrics = require('@overleaf/metrics')
 const { promisify } = require('util')
 const OError = require('@overleaf/o-error')
 const Settings = require('@overleaf/settings')
@@ -94,11 +92,3 @@ const promises = {
 LearnedWordsManager.promises = promises
 
 module.exports = LearnedWordsManager
-;['learnWord', 'unlearnWord', 'getLearnedWords'].map(method =>
-  metrics.timeAsyncMethod(
-    LearnedWordsManager,
-    method,
-    'mongo.LearnedWordsManager',
-    logger
-  )
-)

services/web/app/src/Features/User/UserGetter.js

@@ -1,7 +1,5 @@
 const { callbackify } = require('util')
 const { db } = require('../../infrastructure/mongodb')
-const metrics = require('@overleaf/metrics')
-const logger = require('@overleaf/logger')
 const moment = require('moment')
 const settings = require('@overleaf/settings')
 const { promisifyAll } = require('@overleaf/promise-utils')
@@ -305,16 +303,6 @@ const decorateFullEmails = (
 
   return emailsData
 }
-;[
-  'getUser',
-  'getUserEmail',
-  'getUserByMainEmail',
-  'getUserByAnyEmail',
-  'getUsers',
-  'ensureUniqueEmailAddress',
-].map(method =>
-  metrics.timeAsyncMethod(UserGetter, method, 'mongo.UserGetter', logger)
-)
 
 UserGetter.promises = promisifyAll(UserGetter, {
   without: ['getSsoUsersAtInstitution', 'getUserFullEmails'],

services/web/test/unit/src/Institutions/InstitutionsAPITests.js

@@ -20,9 +20,6 @@ describe('InstitutionsAPI', function () {
     }
     this.InstitutionsAPI = SandboxedModule.require(modulePath, {
       requires: {
-        '@overleaf/metrics': {
-          timeAsyncMethod: sinon.stub(),
-        },
         '@overleaf/settings': this.settings,
         requestretry: this.request,
         '@overleaf/fetch-utils': {

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

@@ -53,9 +53,6 @@ describe('ProjectGetter', function () {
     this.ProjectGetter = SandboxedModule.require(modulePath, {
       requires: {
         '../../infrastructure/mongodb': { db: this.db, ObjectId },
-        '@overleaf/metrics': {
-          timeAsyncMethod: sinon.stub(),
-        },
         '../../models/Project': {
           Project: this.Project,
         },

services/web/test/unit/src/Spelling/LearnedWordsManagerTests.js

@@ -21,7 +21,6 @@ describe('LearnedWordsManager', function () {
       requires: {
         '../../infrastructure/mongodb': { db: this.db },
         '@overleaf/metrics': {
-          timeAsyncMethod: sinon.stub(),
           inc: sinon.stub(),
         },
         '@overleaf/settings': {

services/web/test/unit/src/User/UserCreatorTests.js

@@ -19,7 +19,6 @@ describe('UserCreator', function () {
         '../../models/User': {
           User: this.UserModel,
         },
-        '@overleaf/metrics': { timeAsyncMethod() {} },
         '../../infrastructure/Features': (this.Features = {
           hasFeature: sinon.stub().returns(false),
         }),

services/web/test/unit/src/User/UserGetterTests.js

@@ -49,9 +49,6 @@ describe('UserGetter', function () {
       requires: {
         '../Helpers/Mongo': { normalizeQuery, normalizeMultiQuery },
         '../../infrastructure/mongodb': this.Mongo,
-        '@overleaf/metrics': {
-          timeAsyncMethod: sinon.stub(),
-        },
         '@overleaf/settings': (this.settings = {
           reconfirmNotificationDays: 14,
         }),