diff --git a/services/web/app/src/Features/Analytics/AnalyticsManager.mjs b/services/web/app/src/Features/Analytics/AnalyticsManager.mjs index 7d25613693..202a3446ea 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsManager.mjs +++ b/services/web/app/src/Features/Analytics/AnalyticsManager.mjs @@ -1,5 +1,5 @@ import SessionManager from '../Authentication/SessionManager.mjs' -import UserAnalyticsIdCache from './UserAnalyticsIdCache.mjs' +import UserAnalyticsDataCache from './UserAnalyticsDataCache.mjs' import Settings from '@overleaf/settings' import Metrics from '../../infrastructure/Metrics.mjs' import Queues from '../../infrastructure/Queues.mjs' @@ -34,7 +34,7 @@ const ONE_MINUTE_MS = 60 * 1000 const UUID_REGEXP = /^[\w]{8}(-[\w]{4}){3}-[\w]{12}$/ -function identifyUser(userId, analyticsId, isNewUser) { +function identifyUser(userId, analyticsId, isNewUser, isLabsUser = false) { if (!userId || !analyticsId || !analyticsId.toString().match(UUID_REGEXP)) { return } @@ -46,7 +46,13 @@ function identifyUser(userId, analyticsId, isNewUser) { 'analytics-events', { name: 'identify', - data: { userId, analyticsId, isNewUser, createdAt: new Date() }, + data: { + userId, + analyticsId, + isNewUser, + isLabsUser, + createdAt: new Date(), + }, }, ONE_MINUTE_MS ) @@ -65,12 +71,20 @@ async function recordEventForUser(userId, event, segmentation) { if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) { return } - const analyticsId = await UserAnalyticsIdCache.getWithMetrics( - userId, - `recordEventForUser:${event}` - ) + const { analyticsId, labsProgram } = + await UserAnalyticsDataCache.getAnalyticsData( + userId, + `recordEventForUser:${event}` + ) if (analyticsId) { - _recordEvent({ analyticsId, userId, event, segmentation, isLoggedIn: true }) + _recordEvent({ + analyticsId, + userId, + event, + segmentation, + isLabsUser: Boolean(labsProgram), + isLoggedIn: true, + }) } } @@ -91,10 +105,14 @@ function recordEventForSession(session, event, segmentation) { if (_isAnalyticsDisabled() || _isSmokeTestUser(userId)) { return } + + const isLabsUser = getIsLabsUserFromSession(session) + _recordEvent({ analyticsId, userId, event, + isLabsUser, segmentation, isLoggedIn: !!userId, createdAt: new Date(), @@ -116,12 +134,18 @@ async function setUserPropertyForUser(userId, propertyName, propertyValue) { _checkPropertyValue(propertyValue) - const analyticsId = await UserAnalyticsIdCache.getWithMetrics( - userId, - `setUserPropertyForUser:${propertyName}` - ) + const { analyticsId, labsProgram } = + await UserAnalyticsDataCache.getAnalyticsData( + userId, + `setUserPropertyForUser:${propertyName}` + ) if (analyticsId) { - await _setUserProperty({ analyticsId, propertyName, propertyValue }) + await _setUserProperty({ + analyticsId, + isLabsUser: Boolean(labsProgram), + propertyName, + propertyValue, + }) } } @@ -157,7 +181,13 @@ async function setUserPropertyForSession(session, propertyName, propertyValue) { _checkPropertyValue(propertyValue) if (analyticsId) { - await _setUserProperty({ analyticsId, propertyName, propertyValue }) + const isLabsUser = getIsLabsUserFromSession(session) + await _setUserProperty({ + analyticsId, + isLabsUser, + propertyName, + propertyValue, + }) } } @@ -311,7 +341,7 @@ function updateEditingSession(userId, projectId, countryCode, segmentation) { } function _recordEvent( - { analyticsId, userId, event, segmentation, isLoggedIn }, + { analyticsId, userId, event, segmentation, isLabsUser, isLoggedIn }, { delay } = {} ) { if (!_isAttributeValid(event)) { @@ -333,6 +363,7 @@ function _recordEvent( analyticsId, userId, event, + isLabsUser, segmentation, isLoggedIn: !!userId, createdAt: new Date(), @@ -348,6 +379,7 @@ function _recordEvent( userId, event, segmentation, + isLabsUser, isLoggedIn, createdAt: new Date(), }, @@ -361,7 +393,12 @@ function _recordEvent( }) } -async function _setUserProperty({ analyticsId, propertyName, propertyValue }) { +async function _setUserProperty({ + analyticsId, + isLabsUser, + propertyName, + propertyValue, +}) { if (!_isAttributeValid(propertyName)) { logger.info( { analyticsId, propertyName, propertyValue }, @@ -383,6 +420,7 @@ async function _setUserProperty({ analyticsId, propertyName, propertyValue }) { await analyticsUserPropertiesQueue .add('user-property', { analyticsId, + isLabsUser, propertyName, propertyValue, createdAt: new Date(), @@ -448,6 +486,11 @@ function getIdsFromSession(session) { return { analyticsId, userId } } +function getIsLabsUserFromSession(session) { + const user = SessionManager.getSessionUser(session) + return user?.labsProgram ?? false +} + async function analyticsIdMiddleware(req, res, next) { const session = req.session const sessionUser = SessionManager.getSessionUser(session) @@ -457,7 +500,7 @@ async function analyticsIdMiddleware(req, res, next) { session.analyticsId = sessionUser.analyticsId if (!session.analyticsId) { session.analyticsId = sessionUser.analyticsId = - await UserAnalyticsIdCache.getWithMetrics( + await UserAnalyticsDataCache.getAnalyticsId( sessionUser._id, // Do not drill down further, this middleware is on all endpoints. 'analyticsIdMiddleware' diff --git a/services/web/app/src/Features/Analytics/UserAnalyticsDataCache.mjs b/services/web/app/src/Features/Analytics/UserAnalyticsDataCache.mjs new file mode 100644 index 0000000000..e34b49228f --- /dev/null +++ b/services/web/app/src/Features/Analytics/UserAnalyticsDataCache.mjs @@ -0,0 +1,74 @@ +import UserGetter from '../User/UserGetter.mjs' +import { CacheLoader } from 'cache-flow' +import Metrics from '@overleaf/metrics' + +class UserAnalyticsDataCache extends CacheLoader { + constructor() { + super('user-analytics-id', { + expirationTime: 60, + maxSize: 10000, + }) + } + + async load(userId) { + const user = await UserGetter.promises.getUser(userId, { + analyticsId: 1, + labsProgram: 1, + }) + if (user) { + return { + analyticsId: user.analyticsId || user._id.toString(), + labsProgram: user.labsProgram, + } + } + } + + keyToString(userId) { + if (userId) { + return userId.toString() + } + } + + get() { + throw new Error('use UserAnalyticsDataCache.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 + } + + async getIsLabsUserWithMetrics(userId, path) { + const { value, cached } = await this.getWithMetadata(userId) + Metrics.inc('user_analytics_id_cache', 1, { + status: cached ? 'hit' : 'miss', + path, + }) + return value?.labsProgram + } +} + +const userAnalyticsDataCache = new UserAnalyticsDataCache() +export default { + async getAnalyticsId(userId, path) { + const value = await userAnalyticsDataCache.getWithMetrics(userId, path) + return value?.analyticsId + }, + async getAnalyticsData(userId, path) { + const data = await userAnalyticsDataCache.getWithMetrics(userId, path) + if (!data) { + return {} + } + return data + }, + async reset() { + await userAnalyticsDataCache.reset() + }, + async invalidateCache(userId) { + await userAnalyticsDataCache.delete(userId) + }, +} diff --git a/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs b/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs deleted file mode 100644 index fbfec7a782..0000000000 --- a/services/web/app/src/Features/Analytics/UserAnalyticsIdCache.mjs +++ /dev/null @@ -1,45 +0,0 @@ -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() { - super('user-analytics-id', { - expirationTime: 60, - maxSize: 10000, - }) - } - - async load(userId) { - const user = await UserGetter.promises.getUser(userId, { analyticsId: 1 }) - if (user) { - return user.analyticsId || user._id.toString() - } - } - - keyToString(userId) { - if (userId) { - 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() -userAnalyticsIdCache.callbacks = { - get: callbackify(userAnalyticsIdCache.get).bind(userAnalyticsIdCache), -} -export default userAnalyticsIdCache diff --git a/services/web/app/src/Features/Authentication/AuthenticationController.mjs b/services/web/app/src/Features/Authentication/AuthenticationController.mjs index a598f601c0..8ef0712a37 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationController.mjs +++ b/services/web/app/src/Features/Authentication/AuthenticationController.mjs @@ -84,6 +84,7 @@ const AuthenticationController = { analyticsId: user.analyticsId || user._id, alphaProgram: user.alphaProgram || undefined, // only store if set betaProgram: user.betaProgram || undefined, // only store if set + labsProgram: user.labsProgram || undefined, // only store if set } if (user.isAdmin) { lightUser.isAdmin = true @@ -660,7 +661,12 @@ function _loginAsyncHandlers(req, user, anonymousAnalyticsId, isNewUser) { ? 'saml' : req.user_info?.auth_provider || 'email-password', }) - Analytics.identifyUser(user._id, anonymousAnalyticsId, isNewUser) + Analytics.identifyUser( + user._id, + anonymousAnalyticsId, + isNewUser, + Boolean(user.labsProgram) + ) logger.debug( { email: user.email, userId: user._id.toString() }, diff --git a/services/web/app/src/Features/Compile/CompileManager.mjs b/services/web/app/src/Features/Compile/CompileManager.mjs index e176de4b21..9a4ac41946 100644 --- a/services/web/app/src/Features/Compile/CompileManager.mjs +++ b/services/web/app/src/Features/Compile/CompileManager.mjs @@ -7,7 +7,7 @@ import UserGetter from '../User/UserGetter.mjs' import ClsiManager from './ClsiManager.mjs' import Metrics from '@overleaf/metrics' import { RateLimiter } from '../../infrastructure/RateLimiter.mjs' -import UserAnalyticsIdCache from '../Analytics/UserAnalyticsIdCache.mjs' +import UserAnalyticsDataCache from '../Analytics/UserAnalyticsDataCache.mjs' import { callbackify, callbackifyMultiResult } from '@overleaf/promise-utils' let CompileManager const rclient = RedisWrapper.client('clsi_recently_compiled') @@ -138,7 +138,7 @@ async function _getUserCompileLimits(userId) { ownerFeatures.compileGroup = 'alpha' } - const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + const analyticsId = await UserAnalyticsDataCache.getAnalyticsId( owner._id, '_getUserCompileLimits' ) diff --git a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs index 3c81825aec..383b26763a 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs +++ b/services/web/app/src/Features/SplitTests/SplitTestHandler.mjs @@ -7,7 +7,7 @@ import _ from 'lodash' import { callbackify } from 'node:util' import SplitTestCache from './SplitTestCache.mjs' import { SplitTest } from '../../models/SplitTest.mjs' -import UserAnalyticsIdCache from '../Analytics/UserAnalyticsIdCache.mjs' +import UserAnalyticsDataCache from '../Analytics/UserAnalyticsDataCache.mjs' import Features from '../../infrastructure/Features.mjs' import SplitTestUtils from './SplitTestUtils.mjs' import Settings from '@overleaf/settings' @@ -140,7 +140,7 @@ async function getAssignmentForUser( return _getNonSaasAssignment(splitTestName) } - const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + const analyticsId = await UserAnalyticsDataCache.getAnalyticsId( userId, `getAssignmentForUser:${splitTestName}` ) diff --git a/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs b/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs index cbbe6c9d2d..d3d0738e1b 100755 --- a/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs +++ b/services/web/scripts/stripe/finalize-stripe-subscription-migration.mjs @@ -54,7 +54,7 @@ import { User } from '../../app/src/models/User.mjs' import AnalyticsManager from '../../app/src/Features/Analytics/AnalyticsManager.mjs' import AccountMappingHelper from '../../app/src/Features/Analytics/AccountMappingHelper.mjs' import PlansLocator from '../../app/src/Features/Subscription/PlansLocator.mjs' -import UserAnalyticsIdCache from '../../app/src/Features/Analytics/UserAnalyticsIdCache.mjs' +import UserAnalyticsDataCache from '../../app/src/Features/Analytics/UserAnalyticsDataCache.mjs' import CustomerIoHandler from '../../modules/customer-io/app/src/CustomerIoHandler.mjs' import { ReportError, convertToMinorUnits } from './helpers.mjs' import { compareAccountFields } from '../helpers/migrate_recurly_customers_to_stripe.helpers.mjs' @@ -439,7 +439,7 @@ async function processMigration(input, commit) { } // 7. If commit mode, perform migration - const analyticsId = await UserAnalyticsIdCache.getWithMetrics( + const analyticsId = await UserAnalyticsDataCache.getAnalyticsId( overleafUserId, 'script' // no-op, metrics are not collected from scripts. ) diff --git a/services/web/test/acceptance/src/SessionTests.mjs b/services/web/test/acceptance/src/SessionTests.mjs index c2506b0fc3..133691076c 100644 --- a/services/web/test/acceptance/src/SessionTests.mjs +++ b/services/web/test/acceptance/src/SessionTests.mjs @@ -4,7 +4,7 @@ import UserHelper from './helpers/User.mjs' import redis from './helpers/redis.mjs' import Metrics from './helpers/metrics.mjs' import UserSessionsRedis from '../../../app/src/Features/User/UserSessionsRedis.mjs' -import UserAnalyticsIdCache from '../../../app/src/Features/Analytics/UserAnalyticsIdCache.mjs' +import UserAnalyticsDataCache from '../../../app/src/Features/Analytics/UserAnalyticsDataCache.mjs' import Features from '../../../app/src/infrastructure/Features.mjs' const rclient = UserSessionsRedis.client() @@ -415,7 +415,7 @@ describe('Sessions', function () { }, }, }) - await UserAnalyticsIdCache.reset() + await UserAnalyticsDataCache.reset() for (let i = 0; i < 5; i++) { await this.user1.doRequest('GET', '/project') } diff --git a/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs b/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs index 0aa6a6ce94..85a260371c 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs +++ b/services/web/test/unit/src/Analytics/AnalyticsManager.test.mjs @@ -89,10 +89,13 @@ describe('AnalyticsManager', function () { })) vi.doMock( - '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', + '../../../../app/src/Features/Analytics/UserAnalyticsDataCache', () => ({ - default: (ctx.UserAnalyticsIdCache = { - getWithMetrics: sinon.stub().resolves(ctx.analyticsId), + default: (ctx.UserAnalyticsDataCache = { + getAnalyticsId: sinon.stub().resolves(ctx.analyticsId), + getAnalyticsData: sinon + .stub() + .resolves({ analyticsId: ctx.analyticsId, labsProgram: false }), }), }) ) @@ -388,10 +391,13 @@ describe('AnalyticsManager', function () { })) vi.doMock( - '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', + '../../../../app/src/Features/Analytics/UserAnalyticsDataCache', () => ({ - default: (ctx.UserAnalyticsIdCache = { - getWithMetrics: sinon.stub().resolves(ctx.analyticsId), + default: (ctx.UserAnalyticsDataCache = { + getAnalyticsId: sinon.stub().resolves(ctx.analyticsId), + getAnalyticsData: sinon + .stub() + .resolves({ analyticsId: ctx.analyticsId, labsProgram: false }), }), }) ) @@ -439,7 +445,7 @@ describe('AnalyticsManager', function () { }) it('sets session.analyticsId with a legacy user session without an analyticsId', async function (ctx) { - ctx.UserAnalyticsIdCache.getWithMetrics.resolves(ctx.userId) + ctx.UserAnalyticsDataCache.getAnalyticsId.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, @@ -450,7 +456,7 @@ describe('AnalyticsManager', function () { }) it('updates session.analyticsId with a legacy user session without an analyticsId if different', async function (ctx) { - ctx.UserAnalyticsIdCache.getWithMetrics.resolves(ctx.userId) + ctx.UserAnalyticsDataCache.getAnalyticsId.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, @@ -462,7 +468,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.getWithMetrics.resolves(ctx.userId) + ctx.UserAnalyticsDataCache.getAnalyticsId.resolves(ctx.userId) ctx.req.session.user = { _id: ctx.userId, analyticsId: undefined, diff --git a/services/web/test/unit/src/Analytics/UserAnalyticsDataCache.test.mjs b/services/web/test/unit/src/Analytics/UserAnalyticsDataCache.test.mjs new file mode 100644 index 0000000000..e969c3aebf --- /dev/null +++ b/services/web/test/unit/src/Analytics/UserAnalyticsDataCache.test.mjs @@ -0,0 +1,60 @@ +import { vi } from 'vitest' +import sinon from 'sinon' +import path from 'node:path' + +const MODULE_PATH = path.join( + import.meta.dirname, + '../../../../app/src/Features/Analytics/UserAnalyticsDataCache' +) + +describe('UserAnalyticsDataCache', function () { + beforeEach(async function (ctx) { + ctx.userId = 'abc123def456abc123def456' + ctx.analyticsId = 'ecdb935a-52f3-4f91-aebc-7a70d2ffbb55' + ctx.cacheDeleteSpy = sinon.stub().resolves() + + ctx.UserGetter = { + promises: { + getUser: sinon.stub().resolves({ + _id: ctx.userId, + analyticsId: ctx.analyticsId, + labsProgram: false, + }), + }, + } + + vi.doMock('../../../../app/src/Features/User/UserGetter.mjs', () => ({ + default: ctx.UserGetter, + })) + + vi.doMock('@overleaf/metrics', () => ({ + default: { inc: sinon.stub() }, + })) + + vi.doMock('cache-flow', () => ({ + CacheLoader: class { + async delete(key) { + return ctx.cacheDeleteSpy(key) + } + + async getWithMetadata() { + return { value: undefined, cached: false, time: 0 } + } + + keyToString(key) { + return key?.toString() + } + }, + })) + + ctx.UserAnalyticsDataCache = (await import(MODULE_PATH)).default + }) + + describe('invalidateCache', function () { + it('should delete the cache entry for the userId', async function (ctx) { + await ctx.UserAnalyticsDataCache.invalidateCache(ctx.userId) + sinon.assert.calledOnce(ctx.cacheDeleteSpy) + sinon.assert.calledWith(ctx.cacheDeleteSpy, ctx.userId) + }) + }) +}) diff --git a/services/web/test/unit/src/Compile/CompileManager.test.mjs b/services/web/test/unit/src/Compile/CompileManager.test.mjs index 217e191bbc..556e4ae245 100644 --- a/services/web/test/unit/src/Compile/CompileManager.test.mjs +++ b/services/web/test/unit/src/Compile/CompileManager.test.mjs @@ -65,10 +65,10 @@ describe('CompileManager', function () { })) vi.doMock( - '../../../../app/src/Features/Analytics/UserAnalyticsIdCache', + '../../../../app/src/Features/Analytics/UserAnalyticsDataCache', () => ({ - default: (ctx.UserAnalyticsIdCache = { - getWithMetrics: sinon.stub().resolves('abc'), + default: (ctx.UserAnalyticsDataCache = { + getAnalyticsId: sinon.stub().resolves('abc'), }), }) )