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 29e4d2148a..5e74b2ae9f 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 @@ -113,7 +113,7 @@ test('process metrics properly even when some names are not url friendly, filter ); // only toggle with a bad name gets filtered out - expect(eventBus.emit).not.toHaveBeenCalled(); + expect(eventBus.emit).toHaveBeenCalledTimes(1); expect(lastSeenService.updateLastSeen).not.toHaveBeenCalled(); }); 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 7d51b1a7b4..dbb7fb3dd0 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts +++ b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts @@ -32,6 +32,7 @@ import { MetricsTranslator, } from '../impact/metrics-translator.js'; import { impactRegister } from '../impact/impact-register.js'; +import type { UnknownFlag } from '../unknown-flags/unknown-flags-store.js'; export default class ClientMetricsServiceV2 { private config: IUnleashConfig; @@ -178,12 +179,78 @@ export default class ClientMetricsServiceV2 { return toggleNames; } + private async siftMetrics( + metrics: IClientMetricsEnv[], + ): Promise { + if (!metrics.length) return []; + + const metricsByToggle = new Map(); + for (const m of metrics) { + if (m.yes === 0 && m.no === 0) continue; + let arr = metricsByToggle.get(m.featureName); + if (!arr) { + arr = []; + metricsByToggle.set(m.featureName, arr); + } + arr.push(m); + } + if (metricsByToggle.size === 0) return []; + + const toggleNames = Array.from(metricsByToggle.keys()); + + const { validatedToggleNames, unknownToggleNames } = + await this.filterExistingToggleNames(toggleNames); + + const validatedSet = new Set(validatedToggleNames); + const unknownSet = new Set(unknownToggleNames); + + const invalidCount = toggleNames.length - validatedSet.size; + this.logger.debug( + `Got ${toggleNames.length} metrics (${invalidCount > 0 ? `${invalidCount} invalid` : 'all valid'}).`, + ); + + const unknownFlags: UnknownFlag[] = []; + for (const [featureName, group] of metricsByToggle) { + if (unknownSet.has(featureName)) { + for (const m of group) { + unknownFlags.push({ + name: featureName, + appName: m.appName, + seenAt: m.timestamp, + environment: m.environment, + }); + } + } + } + if (unknownFlags.length) { + const sample = unknownFlags + .slice(0, 10) + .map((f) => `"${f.name}"`) + .join(', '); + this.logger.debug( + `Registering ${unknownFlags.length} unknown flags; sample: ${sample}`, + ); + this.unknownFlagsService.register(unknownFlags); + } + + const siftedMetrics: IClientMetricsEnv[] = []; + for (const [featureName, group] of metricsByToggle) { + if (validatedSet.has(featureName)) { + siftedMetrics.push(...group); + } + } + return siftedMetrics; + } + async registerBulkMetrics(metrics: IClientMetricsEnv[]): Promise { + const siftedMetrics = await this.siftMetrics(metrics); + if (siftedMetrics.length === 0) return; + this.unsavedMetrics = collapseHourlyMetrics([ ...this.unsavedMetrics, - ...metrics, + ...siftedMetrics, ]); - this.lastSeenService.updateLastSeen(metrics); + this.lastSeenService.updateLastSeen(siftedMetrics); } async registerImpactMetrics(impactMetrics: Metric[]) { @@ -202,22 +269,6 @@ export default class ClientMetricsServiceV2 { clientIp: string, ): Promise { const value = await clientMetricsSchema.validateAsync(data); - const toggleNames = Object.keys(value.bucket.toggles).filter( - (name) => - !( - value.bucket.toggles[name].yes === 0 && - value.bucket.toggles[name].no === 0 - ), - ); - - const { validatedToggleNames, unknownToggleNames } = - await this.filterExistingToggleNames(toggleNames); - - const invalidToggleNames = - toggleNames.length - validatedToggleNames.length; - this.logger.debug( - `Got ${toggleNames.length} (${invalidToggleNames > 0 ? `${invalidToggleNames} invalid ones` : 'all valid'}) metrics from ${value.appName}`, - ); if (data.sdkVersion) { const [sdkName, sdkVersion] = data.sdkVersion.split(':'); @@ -235,38 +286,20 @@ export default class ClientMetricsServiceV2 { this.config.eventBus.emit(CLIENT_REGISTER, heartbeatEvent); } - const environment = value.environment ?? 'default'; + const clientMetrics: IClientMetricsEnv[] = Object.keys( + value.bucket.toggles, + ).map((name) => ({ + featureName: name, + appName: value.appName, + environment: value.environment ?? 'default', + timestamp: value.bucket.stop, //we might need to approximate between start/stop... + yes: value.bucket.toggles[name].yes ?? 0, + no: value.bucket.toggles[name].no ?? 0, + variants: value.bucket.toggles[name].variants, + })); - if (unknownToggleNames.length > 0) { - const unknownFlags = unknownToggleNames.map((name) => ({ - name, - appName: value.appName, - seenAt: value.bucket.stop, - environment, - })); - this.logger.debug( - `Registering ${unknownFlags.length} unknown flags from ${value.appName} in the ${environment} environment. Some of the unknown flag names include: ${unknownFlags - .slice(0, 10) - .map(({ name }) => `"${name}"`) - .join(', ')}`, - ); - this.unknownFlagsService.register(unknownFlags); - } - - if (validatedToggleNames.length > 0) { - const clientMetrics: IClientMetricsEnv[] = validatedToggleNames.map( - (name) => ({ - featureName: name, - appName: value.appName, - environment, - timestamp: value.bucket.stop, //we might need to approximate between start/stop... - yes: value.bucket.toggles[name].yes ?? 0, - no: value.bucket.toggles[name].no ?? 0, - variants: value.bucket.toggles[name].variants, - }), - ); + if (clientMetrics.length) { await this.registerBulkMetrics(clientMetrics); - this.config.eventBus.emit(CLIENT_METRICS, clientMetrics); } } diff --git a/src/lib/features/metrics/instance/metrics.test.ts b/src/lib/features/metrics/instance/metrics.test.ts index a3935b1c1e..adfbdcb105 100644 --- a/src/lib/features/metrics/instance/metrics.test.ts +++ b/src/lib/features/metrics/instance/metrics.test.ts @@ -414,6 +414,11 @@ describe('bulk metrics', () => { test('without access to production environment due to no auth setup, we can only access the default env', async () => { const now = new Date(); + // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2 + services.clientMetricsServiceV2.cachedFeatureNames = vi + .fn<() => Promise>() + .mockResolvedValue(['test_feature_one', 'test_feature_two']); + await request .post('/api/client/metrics/bulk') .send({ diff --git a/src/lib/features/metrics/unknown-flags/unknown-flags.e2e.test.ts b/src/lib/features/metrics/unknown-flags/unknown-flags.e2e.test.ts new file mode 100644 index 0000000000..ee2c8d71ac --- /dev/null +++ b/src/lib/features/metrics/unknown-flags/unknown-flags.e2e.test.ts @@ -0,0 +1,158 @@ +import supertest, { type Test } from 'supertest'; +import getApp from '../../../app.js'; +import { createTestConfig } from '../../../../test/config/test-config.js'; +import { + type IUnleashServices, + createServices, +} from '../../../services/index.js'; +import type { + IUnleashConfig, + IUnleashOptions, + IUnleashStores, +} from '../../../types/index.js'; +import dbInit, { + type ITestDb, +} from '../../../../test/e2e/helpers/database-init.js'; +import { startOfHour } from 'date-fns'; +import type TestAgent from 'supertest/lib/agent.d.ts'; +import type { BulkRegistrationSchema } from '../../../openapi/index.js'; + +let db: ITestDb; +let config: IUnleashConfig; + +async function getSetup(opts?: IUnleashOptions) { + config = createTestConfig(opts); + db = await dbInit('unknown_flags', config.getLogger); + + const services = createServices(db.stores, config, db.rawDatabase); + const app = await getApp(config, db.stores, services); + return { + request: supertest(app), + stores: db.stores, + services, + db: db.rawDatabase, + destroy: db.destroy, + }; +} + +let request: TestAgent; +let stores: IUnleashStores; +let services: IUnleashServices; +let destroy: () => Promise; + +beforeAll(async () => { + const setup = await getSetup({ + experimental: { + flags: { + reportUnknownFlags: true, + }, + }, + }); + request = setup.request; + stores = setup.stores; + destroy = setup.destroy; + services = setup.services; +}); + +afterAll(async () => { + await destroy(); +}); + +afterEach(async () => { + await stores.unknownFlagsStore.deleteAll(); +}); + +describe('should register unknown flags', () => { + test('/metrics endpoint', async () => { + // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2 + services.clientMetricsServiceV2.cachedFeatureNames = vi + .fn<() => Promise>() + .mockResolvedValue(['existing_flag']); + + await request + .post('/api/client/metrics') + .send({ + appName: 'demo', + instanceId: '1', + bucket: { + start: Date.now(), + stop: Date.now(), + toggles: { + existing_flag: { + yes: 200, + no: 0, + }, + unknown_flag: { + yes: 100, + no: 50, + }, + }, + }, + }) + .expect(202); + + await services.unknownFlagsService.flush(); + const unknownFlags = await services.unknownFlagsService.getAll(); + + expect(unknownFlags).toHaveLength(1); + expect(unknownFlags[0]).toMatchObject({ + name: 'unknown_flag', + environment: 'development', + appName: 'demo', + seenAt: expect.any(Date), + }); + }); + + test('/metrics/bulk endpoint', async () => { + // @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2 + services.clientMetricsServiceV2.cachedFeatureNames = vi + .fn<() => Promise>() + .mockResolvedValue(['existing_flag']); + + const unknownFlag: BulkRegistrationSchema = { + appName: 'demo', + instanceId: '1', + environment: 'development', + sdkVersion: 'unleash-client-js:1.0.0', + sdkType: 'frontend', + }; + + await request + .post('/api/client/metrics/bulk') + .send({ + applications: [unknownFlag], + metrics: [ + { + featureName: 'existing_flag', + environment: 'development', + appName: 'demo', + timestamp: startOfHour(new Date()), + yes: 200, + no: 0, + variants: {}, + }, + { + featureName: 'unknown_flag', + environment: 'development', + appName: 'demo', + timestamp: startOfHour(new Date()), + yes: 100, + no: 50, + variants: {}, + }, + ], + }) + .expect(202); + + await services.unknownFlagsService.flush(); + const unknownFlags = await services.unknownFlagsService.getAll(); + + expect(unknownFlags).toHaveLength(1); + expect(unknownFlags[0]).toMatchObject({ + name: 'unknown_flag', + environment: 'development', + appName: 'demo', + seenAt: expect.any(Date), + }); + }); +});