From 4d92d54f9a3366ecc73bb1e53b0288da8f7f34b1 Mon Sep 17 00:00:00 2001 From: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com> Date: Thu, 15 May 2025 13:06:54 +0200 Subject: [PATCH] feat: track last seen clients using bulk update (#9981) Let's not update `lastSeen` in the db on each client call --- src/lib/db/client-instance-store.ts | 12 ++++++++++ .../feature-lifecycle.e2e.test.ts | 4 ++++ .../metrics/instance/instance-service.ts | 22 +++++++++++++----- .../features/metrics/instance/metrics.test.ts | 23 ------------------- src/lib/types/experimental.ts | 5 ++++ src/lib/types/stores/client-instance-store.ts | 4 ++++ .../e2e/api/admin/applications.e2e.test.ts | 1 + src/test/e2e/api/client/metrics.e2e.test.ts | 2 ++ 8 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/lib/db/client-instance-store.ts b/src/lib/db/client-instance-store.ts index 3d9be8437b..bf61a076cb 100644 --- a/src/lib/db/client-instance-store.ts +++ b/src/lib/db/client-instance-store.ts @@ -71,12 +71,18 @@ export default class ClientInstanceStore implements IClientInstanceStore { } } + /** + * @deprecated + * `bulkUpsert` is beeing used instead. remove with `lastSeenBulkQuery` flag + */ async setLastSeen({ appName, instanceId, environment, clientIp, }: INewClientInstance): Promise { + const stopTimer = this.metricTimer('setLastSeen'); + await this.db(TABLE) .insert({ app_name: appName, @@ -90,14 +96,20 @@ export default class ClientInstanceStore implements IClientInstanceStore { last_seen: new Date(), client_ip: clientIp, }); + + stopTimer(); } async bulkUpsert(instances: INewClientInstance[]): Promise { + const stopTimer = this.metricTimer('bulkUpsert'); + const rows = instances.map(mapToDb); await this.db(TABLE) .insert(rows) .onConflict(['app_name', 'instance_id', 'environment']) .merge(); + + stopTimer(); } async delete({ diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts b/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts index a5ceb97b54..173a0aed64 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts @@ -22,6 +22,7 @@ import type { FeatureLifecycleCompletedSchema } from '../../openapi/index.js'; import { FeatureLifecycleReadModel } from './feature-lifecycle-read-model.js'; import type { IFeatureLifecycleReadModel } from './feature-lifecycle-read-model-type.js'; import { STAGE_ENTERED } from '../../metric-events.js'; +import type ClientInstanceService from '../metrics/instance/instance-service.js'; let app: IUnleashTest; let db: ITestDb; @@ -29,6 +30,7 @@ let featureLifecycleStore: IFeatureLifecycleStore; let eventStore: IEventStore; let eventBus: EventEmitter; let featureLifecycleReadModel: IFeatureLifecycleReadModel; +let clientInstanceService: ClientInstanceService; beforeAll(async () => { db = await dbInit('feature_lifecycle', getLogger, { @@ -47,6 +49,7 @@ beforeAll(async () => { eventBus = app.config.eventBus; featureLifecycleReadModel = new FeatureLifecycleReadModel(db.rawDatabase); featureLifecycleStore = db.stores.featureLifecycleStore; + clientInstanceService = app.services.clientInstanceService; await app.request .post(`/auth/demo/login`) @@ -62,6 +65,7 @@ afterAll(async () => { }); beforeEach(async () => { + await clientInstanceService.bulkAdd(); // flush await featureLifecycleStore.deleteAll(); }); diff --git a/src/lib/features/metrics/instance/instance-service.ts b/src/lib/features/metrics/instance/instance-service.ts index 4e26278bb6..cad52761e3 100644 --- a/src/lib/features/metrics/instance/instance-service.ts +++ b/src/lib/features/metrics/instance/instance-service.ts @@ -100,12 +100,22 @@ export default class ClientInstanceService { clientIp: string, ): Promise { const value = await clientMetricsSchema.validateAsync(data); - await this.clientInstanceStore.setLastSeen({ - appName: value.appName, - instanceId: value.instanceId, - environment: value.environment, - clientIp: clientIp, - }); + + if (this.flagResolver.isEnabled('lastSeenBulkQuery')) { + this.seenClients[this.clientKey(value)] = { + appName: value.appName, + instanceId: value.instanceId, + environment: value.environment, + clientIp: clientIp, + }; + } else { + await this.clientInstanceStore.setLastSeen({ + appName: value.appName, + instanceId: value.instanceId, + environment: value.environment, + clientIp: clientIp, + }); + } } public registerFrontendClient(data: IFrontendClientApp): void { diff --git a/src/lib/features/metrics/instance/metrics.test.ts b/src/lib/features/metrics/instance/metrics.test.ts index ef2eb90b7e..69c7690010 100644 --- a/src/lib/features/metrics/instance/metrics.test.ts +++ b/src/lib/features/metrics/instance/metrics.test.ts @@ -108,29 +108,6 @@ test('should accept client metrics with yes/no', () => { .expect(202); }); -test('should accept client metrics with yes/no with metricsV2', async () => { - const testRunner = await getSetup(); - await testRunner.request - .post('/api/client/metrics') - .send({ - appName: 'demo', - instanceId: '1', - bucket: { - start: Date.now(), - stop: Date.now(), - toggles: { - toggleA: { - yes: 200, - no: 0, - }, - }, - }, - }) - .expect(202); - - await testRunner.destroy(); -}); - test('should accept client metrics with variants', () => { return request .post('/api/client/metrics') diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index c0a92fc085..eaf1bfb38d 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -67,6 +67,7 @@ export type IFlagKey = | 'featureLinks' | 'projectLinkTemplates' | 'reportUnknownFlags' + | 'lastSeenBulkQuery' | 'newGettingStartedEmail'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -316,6 +317,10 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_REPORT_UNKNOWN_FLAGS, false, ), + lastSeenBulkQuery: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_LAST_SEEN_BULK_QUERY, + false, + ), newGettingStartedEmail: parseEnvVarBoolean( process.env.UNLEASH_EXPERIMENTAL_NEW_GETTING_STARTED_EMAIL, false, diff --git a/src/lib/types/stores/client-instance-store.ts b/src/lib/types/stores/client-instance-store.ts index 69de4d5308..e835664cce 100644 --- a/src/lib/types/stores/client-instance-store.ts +++ b/src/lib/types/stores/client-instance-store.ts @@ -18,6 +18,10 @@ export interface IClientInstanceStore Pick > { bulkUpsert(instances: INewClientInstance[]): Promise; + /** + * @deprecated + * `bulkUpsert` is beeing used instead. remove with `lastSeenBulkQuery` flag + */ setLastSeen(INewClientInstance): Promise; insert(details: INewClientInstance): Promise; getByAppName(appName: string): Promise; diff --git a/src/test/e2e/api/admin/applications.e2e.test.ts b/src/test/e2e/api/admin/applications.e2e.test.ts index d2e81223fd..2f32f7a026 100644 --- a/src/test/e2e/api/admin/applications.e2e.test.ts +++ b/src/test/e2e/api/admin/applications.e2e.test.ts @@ -268,6 +268,7 @@ test('should not return instances older than 24h', async () => { .expect(202); await app.services.clientMetricsServiceV2.bulkAdd(); + await app.services.clientInstanceService.bulkAdd(); await db.stores.clientApplicationsStore.upsert({ appName: metrics.appName, diff --git a/src/test/e2e/api/client/metrics.e2e.test.ts b/src/test/e2e/api/client/metrics.e2e.test.ts index 30e4fd7b0e..8c0dae3688 100644 --- a/src/test/e2e/api/client/metrics.e2e.test.ts +++ b/src/test/e2e/api/client/metrics.e2e.test.ts @@ -24,6 +24,7 @@ beforeAll(async () => { }); afterEach(async () => { + await app.services.clientInstanceService.bulkAdd(); // flush await Promise.all([ db.stores.clientMetricsStoreV2.deleteAll(), db.stores.clientInstanceStore.deleteAll(), @@ -73,6 +74,7 @@ test('should create instance if does not exist', async () => { .post('/api/client/metrics') .send(metricsExample) .expect(202); + await app.services.clientInstanceService.bulkAdd(); const finalInstances = await db.stores.clientInstanceStore.getAll(); expect(finalInstances.length).toBe(1); });