diff --git a/src/lib/features/frontend-api/frontend-api.e2e.test.ts b/src/lib/features/frontend-api/frontend-api.e2e.test.ts index 915e710afb..5b8d9ac344 100644 --- a/src/lib/features/frontend-api/frontend-api.e2e.test.ts +++ b/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>() + .mockResolvedValue([featureName]); + await app.request .get(`/api/admin/client-metrics/features/${featureName}`) .set('Authorization', adminToken.secret) diff --git a/src/lib/features/metrics/client-metrics/fake-client-metrics-store-v2.ts b/src/lib/features/metrics/client-metrics/fake-client-metrics-store-v2.ts index 4021cf5e04..3582c9a4cc 100644 --- a/src/lib/features/metrics/client-metrics/fake-client-metrics-store-v2.ts +++ b/src/lib/features/metrics/client-metrics/fake-client-metrics-store-v2.ts @@ -19,7 +19,7 @@ export default class FakeClientMetricsStoreV2 } getFeatureFlagNames(): Promise { - throw new Error('Method not implemented.'); + return Promise.resolve([]); } getSeenTogglesForApp( diff --git a/src/lib/features/metrics/client-metrics/metrics-service-v2.test.ts b/src/lib/features/metrics/client-metrics/metrics-service-v2.test.ts index 818cc5f167..7460063202 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.test.ts +++ b/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>() + .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); diff --git a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts index a981ac19e9..0fb970da0b 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts +++ b/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 }; } diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index eaf1bfb38d..f47c88164a 100644 --- a/src/lib/types/experimental.ts +++ b/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, diff --git a/src/server-dev.ts b/src/server-dev.ts index 92c92da34b..82372e2607 100644 --- a/src/server-dev.ts +++ b/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, diff --git a/src/test/e2e/api/client/metrics.access.e2e.test.ts b/src/test/e2e/api/client/metrics.access.e2e.test.ts index 8b1ac1e68a..cf1306eb02 100644 --- a/src/test/e2e/api/client/metrics.access.e2e.test.ts +++ b/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>() + .mockResolvedValue([featureName]); + await app.request .post('/api/client/metrics') .set('Authorization', token.secret) diff --git a/src/test/e2e/api/client/metricsV2.e2e.test.ts b/src/test/e2e/api/client/metricsV2.e2e.test.ts index 94032c9b18..d80992f765 100644 --- a/src/test/e2e/api/client/metricsV2.e2e.test.ts +++ b/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>() + .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>() + .mockResolvedValue(['t1', 't2']); + const token = await app.services.apiTokenService.createApiToken({ type: ApiTokenType.CLIENT, project: 'default',