From d1b9ca00a0b0cf73a824d7ad44909e213c998113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Sun, 9 Feb 2025 22:45:44 +0100 Subject: [PATCH] fix: Killwitch to block usage-metrics from non-exiting flag-names (#9266) Adds a killswitch called "filterExistingFlagNames". When enabled it will filter out reported SDK metrics and remove all reported metrics for names that does not match an exiting feature flag in Unleash. This have proven critical in the rare case of an SDK that start sending random flag-names back to unleash, and thus filling up the database. At some point the database will start slowing down due to the noisy data. In order to not resolve the flagNames all the time we have added a small cache (10s) for feature flag names. This gives a small delay (10s) from flag is created until we start allow metrics for the flag when kill-switch is enabled. We should probably listen to the event-stream and use that invalidate the cache when a flag is created. --- .../client-metrics-store-v2-type.ts | 1 + .../client-metrics/client-metrics-store-v2.ts | 7 ++++ .../fake-client-metrics-store-v2.ts | 5 +++ .../client-metrics/metrics-service-v2.ts | 35 +++++++++++++++++-- src/lib/types/experimental.ts | 5 +++ src/server-dev.ts | 1 + 6 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/lib/features/metrics/client-metrics/client-metrics-store-v2-type.ts b/src/lib/features/metrics/client-metrics/client-metrics-store-v2-type.ts index 18d1900ed1..b4a7c6e90e 100644 --- a/src/lib/features/metrics/client-metrics/client-metrics-store-v2-type.ts +++ b/src/lib/features/metrics/client-metrics/client-metrics-store-v2-type.ts @@ -48,4 +48,5 @@ export interface IClientMetricsStoreV2 variantCount: number; }>; aggregateDailyMetrics(): Promise; + getFeatureFlagNames(): Promise; } diff --git a/src/lib/features/metrics/client-metrics/client-metrics-store-v2.ts b/src/lib/features/metrics/client-metrics/client-metrics-store-v2.ts index 4ec3abde5e..79bf70b825 100644 --- a/src/lib/features/metrics/client-metrics/client-metrics-store-v2.ts +++ b/src/lib/features/metrics/client-metrics/client-metrics-store-v2.ts @@ -33,6 +33,8 @@ const DAILY_TABLE = 'client_metrics_env_daily'; const HOURLY_TABLE_VARIANTS = 'client_metrics_env_variants'; const DAILY_TABLE_VARIANTS = 'client_metrics_env_variants_daily'; +const FEATURES_TABLE = 'features'; + const fromRow = (row: ClientMetricsEnvTable) => ({ featureName: row.feature_name, appName: row.app_name, @@ -153,6 +155,11 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { throw new NotFoundError(`Could not find metric`); } + //TODO: Consider moving this to a specific feature store + async getFeatureFlagNames(): Promise { + return this.db(FEATURES_TABLE).distinct('name').pluck('name'); + } + async getAll(query: Object = {}): Promise { const rows = await this.db(HOURLY_TABLE) .select('*') 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 5af1a473f6..caabf77956 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 @@ -17,6 +17,11 @@ export default class FakeClientMetricsStoreV2 super(); this.setMaxListeners(0); } + + getFeatureFlagNames(): Promise { + throw new Error('Method not implemented.'); + } + getSeenTogglesForApp( appName: string, hoursBack?: number, 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 c002ef3fbe..9e8114b605 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts +++ b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts @@ -11,7 +11,7 @@ import type { IClientMetricsStoreV2, } from './client-metrics-store-v2-type'; import { clientMetricsSchema } from '../shared/schema'; -import { compareAsc } from 'date-fns'; +import { compareAsc, secondsToMilliseconds } from 'date-fns'; import { CLIENT_METRICS, CLIENT_REGISTER } from '../../../types/events'; import ApiUser, { type IApiUser } from '../../../types/api-user'; import { ALL } from '../../../types/models/api-token'; @@ -25,6 +25,7 @@ import { } from '../../../util/time-utils'; import type { ClientMetricsSchema } from '../../../../lib/openapi'; import { nameSchema } from '../../../schema/feature-schema'; +import memoizee from 'memoizee'; export default class ClientMetricsServiceV2 { private config: IUnleashConfig; @@ -39,6 +40,8 @@ export default class ClientMetricsServiceV2 { private logger: Logger; + private cachedFeatureNames: () => Promise; + constructor( { clientMetricsStoreV2 }: Pick, config: IUnleashConfig, @@ -51,6 +54,13 @@ export default class ClientMetricsServiceV2 { '/services/client-metrics/client-metrics-service-v2.ts', ); this.flagResolver = config.flagResolver; + this.cachedFeatureNames = memoizee( + async () => this.clientMetricsStoreV2.getFeatureFlagNames(), + { + promise: true, + maxAge: secondsToMilliseconds(10), + }, + ); } async clearMetrics(hoursAgo: number) { @@ -103,6 +113,27 @@ export default class ClientMetricsServiceV2 { } } + async filterExistingToggleNames(toggleNames: string[]): Promise { + if (this.flagResolver.isEnabled('filterExistingFlagNames')) { + try { + const validNames = await this.cachedFeatureNames(); + + const existingNames = toggleNames.filter((name) => + validNames.includes(name), + ); + if (existingNames.length !== toggleNames.length) { + this.logger.warn( + `Filtered out ${toggleNames.length - existingNames.length} toggles with non-existing names`, + ); + } + return this.filterValidToggleNames(existingNames); + } catch (e) { + this.logger.error(e); + } + } + return this.filterValidToggleNames(toggleNames); + } + async filterValidToggleNames(toggleNames: string[]): Promise { const nameValidations: Promise< PromiseFulfilledResult<{ name: string }> | PromiseRejectedResult @@ -151,7 +182,7 @@ export default class ClientMetricsServiceV2 { ); const validatedToggleNames = - await this.filterValidToggleNames(toggleNames); + await this.filterExistingToggleNames(toggleNames); this.logger.debug( `Got ${toggleNames.length} (${validatedToggleNames.length} valid) metrics from ${clientIp}`, diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 67727f239b..fcd353b0ad 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -23,6 +23,7 @@ export type IFlagKey = | 'disableNotifications' | 'advancedPlayground' | 'filterInvalidClientMetrics' + | 'filterExistingFlagNames' | 'disableMetrics' | 'signals' | 'automatedActions' @@ -129,6 +130,10 @@ 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 a7c7cb60b5..41ef4e1214 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -58,6 +58,7 @@ process.nextTick(async () => { uniqueSdkTracking: true, frontendHeaderRedesign: true, dataUsageMultiMonthView: true, + filterExistingFlagNames: true, }, }, authentication: {