diff --git a/libraries/metrics/http.js b/libraries/metrics/http.js index a5c6e54779..8fba2a6310 100644 --- a/libraries/metrics/http.js +++ b/libraries/metrics/http.js @@ -75,4 +75,4 @@ class RequestLogger { } } -module.exports = { monitor, RequestLogger } +module.exports = { monitor, RequestLogger, getRoutePath } diff --git a/services/web/app/src/Features/Analytics/AnalyticsManager.mjs b/services/web/app/src/Features/Analytics/AnalyticsManager.mjs index b763ae494b..915799d02c 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsManager.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsManager.mjs @@ -65,7 +65,10 @@ async function recordEventForUser(userId, event, segmentation) { if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) { return } - const analyticsId = await UserAnalyticsIdCache.get(userId) + const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + userId, + `recordEventForUser:${event}` + ) if (analyticsId) { _recordEvent({ analyticsId, userId, event, segmentation, isLoggedIn: true }) } @@ -113,7 +116,10 @@ async function setUserPropertyForUser(userId, propertyName, propertyValue) { _checkPropertyValue(propertyValue) - const analyticsId = await UserAnalyticsIdCache.get(userId) + const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + userId, + `setUserPropertyForUser:${propertyName}` + ) if (analyticsId) { await _setUserProperty({ analyticsId, propertyName, propertyValue }) } @@ -447,7 +453,11 @@ async function analyticsIdMiddleware(req, res, next) { const sessionUser = SessionManager.getSessionUser(session) if (sessionUser) { - session.analyticsId = await UserAnalyticsIdCache.get(sessionUser._id) + session.analyticsId = await UserAnalyticsIdCache.getWithMetrics( + sessionUser._id, + // Do not drill down further, this middleware is on all endpoints. + 'analyticsIdMiddleware' + ) } else if (!session.analyticsId) { // generate an `analyticsId` if needed session.analyticsId = crypto.randomUUID() diff --git a/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs b/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs index 3a29f7fc43..fbfec7a782 100644 --- a/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs +++ b/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs @@ -1,6 +1,7 @@ import UserGetter from '../User/UserGetter.mjs' import { CacheLoader } from 'cache-flow' import { callbackify } from 'node:util' +import Metrics from '@overleaf/metrics' class UserAnalyticsIdCache extends CacheLoader { constructor() { @@ -22,6 +23,19 @@ class UserAnalyticsIdCache extends CacheLoader { return userId.toString() } } + + get() { + throw new Error('use UserAnalyticsIdCache.getWithMetrics') + } + + async getWithMetrics(userId, path) { + const { value, cached } = await this.getWithMetadata(userId) + Metrics.inc('user_analytics_id_cache', 1, { + status: cached ? 'hit' : 'miss', + path, + }) + return value + } } const userAnalyticsIdCache = new UserAnalyticsIdCache() diff --git a/services/web/app/src/Features/Compile/CompileManager.mjs b/services/web/app/src/Features/Compile/CompileManager.mjs index b10f05945c..e176de4b21 100644 --- a/services/web/app/src/Features/Compile/CompileManager.mjs +++ b/services/web/app/src/Features/Compile/CompileManager.mjs @@ -138,7 +138,10 @@ async function _getUserCompileLimits(userId) { ownerFeatures.compileGroup = 'alpha' } - const analyticsId = await UserAnalyticsIdCache.get(owner._id) + const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + owner._id, + '_getUserCompileLimits' + ) const compileGroup = ownerFeatures.compileGroup || Settings.defaultFeatures.compileGroup diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs index a1ebb7c5dd..fd70276de1 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs @@ -120,7 +120,10 @@ async function getAssignmentForUser( return _getNonSaasAssignment(splitTestName) } - const analyticsId = await UserAnalyticsIdCache.get(userId) + const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + userId, + `getAssignmentForUser:${splitTestName}` + ) return _getAssignment(splitTestName, { analyticsId, userId, sync }) } catch (error) { logger.error({ err: error }, 'Failed to get split test assignment for user') @@ -227,7 +230,11 @@ async function getActiveAssignmentsForUser( return {} } - const user = await SplitTestUserGetter.promises.getUser(userId) + const user = await SplitTestUserGetter.promises.getUser( + userId, + null, + 'getActiveAssignmentsForUser' + ) if (user == null) { return {} } @@ -402,7 +409,11 @@ async function _getAssignment( user = user || (userId && - (await SplitTestUserGetter.promises.getUser(userId, splitTestName))) + (await SplitTestUserGetter.promises.getUser( + userId, + splitTestName, + `_getAssignment:${splitTestName}` + ))) const metadata = await _getAssignmentMetadata(analyticsId, user, splitTest) const { activeForUser, selectedVariantName, phase, versionNumber } = metadata @@ -651,7 +662,12 @@ async function _recordAssignment({ assignedAt: new Date(), } user = - user || (await SplitTestUserGetter.promises.getUser(userId, splitTestName)) + user || + (await SplitTestUserGetter.promises.getUser( + userId, + splitTestName, + `_recordAssignment:${splitTestName}` + )) if (user) { const assignedSplitTests = user.splitTests || [] const assignmentLog = assignedSplitTests[splitTestName] || [] diff --git a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs index d6248b3771..a392d7ed30 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.mjs @@ -164,7 +164,13 @@ async function sessionMaintenance(req, user) { Metrics.inc('split_test_session_maintenance', 1, { status: 'start' }) if (sessionUser) { - user = user || (await SplitTestUserGetter.promises.getUser(sessionUser._id)) + user = + user || + (await SplitTestUserGetter.promises.getUser( + sessionUser._id, + null, + `sessionMaintenance:${Metrics.http.getRoutePath(req)}` + )) if ( Boolean(sessionUser.alphaProgram) !== Boolean(user.alphaProgram) || Boolean(sessionUser.betaProgram) !== Boolean(user.betaProgram) diff --git a/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs b/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs index e77150c2ce..b7efe1c529 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestUserGetter.mjs @@ -2,7 +2,8 @@ import { callbackify } from 'node:util' import Metrics from '@overleaf/metrics' import UserGetter from '../User/UserGetter.mjs' -async function getUser(id, splitTestName) { +async function getUser(id, splitTestName, path) { + Metrics.inc('split_test_get_user', 1, { path }) const projection = { analyticsId: 1, alphaProgram: 1, diff --git a/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs b/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs index eaf1b28fe3..2ddbe01008 100755 --- a/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs +++ b/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs @@ -437,7 +437,10 @@ async function processMigration(input, commit) { } // 7. If commit mode, perform migration - const analyticsId = await UserAnalyticsIdCache.get(overleafUserId) + const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + overleafUserId, + 'script' // no-op, metrics are not collected from scripts. + ) const mongoUser = await User.findOne({ _id: overleafUserId, }).exec() diff --git a/services/web/test/unit/bootstrap.mjs b/services/web/test/unit/bootstrap.mjs index 085a907664..99b1afad5a 100644 --- a/services/web/test/unit/bootstrap.mjs +++ b/services/web/test/unit/bootstrap.mjs @@ -58,6 +58,9 @@ vi.mock('@overleaf/metrics', () => { return 1 } }, + http: { + getRoutePath: sinon.stub().returns('project_Project_id'), + }, prom: { Counter: sinon.stub(), Histogram: sinon.stub() }, mongodb: { monitor: sinon.stub() }, }, diff --git a/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs b/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs index a44690dd6d..0aa6a6ce94 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs +++ b/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs @@ -92,7 +92,7 @@ describe('AnalyticsManager', function () { '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', () => ({ default: (ctx.UserAnalyticsIdCache = { - get: sinon.stub().resolves(ctx.analyticsId), + getWithMetrics: sinon.stub().resolves(ctx.analyticsId), }), }) ) @@ -391,7 +391,7 @@ describe('AnalyticsManager', function () { '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', () => ({ default: (ctx.UserAnalyticsIdCache = { - get: sinon.stub().resolves(ctx.analyticsId), + getWithMetrics: sinon.stub().resolves(ctx.analyticsId), }), }) ) @@ -439,7 +439,7 @@ describe('AnalyticsManager', function () { }) it('sets session.analyticsId with a legacy user session without an analyticsId', async function (ctx) { - ctx.UserAnalyticsIdCache.get.resolves(ctx.userId) + ctx.UserAnalyticsIdCache.getWithMetrics.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, @@ -450,7 +450,7 @@ describe('AnalyticsManager', function () { }) it('updates session.analyticsId with a legacy user session without an analyticsId if different', async function (ctx) { - ctx.UserAnalyticsIdCache.get.resolves(ctx.userId) + ctx.UserAnalyticsIdCache.getWithMetrics.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, @@ -462,7 +462,7 @@ describe('AnalyticsManager', function () { }) it('does not update session.analyticsId with a legacy user session without an analyticsId if same', async function (ctx) { - ctx.UserAnalyticsIdCache.get.resolves(ctx.userId) + ctx.UserAnalyticsIdCache.getWithMetrics.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, diff --git a/services/web/test/unit/src/Compile/CompileManager.test.mjs b/services/web/test/unit/src/Compile/CompileManager.test.mjs index 0274d98bd3..217e191bbc 100644 --- a/services/web/test/unit/src/Compile/CompileManager.test.mjs +++ b/services/web/test/unit/src/Compile/CompileManager.test.mjs @@ -68,7 +68,7 @@ describe('CompileManager', function () { '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', () => ({ default: (ctx.UserAnalyticsIdCache = { - get: sinon.stub().resolves('abc'), + getWithMetrics: sinon.stub().resolves('abc'), }), }) )