chore: remove filterExistingFlagNames feature flag (#9957)

https://linear.app/unleash/issue/2-3564/remove-filterexistingflagnames-feature-flag

We're removing the `filterExistingFlagNames` feature flag since we've
decided we want this to be the default behavior.

We don't need to rush to merge it, just in case we need to disable this
for any reason. However it should also be pretty easy to just revert if
needed.

Changes in tests are a bit tricky since they assumed the previous
behavior where we always registered metrics, even for non existing flag
names. `cachedFeatureNames` is also memoized with a TTL of 10s, so the
easiest way to overcome this was to override `cachedFeatureNames` to
return what we expected. As long as they return the same flag names that
we expect, we're able to register their metrics.

Let me know if you can think of a better approach.

src/lib/features/frontend-api/frontend-api.e2e.test.ts

@@ -360,6 +360,12 @@ test('should store frontend api client metrics', async () => {
         projects: ['*'],
         environment: '*',
     });
+
+    // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2
+    app.services.clientMetricsServiceV2.cachedFeatureNames = jest
+        .fn<() => Promise<string[]>>()
+        .mockResolvedValue([featureName]);
+
     await app.request
         .get(`/api/admin/client-metrics/features/${featureName}`)
         .set('Authorization', adminToken.secret)

src/lib/features/metrics/client-metrics/fake-client-metrics-store-v2.ts

@@ -19,7 +19,7 @@ export default class FakeClientMetricsStoreV2
     }
 
     getFeatureFlagNames(): Promise<string[]> {
-        throw new Error('Method not implemented.');
+        return Promise.resolve([]);
     }
 
     getSeenTogglesForApp(

src/lib/features/metrics/client-metrics/metrics-service-v2.test.ts

@@ -15,7 +15,7 @@ import { UnknownFlagsService } from '../unknown-flags/unknown-flags-service.js';
 
 import { jest } from '@jest/globals';
 
-function initClientMetrics(flagEnabled = true) {
+function initClientMetrics() {
     const stores = createStores();
 
     const eventBus = new EventEmitter();
@@ -25,9 +25,7 @@ function initClientMetrics(flagEnabled = true) {
         eventBus,
         getLogger,
         flagResolver: {
-            isEnabled: () => {
-                return flagEnabled;
-            },
+            isEnabled: () => true,
         },
     } as unknown as IUnleashConfig;
 
@@ -51,12 +49,17 @@ function initClientMetrics(flagEnabled = true) {
         lastSeenService,
         unknownFlagsService,
     );
-    return { clientMetricsService: service, eventBus, lastSeenService };
+    return { clientMetricsService: service, eventBus, lastSeenService, stores };
 }
 
 test('process metrics properly', async () => {
-    const { clientMetricsService, eventBus, lastSeenService } =
+    const { clientMetricsService, eventBus, lastSeenService, stores } =
         initClientMetrics();
+
+    stores.clientMetricsStoreV2.getFeatureFlagNames = jest
+        .fn<() => Promise<string[]>>()
+        .mockResolvedValue(['myCoolToggle', 'myOtherToggle']);
+
     await clientMetricsService.registerClientMetrics(
         {
             appName: 'test',
@@ -114,31 +117,6 @@ test('process metrics properly even when some names are not url friendly, filter
     expect(lastSeenService.updateLastSeen).not.toHaveBeenCalled();
 });
 
-test('process metrics properly even when some names are not url friendly, with default behavior when flag is off', async () => {
-    const { clientMetricsService, eventBus, lastSeenService } =
-        initClientMetrics(false);
-    await clientMetricsService.registerClientMetrics(
-        {
-            appName: 'test',
-            bucket: {
-                start: '1982-07-25T12:00:00.000Z',
-                stop: '2023-07-25T12:00:00.000Z',
-                toggles: {
-                    'not url friendly ☹': {
-                        yes: 0,
-                        no: 100,
-                    },
-                },
-            },
-            environment: 'test',
-        },
-        '127.0.0.1',
-    );
-
-    expect(eventBus.emit).toHaveBeenCalledTimes(1);
-    expect(lastSeenService.updateLastSeen).toHaveBeenCalledTimes(1);
-});
-
 test('get daily client metrics for a toggle', async () => {
     const yesterday = subDays(new Date(), 1);
     const twoDaysAgo = subDays(new Date(), 2);

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

@@ -125,49 +125,36 @@ export default class ClientMetricsServiceV2 {
         validatedToggleNames: string[];
         unknownToggleNames: string[];
     }> {
-        let toggleNamesToValidate: string[] = toggleNames;
         let unknownToggleNames: string[] = [];
 
-        const shouldFilter = this.flagResolver.isEnabled(
-            'filterExistingFlagNames',
+        const existingFlags = await this.cachedFeatureNames();
+        const existingNames = toggleNames.filter((name) =>
+            existingFlags.includes(name),
         );
-        const shouldReport = this.flagResolver.isEnabled('reportUnknownFlags');
 
-        if (shouldFilter || shouldReport) {
+        if (existingNames.length !== toggleNames.length) {
+            this.logger.info(
+                `Filtered out ${toggleNames.length - existingNames.length} toggles with non-existing names`,
+            );
+        }
+
+        if (this.flagResolver.isEnabled('reportUnknownFlags')) {
             try {
-                const existingFlags = await this.cachedFeatureNames();
-
-                const existingNames = toggleNames.filter((name) =>
-                    existingFlags.includes(name),
-                );
                 const nonExistingNames = toggleNames.filter(
                     (name) => !existingFlags.includes(name),
                 );
 
-                if (shouldFilter) {
-                    toggleNamesToValidate = existingNames;
-
-                    if (existingNames.length !== toggleNames.length) {
-                        this.logger.info(
-                            `Filtered out ${toggleNames.length - existingNames.length} toggles with non-existing names`,
-                        );
-                    }
-                }
-
-                if (shouldReport) {
-                    unknownToggleNames = nonExistingNames.slice(
-                        0,
-                        MAX_UNKNOWN_FLAGS,
-                    );
-                }
+                unknownToggleNames = nonExistingNames.slice(
+                    0,
+                    MAX_UNKNOWN_FLAGS,
+                );
             } catch (e) {
                 this.logger.error(e);
             }
         }
 
-        const validatedToggleNames = await this.filterValidToggleNames(
-            toggleNamesToValidate,
-        );
+        const validatedToggleNames =
+            await this.filterValidToggleNames(existingNames);
 
         return { validatedToggleNames, unknownToggleNames };
     }

src/lib/types/experimental.ts

@@ -20,7 +20,6 @@ export type IFlagKey =
     | 'disableBulkToggle'
     | 'advancedPlayground'
     | 'filterInvalidClientMetrics'
-    | 'filterExistingFlagNames'
     | 'disableMetrics'
     | 'signals'
     | 'automatedActions'
@@ -118,10 +117,6 @@ const flags: IFlags = {
         process.env.FILTER_INVALID_CLIENT_METRICS,
         false,
     ),
-    filterExistingFlagNames: parseEnvVarBoolean(
-        process.env.FILTER_INVALID_CLIENT_METRICS,
-        false,
-    ),
     disableMetrics: parseEnvVarBoolean(
         process.env.UNLEASH_EXPERIMENTAL_DISABLE_METRICS,
         false,

src/server-dev.ts

@@ -50,7 +50,6 @@ process.nextTick(async () => {
                         showUserDeviceCount: true,
                         deltaApi: true,
                         uniqueSdkTracking: true,
-                        filterExistingFlagNames: true,
                         teamsIntegrationChangeRequests: true,
                         addEditStrategy: true,
                         flagsOverviewSearch: true,

src/test/e2e/api/client/metrics.access.e2e.test.ts

@@ -9,6 +9,8 @@ import dbInit, { type ITestDb } from '../../helpers/database-init.js';
 import getLogger from '../../../fixtures/no-logger.js';
 import { ApiTokenType } from '../../../../lib/types/model.js';
 
+import { jest } from '@jest/globals';
+
 let app: IUnleashTest;
 let db: ITestDb;
 
@@ -38,6 +40,12 @@ test('should enrich metrics with environment from api-token', async () => {
         project: '*',
     });
 
+    const featureName = Object.keys(metricsExample.bucket.toggles)[0];
+    // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2
+    app.services.clientMetricsServiceV2.cachedFeatureNames = jest
+        .fn<() => Promise<string[]>>()
+        .mockResolvedValue([featureName]);
+
     await app.request
         .post('/api/client/metrics')
         .set('Authorization', token.secret)

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

@@ -10,6 +10,8 @@ import getLogger from '../../../fixtures/no-logger.js';
 import { ApiTokenType, type IApiToken } from '../../../../lib/types/model.js';
 import { TEST_AUDIT_USER } from '../../../../lib/types/index.js';
 
+import { jest } from '@jest/globals';
+
 let app: IUnleashTest;
 let db: ITestDb;
 
@@ -81,6 +83,11 @@ test('should pick up environment from token', async () => {
         tokenName: 'tester',
     });
 
+    // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2
+    app.services.clientMetricsServiceV2.cachedFeatureNames = jest
+        .fn<() => Promise<string[]>>()
+        .mockResolvedValue(['test']);
+
     await app.request
         .post('/api/client/metrics')
         .set('Authorization', token.secret)
@@ -119,6 +126,11 @@ test('should set lastSeen for toggles with metrics both for toggle and toggle en
         TEST_AUDIT_USER,
     );
 
+    // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2
+    app.services.clientMetricsServiceV2.cachedFeatureNames = jest
+        .fn<() => Promise<string[]>>()
+        .mockResolvedValue(['t1', 't2']);
+
     const token = await app.services.apiTokenService.createApiToken({
         type: ApiTokenType.CLIENT,
         project: 'default',