From 574eb284b93aa05bccaef52db5150c978c7a5f1b Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Thu, 25 Apr 2024 09:27:20 +0200 Subject: [PATCH] fix: client metrics structure lifecycle (#6924) --- .../fake-feature-lifecycle-store.ts | 12 +++- .../feature-lifecycle-service.test.ts | 7 +- .../feature-lifecycle-service.ts | 65 +++++++++++-------- .../feature-lifecycle-store-type.ts | 2 +- .../feature-lifecycle-store.ts | 14 ++-- .../feature-lifecycle.e2e.test.ts | 9 ++- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/lib/features/feature-lifecycle/fake-feature-lifecycle-store.ts b/src/lib/features/feature-lifecycle/fake-feature-lifecycle-store.ts index ee734caf713..46a813c5581 100644 --- a/src/lib/features/feature-lifecycle/fake-feature-lifecycle-store.ts +++ b/src/lib/features/feature-lifecycle/fake-feature-lifecycle-store.ts @@ -7,7 +7,17 @@ import type { export class FakeFeatureLifecycleStore implements IFeatureLifecycleStore { private lifecycles: Record = {}; - async insert(featureLifecycleStage: FeatureLifecycleStage): Promise { + async insert( + featureLifecycleStages: FeatureLifecycleStage[], + ): Promise { + await Promise.all( + featureLifecycleStages.map((stage) => this.insertOne(stage)), + ); + } + + private async insertOne( + featureLifecycleStage: FeatureLifecycleStage, + ): Promise { if (await this.stageExists(featureLifecycleStage)) { return; } diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle-service.test.ts b/src/lib/features/feature-lifecycle/feature-lifecycle-service.test.ts index 849432f3cb3..6c7cfacdab7 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle-service.test.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle-service.test.ts @@ -23,7 +23,10 @@ test('can insert and read lifecycle stages', async () => { const featureName = 'testFeature'; function emitMetricsEvent(environment: string) { - eventBus.emit(CLIENT_METRICS, { featureName, environment }); + eventBus.emit(CLIENT_METRICS, { + bucket: { toggles: { [featureName]: 'irrelevant' } }, + environment, + }); } function reachedStage(name: StageName) { return new Promise((resolve) => @@ -100,7 +103,7 @@ test('ignores lifecycle state updates when flag disabled', async () => { await eventStore.emit(FEATURE_CREATED, { featureName }); await eventStore.emit(FEATURE_COMPLETED, { featureName }); await eventBus.emit(CLIENT_METRICS, { - featureName, + bucket: { toggles: { [featureName]: 'irrelevant' } }, environment: 'development', }); await eventStore.emit(FEATURE_ARCHIVED, { featureName }); diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle-service.ts b/src/lib/features/feature-lifecycle/feature-lifecycle-service.ts index 7ebf6e706f1..7d21fe244b9 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle-service.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle-service.ts @@ -14,6 +14,7 @@ import type { } from './feature-lifecycle-store-type'; import EventEmitter from 'events'; import type { Logger } from '../../logger'; +import type { ValidatedClientMetrics } from '../metrics/shared/schema'; export const STAGE_ENTERED = 'STAGE_ENTERED'; @@ -70,15 +71,18 @@ export class FeatureLifecycleService extends EventEmitter { this.featureInitialized(event.featureName), ); }); - this.eventBus.on(CLIENT_METRICS, async (event) => { - if (!event.featureName || !event.environment) return; - await this.checkEnabled(() => - this.featureReceivedMetrics( - event.featureName, - event.environment, - ), - ); - }); + this.eventBus.on( + CLIENT_METRICS, + async (event: ValidatedClientMetrics) => { + if (event.environment) { + const features = Object.keys(event.bucket.toggles); + const environment = event.environment; + await this.checkEnabled(() => + this.featuresReceivedMetrics(features, environment), + ); + } + }, + ); this.eventStore.on(FEATURE_COMPLETED, async (event) => { await this.checkEnabled(() => this.featureCompleted(event.featureName), @@ -96,25 +100,26 @@ export class FeatureLifecycleService extends EventEmitter { } private async featureInitialized(feature: string) { - await this.featureLifecycleStore.insert({ feature, stage: 'initial' }); + await this.featureLifecycleStore.insert([ + { feature, stage: 'initial' }, + ]); this.emit(STAGE_ENTERED, { stage: 'initial' }); } private async stageReceivedMetrics( - feature: string, + features: string[], stage: 'live' | 'pre-live', ) { - const stageExists = await this.featureLifecycleStore.stageExists({ - stage, - feature, - }); - if (!stageExists) { - await this.featureLifecycleStore.insert({ feature, stage }); - this.emit(STAGE_ENTERED, { stage }); - } + await this.featureLifecycleStore.insert( + features.map((feature) => ({ feature, stage })), + ); + this.emit(STAGE_ENTERED, { stage }); } - private async featureReceivedMetrics(feature: string, environment: string) { + private async featuresReceivedMetrics( + features: string[], + environment: string, + ) { try { const env = await this.environmentStore.get(environment); @@ -122,28 +127,32 @@ export class FeatureLifecycleService extends EventEmitter { return; } if (env.type === 'production') { - await this.stageReceivedMetrics(feature, 'live'); + await this.stageReceivedMetrics(features, 'live'); } else if (env.type === 'development') { - await this.stageReceivedMetrics(feature, 'pre-live'); + await this.stageReceivedMetrics(features, 'pre-live'); } } catch (e) { this.logger.warn( - `Error handling metrics for ${feature} in ${environment}`, + `Error handling ${features.length} metrics in ${environment}`, e, ); } } private async featureCompleted(feature: string) { - await this.featureLifecycleStore.insert({ - feature, - stage: 'completed', - }); + await this.featureLifecycleStore.insert([ + { + feature, + stage: 'completed', + }, + ]); this.emit(STAGE_ENTERED, { stage: 'completed' }); } private async featureArchived(feature: string) { - await this.featureLifecycleStore.insert({ feature, stage: 'archived' }); + await this.featureLifecycleStore.insert([ + { feature, stage: 'archived' }, + ]); this.emit(STAGE_ENTERED, { stage: 'archived' }); } } diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle-store-type.ts b/src/lib/features/feature-lifecycle/feature-lifecycle-store-type.ts index ac73b0b3d8e..963a451d0c9 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle-store-type.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle-store-type.ts @@ -8,7 +8,7 @@ export type FeatureLifecycleStage = { export type FeatureLifecycleView = IFeatureLifecycleStage[]; export interface IFeatureLifecycleStore { - insert(featureLifecycleStage: FeatureLifecycleStage): Promise; + insert(featureLifecycleStages: FeatureLifecycleStage[]): Promise; get(feature: string): Promise; stageExists(stage: FeatureLifecycleStage): Promise; } diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle-store.ts b/src/lib/features/feature-lifecycle/feature-lifecycle-store.ts index 21ba4d6b025..f7bbb3036c4 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle-store.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle-store.ts @@ -19,12 +19,16 @@ export class FeatureLifecycleStore implements IFeatureLifecycleStore { this.db = db; } - async insert(featureLifecycleStage: FeatureLifecycleStage): Promise { + async insert( + featureLifecycleStages: FeatureLifecycleStage[], + ): Promise { await this.db('feature_lifecycles') - .insert({ - feature: featureLifecycleStage.feature, - stage: featureLifecycleStage.stage, - }) + .insert( + featureLifecycleStages.map((stage) => ({ + feature: stage.feature, + stage: stage.stage, + })), + ) .returning('*') .onConflict(['feature', 'stage']) .ignore(); diff --git a/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts b/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts index 72f74fe994e..c4fd946af9c 100644 --- a/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts +++ b/src/lib/features/feature-lifecycle/feature-lifecycle.e2e.test.ts @@ -86,16 +86,21 @@ test('should return lifecycle stages', async () => { await reachedStage('initial'); await expectFeatureStage('initial'); eventBus.emit(CLIENT_METRICS, { - featureName: 'my_feature_a', + bucket: { toggles: { my_feature_a: 'irrelevant' } }, environment: 'default', }); // missing feature eventBus.emit(CLIENT_METRICS, { environment: 'default', + bucket: { toggles: {} }, }); // non existent env eventBus.emit(CLIENT_METRICS, { - featureName: 'my_feature_a', + bucket: { + toggles: { + my_feature_a: 'irrelevant', + }, + }, environment: 'non-existent', }); await reachedStage('live');