From 1094b0570b22a3b466e960cae395359bc58fbaec Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Tue, 31 Aug 2021 14:46:23 +0200 Subject: [PATCH] Merge pull request #4922 from overleaf/jpa-web-clsi-metrics [web] add new metrics for tracking initial assign/switch of clsi backend GitOrigin-RevId: 4fb15ee8727cb397d1e44a86efb7d4833626bc6b --- .../src/Features/Compile/ClsiCookieManager.js | 31 +++++++++++++------ .../app/src/Features/Compile/ClsiManager.js | 4 ++- .../src/Compile/ClsiCookieManagerTests.js | 7 ++++- .../test/unit/src/Compile/ClsiManagerTests.js | 2 +- 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiCookieManager.js b/services/web/app/src/Features/Compile/ClsiCookieManager.js index 332b7d85a6..620f27b450 100644 --- a/services/web/app/src/Features/Compile/ClsiCookieManager.js +++ b/services/web/app/src/Features/Compile/ClsiCookieManager.js @@ -22,6 +22,7 @@ if (Settings.redis.clsi_cookie_secondary != null) { } const Cookie = require('cookie') const logger = require('logger-sharelatex') +const Metrics = require('@overleaf/metrics') const clsiCookiesEnabled = (Settings.clsiCookie != null ? Settings.clsiCookie.key : undefined) != null && @@ -72,15 +73,21 @@ module.exports = function (backendGroup) { }) return callback(err) } - this.setServerId(project_id, user_id, res, function (err, serverId) { - if (err != null) { - logger.warn( - { err, project_id }, - 'error setting server id via populate request' - ) + this.setServerId( + project_id, + user_id, + res, + null, + function (err, serverId) { + if (err != null) { + logger.warn( + { err, project_id }, + 'error setting server id via populate request' + ) + } + return callback(err, serverId) } - return callback(err, serverId) - }) + ) }) }, @@ -93,7 +100,7 @@ module.exports = function (backendGroup) { return cookies != null ? cookies[Settings.clsiCookie.key] : undefined }, - setServerId(project_id, user_id, response, callback) { + setServerId(project_id, user_id, response, previous, callback) { if (callback == null) { callback = function (err, serverId) {} } @@ -109,6 +116,12 @@ module.exports = function (backendGroup) { err => callback(err, undefined) ) } + if (!previous) { + // Initial assignment of a user+project or after clearing cache. + Metrics.inc('clsi-lb-assign-initial-backend') + } else { + Metrics.inc('clsi-lb-switch-backend') + } if (rclient_secondary != null) { this._setServerIdInRedis( rclient_secondary, diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 7c395f49b0..fb4fdce487 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -328,6 +328,7 @@ const ClsiManager = { projectId, userId, response, + clsiServerId, (err, newClsiServerId) => { if (err != null) { callback( @@ -420,7 +421,7 @@ const ClsiManager = { NewBackendCloudClsiCookieManager.getCookieJar( projectId, userId, - (err, jar) => { + (err, jar, clsiServerId) => { if (err != null) { return callback( OError.tag(err, 'error getting cookie jar for CLSI request', { @@ -444,6 +445,7 @@ const ClsiManager = { projectId, userId, response, + clsiServerId, err => { if (err != null) { return callback( diff --git a/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js b/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js index 6593c8ffe3..5fe7b2bc50 100644 --- a/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js @@ -118,7 +118,7 @@ describe('ClsiCookieManager', function () { this.request.post.callsArgWith(1, null, this.response) return (this.ClsiCookieManager.setServerId = sinon .stub() - .callsArgWith(3, null, 'clsi-9')) + .yields(null, 'clsi-9')) }) it('should make a request to the clsi', function (done) { @@ -160,6 +160,7 @@ describe('ClsiCookieManager', function () { this.project_id, this.user_id, this.response, + null, err => { this.redis.setex .calledWith( @@ -178,6 +179,7 @@ describe('ClsiCookieManager', function () { this.project_id, this.user_id, this.response, + null, (err, serverId) => { serverId.should.equal('clsi-8') return done() @@ -197,6 +199,7 @@ describe('ClsiCookieManager', function () { this.project_id, this.user_id, this.response, + null, (err, serverId) => { this.redis.setex.called.should.equal(false) return done() @@ -212,6 +215,7 @@ describe('ClsiCookieManager', function () { this.project_id, this.user_id, this.response, + null, (err, serverId) => { this.redis.setex.called.should.equal(false) return done() @@ -240,6 +244,7 @@ describe('ClsiCookieManager', function () { this.project_id, this.user_id, this.response, + null, (err, serverId) => { this.redis_secondary.setex .calledWith( diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index 6cc26bc243..b4389ab59e 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -9,7 +9,7 @@ describe('ClsiManager', function () { this.ClsiCookieManager = { clearServerId: sinon.stub().yields(), getCookieJar: sinon.stub().callsArgWith(2, null, this.jar), - setServerId: sinon.stub().callsArgWith(3), + setServerId: sinon.stub().yields(null), _getServerId: sinon.stub(), } this.ClsiStateManager = {