diff --git a/src/lib/db/client-metrics-store-v2.test.ts b/src/lib/db/client-metrics-store-v2.test.ts index 2b83c3c17a..e4c3b2a050 100644 --- a/src/lib/db/client-metrics-store-v2.test.ts +++ b/src/lib/db/client-metrics-store-v2.test.ts @@ -186,3 +186,37 @@ test('clear daily metrics', async () => { .select('*'); expect(variantResults.length).toBe(2); }); + +test('count previous day metrics', async () => { + const yesterday = subDays(new Date(), 1); + await clientMetricsStore.batchInsertMetrics([ + { + appName: 'test', + featureName: 'feature', + environment: 'development', + timestamp: setHours(yesterday, 10), + no: 0, + yes: 1, + variants: { + a: 1, + b: 0, + }, + }, + { + appName: 'test', + featureName: 'feature', + environment: 'development', + timestamp: setHours(yesterday, 11), + no: 1, + yes: 1, + variants: { + a: 0, + b: 1, + }, + }, + ]); + + const result = await clientMetricsStore.countPreviousDayMetrics(); + + expect(result).toMatchObject({ enabledCount: 2, variantCount: 4 }); +}); diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts index ee4218b8d0..fdb537a9b7 100644 --- a/src/lib/db/client-metrics-store-v2.ts +++ b/src/lib/db/client-metrics-store-v2.ts @@ -31,9 +31,9 @@ interface ClientMetricsEnvVariantTable extends ClientMetricsBaseTable { count: number; } -const TABLE = 'client_metrics_env'; +const HOURLY_TABLE = 'client_metrics_env'; const DAILY_TABLE = 'client_metrics_env_daily'; -const TABLE_VARIANTS = 'client_metrics_env_variants'; +const HOURLY_TABLE_VARIANTS = 'client_metrics_env_variants'; const DAILY_TABLE_VARIANTS = 'client_metrics_env_variants_daily'; const fromRow = (row: ClientMetricsEnvTable) => ({ @@ -142,7 +142,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { } async get(key: IClientMetricsEnvKey): Promise<IClientMetricsEnv> { - const row = await this.db<ClientMetricsEnvTable>(TABLE) + const row = await this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .where({ feature_name: key.featureName, app_name: key.appName, @@ -157,7 +157,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { } async getAll(query: Object = {}): Promise<IClientMetricsEnv[]> { - const rows = await this.db<ClientMetricsEnvTable>(TABLE) + const rows = await this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .select('*') .where(query); return rows.map(fromRow); @@ -173,7 +173,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { } async delete(key: IClientMetricsEnvKey): Promise<void> { - return this.db<ClientMetricsEnvTable>(TABLE) + return this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .where({ feature_name: key.featureName, app_name: key.appName, @@ -184,7 +184,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { } deleteAll(): Promise<void> { - return this.db(TABLE).del(); + return this.db(HOURLY_TABLE).del(); } destroy(): void { @@ -207,7 +207,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { ); // Consider rewriting to SQL batch! - const insert = this.db<ClientMetricsEnvTable>(TABLE) + const insert = this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .insert(sortedRows) .toQuery(); const query = `${insert.toString()} ON CONFLICT (feature_name, app_name, environment, timestamp) DO UPDATE SET "yes" = "client_metrics_env"."yes" + EXCLUDED.yes, "no" = "client_metrics_env"."no" + EXCLUDED.no`; @@ -226,7 +226,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { if (sortedVariantRows.length > 0) { const insertVariants = this.db<ClientMetricsEnvVariantTable>( - TABLE_VARIANTS, + HOURLY_TABLE_VARIANTS, ) .insert(sortedVariantRows) .toQuery(); @@ -239,20 +239,29 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { featureName: string, hoursBack: number = 24, ): Promise<IClientMetricsEnv[]> { - const rows = await this.db<ClientMetricsEnvTable>(TABLE) - .select([`${TABLE}.*`, 'variant', 'count']) - .leftJoin(TABLE_VARIANTS, function () { + const rows = await this.db<ClientMetricsEnvTable>(HOURLY_TABLE) + .select([`${HOURLY_TABLE}.*`, 'variant', 'count']) + .leftJoin(HOURLY_TABLE_VARIANTS, function () { this.on( - `${TABLE_VARIANTS}.feature_name`, - `${TABLE}.feature_name`, + `${HOURLY_TABLE_VARIANTS}.feature_name`, + `${HOURLY_TABLE}.feature_name`, ) - .on(`${TABLE_VARIANTS}.app_name`, `${TABLE}.app_name`) - .on(`${TABLE_VARIANTS}.environment`, `${TABLE}.environment`) - .on(`${TABLE_VARIANTS}.timestamp`, `${TABLE}.timestamp`); + .on( + `${HOURLY_TABLE_VARIANTS}.app_name`, + `${HOURLY_TABLE}.app_name`, + ) + .on( + `${HOURLY_TABLE_VARIANTS}.environment`, + `${HOURLY_TABLE}.environment`, + ) + .on( + `${HOURLY_TABLE_VARIANTS}.timestamp`, + `${HOURLY_TABLE}.timestamp`, + ); }) - .where(`${TABLE}.feature_name`, featureName) + .where(`${HOURLY_TABLE}.feature_name`, featureName) .andWhereRaw( - `${TABLE}.timestamp >= NOW() - INTERVAL '${hoursBack} hours'`, + `${HOURLY_TABLE}.timestamp >= NOW() - INTERVAL '${hoursBack} hours'`, ); const tokens = rows.reduce(variantRowReducer, {}); @@ -263,9 +272,9 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { featureName: string, hoursBack: number = 24, ): Promise<IClientMetricsEnv[]> { - const mainTable = hoursBack <= 48 ? TABLE : DAILY_TABLE; + const mainTable = hoursBack <= 48 ? HOURLY_TABLE : DAILY_TABLE; const variantsTable = - hoursBack <= 48 ? TABLE_VARIANTS : DAILY_TABLE_VARIANTS; + hoursBack <= 48 ? HOURLY_TABLE_VARIANTS : DAILY_TABLE_VARIANTS; const dateTime = hoursBack <= 48 ? 'timestamp' : 'date'; const rows = await this.db<ClientMetricsEnvTable>(mainTable) @@ -298,7 +307,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { featureName: string, hoursBack: number = 24, ): Promise<string[]> { - return this.db<ClientMetricsEnvTable>(TABLE) + return this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .distinct() .where({ feature_name: featureName }) .andWhereRaw(`timestamp >= NOW() - INTERVAL '${hoursBack} hours'`) @@ -310,7 +319,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { appName: string, hoursBack: number = 24, ): Promise<string[]> { - return this.db<ClientMetricsEnvTable>(TABLE) + return this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .distinct() .where({ app_name: appName }) .andWhereRaw(`timestamp >= NOW() - INTERVAL '${hoursBack} hours'`) @@ -319,7 +328,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { } async clearMetrics(hoursAgo: number): Promise<void> { - return this.db<ClientMetricsEnvTable>(TABLE) + return this.db<ClientMetricsEnvTable>(HOURLY_TABLE) .whereRaw(`timestamp <= NOW() - INTERVAL '${hoursAgo} hours'`) .del(); } @@ -330,6 +339,30 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { .del(); } + async countPreviousDayMetrics(): Promise<{ + enabledCount: number; + variantCount: number; + }> { + const enabledCountQuery = this.db(HOURLY_TABLE) + .whereRaw("timestamp >= CURRENT_DATE - INTERVAL '1 day'") + .andWhereRaw('timestamp < CURRENT_DATE') + .count() + .first(); + const variantCountQuery = this.db(HOURLY_TABLE_VARIANTS) + .whereRaw("timestamp >= CURRENT_DATE - INTERVAL '1 day'") + .andWhereRaw('timestamp < CURRENT_DATE') + .count() + .first(); + const [enabledCount, variantCount] = await Promise.all([ + enabledCountQuery, + variantCountQuery, + ]); + return { + enabledCount: Number(enabledCount?.count || 0), + variantCount: Number(variantCount?.count || 0), + }; + } + // aggregates all hourly metrics from a previous day into daily metrics async aggregateDailyMetrics(): Promise<void> { const rawQuery: string = ` @@ -342,7 +375,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { SUM(yes) as yes, SUM(no) as no FROM - ${TABLE} + ${HOURLY_TABLE} WHERE timestamp >= CURRENT_DATE - INTERVAL '1 day' AND timestamp < CURRENT_DATE @@ -361,7 +394,7 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { variant, SUM(count) as count FROM - ${TABLE_VARIANTS} + ${HOURLY_TABLE_VARIANTS} WHERE timestamp >= CURRENT_DATE - INTERVAL '1 day' AND timestamp < CURRENT_DATE diff --git a/src/lib/services/client-metrics/metrics-service-v2.test.ts b/src/lib/services/client-metrics/metrics-service-v2.test.ts index 0995bc8515..b414bb2ebf 100644 --- a/src/lib/services/client-metrics/metrics-service-v2.test.ts +++ b/src/lib/services/client-metrics/metrics-service-v2.test.ts @@ -234,3 +234,57 @@ test('get hourly client metrics for a toggle', async () => { }, ]); }); + +test('aggregate previous day metrics when metrics count is below limit', async () => { + const enabledCount = 2; + const variantCount = 4; + let limit = 5; + let aggregationCalled = false; + let recordedWarning = ''; + const clientMetricsStoreV2 = { + aggregateDailyMetrics() { + aggregationCalled = true; + return Promise.resolve(); + }, + countPreviousDayMetrics() { + return { enabledCount, variantCount }; + }, + } as unknown as IClientMetricsStoreV2; + const config = { + flagResolver: { + isEnabled() { + return true; + }, + getVariant() { + return { payload: { value: limit } }; + }, + }, + getLogger() { + return { + warn(message: string) { + recordedWarning = message; + }, + }; + }, + } as unknown as IUnleashConfig; + const lastSeenService = {} as LastSeenService; + const service = new ClientMetricsServiceV2( + { clientMetricsStoreV2 }, + config, + lastSeenService, + ); + + await service.aggregateDailyMetrics(); + + expect(recordedWarning).toBe( + 'Skipping previous day metrics aggregation. Too many results. Expected max value: 5, Actual value: 6', + ); + expect(aggregationCalled).toBe(false); + + recordedWarning = ''; + limit = 6; + await service.aggregateDailyMetrics(); + + expect(recordedWarning).toBe(''); + expect(aggregationCalled).toBe(true); +}); diff --git a/src/lib/services/client-metrics/metrics-service-v2.ts b/src/lib/services/client-metrics/metrics-service-v2.ts index 7f64b00924..e3a8910d5c 100644 --- a/src/lib/services/client-metrics/metrics-service-v2.ts +++ b/src/lib/services/client-metrics/metrics-service-v2.ts @@ -31,7 +31,7 @@ export default class ClientMetricsServiceV2 { private lastSeenService: LastSeenService; - private flagResolver: Pick<IFlagResolver, 'isEnabled'>; + private flagResolver: Pick<IFlagResolver, 'isEnabled' | 'getVariant'>; private logger: Logger; @@ -61,7 +61,26 @@ export default class ClientMetricsServiceV2 { async aggregateDailyMetrics() { if (this.flagResolver.isEnabled('extendedUsageMetrics')) { - await this.clientMetricsStoreV2.aggregateDailyMetrics(); + const { enabledCount, variantCount } = + await this.clientMetricsStoreV2.countPreviousDayMetrics(); + const { payload } = this.flagResolver.getVariant( + 'extendedUsageMetrics', + ); + + const limit = + payload?.value && Number.isInteger(parseInt(payload?.value)) + ? parseInt(payload?.value) + : 3600000; + + const totalCount = enabledCount + variantCount; + + if (totalCount <= limit) { + await this.clientMetricsStoreV2.aggregateDailyMetrics(); + } else { + this.logger.warn( + `Skipping previous day metrics aggregation. Too many results. Expected max value: ${limit}, Actual value: ${totalCount}`, + ); + } } } diff --git a/src/lib/types/stores/client-metrics-store-v2.ts b/src/lib/types/stores/client-metrics-store-v2.ts index 9f148c8879..8d0d70f264 100644 --- a/src/lib/types/stores/client-metrics-store-v2.ts +++ b/src/lib/types/stores/client-metrics-store-v2.ts @@ -39,5 +39,9 @@ export interface IClientMetricsStoreV2 ): Promise<string[]>; clearMetrics(hoursAgo: number): Promise<void>; clearDailyMetrics(daysAgo: number): Promise<void>; + countPreviousDayMetrics(): Promise<{ + enabledCount: number; + variantCount: number; + }>; aggregateDailyMetrics(): Promise<void>; } diff --git a/src/test/fixtures/fake-client-metrics-store-v2.ts b/src/test/fixtures/fake-client-metrics-store-v2.ts index 19a35293a3..c06eb0e9ed 100644 --- a/src/test/fixtures/fake-client-metrics-store-v2.ts +++ b/src/test/fixtures/fake-client-metrics-store-v2.ts @@ -29,6 +29,12 @@ export default class FakeClientMetricsStoreV2 clearDailyMetrics(daysBack: number): Promise<void> { return Promise.resolve(); } + countPreviousDayMetrics(): Promise<{ + enabledCount: number; + variantCount: number; + }> { + return Promise.resolve({ enabledCount: 0, variantCount: 0 }); + } aggregateDailyMetrics(): Promise<void> { return Promise.resolve(); }