From 6c5df9f2c7397ed473144b3866f809f766c68fc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Fri, 12 May 2023 19:52:11 +0200 Subject: [PATCH] feat: improve frontend config freshness to < 1s (#3749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR reuses the revision Id information from the "optimal 304 for server SDKs" to improve the freshness of the frontend API config data. In addition it allows us to reduce the polling (and eventually remove it when we are confident). --------- Co-authored-by: Gastón Fournier --- .../__snapshots__/create-config.test.ts.snap | 2 +- src/lib/create-config.ts | 2 +- .../configuration-revision-service.ts | 46 +++++++++++++++++++ src/lib/proxy/proxy-repository.ts | 15 ++++-- src/lib/routes/client-api/feature.test.ts | 8 ++-- src/lib/routes/client-api/feature.ts | 15 +++--- src/lib/services/event-service.ts | 19 -------- src/lib/services/index.ts | 15 +++++- src/lib/services/proxy-service.ts | 1 + src/lib/types/services.ts | 4 +- .../api/client/feature.optimal304.e2e.test.ts | 2 +- 11 files changed, 89 insertions(+), 40 deletions(-) create mode 100644 src/lib/features/feature-toggle/configuration-revision-service.ts diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 8b88ce1d77..5b300af392 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -112,7 +112,7 @@ exports[`should create default config 1`] = ` }, }, "frontendApi": { - "refreshIntervalInMs": 10000, + "refreshIntervalInMs": 20000, }, "frontendApiOrigins": [ "*", diff --git a/src/lib/create-config.ts b/src/lib/create-config.ts index 0e12436da0..0aacf5efe3 100644 --- a/src/lib/create-config.ts +++ b/src/lib/create-config.ts @@ -431,7 +431,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig { const frontendApi = options.frontendApi || { refreshIntervalInMs: parseEnvVarNumber( process.env.FRONTEND_API_REFRESH_INTERVAL_MS, - 10000, + 20000, ), }; diff --git a/src/lib/features/feature-toggle/configuration-revision-service.ts b/src/lib/features/feature-toggle/configuration-revision-service.ts new file mode 100644 index 0000000000..b3b229f2cc --- /dev/null +++ b/src/lib/features/feature-toggle/configuration-revision-service.ts @@ -0,0 +1,46 @@ +import { EventEmitter } from 'stream'; +import { Logger } from '../../logger'; +import { IEventStore, IUnleashConfig, IUnleashStores } from '../../types'; + +export const UPDATE_REVISION = 'UPDATE_REVISION'; + +export default class ConfigurationRevisionService extends EventEmitter { + private logger: Logger; + + private eventStore: IEventStore; + + private revisionId: number; + + constructor( + { eventStore }: Pick, + { getLogger }: Pick, + ) { + super(); + this.logger = getLogger('configuration-revision-service.ts'); + this.eventStore = eventStore; + } + + async getMaxRevisionId(): Promise { + if (this.revisionId) { + return this.revisionId; + } else { + return this.updateMaxRevisionId(); + } + } + + async updateMaxRevisionId(): Promise { + const revisionId = await this.eventStore.getMaxRevisionId( + this.revisionId, + ); + if (this.revisionId !== revisionId) { + this.logger.debug( + 'Updating feature configuration with new revision Id', + revisionId, + ); + this.emit(UPDATE_REVISION, revisionId); + this.revisionId = revisionId; + } + + return this.revisionId; + } +} diff --git a/src/lib/proxy/proxy-repository.ts b/src/lib/proxy/proxy-repository.ts index a3c185c5e1..821c11cebd 100644 --- a/src/lib/proxy/proxy-repository.ts +++ b/src/lib/proxy/proxy-repository.ts @@ -10,8 +10,10 @@ import { } from '../util/offline-unleash-client'; import { ALL_ENVS, ALL_PROJECTS } from '../util/constants'; import { UnleashEvents } from 'unleash-client'; -import { ANY_EVENT } from '../util/anyEventEmitter'; import { Logger } from '../logger'; +import ConfigurationRevisionService, { + UPDATE_REVISION, +} from '../features/feature-toggle/configuration-revision-service'; type Config = Pick; @@ -19,7 +21,7 @@ type Stores = Pick; type Services = Pick< IUnleashServices, - 'featureToggleServiceV2' | 'segmentService' + 'featureToggleServiceV2' | 'segmentService' | 'configurationRevisionService' >; export class ProxyRepository @@ -34,6 +36,8 @@ export class ProxyRepository private readonly services: Services; + private readonly configurationRevisionService: ConfigurationRevisionService; + private readonly token: ApiUser; private features: FeatureInterface[]; @@ -57,6 +61,8 @@ export class ProxyRepository this.logger = config.getLogger('proxy-repository.ts'); this.stores = stores; this.services = services; + this.configurationRevisionService = + services.configurationRevisionService; this.token = token; this.onAnyEvent = this.onAnyEvent.bind(this); this.interval = config.frontendApi.refreshIntervalInMs; @@ -67,6 +73,7 @@ export class ProxyRepository } getToggle(name: string): FeatureInterface { + //@ts-ignore (we must update the node SDK to allow undefined) return this.features.find((feature) => feature.name === name); } @@ -80,14 +87,14 @@ export class ProxyRepository // Reload cached token data whenever something relevant has changed. // For now, simply reload all the data on any EventStore event. - this.stores.eventStore.on(ANY_EVENT, this.onAnyEvent); + this.configurationRevisionService.on(UPDATE_REVISION, this.onAnyEvent); this.emit(UnleashEvents.Ready); this.emit(UnleashEvents.Changed); } stop(): void { - this.stores.eventStore.off(ANY_EVENT, this.onAnyEvent); + this.configurationRevisionService.off(UPDATE_REVISION, this.onAnyEvent); this.running = false; } diff --git a/src/lib/routes/client-api/feature.test.ts b/src/lib/routes/client-api/feature.test.ts index b4930b1c27..83ffa23a31 100644 --- a/src/lib/routes/client-api/feature.test.ts +++ b/src/lib/routes/client-api/feature.test.ts @@ -82,7 +82,7 @@ test('if caching is enabled should memoize', async () => { const openApiService = { respondWithValidation, validPath }; const featureToggleServiceV2 = { getClientFeatures }; const segmentService = { getActive }; - const eventService = { getMaxRevisionId: () => 1 }; + const configurationRevisionService = { getMaxRevisionId: () => 1 }; const controller = new FeatureController( { @@ -94,7 +94,7 @@ test('if caching is enabled should memoize', async () => { // @ts-expect-error due to partial implementation segmentService, // @ts-expect-error due to partial implementation - eventService, + configurationRevisionService, }, { getLogger, @@ -120,7 +120,7 @@ test('if caching is not enabled all calls goes to service', async () => { const featureToggleServiceV2 = { getClientFeatures }; const segmentService = { getActive }; const openApiService = { respondWithValidation, validPath }; - const eventService = { getMaxRevisionId: () => 1 }; + const configurationRevisionService = { getMaxRevisionId: () => 1 }; const controller = new FeatureController( { @@ -132,7 +132,7 @@ test('if caching is not enabled all calls goes to service', async () => { // @ts-expect-error due to partial implementation segmentService, // @ts-expect-error due to partial implementation - eventService, + configurationRevisionService, }, { getLogger, diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index 442aa070be..9cbbc99696 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -26,8 +26,8 @@ import { clientFeaturesSchema, ClientFeaturesSchema, } from '../../openapi/spec/client-features-schema'; -import { ISegmentService } from 'lib/segments/segment-service-interface'; -import { EventService } from 'lib/services'; +import { ISegmentService } from '../../segments/segment-service-interface'; +import ConfigurationRevisionService from '../../features/feature-toggle/configuration-revision-service'; const version = 2; @@ -53,7 +53,7 @@ export default class FeatureController extends Controller { private openApiService: OpenApiService; - private eventService: EventService; + private configurationRevisionService: ConfigurationRevisionService; private featuresAndSegments: ( query: IFeatureToggleQuery, @@ -66,14 +66,14 @@ export default class FeatureController extends Controller { segmentService, clientSpecService, openApiService, - eventService, + configurationRevisionService, }: Pick< IUnleashServices, | 'featureToggleServiceV2' | 'segmentService' | 'clientSpecService' | 'openApiService' - | 'eventService' + | 'configurationRevisionService' >, config: IUnleashConfig, ) { @@ -83,7 +83,7 @@ export default class FeatureController extends Controller { this.segmentService = segmentService; this.clientSpecService = clientSpecService; this.openApiService = openApiService; - this.eventService = eventService; + this.configurationRevisionService = configurationRevisionService; this.logger = config.getLogger('client-api/feature.js'); this.route({ @@ -265,7 +265,8 @@ export default class FeatureController extends Controller { async calculateMeta(query: IFeatureToggleQuery): Promise { // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). - const revisionId = await this.eventService.getMaxRevisionId(); + const revisionId = + await this.configurationRevisionService.getMaxRevisionId(); // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). const queryHash = hashSum(query); diff --git a/src/lib/services/event-service.ts b/src/lib/services/event-service.ts index ec507b43a0..d8dd290628 100644 --- a/src/lib/services/event-service.ts +++ b/src/lib/services/event-service.ts @@ -10,8 +10,6 @@ export default class EventService { private eventStore: IEventStore; - private revisionId: number; - constructor( { eventStore }: Pick, { getLogger }: Pick, @@ -37,21 +35,4 @@ export default class EventService { totalEvents, }; } - - async getMaxRevisionId(): Promise { - if (this.revisionId) { - return this.revisionId; - } else { - return this.updateMaxRevisionId(); - } - } - - async updateMaxRevisionId(): Promise { - this.revisionId = await this.eventStore.getMaxRevisionId( - this.revisionId, - ); - return this.revisionId; - } } - -module.exports = EventService; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 9b731b1e45..22c3be42c1 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -56,6 +56,7 @@ import { createChangeRequestAccessReadModel, createFakeChangeRequestAccessService, } from '../features/change-request-access-service/createChangeRequestAccessReadModel'; +import ConfigurationRevisionService from '../features/feature-toggle/configuration-revision-service'; // TODO: will be moved to scheduler feature directory export const scheduleServices = (services: IUnleashServices): void => { @@ -66,7 +67,7 @@ export const scheduleServices = (services: IUnleashServices): void => { clientInstanceService, projectService, projectHealthService, - eventService, + configurationRevisionService, } = services; schedulerService.schedule( @@ -102,7 +103,9 @@ export const scheduleServices = (services: IUnleashServices): void => { ); schedulerService.schedule( - eventService.updateMaxRevisionId.bind(eventService), + configurationRevisionService.updateMaxRevisionId.bind( + configurationRevisionService, + ), secondsToMilliseconds(1), ); }; @@ -188,11 +191,18 @@ export const createServices = ( featureToggleServiceV2, segmentService, }); + + const configurationRevisionService = new ConfigurationRevisionService( + stores, + config, + ); + const proxyService = new ProxyService(config, stores, { featureToggleServiceV2, clientMetricsServiceV2, segmentService, settingService, + configurationRevisionService, }); const edgeService = new EdgeService(stores, config); @@ -264,6 +274,7 @@ export const createServices = ( exportImportService, transactionalExportImportService, schedulerService, + configurationRevisionService, }; }; diff --git a/src/lib/services/proxy-service.ts b/src/lib/services/proxy-service.ts index b8965fd0f0..c583ed8d01 100644 --- a/src/lib/services/proxy-service.ts +++ b/src/lib/services/proxy-service.ts @@ -31,6 +31,7 @@ type Services = Pick< | 'segmentService' | 'clientMetricsServiceV2' | 'settingService' + | 'configurationRevisionService' >; export class ProxyService { diff --git a/src/lib/types/services.ts b/src/lib/types/services.ts index cca5871037..a21b93def8 100644 --- a/src/lib/types/services.ts +++ b/src/lib/types/services.ts @@ -40,7 +40,8 @@ import { AccountService } from '../services/account-service'; import { SchedulerService } from '../services/scheduler-service'; import { Knex } from 'knex'; import ExportImportService from '../features/export-import-toggles/export-import-service'; -import { ISegmentService } from 'lib/segments/segment-service-interface'; +import { ISegmentService } from '../segments/segment-service-interface'; +import ConfigurationRevisionService from '../features/feature-toggle/configuration-revision-service'; export interface IUnleashServices { accessService: AccessService; @@ -85,6 +86,7 @@ export interface IUnleashServices { favoritesService: FavoritesService; maintenanceService: MaintenanceService; exportImportService: ExportImportService; + configurationRevisionService: ConfigurationRevisionService; schedulerService: SchedulerService; transactionalExportImportService: ( db: Knex.Transaction, diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts index 3ce6f411fd..1effc282fe 100644 --- a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -142,7 +142,7 @@ test('returns 200 when content updates and hash does not match anymore', async ( }, 'test', ); - await app.services.eventService.updateMaxRevisionId(); + await app.services.configurationRevisionService.updateMaxRevisionId(); const res = await app.request .get('/api/client/features')