From 50fe3ebcaf7b3bf54510d17e09fd4d82146fcb48 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Fri, 5 May 2023 11:10:54 +0300 Subject: [PATCH] feat: metrics for variants (#3685) --- frontend/src/interfaces/uiConfig.ts | 1 + .../__snapshots__/create-config.test.ts.snap | 2 + src/lib/db/client-metrics-store-v2.ts | 118 ++++++++++++++++-- src/lib/db/index.ts | 6 +- .../spec/client-metrics-schema.test.ts | 2 +- .../feature-environment-metrics-schema.ts | 13 ++ .../client-metrics/metrics-service-v2.ts | 4 +- src/lib/types/experimental.ts | 4 + .../types/stores/client-metrics-store-v2.ts | 6 + src/lib/util/collapseHourlyMetrics.ts | 49 +++++++- .../20230504145945-variant-metrics.js | 38 ++++++ src/server-dev.ts | 1 + .../e2e/api/admin/client-metrics.e2e.test.ts | 61 +++++++-- .../__snapshots__/openapi.e2e.test.ts.snap | 13 ++ src/test/e2e/migrator.e2e.test.ts | 1 + src/test/examples/client-metrics.json | 12 +- 16 files changed, 303 insertions(+), 28 deletions(-) create mode 100644 src/migrations/20230504145945-variant-metrics.js diff --git a/frontend/src/interfaces/uiConfig.ts b/frontend/src/interfaces/uiConfig.ts index 9c4a27014f..a979435496 100644 --- a/frontend/src/interfaces/uiConfig.ts +++ b/frontend/src/interfaces/uiConfig.ts @@ -50,6 +50,7 @@ export interface IFlags { groupRootRoles?: boolean; strategyDisable?: boolean; googleAuthEnabled?: boolean; + variantMetrics?: boolean; } export interface IVersionInfo { diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 82404106d5..5c79e39800 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -85,6 +85,7 @@ exports[`should create default config 1`] = ` "strategyDisable": false, "strategyTitle": false, "strictSchemaValidation": false, + "variantMetrics": false, }, }, "flagResolver": FlagResolver { @@ -108,6 +109,7 @@ exports[`should create default config 1`] = ` "strategyDisable": false, "strategyTitle": false, "strictSchemaValidation": false, + "variantMetrics": false, }, "externalResolver": { "isEnabled": [Function], diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts index 271dbf4f25..df75a7f45c 100644 --- a/src/lib/db/client-metrics-store-v2.ts +++ b/src/lib/db/client-metrics-store-v2.ts @@ -2,23 +2,37 @@ import { Logger, LogProvider } from '../logger'; import { IClientMetricsEnv, IClientMetricsEnvKey, + IClientMetricsEnvVariant, IClientMetricsStoreV2, } from '../types/stores/client-metrics-store-v2'; import NotFoundError from '../error/notfound-error'; import { startOfHour } from 'date-fns'; -import { collapseHourlyMetrics } from '../util/collapseHourlyMetrics'; +import { + collapseHourlyMetrics, + spreadVariants, +} from '../util/collapseHourlyMetrics'; import { Db } from './db'; +import { IFlagResolver } from '../types'; -interface ClientMetricsEnvTable { +interface ClientMetricsBaseTable { feature_name: string; app_name: string; environment: string; timestamp: Date; +} + +interface ClientMetricsEnvTable extends ClientMetricsBaseTable { yes: number; no: number; } +interface ClientMetricsEnvVariantTable extends ClientMetricsBaseTable { + variant: string; + count: number; +} + const TABLE = 'client_metrics_env'; +const TABLE_VARIANTS = 'client_metrics_env_variants'; const fromRow = (row: ClientMetricsEnvTable) => ({ featureName: row.feature_name, @@ -38,14 +52,58 @@ const toRow = (metric: IClientMetricsEnv): ClientMetricsEnvTable => ({ no: metric.no, }); +const toVariantRow = ( + metric: IClientMetricsEnvVariant, +): ClientMetricsEnvVariantTable => ({ + feature_name: metric.featureName, + app_name: metric.appName, + environment: metric.environment, + timestamp: startOfHour(metric.timestamp), + variant: metric.variant, + count: metric.count, +}); + +const variantRowReducer = (acc, tokenRow) => { + const { + feature_name: featureName, + app_name: appName, + environment, + timestamp, + yes, + no, + variant, + count, + } = tokenRow; + const key = `${featureName}_${appName}_${environment}_${timestamp}_${yes}_${no}`; + if (!acc[key]) { + acc[key] = { + featureName, + appName, + environment, + timestamp, + yes: Number(yes), + no: Number(no), + variants: {}, + }; + } + if (variant) { + acc[key].variants[variant] = count; + } + + return acc; +}; + export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { private db: Db; private logger: Logger; - constructor(db: Db, getLogger: LogProvider) { + private flagResolver: IFlagResolver; + + constructor(db: Db, getLogger: LogProvider, flagResolver: IFlagResolver) { this.db = db; this.logger = getLogger('client-metrics-store-v2.js'); + this.flagResolver = flagResolver; } async get(key: IClientMetricsEnvKey): Promise { @@ -103,7 +161,6 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { if (!metrics || metrics.length == 0) { return; } - const rows = collapseHourlyMetrics(metrics).map(toRow); // Sort the rows to avoid deadlocks @@ -118,20 +175,61 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 { const insert = this.db(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`; await this.db.raw(query); + + if (this.flagResolver.isEnabled('variantMetrics')) { + const variantRows = spreadVariants(metrics).map(toVariantRow); + if (variantRows.length > 0) { + const insertVariants = this.db( + TABLE_VARIANTS, + ) + .insert(variantRows) + .toQuery(); + const variantsQuery = `${insertVariants.toString()} ON CONFLICT (feature_name, app_name, environment, timestamp, variant) DO UPDATE SET "count" = "client_metrics_env_variants"."count" + EXCLUDED.count`; + await this.db.raw(variantsQuery); + } + } } async getMetricsForFeatureToggle( featureName: string, hoursBack: number = 24, ): Promise { - const rows = await this.db(TABLE) - .select('*') - .where({ feature_name: featureName }) - .andWhereRaw(`timestamp >= NOW() - INTERVAL '${hoursBack} hours'`); - return rows.map(fromRow); + if (this.flagResolver.isEnabled('variantMetrics')) { + const rows = await this.db(TABLE) + .select([`${TABLE}.*`, 'variant', 'count']) + .leftJoin(TABLE_VARIANTS, function () { + this.on( + `${TABLE_VARIANTS}.feature_name`, + `${TABLE}.feature_name`, + ) + .on(`${TABLE_VARIANTS}.app_name`, `${TABLE}.app_name`) + .on( + `${TABLE_VARIANTS}.environment`, + `${TABLE}.environment`, + ) + .on( + `${TABLE_VARIANTS}.timestamp`, + `${TABLE}.timestamp`, + ); + }) + .where(`${TABLE}.feature_name`, featureName) + .andWhereRaw( + `${TABLE}.timestamp >= NOW() - INTERVAL '${hoursBack} hours'`, + ); + + const tokens = rows.reduce(variantRowReducer, {}); + return Object.values(tokens); + } else { + const rows = await this.db(TABLE) + .select('*') + .where({ feature_name: featureName }) + .andWhereRaw( + `timestamp >= NOW() - INTERVAL '${hoursBack} hours'`, + ); + return rows.map(fromRow); + } } async getSeenAppsForFeatureToggle( diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 0baefaad43..f48557d450 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -56,7 +56,11 @@ export const createStores = ( getLogger, ), clientInstanceStore: new ClientInstanceStore(db, eventBus, getLogger), - clientMetricsStoreV2: new ClientMetricsStoreV2(db, getLogger), + clientMetricsStoreV2: new ClientMetricsStoreV2( + db, + getLogger, + config.flagResolver, + ), contextFieldStore: new ContextFieldStore(db, getLogger), settingStore: new SettingStore(db, getLogger), userStore: new UserStore(db, getLogger), diff --git a/src/lib/openapi/spec/client-metrics-schema.test.ts b/src/lib/openapi/spec/client-metrics-schema.test.ts index 05dd2f405d..b7361cb6e0 100644 --- a/src/lib/openapi/spec/client-metrics-schema.test.ts +++ b/src/lib/openapi/spec/client-metrics-schema.test.ts @@ -13,7 +13,7 @@ test('clientMetricsSchema full', () => { someToggle: { yes: 52, no: 2, - variants: {}, + variants: { someVariant: 52, newVariant: 33 }, }, }, }, diff --git a/src/lib/openapi/spec/feature-environment-metrics-schema.ts b/src/lib/openapi/spec/feature-environment-metrics-schema.ts index 00a3b8690e..4a63d536a6 100644 --- a/src/lib/openapi/spec/feature-environment-metrics-schema.ts +++ b/src/lib/openapi/spec/feature-environment-metrics-schema.ts @@ -42,6 +42,19 @@ export const featureEnvironmentMetricsSchema = { example: 50, minimum: 0, }, + variants: { + description: 'How many times each variant was returned', + type: 'object', + additionalProperties: { + type: 'integer', + minimum: 0, + }, + example: { + variantA: 15, + variantB: 25, + variantC: 5, + }, + }, }, components: { dateSchema, diff --git a/src/lib/services/client-metrics/metrics-service-v2.ts b/src/lib/services/client-metrics/metrics-service-v2.ts index e3d78f3e7b..0b97023865 100644 --- a/src/lib/services/client-metrics/metrics-service-v2.ts +++ b/src/lib/services/client-metrics/metrics-service-v2.ts @@ -1,5 +1,5 @@ import { Logger } from '../../logger'; -import { IUnleashConfig } from '../../server-impl'; +import { IUnleashConfig } from '../../types'; import { IUnleashStores } from '../../types'; import { ToggleMetricsSummary } from '../../types/models/metrics'; import { @@ -90,8 +90,10 @@ export default class ClientMetricsServiceV2 { timestamp: value.bucket.start, //we might need to approximate between start/stop... yes: value.bucket.toggles[name].yes, no: value.bucket.toggles[name].no, + variants: value.bucket.toggles[name].variants, })); await this.registerBulkMetrics(clientMetrics); + this.config.eventBus.emit(CLIENT_METRICS, value); } diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 60f5a467b5..8f4dc22f03 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -68,6 +68,10 @@ const flags = { process.env.GOOGLE_AUTH_ENABLED, false, ), + variantMetrics: parseEnvVarBoolean( + process.env.UNLEASH_VARIANT_METRICS, + false, + ), }; export const defaultExperimentalOptions: IExperimentalOptions = { diff --git a/src/lib/types/stores/client-metrics-store-v2.ts b/src/lib/types/stores/client-metrics-store-v2.ts index 52bec0c5cf..8d99b28265 100644 --- a/src/lib/types/stores/client-metrics-store-v2.ts +++ b/src/lib/types/stores/client-metrics-store-v2.ts @@ -10,6 +10,12 @@ export interface IClientMetricsEnvKey { export interface IClientMetricsEnv extends IClientMetricsEnvKey { yes: number; no: number; + variants?: Record; +} + +export interface IClientMetricsEnvVariant extends IClientMetricsEnvKey { + variant: string; + count: number; } export interface IClientMetricsStoreV2 diff --git a/src/lib/util/collapseHourlyMetrics.ts b/src/lib/util/collapseHourlyMetrics.ts index f28c3bce45..c18bea26bd 100644 --- a/src/lib/util/collapseHourlyMetrics.ts +++ b/src/lib/util/collapseHourlyMetrics.ts @@ -1,4 +1,7 @@ -import { IClientMetricsEnv } from '../types/stores/client-metrics-store-v2'; +import { + IClientMetricsEnv, + IClientMetricsEnvVariant, +} from '../types/stores/client-metrics-store-v2'; import { startOfHour } from 'date-fns'; const createMetricKey = (metric: IClientMetricsEnv): string => { @@ -10,6 +13,25 @@ const createMetricKey = (metric: IClientMetricsEnv): string => { ].join(); }; +const mergeRecords = ( + firstRecord: Record, + secondRecord: Record, +): Record => { + const result: Record = {}; + + for (const key in firstRecord) { + result[key] = firstRecord[key] + (secondRecord[key] ?? 0); + } + + for (const key in secondRecord) { + if (!(key in result)) { + result[key] = secondRecord[key]; + } + } + + return result; +}; + export const collapseHourlyMetrics = ( metrics: IClientMetricsEnv[], ): IClientMetricsEnv[] => { @@ -25,7 +47,32 @@ export const collapseHourlyMetrics = ( } else { grouped[key].yes = metric.yes + (grouped[key].yes || 0); grouped[key].no = metric.no + (grouped[key].no || 0); + + if (metric.variants) { + grouped[key].variants = mergeRecords( + metric.variants, + grouped[key].variants, + ); + } } }); return Object.values(grouped); }; + +export const spreadVariants = ( + metrics: IClientMetricsEnv[], +): IClientMetricsEnvVariant[] => { + return metrics.flatMap((item) => { + if (!item.variants) { + return []; + } + return Object.entries(item.variants).map(([variant, count]) => ({ + featureName: item.featureName, + appName: item.appName, + environment: item.environment, + timestamp: item.timestamp, + variant, + count, + })); + }); +}; diff --git a/src/migrations/20230504145945-variant-metrics.js b/src/migrations/20230504145945-variant-metrics.js new file mode 100644 index 0000000000..e345635f1a --- /dev/null +++ b/src/migrations/20230504145945-variant-metrics.js @@ -0,0 +1,38 @@ +'use strict'; + +exports.up = function (db, callback) { + db.runSql( + ` + CREATE TABLE IF NOT EXISTS client_metrics_env_variants ( + feature_name VARCHAR(255), + app_name VARCHAR(255), + environment VARCHAR(100), + timestamp TIMESTAMP WITH TIME ZONE, + variant text, + count INTEGER DEFAULT 0, + FOREIGN KEY ( + feature_name, app_name, environment, + timestamp + ) REFERENCES client_metrics_env ( + feature_name, app_name, environment, + timestamp + ) ON UPDATE CASCADE ON DELETE CASCADE, + PRIMARY KEY( + feature_name, app_name, environment, + timestamp, variant + ) + ); + + `, + callback, + ); +}; + +exports.down = function (db, callback) { + db.runSql( + ` + DROP TABLE IF EXISTS client_metrics_env_variants; + `, + callback, + ); +}; diff --git a/src/server-dev.ts b/src/server-dev.ts index a0f2123ef7..3781a926d4 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -38,6 +38,7 @@ process.nextTick(async () => { embedProxyFrontend: true, anonymiseEventLog: false, responseTimeWithAppNameKillSwitch: false, + variantMetrics: true, }, }, authentication: { diff --git a/src/test/e2e/api/admin/client-metrics.e2e.test.ts b/src/test/e2e/api/admin/client-metrics.e2e.test.ts index d465e85c84..d851a6a971 100644 --- a/src/test/e2e/api/admin/client-metrics.e2e.test.ts +++ b/src/test/e2e/api/admin/client-metrics.e2e.test.ts @@ -7,9 +7,36 @@ import { subHours } from 'date-fns'; let app; let db: ITestDb; +const fetchHoursBack = (hoursBack: number, feature: string = 'demo') => { + return app.request + .get( + `/api/admin/client-metrics/features/${feature}/raw?hoursBack=${hoursBack}`, + ) + .expect('Content-Type', /json/) + .expect(200) + .then((res) => res.body); +}; + beforeAll(async () => { - db = await dbInit('client_metrics_serial', getLogger); - app = await setupAppWithCustomConfig(db.stores, {}); + db = await dbInit('client_metrics_serial', getLogger, { + experimental: { + flags: { + variantMetrics: true, + }, + }, + }); + app = await setupAppWithCustomConfig( + db.stores, + { + experimental: { + flags: { + variantMetrics: true, + strictSchemaValidation: true, + }, + }, + }, + db.rawDatabase, + ); }); afterAll(async () => { @@ -124,16 +151,6 @@ test('should support the hoursBack query param for raw metrics', async () => { await db.stores.clientMetricsStoreV2.batchInsertMetrics(metrics); - const fetchHoursBack = (hoursBack: number) => { - return app.request - .get( - `/api/admin/client-metrics/features/demo/raw?hoursBack=${hoursBack}`, - ) - .expect('Content-Type', /json/) - .expect(200) - .then((res) => res.body); - }; - const hours1 = await fetchHoursBack(1); const hours24 = await fetchHoursBack(24); const hours48 = await fetchHoursBack(48); @@ -291,3 +308,23 @@ test('should only include last hour of metrics return toggle summary', async () expect(test.no).toBe(6); expect(demo.seenApplications).toStrictEqual(['backend-api', 'web']); }); + +test('should support posting and receiving variants data', async () => { + const date = new Date(); + const metric = { + featureName: 'demo', + appName: 'web', + environment: 'default', + timestamp: date, + yes: 7, + no: 1, + variants: { red: 3, blue: 4 }, + }; + const metrics: IClientMetricsEnv[] = [metric]; + + await db.stores.clientMetricsStoreV2.batchInsertMetrics(metrics); + + const hours1 = await fetchHoursBack(1); + + expect(hours1.data[0].variants).toMatchObject(metric.variants); +}); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 941bb7d666..78bcb1fc8a 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -2034,6 +2034,19 @@ The provider you choose for your addon dictates what properties the \`parameters "description": "The start of the time window these metrics are valid for. The window is usually 1 hour wide", "example": "1926-05-08T12:00:00.000Z", }, + "variants": { + "additionalProperties": { + "minimum": 0, + "type": "integer", + }, + "description": "How many times each variant was returned", + "example": { + "variantA": 15, + "variantB": 25, + "variantC": 5, + }, + "type": "object", + }, "yes": { "description": "How many times the toggle evaluated to true", "example": 974, diff --git a/src/test/e2e/migrator.e2e.test.ts b/src/test/e2e/migrator.e2e.test.ts index 21cf1ed2ba..9e7d6d2751 100644 --- a/src/test/e2e/migrator.e2e.test.ts +++ b/src/test/e2e/migrator.e2e.test.ts @@ -13,6 +13,7 @@ async function initSchema(db: IDBOption): Promise { } test('Up & down migrations work', async () => { + jest.setTimeout(15000); const config = createTestConfig({ db: { ...getDbConfig(), diff --git a/src/test/examples/client-metrics.json b/src/test/examples/client-metrics.json index 2e035a4bc5..f441928d69 100644 --- a/src/test/examples/client-metrics.json +++ b/src/test/examples/client-metrics.json @@ -7,11 +7,19 @@ "toggles": { "toggle-name-1": { "yes": 123, - "no": 321 + "no": 321, + "variants": { + "variant-1": 123, + "variant-2": 321 + } }, "toggle-name-2": { "yes": 111, - "no": 0 + "no": 0, + "variants": { + "variant-3": 111, + "variant-4": 0 + } } } }