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 d089f00604..f4998508df 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts +++ b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts @@ -26,10 +26,7 @@ import { import type { ClientMetricsSchema } from '../../../../lib/openapi/index.js'; import { nameSchema } from '../../../schema/feature-schema.js'; import memoizee from 'memoizee'; -import { - MAX_UNKNOWN_FLAGS, - type UnknownFlagsService, -} from '../unknown-flags/unknown-flags-service.js'; +import type { UnknownFlagsService } from '../unknown-flags/unknown-flags-service.js'; import { type Metric, MetricsTranslator, @@ -133,23 +130,17 @@ export default class ClientMetricsServiceV2 { validatedToggleNames: string[]; unknownToggleNames: string[]; }> { - let unknownToggleNames: string[] = []; - const existingFlags = await this.cachedFeatureNames(); const existingNames = toggleNames.filter((name) => existingFlags.includes(name), ); + let unknownToggleNames: string[] = []; if (this.flagResolver.isEnabled('reportUnknownFlags')) { try { - const nonExistingNames = toggleNames.filter( + unknownToggleNames = toggleNames.filter( (name) => !existingFlags.includes(name), ); - - unknownToggleNames = nonExistingNames.slice( - 0, - MAX_UNKNOWN_FLAGS, - ); } catch (e) { this.logger.error(e); } @@ -244,16 +235,19 @@ export default class ClientMetricsServiceV2 { this.config.eventBus.emit(CLIENT_REGISTER, heartbeatEvent); } + const environment = value.environment ?? 'default'; + if (unknownToggleNames.length > 0) { const unknownFlags = unknownToggleNames.map((name) => ({ name, appName: value.appName, seenAt: value.bucket.stop, + environment, })); this.logger.info( - `Registering ${unknownFlags.length} unknown flags from ${value.appName}, i.e.: ${unknownFlags + `Registering ${unknownFlags.length} unknown flags from ${value.appName} in the ${environment} environment. Some of the unknown flag names include: ${unknownFlags .slice(0, 10) - .map((f) => f.name) + .map(({ name }) => `"${name}"`) .join(', ')}`, ); this.unknownFlagsService.register(unknownFlags); @@ -264,7 +258,7 @@ export default class ClientMetricsServiceV2 { (name) => ({ featureName: name, appName: value.appName, - environment: value.environment ?? 'default', + 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, diff --git a/src/lib/features/metrics/unknown-flags/fake-unknown-flags-store.ts b/src/lib/features/metrics/unknown-flags/fake-unknown-flags-store.ts index 04eabfc21f..db1f8160f4 100644 --- a/src/lib/features/metrics/unknown-flags/fake-unknown-flags-store.ts +++ b/src/lib/features/metrics/unknown-flags/fake-unknown-flags-store.ts @@ -1,21 +1,27 @@ -import type { IUnknownFlagsStore, UnknownFlag } from './unknown-flags-store.js'; +import type { + IUnknownFlagsStore, + UnknownFlag, + QueryParams, +} from './unknown-flags-store.js'; export class FakeUnknownFlagsStore implements IUnknownFlagsStore { private unknownFlagMap = new Map(); private getKey(flag: UnknownFlag): string { - return `${flag.name}:${flag.appName}`; + return `${flag.name}:${flag.appName}:${flag.environment}`; } - async replaceAll(flags: UnknownFlag[]): Promise { + async insert(flags: UnknownFlag[]): Promise { this.unknownFlagMap.clear(); for (const flag of flags) { this.unknownFlagMap.set(this.getKey(flag), flag); } } - async getAll(): Promise { - return Array.from(this.unknownFlagMap.values()); + async getAll({ limit }: QueryParams = {}): Promise { + const flags = Array.from(this.unknownFlagMap.values()); + if (!limit) return flags; + return flags.slice(0, limit); } async clear(hoursAgo: number): Promise { diff --git a/src/lib/features/metrics/unknown-flags/unknown-flags-controller.ts b/src/lib/features/metrics/unknown-flags/unknown-flags-controller.ts index 8d7714535a..495829c186 100644 --- a/src/lib/features/metrics/unknown-flags/unknown-flags-controller.ts +++ b/src/lib/features/metrics/unknown-flags/unknown-flags-controller.ts @@ -45,7 +45,7 @@ export default class UnknownFlagsController extends Controller { tags: ['Unstable'], summary: 'Get latest reported unknown flag names', description: - 'Returns a list of unknown flag names reported in the last 24 hours, if any. Maximum of 10.', + 'Returns a list of unknown flag reports from the last 7 days, if any. Maximum of 1000.', responses: { 200: createResponseSchema('unknownFlagsResponseSchema'), }, @@ -61,8 +61,9 @@ export default class UnknownFlagsController extends Controller { if (!this.flagResolver.isEnabled('reportUnknownFlags')) { throw new NotFoundError(); } - const unknownFlags = - await this.unknownFlagsService.getGroupedUnknownFlags(); + const unknownFlags = await this.unknownFlagsService.getAll({ + limit: 1000, + }); this.openApiService.respondWithValidation( 200, diff --git a/src/lib/features/metrics/unknown-flags/unknown-flags-service.ts b/src/lib/features/metrics/unknown-flags/unknown-flags-service.ts index 5675a0c161..99d6192244 100644 --- a/src/lib/features/metrics/unknown-flags/unknown-flags-service.ts +++ b/src/lib/features/metrics/unknown-flags/unknown-flags-service.ts @@ -7,8 +7,6 @@ import type { import type { IUnleashStores } from '../../../types/index.js'; import type { UnknownFlag } from './unknown-flags-store.js'; -export const MAX_UNKNOWN_FLAGS = 10; - export class UnknownFlagsService { private logger: Logger; @@ -31,25 +29,13 @@ export class UnknownFlagsService { } private getKey(flag: UnknownFlag) { - return `${flag.name}:${flag.appName}`; + return `${flag.name}:${flag.appName}:${flag.environment}`; } register(unknownFlags: UnknownFlag[]) { + if (!this.flagResolver.isEnabled('reportUnknownFlags')) return; for (const flag of unknownFlags) { const key = this.getKey(flag); - - if (this.unknownFlagsCache.has(key)) { - this.unknownFlagsCache.set(key, flag); - continue; - } - - if (this.unknownFlagsCache.size >= MAX_UNKNOWN_FLAGS) { - const oldestKey = [...this.unknownFlagsCache.entries()].sort( - (a, b) => a[1].seenAt.getTime() - b[1].seenAt.getTime(), - )[0][0]; - this.unknownFlagsCache.delete(oldestKey); - } - this.unknownFlagsCache.set(key, flag); } } @@ -58,46 +44,15 @@ export class UnknownFlagsService { if (!this.flagResolver.isEnabled('reportUnknownFlags')) return; if (this.unknownFlagsCache.size === 0) return; - const existing = await this.unknownFlagsStore.getAll(); const cached = Array.from(this.unknownFlagsCache.values()); - const merged = [...existing, ...cached]; - const mergedMap = new Map(); - - for (const flag of merged) { - const key = this.getKey(flag); - const existing = mergedMap.get(key); - if (!existing || flag.seenAt > existing.seenAt) { - mergedMap.set(key, flag); - } - } - - const latest = Array.from(mergedMap.values()) - .sort((a, b) => b.seenAt.getTime() - a.seenAt.getTime()) - .slice(0, MAX_UNKNOWN_FLAGS); - - await this.unknownFlagsStore.replaceAll(latest); + await this.unknownFlagsStore.insert(cached); this.unknownFlagsCache.clear(); } - async getGroupedUnknownFlags(): Promise< - { name: string; reportedBy: { appName: string; seenAt: Date }[] }[] - > { - const unknownFlags = await this.unknownFlagsStore.getAll(); - - const grouped = new Map(); - - for (const { name, appName, seenAt } of unknownFlags) { - if (!grouped.has(name)) { - grouped.set(name, []); - } - grouped.get(name)!.push({ appName, seenAt }); - } - - return Array.from(grouped.entries()).map(([name, reportedBy]) => ({ - name, - reportedBy, - })); + async getAll({ limit }: { limit?: number }): Promise { + if (!this.flagResolver.isEnabled('reportUnknownFlags')) return []; + return this.unknownFlagsStore.getAll({ limit }); } async clear(hoursAgo: number) { diff --git a/src/lib/features/metrics/unknown-flags/unknown-flags-store.ts b/src/lib/features/metrics/unknown-flags/unknown-flags-store.ts index 5ef8936b2e..db9e50a616 100644 --- a/src/lib/features/metrics/unknown-flags/unknown-flags-store.ts +++ b/src/lib/features/metrics/unknown-flags/unknown-flags-store.ts @@ -1,5 +1,4 @@ import type { Db } from '../../../db/db.js'; -import { MAX_UNKNOWN_FLAGS } from './unknown-flags-service.js'; const TABLE = 'unknown_flags'; @@ -7,11 +6,16 @@ export type UnknownFlag = { name: string; appName: string; seenAt: Date; + environment: string; +}; + +export type QueryParams = { + limit?: number; }; export interface IUnknownFlagsStore { - replaceAll(flags: UnknownFlag[]): Promise; - getAll(): Promise; + insert(flags: UnknownFlag[]): Promise; + getAll(params?: QueryParams): Promise; clear(hoursAgo: number): Promise; deleteAll(): Promise; count(): Promise; @@ -24,32 +28,40 @@ export class UnknownFlagsStore implements IUnknownFlagsStore { this.db = db; } - async replaceAll(flags: UnknownFlag[]): Promise { - await this.db.transaction(async (tx) => { - await tx(TABLE).delete(); - if (flags.length > 0) { - const rows = flags.map((flag) => ({ - name: flag.name, - app_name: flag.appName, - seen_at: flag.seenAt, - })); - await tx(TABLE) - .insert(rows) - .onConflict(['name', 'app_name']) - .merge(['seen_at']); - } - }); + async insert(flags: UnknownFlag[]): Promise { + if (flags.length > 0) { + const rows = flags.map((flag) => ({ + name: flag.name, + app_name: flag.appName, + seen_at: flag.seenAt, + environment: flag.environment, + })); + await this.db(TABLE) + .insert(rows) + .onConflict(['name', 'app_name', 'environment']) + .merge(['seen_at']); + } } - async getAll(): Promise { - const rows = await this.db(TABLE) - .select('name', 'app_name', 'seen_at') - .orderBy('seen_at', 'desc') - .limit(MAX_UNKNOWN_FLAGS); + async getAll({ limit }: QueryParams = {}): Promise { + let query = this.db(TABLE).select( + 'name', + 'app_name', + 'seen_at', + 'environment', + ); + + if (limit) { + query = query.limit(limit); + } + + const rows = await query; + return rows.map((row) => ({ name: row.name, appName: row.app_name, seenAt: new Date(row.seen_at), + environment: row.environment, })); } diff --git a/src/lib/features/scheduler/schedule-services.ts b/src/lib/features/scheduler/schedule-services.ts index de64a296e7..6e2fef72b7 100644 --- a/src/lib/features/scheduler/schedule-services.ts +++ b/src/lib/features/scheduler/schedule-services.ts @@ -203,7 +203,7 @@ export const scheduleServices = ( ); schedulerService.schedule( - unknownFlagsService.clear.bind(unknownFlagsService, 24), + unknownFlagsService.clear.bind(unknownFlagsService, 24 * 7), hoursToMilliseconds(24), 'clearUnknownFlags', ); diff --git a/src/lib/openapi/spec/unknown-flag-schema.ts b/src/lib/openapi/spec/unknown-flag-schema.ts index dc40089d64..553ddde708 100644 --- a/src/lib/openapi/spec/unknown-flag-schema.ts +++ b/src/lib/openapi/spec/unknown-flag-schema.ts @@ -4,7 +4,7 @@ export const unknownFlagSchema = { $id: '#/components/schemas/unknownFlagSchema', type: 'object', additionalProperties: false, - required: ['name', 'reportedBy'], + required: ['name', 'appName', 'seenAt', 'environment'], description: 'An unknown flag that has been reported by the system', properties: { name: { @@ -12,30 +12,24 @@ export const unknownFlagSchema = { description: 'The name of the unknown flag.', example: 'my-unknown-flag', }, - reportedBy: { + appName: { + type: 'string', description: - 'Details about the application that reported the unknown flag.', - type: 'array', - items: { - type: 'object', - additionalProperties: false, - required: ['appName', 'seenAt'], - properties: { - appName: { - type: 'string', - description: - 'The name of the application that reported the unknown flag.', - example: 'my-app', - }, - seenAt: { - type: 'string', - format: 'date-time', - description: - 'The date and time when the unknown flag was reported.', - example: '2023-10-01T12:00:00Z', - }, - }, - }, + 'The name of the application that reported the unknown flag.', + example: 'my-app', + }, + seenAt: { + type: 'string', + format: 'date-time', + description: + 'The date and time when the unknown flag was reported.', + example: '2023-10-01T12:00:00Z', + }, + environment: { + type: 'string', + description: + 'The environment in which the unknown flag was reported.', + example: 'production', }, }, components: {}, diff --git a/src/migrations/20250707153020-unknown-flags-environment.js b/src/migrations/20250707153020-unknown-flags-environment.js new file mode 100644 index 0000000000..7708392676 --- /dev/null +++ b/src/migrations/20250707153020-unknown-flags-environment.js @@ -0,0 +1,17 @@ +exports.up = function(db, cb) { + db.runSql(` + ALTER TABLE unknown_flags + ADD COLUMN IF NOT EXISTS environment TEXT NOT NULL DEFAULT 'default', + DROP CONSTRAINT unknown_flags_pkey, + ADD PRIMARY KEY (name, app_name, environment); + `, cb); +}; + +exports.down = function(db, cb) { + db.runSql(` + ALTER TABLE unknown_flags + DROP CONSTRAINT unknown_flags_pkey, + ADD PRIMARY KEY (name, app_name), + DROP COLUMN IF EXISTS environment; + `, cb); +};