From 08a1d053dc59e4a652f3b09da88aa9fe66f046b2 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Mon, 23 Oct 2023 11:26:48 +0200 Subject: [PATCH] feat: add job that cleans last seen every 24 hours (#5114) This PR adds a cleanup job that removes unknown feature flags from last_seen_at_metrics table every 24 hours since we no longer have a foreign key on the name column in the features table. --- src/lib/server-impl.ts | 2 +- .../last-seen/fake-last-seen-store.ts | 4 + .../last-seen/last-seen-service.ts | 9 +- .../last-seen/last-seen-store.ts | 6 ++ .../tests/last-seen-service.e2e.test.ts | 82 +++++++++++++++++++ .../last-seen/types/last-seen-store-type.ts | 1 + src/lib/services/index.ts | 23 +++++- .../services/last-seen-service.e2e.test.ts | 2 - src/test/fixtures/store.ts | 3 +- 9 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 src/lib/services/client-metrics/last-seen/tests/last-seen-service.e2e.test.ts diff --git a/src/lib/server-impl.ts b/src/lib/server-impl.ts index 2b59984822..b705c142c1 100644 --- a/src/lib/server-impl.ts +++ b/src/lib/server-impl.ts @@ -44,7 +44,7 @@ async function createApp( const stores = createStores(config, db); const services = createServices(stores, config, db); if (!config.disableScheduler) { - await scheduleServices(services); + await scheduleServices(services, config.flagResolver); } const metricsMonitor = createMetricsMonitor(); diff --git a/src/lib/services/client-metrics/last-seen/fake-last-seen-store.ts b/src/lib/services/client-metrics/last-seen/fake-last-seen-store.ts index 94b6be1445..eb11764aef 100644 --- a/src/lib/services/client-metrics/last-seen/fake-last-seen-store.ts +++ b/src/lib/services/client-metrics/last-seen/fake-last-seen-store.ts @@ -6,4 +6,8 @@ export class FakeLastSeenStore implements ILastSeenStore { data.map((lastSeen) => lastSeen); return Promise.resolve(); } + + cleanLastSeen(): Promise { + return Promise.resolve(); + } } diff --git a/src/lib/services/client-metrics/last-seen/last-seen-service.ts b/src/lib/services/client-metrics/last-seen/last-seen-service.ts index 2f438a95c8..555b5eb930 100644 --- a/src/lib/services/client-metrics/last-seen/last-seen-service.ts +++ b/src/lib/services/client-metrics/last-seen/last-seen-service.ts @@ -29,7 +29,6 @@ export class LastSeenService { lastSeenStore, }: Pick, config: IUnleashConfig, - lastSeenInterval = secondsToMilliseconds(30), ) { this.lastSeenStore = lastSeenStore; this.featureToggleStore = featureToggleStore; @@ -37,10 +36,6 @@ export class LastSeenService { '/services/client-metrics/last-seen-service.ts', ); this.config = config; - - this.timers.push( - setInterval(() => this.store(), lastSeenInterval).unref(), - ); } async store(): Promise { @@ -81,6 +76,10 @@ export class LastSeenService { }); } + async cleanLastSeen() { + await this.lastSeenStore.cleanLastSeen(); + } + destroy(): void { this.timers.forEach(clearInterval); } diff --git a/src/lib/services/client-metrics/last-seen/last-seen-store.ts b/src/lib/services/client-metrics/last-seen/last-seen-store.ts index 183cde017c..9dbd4059cc 100644 --- a/src/lib/services/client-metrics/last-seen/last-seen-store.ts +++ b/src/lib/services/client-metrics/last-seen/last-seen-store.ts @@ -58,6 +58,12 @@ export default class LastSeenStore implements ILastSeenStore { this.logger.error('Could not update lastSeen, error: ', err); } } + + async cleanLastSeen() { + await this.db(TABLE) + .whereNotIn('feature_name', this.db.select('name').from('features')) + .del(); + } } module.exports = LastSeenStore; diff --git a/src/lib/services/client-metrics/last-seen/tests/last-seen-service.e2e.test.ts b/src/lib/services/client-metrics/last-seen/tests/last-seen-service.e2e.test.ts new file mode 100644 index 0000000000..ea99f96500 --- /dev/null +++ b/src/lib/services/client-metrics/last-seen/tests/last-seen-service.e2e.test.ts @@ -0,0 +1,82 @@ +import dbInit, { ITestDb } from '../../../../../test/e2e/helpers/database-init'; +import { + IUnleashTest, + setupAppWithCustomConfig, +} from '../../../../../test/e2e/helpers/test-helper'; +import getLogger from '../../../../../test/fixtures/no-logger'; + +let app: IUnleashTest; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('last_seen_at_service_e2e', getLogger); + app = await setupAppWithCustomConfig( + db.stores, + { + experimental: { + flags: { + strictSchemaValidation: true, + disableEnvsOnRevive: true, + useLastSeenRefactor: true, + }, + }, + }, + db.rawDatabase, + ); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('should clean unknown feature toggle names from last seen store', async () => { + const { lastSeenService, featureToggleService } = app.services; + + const clean = ['clean1', 'clean2', 'clean3', 'clean4']; + const dirty = ['dirty1', 'dirty2', 'dirty3', 'dirty4']; + + await Promise.all( + clean.map((featureName) => + featureToggleService.createFeatureToggle( + 'default', + { name: featureName }, + 'user', + ), + ), + ); + + const inserts = [...clean, ...dirty].map((feature) => { + return { + featureName: feature, + environment: 'default', + yes: 1, + no: 0, + appName: 'test', + timestamp: new Date(), + }; + }); + + lastSeenService.updateLastSeen(inserts); + await lastSeenService.store(); + + // We have no method to get these from the last seen service or any other service or store + let stored = await db.rawDatabase.raw( + 'SELECT * FROM last_seen_at_metrics;', + ); + + expect(stored.rows.length).toBe(8); + + await lastSeenService.cleanLastSeen(); + + stored = await db.rawDatabase.raw('SELECT * FROM last_seen_at_metrics;'); + + expect(stored.rows.length).toBe(4); + expect(stored.rows).toMatch; + + const notInDirty = stored.rows.filter( + (row) => !dirty.includes(row.feature_name), + ); + + expect(notInDirty.length).toBe(4); +}); diff --git a/src/lib/services/client-metrics/last-seen/types/last-seen-store-type.ts b/src/lib/services/client-metrics/last-seen/types/last-seen-store-type.ts index 8670792f30..38beef583c 100644 --- a/src/lib/services/client-metrics/last-seen/types/last-seen-store-type.ts +++ b/src/lib/services/client-metrics/last-seen/types/last-seen-store-type.ts @@ -2,4 +2,5 @@ import { LastSeenInput } from '../last-seen-service'; export interface ILastSeenStore { setLastSeen(data: LastSeenInput[]): Promise; + cleanLastSeen: () => Promise; } diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index dbca8cd484..cb2e1f556e 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -1,4 +1,9 @@ -import { IUnleashConfig, IUnleashStores, IUnleashServices } from '../types'; +import { + IUnleashConfig, + IUnleashStores, + IUnleashServices, + IFlagResolver, +} from '../types'; import FeatureTypeService from './feature-type-service'; import EventService from './event-service'; import HealthService from './health-service'; @@ -98,6 +103,7 @@ import { ClientFeatureToggleService } from '../features/client-feature-toggles/c // TODO: will be moved to scheduler feature directory export const scheduleServices = async ( services: IUnleashServices, + flagResolver: IFlagResolver, ): Promise => { const { schedulerService, @@ -110,12 +116,27 @@ export const scheduleServices = async ( maintenanceService, eventAnnouncerService, featureToggleService, + lastSeenService, } = services; if (await maintenanceService.isMaintenanceMode()) { schedulerService.pause(); } + if (flagResolver.isEnabled('useLastSeenRefactor')) { + schedulerService.schedule( + lastSeenService.cleanLastSeen.bind(lastSeenService), + hoursToMilliseconds(1), + 'cleanLastSeen', + ); + } + + schedulerService.schedule( + lastSeenService.store.bind(lastSeenService), + secondsToMilliseconds(30), + 'storeLastSeen', + ); + schedulerService.schedule( apiTokenService.fetchActiveTokens.bind(apiTokenService), minutesToMilliseconds(1), diff --git a/src/test/e2e/services/last-seen-service.e2e.test.ts b/src/test/e2e/services/last-seen-service.e2e.test.ts index c07cd061ec..e1745a8b1d 100644 --- a/src/test/e2e/services/last-seen-service.e2e.test.ts +++ b/src/test/e2e/services/last-seen-service.e2e.test.ts @@ -68,7 +68,6 @@ test('Should not update last seen toggles with 0 metrics', async () => { featureToggleStore: stores.featureToggleStore, }, config, - 30, ); const time = Date.now(); await stores.featureToggleStore.create('default', { name: 'tb1' }); @@ -115,7 +114,6 @@ test('Should not update anything for 0 toggles', async () => { featureToggleStore: stores.featureToggleStore, }, config, - 30, ); const time = Date.now(); await stores.featureToggleStore.create('default', { name: 'tb1' }); diff --git a/src/test/fixtures/store.ts b/src/test/fixtures/store.ts index 039afedae9..61228b9632 100644 --- a/src/test/fixtures/store.ts +++ b/src/test/fixtures/store.ts @@ -38,6 +38,7 @@ import FakeFavoriteProjectsStore from './fake-favorite-projects-store'; import { FakeAccountStore } from './fake-account-store'; import FakeProjectStatsStore from './fake-project-stats-store'; import { FakeDependentFeaturesStore } from '../../lib/features/dependent-features/fake-dependent-features-store'; +import { FakeLastSeenStore } from '../../lib/services/client-metrics/last-seen/fake-last-seen-store'; const db = { select: () => ({ @@ -85,7 +86,7 @@ const createStores: () => IUnleashStores = () => { importTogglesStore: {} as IImportTogglesStore, privateProjectStore: {} as IPrivateProjectStore, dependentFeaturesStore: new FakeDependentFeaturesStore(), - lastSeenStore: { setLastSeen: async () => {} }, + lastSeenStore: new FakeLastSeenStore(), }; };