fix: filter empty metrics before we collect last seen toggles. (#2172)

* fix: filter empty metrics before we collect last seen toggles.

fixes: #2104

* fix: add a last-seen service to batch last-seen toggle updates

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

src/lib/routes/client-api/metrics.test.ts

@@ -4,7 +4,7 @@ import getApp from '../../app';
 import { createTestConfig } from '../../../test/config/test-config';
 import { clientMetricsSchema } from '../../services/client-metrics/schema';
 import { createServices } from '../../services';
-import { IUnleashOptions, IUnleashStores } from '../../types';
+import { IUnleashOptions, IUnleashServices, IUnleashStores } from '../../types';
 
 async function getSetup(opts?: IUnleashOptions) {
     const stores = createStores();
@@ -16,6 +16,7 @@ async function getSetup(opts?: IUnleashOptions) {
     return {
         request: supertest(app),
         stores,
+        services,
         destroy: () => {
             services.versionService.destroy();
             services.clientInstanceService.destroy();
@@ -26,6 +27,7 @@ async function getSetup(opts?: IUnleashOptions) {
 
 let request;
 let stores: IUnleashStores;
+let services: IUnleashServices;
 let destroy;
 
 beforeEach(async () => {
@@ -33,6 +35,7 @@ beforeEach(async () => {
     request = setup.request;
     stores = setup.stores;
     destroy = setup.destroy;
+    services = setup.services;
 });
 
 afterEach(() => {
@@ -202,6 +205,7 @@ test('should set lastSeen on toggle', async () => {
         })
         .expect(202);
 
+    await services.lastSeenService.store();
     const toggle = await stores.featureToggleStore.get('toggleLastSeen');
 
     expect(toggle.lastSeenAt).toBeTruthy();

src/lib/services/client-metrics/last-seen-service.ts

@@ -0,0 +1,58 @@
+import { secondsToMilliseconds } from 'date-fns';
+import { Logger } from '../../logger';
+import { IUnleashConfig } from '../../server-impl';
+import { IUnleashStores } from '../../types';
+import { IClientMetricsEnv } from '../../types/stores/client-metrics-store-v2';
+import { IFeatureToggleStore } from '../../types/stores/feature-toggle-store';
+
+export class LastSeenService {
+    private timers: NodeJS.Timeout[] = [];
+
+    private lastSeenToggles: Set<string> = new Set();
+
+    private logger: Logger;
+
+    private featureToggleStore: IFeatureToggleStore;
+
+    constructor(
+        { featureToggleStore }: Pick<IUnleashStores, 'featureToggleStore'>,
+        config: IUnleashConfig,
+        lastSeenInterval = secondsToMilliseconds(30),
+    ) {
+        this.featureToggleStore = featureToggleStore;
+        this.logger = config.getLogger(
+            '/services/client-metrics/last-seen-service.ts',
+        );
+
+        this.timers.push(
+            setInterval(() => this.store(), lastSeenInterval).unref(),
+        );
+    }
+
+    async store(): Promise<number> {
+        const count = this.lastSeenToggles.size;
+        if (count > 0) {
+            const lastSeenToggles = [...this.lastSeenToggles];
+            this.lastSeenToggles = new Set();
+            this.logger.debug(
+                `Updating last seen for ${lastSeenToggles.length} toggles`,
+            );
+            await this.featureToggleStore.setLastSeen(lastSeenToggles);
+        }
+        return count;
+    }
+
+    updateLastSeen(clientMetrics: IClientMetricsEnv[]): void {
+        clientMetrics
+            .filter(
+                (clientMetric) => clientMetric.yes > 0 || clientMetric.no > 0,
+            )
+            .forEach((clientMetric) =>
+                this.lastSeenToggles.add(clientMetric.featureName),
+            );
+    }
+
+    destroy(): void {
+        this.timers.forEach(clearInterval);
+    }
+}

src/lib/services/client-metrics/metrics-service-v2.ts

@@ -15,6 +15,7 @@ import ApiUser from '../../types/api-user';
 import { ALL } from '../../types/models/api-token';
 import User from '../../types/user';
 import { collapseHourlyMetrics } from '../../util/collapseHourlyMetrics';
+import { LastSeenService } from './last-seen-service';
 
 export default class ClientMetricsServiceV2 {
     private config: IUnleashConfig;
@@ -27,6 +28,8 @@ export default class ClientMetricsServiceV2 {
 
     private featureToggleStore: IFeatureToggleStore;
 
+    private lastSeenService: LastSeenService;
+
     private logger: Logger;
 
     constructor(
@@ -35,10 +38,12 @@ export default class ClientMetricsServiceV2 {
             clientMetricsStoreV2,
         }: Pick<IUnleashStores, 'featureToggleStore' | 'clientMetricsStoreV2'>,
         config: IUnleashConfig,
+        lastSeenService: LastSeenService,
         bulkInterval = secondsToMilliseconds(5),
     ) {
         this.featureToggleStore = featureToggleStore;
         this.clientMetricsStoreV2 = clientMetricsStoreV2;
+        this.lastSeenService = lastSeenService;
         this.config = config;
         this.logger = config.getLogger(
             '/services/client-metrics/client-metrics-service-v2.ts',
@@ -62,30 +67,35 @@ export default class ClientMetricsServiceV2 {
         clientIp: string,
     ): Promise<void> {
         const value = await clientMetricsSchema.validateAsync(data);
-        const toggleNames = Object.keys(value.bucket.toggles);
-        if (toggleNames.length > 0) {
-            await this.featureToggleStore.setLastSeen(toggleNames);
-        }
+        const toggleNames = Object.keys(value.bucket.toggles).filter(
+            (name) =>
+                !(
+                    value.bucket.toggles[name].yes === 0 &&
+                    value.bucket.toggles[name].no === 0
+                ),
+        );
 
         this.logger.debug(`got metrics from ${clientIp}`);
 
-        const clientMetrics: IClientMetricsEnv[] = toggleNames
-            .map((name) => ({
+        const clientMetrics: IClientMetricsEnv[] = toggleNames.map((name) => ({
             featureName: name,
             appName: value.appName,
             environment: value.environment,
             timestamp: value.bucket.start, //we might need to approximate between start/stop...
             yes: value.bucket.toggles[name].yes,
             no: value.bucket.toggles[name].no,
-            }))
-            .filter((item) => !(item.yes === 0 && item.no === 0));
+        }));
 
         if (this.config.flagResolver.isEnabled('batchMetrics')) {
             this.unsavedMetrics = collapseHourlyMetrics([
                 ...this.unsavedMetrics,
                 ...clientMetrics,
             ]);
+            this.lastSeenService.updateLastSeen(clientMetrics);
         } else {
+            if (toggleNames.length > 0) {
+                await this.featureToggleStore.setLastSeen(toggleNames);
+            }
             await this.clientMetricsStoreV2.batchInsertMetrics(clientMetrics);
         }
 
@@ -161,5 +171,6 @@ export default class ClientMetricsServiceV2 {
 
     destroy(): void {
         this.timers.forEach(clearInterval);
+        this.lastSeenService.destroy();
     }
 }

src/lib/services/index.ts

@@ -35,6 +35,7 @@ import { ProxyService } from './proxy-service';
 import EdgeService from './edge-service';
 import PatService from './pat-service';
 import { PublicSignupTokenService } from './public-signup-token-service';
+import { LastSeenService } from './client-metrics/last-seen-service';
 export const createServices = (
     stores: IUnleashStores,
     config: IUnleashConfig,
@@ -43,7 +44,12 @@ export const createServices = (
     const accessService = new AccessService(stores, config, groupService);
     const apiTokenService = new ApiTokenService(stores, config);
     const clientInstanceService = new ClientInstanceService(stores, config);
-    const clientMetricsServiceV2 = new ClientMetricsServiceV2(stores, config);
+    const lastSeenService = new LastSeenService(stores, config);
+    const clientMetricsServiceV2 = new ClientMetricsServiceV2(
+        stores,
+        config,
+        lastSeenService,
+    );
     const contextService = new ContextService(stores, config);
     const emailService = new EmailService(config.email, config.getLogger);
     const eventService = new EventService(stores, config);
@@ -147,6 +153,7 @@ export const createServices = (
         edgeService,
         patService,
         publicSignupTokenService,
+        lastSeenService,
     };
 };
 

src/lib/types/services.ts

@@ -33,6 +33,7 @@ import { ProxyService } from '../services/proxy-service';
 import EdgeService from '../services/edge-service';
 import PatService from '../services/pat-service';
 import { PublicSignupTokenService } from '../services/public-signup-token-service';
+import { LastSeenService } from '../services/client-metrics/last-seen-service';
 
 export interface IUnleashServices {
     accessService: AccessService;
@@ -71,4 +72,5 @@ export interface IUnleashServices {
     openApiService: OpenApiService;
     clientSpecService: ClientSpecService;
     patService: PatService;
+    lastSeenService: LastSeenService;
 }

src/test/e2e/api/client/metricsV2.e2e.test.ts

@@ -97,3 +97,53 @@ test('should pick up environment from token', async () => {
     expect(metrics[0].environment).toBe('test');
     expect(metrics[0].appName).toBe('some-fancy-app');
 });
+
+test('should set lastSeen for toggles with metrics', async () => {
+    const start = Date.now();
+    await app.services.featureToggleServiceV2.createFeatureToggle(
+        'default',
+        { name: 't1' },
+        'tester',
+    );
+    await app.services.featureToggleServiceV2.createFeatureToggle(
+        'default',
+        { name: 't2' },
+        'tester',
+    );
+    const token = await app.services.apiTokenService.createApiToken({
+        type: ApiTokenType.CLIENT,
+        project: 'default',
+        environment: 'default',
+        username: 'tester',
+    });
+
+    await app.request
+        .post('/api/client/metrics')
+        .set('Authorization', token.secret)
+        .send({
+            appName: 'some-fancy-app',
+            instanceId: '1',
+            bucket: {
+                start: Date.now(),
+                stop: Date.now(),
+                toggles: {
+                    t1: {
+                        yes: 100,
+                        no: 50,
+                    },
+                    t2: {
+                        yes: 0,
+                        no: 0,
+                    },
+                },
+            },
+        })
+        .expect(202);
+
+    await app.services.clientMetricsServiceV2.bulkAdd();
+    await app.services.lastSeenService.store();
+    const t1 = await db.stores.featureToggleStore.get('t1');
+    const t2 = await db.stores.featureToggleStore.get('t2');
+    expect(t1.lastSeenAt.getTime()).toBeGreaterThanOrEqual(start);
+    expect(t2.lastSeenAt).toBeDefined();
+});

src/test/e2e/services/last-seen-service.e2e.test.ts

@@ -0,0 +1,131 @@
+import { createTestConfig } from '../../config/test-config';
+import dbInit from '../helpers/database-init';
+import { IUnleashStores } from '../../../lib/types/stores';
+import { LastSeenService } from '../../../lib/services/client-metrics/last-seen-service';
+import { IClientMetricsEnv } from '../../../lib/types/stores/client-metrics-store-v2';
+
+let stores: IUnleashStores;
+let db;
+let config;
+
+beforeAll(async () => {
+    config = createTestConfig();
+    db = await dbInit('last_seen_service_serial', config.getLogger);
+    stores = db.stores;
+});
+beforeEach(async () => {
+    await stores.featureToggleStore.deleteAll();
+});
+afterAll(async () => {
+    await db.destroy();
+});
+
+test('Should update last seen for known toggles', async () => {
+    const service = new LastSeenService(stores, config);
+    const time = Date.now();
+    await stores.featureToggleStore.create('default', { name: 'ta1' });
+
+    const metrics: IClientMetricsEnv[] = [
+        {
+            featureName: 'ta1',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 1,
+            no: 0,
+        },
+        {
+            featureName: 'ta2',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 1,
+            no: 0,
+        },
+    ];
+
+    service.updateLastSeen(metrics);
+    await service.store();
+
+    const t1 = await stores.featureToggleStore.get('ta1');
+
+    expect(t1.lastSeenAt.getTime()).toBeGreaterThan(time);
+
+    service.destroy();
+});
+
+test('Should not update last seen toggles with 0 metrics', async () => {
+    // jest.useFakeTimers();
+    const service = new LastSeenService(stores, config, 30);
+    const time = Date.now();
+    await stores.featureToggleStore.create('default', { name: 'tb1' });
+    await stores.featureToggleStore.create('default', { name: 'tb2' });
+
+    const metrics: IClientMetricsEnv[] = [
+        {
+            featureName: 'tb1',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 1,
+            no: 0,
+        },
+        {
+            featureName: 'tb2',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 0,
+            no: 0,
+        },
+    ];
+
+    service.updateLastSeen(metrics);
+
+    // bypass interval waiting
+    await service.store();
+
+    const t1 = await stores.featureToggleStore.get('tb1');
+    const t2 = await stores.featureToggleStore.get('tb2');
+
+    expect(t2.lastSeenAt).toBeNull();
+    expect(t1.lastSeenAt.getTime()).toBeGreaterThanOrEqual(time);
+
+    service.destroy();
+});
+
+test('Should not update anything for 0 toggles', async () => {
+    // jest.useFakeTimers();
+    const service = new LastSeenService(stores, config, 30);
+    const time = Date.now();
+    await stores.featureToggleStore.create('default', { name: 'tb1' });
+    await stores.featureToggleStore.create('default', { name: 'tb2' });
+
+    const metrics: IClientMetricsEnv[] = [
+        {
+            featureName: 'tb1',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 0,
+            no: 0,
+        },
+        {
+            featureName: 'tb2',
+            appName: 'some-App',
+            environment: 'default',
+            timestamp: new Date(time),
+            yes: 0,
+            no: 0,
+        },
+    ];
+
+    service.updateLastSeen(metrics);
+
+    // bypass interval waiting
+    const count = await service.store();
+
+    expect(count).toBe(0);
+
+    service.destroy();
+});