Skip to content

Commit d1b9ca0

Browse files
authored
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.
1 parent 6413a66 commit d1b9ca0

6 files changed

+52
-2
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,5 @@ export interface IClientMetricsStoreV2
4848
variantCount: number;
4949
}>;
5050
aggregateDailyMetrics(): Promise<void>;
51+
getFeatureFlagNames(): Promise<string[]>;
5152
}

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

+7
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ const DAILY_TABLE = 'client_metrics_env_daily';
3333
const HOURLY_TABLE_VARIANTS = 'client_metrics_env_variants';
3434
const DAILY_TABLE_VARIANTS = 'client_metrics_env_variants_daily';
3535

36+
const FEATURES_TABLE = 'features';
37+
3638
const fromRow = (row: ClientMetricsEnvTable) => ({
3739
featureName: row.feature_name,
3840
appName: row.app_name,
@@ -153,6 +155,11 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 {
153155
throw new NotFoundError(`Could not find metric`);
154156
}
155157

158+
//TODO: Consider moving this to a specific feature store
159+
async getFeatureFlagNames(): Promise<string[]> {
160+
return this.db(FEATURES_TABLE).distinct('name').pluck('name');
161+
}
162+
156163
async getAll(query: Object = {}): Promise<IClientMetricsEnv[]> {
157164
const rows = await this.db<ClientMetricsEnvTable>(HOURLY_TABLE)
158165
.select('*')

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

+5
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ export default class FakeClientMetricsStoreV2
1717
super();
1818
this.setMaxListeners(0);
1919
}
20+
21+
getFeatureFlagNames(): Promise<string[]> {
22+
throw new Error('Method not implemented.');
23+
}
24+
2025
getSeenTogglesForApp(
2126
appName: string,
2227
hoursBack?: number,

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

+33-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import type {
1111
IClientMetricsStoreV2,
1212
} from './client-metrics-store-v2-type';
1313
import { clientMetricsSchema } from '../shared/schema';
14-
import { compareAsc } from 'date-fns';
14+
import { compareAsc, secondsToMilliseconds } from 'date-fns';
1515
import { CLIENT_METRICS, CLIENT_REGISTER } from '../../../types/events';
1616
import ApiUser, { type IApiUser } from '../../../types/api-user';
1717
import { ALL } from '../../../types/models/api-token';
@@ -25,6 +25,7 @@ import {
2525
} from '../../../util/time-utils';
2626
import type { ClientMetricsSchema } from '../../../../lib/openapi';
2727
import { nameSchema } from '../../../schema/feature-schema';
28+
import memoizee from 'memoizee';
2829

2930
export default class ClientMetricsServiceV2 {
3031
private config: IUnleashConfig;
@@ -39,6 +40,8 @@ export default class ClientMetricsServiceV2 {
3940

4041
private logger: Logger;
4142

43+
private cachedFeatureNames: () => Promise<string[]>;
44+
4245
constructor(
4346
{ clientMetricsStoreV2 }: Pick<IUnleashStores, 'clientMetricsStoreV2'>,
4447
config: IUnleashConfig,
@@ -51,6 +54,13 @@ export default class ClientMetricsServiceV2 {
5154
'/services/client-metrics/client-metrics-service-v2.ts',
5255
);
5356
this.flagResolver = config.flagResolver;
57+
this.cachedFeatureNames = memoizee(
58+
async () => this.clientMetricsStoreV2.getFeatureFlagNames(),
59+
{
60+
promise: true,
61+
maxAge: secondsToMilliseconds(10),
62+
},
63+
);
5464
}
5565

5666
async clearMetrics(hoursAgo: number) {
@@ -103,6 +113,27 @@ export default class ClientMetricsServiceV2 {
103113
}
104114
}
105115

116+
async filterExistingToggleNames(toggleNames: string[]): Promise<string[]> {
117+
if (this.flagResolver.isEnabled('filterExistingFlagNames')) {
118+
try {
119+
const validNames = await this.cachedFeatureNames();
120+
121+
const existingNames = toggleNames.filter((name) =>
122+
validNames.includes(name),
123+
);
124+
if (existingNames.length !== toggleNames.length) {
125+
this.logger.warn(
126+
`Filtered out ${toggleNames.length - existingNames.length} toggles with non-existing names`,
127+
);
128+
}
129+
return this.filterValidToggleNames(existingNames);
130+
} catch (e) {
131+
this.logger.error(e);
132+
}
133+
}
134+
return this.filterValidToggleNames(toggleNames);
135+
}
136+
106137
async filterValidToggleNames(toggleNames: string[]): Promise<string[]> {
107138
const nameValidations: Promise<
108139
PromiseFulfilledResult<{ name: string }> | PromiseRejectedResult
@@ -151,7 +182,7 @@ export default class ClientMetricsServiceV2 {
151182
);
152183

153184
const validatedToggleNames =
154-
await this.filterValidToggleNames(toggleNames);
185+
await this.filterExistingToggleNames(toggleNames);
155186

156187
this.logger.debug(
157188
`Got ${toggleNames.length} (${validatedToggleNames.length} valid) metrics from ${clientIp}`,

src/lib/types/experimental.ts

+5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type IFlagKey =
2323
| 'disableNotifications'
2424
| 'advancedPlayground'
2525
| 'filterInvalidClientMetrics'
26+
| 'filterExistingFlagNames'
2627
| 'disableMetrics'
2728
| 'signals'
2829
| 'automatedActions'
@@ -129,6 +130,10 @@ const flags: IFlags = {
129130
process.env.FILTER_INVALID_CLIENT_METRICS,
130131
false,
131132
),
133+
filterExistingFlagNames: parseEnvVarBoolean(
134+
process.env.FILTER_INVALID_CLIENT_METRICS,
135+
false,
136+
),
132137
disableMetrics: parseEnvVarBoolean(
133138
process.env.UNLEASH_EXPERIMENTAL_DISABLE_METRICS,
134139
false,

src/server-dev.ts

+1
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ process.nextTick(async () => {
5858
uniqueSdkTracking: true,
5959
frontendHeaderRedesign: true,
6060
dataUsageMultiMonthView: true,
61+
filterExistingFlagNames: true,
6162
},
6263
},
6364
authentication: {

0 commit comments

Comments
 (0)