Skip to content

Commit

Permalink
feat: metrics calculation limit (#5853)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kwasniew
kwasniew committed Jan 12, 2024
1 parent 8ae267e commit 8eb5a53
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 27 deletions.
34 changes: 34 additions & 0 deletions src/lib/db/client-metrics-store-v2.test.ts
View file
Expand Up @@ -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 });
});
83 changes: 58 additions & 25 deletions src/lib/db/client-metrics-store-v2.ts
View file
Expand Up @@ -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) => ({
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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`;
Expand All @@ -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();
Expand All @@ -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, {});
Expand All @@ -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)
Expand Down Expand Up @@ -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'`)
Expand All @@ -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'`)
Expand All @@ -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();
}
Expand All @@ -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 = `
Expand All @@ -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
Expand All @@ -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
Expand Down
54 changes: 54 additions & 0 deletions src/lib/services/client-metrics/metrics-service-v2.test.ts
View file
Expand Up @@ -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);
});
23 changes: 21 additions & 2 deletions src/lib/services/client-metrics/metrics-service-v2.ts
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -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}`,
);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/lib/types/stores/client-metrics-store-v2.ts
View file
Expand Up @@ -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>;
}
6 changes: 6 additions & 0 deletions src/test/fixtures/fake-client-metrics-store-v2.ts
View file
Expand Up @@ -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();
}
Expand Down

0 comments on commit 8eb5a53

Please sign in to comment.