diff --git a/package.json b/package.json index 4ec87bb9fa..78c8b75eb8 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "db-migrate": "0.11.13", "db-migrate-pg": "1.2.2", "db-migrate-shared": "1.2.0", + "deep-object-diff": "^1.1.9", "deepmerge": "^4.2.2", "errorhandler": "^1.5.1", "express": "^4.18.2", @@ -106,6 +107,7 @@ "express-session": "^1.17.1", "fast-json-patch": "^3.1.0", "gravatar-url": "^3.1.0", + "hash-sum": "^2.0.0", "helmet": "^6.0.0", "ip": "^1.1.8", "joi": "^17.3.0", @@ -149,6 +151,7 @@ "@types/express": "4.17.17", "@types/express-session": "1.17.6", "@types/faker": "5.5.9", + "@types/hash-sum": "^1.0.0", "@types/jest": "29.4.0", "@types/js-yaml": "4.0.5", "@types/make-fetch-happen": "10.0.1", diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 68828eb3cc..1327f4a52e 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -81,6 +81,8 @@ exports[`should create default config 1`] = ` "messageBanner": false, "newProjectOverview": false, "notifications": false, + "optimal304": false, + "optimal304Differ": false, "proPlanAutoCharge": false, "projectMode": false, "projectScopedSegments": false, @@ -107,6 +109,8 @@ exports[`should create default config 1`] = ` "messageBanner": false, "newProjectOverview": false, "notifications": false, + "optimal304": false, + "optimal304Differ": false, "proPlanAutoCharge": false, "projectMode": false, "projectScopedSegments": false, diff --git a/src/lib/db/event-store.ts b/src/lib/db/event-store.ts index 344a79ca76..89913c871b 100644 --- a/src/lib/db/event-store.ts +++ b/src/lib/db/event-store.ts @@ -149,6 +149,16 @@ class EventStore implements IEventStore { } } + async getMaxRevisionId(largerThan: number = 0): Promise { + const row = await this.db(TABLE) + .max('id') + .whereNotNull('feature_name') + .orWhere('type', 'segment-update') + .andWhere('id', '>=', largerThan) + .first(); + return row ? row.max : -1; + } + async delete(key: number): Promise { await this.db(TABLE).where({ id: key }).del(); } diff --git a/src/lib/metrics.ts b/src/lib/metrics.ts index 288af655e9..d8b04efc1d 100644 --- a/src/lib/metrics.ts +++ b/src/lib/metrics.ts @@ -141,6 +141,12 @@ export default class MetricsMonitor { labelNames: ['sdk_name', 'sdk_version'], }); + const optimal304DiffingCounter = new client.Counter({ + name: 'optimal_304_diffing', + help: 'Count the Optimal 304 diffing with status', + labelNames: ['status'], + }); + async function collectStaticCounters() { try { const stats = await instanceStatsService.getStats(); @@ -204,6 +210,10 @@ export default class MetricsMonitor { }, ); + eventBus.on('optimal304Differ', ({ status }) => { + optimal304DiffingCounter.labels(status).inc(); + }); + eventBus.on(events.DB_TIME, ({ store, action, time }) => { dbDuration.labels(store, action).observe(time); }); diff --git a/src/lib/routes/client-api/feature.test.ts b/src/lib/routes/client-api/feature.test.ts index ea4093ef3b..57b34bb8ae 100644 --- a/src/lib/routes/client-api/feature.test.ts +++ b/src/lib/routes/client-api/feature.test.ts @@ -43,12 +43,19 @@ let request; let destroy; let featureToggleClientStore; +let flagResolver; + beforeEach(async () => { const setup = await getSetup(); base = setup.base; request = setup.request; featureToggleClientStore = setup.featureToggleClientStore; destroy = setup.destroy; + flagResolver = { + isEnabled: () => { + return false; + }, + }; }); afterEach(() => { @@ -92,6 +99,7 @@ test('if caching is enabled should memoize', async () => { enabled: true, maxAge: secondsToMilliseconds(10), }, + flagResolver, }, ); @@ -126,6 +134,7 @@ test('if caching is not enabled all calls goes to service', async () => { enabled: false, maxAge: secondsToMilliseconds(10), }, + flagResolver, }, ); diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index e5cb763321..d99f2ef9e5 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -1,7 +1,11 @@ import memoizee from 'memoizee'; import { Response } from 'express'; +// eslint-disable-next-line import/no-extraneous-dependencies +import hasSum from 'hash-sum'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { diff } from 'deep-object-diff'; import Controller from '../controller'; -import { IUnleashConfig, IUnleashServices } from '../../types'; +import { IFlagResolver, IUnleashConfig, IUnleashServices } from '../../types'; import FeatureToggleService from '../../services/feature-toggle-service'; import { Logger } from '../../logger'; import { querySchema } from '../../schema/feature-schema'; @@ -25,6 +29,10 @@ import { ClientFeaturesSchema, } from '../../openapi/spec/client-features-schema'; import { ISegmentService } from 'lib/segments/segment-service-interface'; +import { EventService } from 'lib/services'; +import { hoursToMilliseconds } from 'date-fns'; +import { isEmpty } from '../../util/isEmpty'; +import EventEmitter from 'events'; const version = 2; @@ -33,9 +41,17 @@ interface QueryOverride { environment?: string; } +interface IMeta { + revisionId: number; + etag: string; + queryHash: string; +} + export default class FeatureController extends Controller { private readonly logger: Logger; + private eventBus: EventEmitter; + private featureToggleServiceV2: FeatureToggleService; private segmentService: ISegmentService; @@ -44,32 +60,43 @@ export default class FeatureController extends Controller { private openApiService: OpenApiService; + private eventService: EventService; + private readonly cache: boolean; private cachedFeatures: any; + private cachedFeatures2: any; + + private flagResolver: IFlagResolver; + constructor( { featureToggleServiceV2, segmentService, clientSpecService, openApiService, + eventService, }: Pick< IUnleashServices, | 'featureToggleServiceV2' | 'segmentService' | 'clientSpecService' | 'openApiService' + | 'eventService' >, config: IUnleashConfig, ) { super(config); - const { clientFeatureCaching } = config; + const { clientFeatureCaching, flagResolver, eventBus } = config; this.featureToggleServiceV2 = featureToggleServiceV2; this.segmentService = segmentService; this.clientSpecService = clientSpecService; this.openApiService = openApiService; + this.flagResolver = flagResolver; + this.eventService = eventService; this.logger = config.getLogger('client-api/feature.js'); + this.eventBus = eventBus; this.route({ method: 'get', @@ -117,11 +144,25 @@ export default class FeatureController extends Controller { }, ); } + this.cachedFeatures2 = memoizee( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + (query: IFeatureToggleQuery, etag: string) => + this.resolveFeaturesAndSegments(query), + { + promise: true, + maxAge: hoursToMilliseconds(1), + normalizer(args) { + // args is arguments object as accessible in memoized function + return args[1]; + }, + }, + ); } private async resolveFeaturesAndSegments( query?: IFeatureToggleQuery, ): Promise<[FeatureConfigurationClient[], ISegment[]]> { + this.logger.debug('bypass cache'); return Promise.all([ this.featureToggleServiceV2.getClientFeatures(query), this.segmentService.getActive(), @@ -175,7 +216,7 @@ export default class FeatureController extends Controller { !environment && !inlineSegmentConstraints ) { - return null; + return {}; } const tagQuery = this.paramToArray(tag); @@ -199,12 +240,22 @@ export default class FeatureController extends Controller { req: IAuthRequest, res: Response, ): Promise { + if (this.flagResolver.isEnabled('optimal304')) { + return this.optimal304(req, res); + } + const query = await this.resolveQuery(req); const [features, segments] = this.cache ? await this.cachedFeatures(query) : await this.resolveFeaturesAndSegments(query); + if (this.flagResolver.isEnabled('optimal304Differ')) { + process.nextTick(async () => + this.doOptimal304Diffing(features, query), + ); + } + if (this.clientSpecService.requestSupportsSpec(req, 'segments')) { this.openApiService.respondWithValidation( 200, @@ -222,6 +273,103 @@ export default class FeatureController extends Controller { } } + /** + * This helper method is used to validate that the new way of calculating + * cache-key based on query hash and revision id, with an internal memoization + * of 1hr still ends up producing the same result. + * + * It's worth to note that it is expected that a diff will occur immediately after + * a toggle changes due to the nature of two individual caches and how fast they + * detect the change. The diffs should however go away as soon as they both have + * the latest feature toggle configuration, which will happen within 600ms on the + * default configurations. + * + * This method is experimental and will only be used to validate our internal state + * to make sure our new way of caching is correct and stable. + * + * @deprecated + */ + async doOptimal304Diffing( + features: FeatureConfigurationClient[], + query: IFeatureToggleQuery, + ): Promise { + try { + const { etag } = await this.calculateMeta(query); + const [featuresNew] = await this.cachedFeatures2(query, etag); + const theDiffedObject = diff(features, featuresNew); + + if (isEmpty(theDiffedObject)) { + this.logger.warn('The diff is: '); + this.eventBus.emit('optimal304Differ', { status: 'equal' }); + } else { + this.logger.warn( + `The diff is: ${JSON.stringify(theDiffedObject)}`, + ); + this.eventBus.emit('optimal304Differ', { status: 'diff' }); + } + } catch (e) { + this.logger.error('The diff checker crashed', e); + this.eventBus.emit('optimal304Differ', { status: 'crash' }); + } + } + + 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(); + + // TODO: We will need to standardize this to be able to implement this a cross languages (Edge in Rust?). + const queryHash = hasSum(query); + const etag = `${queryHash}:${revisionId}`; + return { revisionId, etag, queryHash }; + } + + async optimal304( + req: IAuthRequest, + res: Response, + ): Promise { + const query = await this.resolveQuery(req); + + const userVersion = req.headers['if-none-match']; + const meta = await this.calculateMeta(query); + const { etag } = meta; + + res.setHeader('etag', etag); + + if (etag === userVersion) { + res.status(304); + res.end(); + return; + } else { + this.logger.debug( + `Provided revision: ${userVersion}, calculated revision: ${etag}`, + ); + } + + const [features, segments] = await this.cachedFeatures2(query, etag); + + if (this.clientSpecService.requestSupportsSpec(req, 'segments')) { + this.openApiService.respondWithValidation( + 200, + res, + clientFeaturesSchema.$id, + { + version, + features, + query: { ...query }, + segments, + meta, + }, + ); + } else { + this.openApiService.respondWithValidation( + 200, + res, + clientFeaturesSchema.$id, + { version, features, query, meta }, + ); + } + } + async getFeatureToggle( req: IAuthRequest<{ featureName: string }, ClientFeaturesQuerySchema>, res: Response, diff --git a/src/lib/services/event-service.ts b/src/lib/services/event-service.ts index dd81c52968..ec507b43a0 100644 --- a/src/lib/services/event-service.ts +++ b/src/lib/services/event-service.ts @@ -10,6 +10,8 @@ export default class EventService { private eventStore: IEventStore; + private revisionId: number; + constructor( { eventStore }: Pick, { getLogger }: Pick, @@ -35,6 +37,21 @@ 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 ccda3d1b48..c4378a6345 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -39,7 +39,11 @@ import { LastSeenService } from './client-metrics/last-seen-service'; import { InstanceStatsService } from './instance-stats-service'; import { FavoritesService } from './favorites-service'; import MaintenanceService from './maintenance-service'; -import { hoursToMilliseconds, minutesToMilliseconds } from 'date-fns'; +import { + hoursToMilliseconds, + minutesToMilliseconds, + secondsToMilliseconds, +} from 'date-fns'; import { AccountService } from './account-service'; import { SchedulerService } from './scheduler-service'; import { Knex } from 'knex'; @@ -61,6 +65,7 @@ export const scheduleServices = ( clientInstanceService, projectService, projectHealthService, + eventService, } = services; schedulerService.schedule( @@ -96,6 +101,11 @@ export const scheduleServices = ( projectHealthService.setHealthRating.bind(projectHealthService), hoursToMilliseconds(1), ); + + schedulerService.schedule( + eventService.updateMaxRevisionId.bind(eventService), + secondsToMilliseconds(1), + ); }; export const createServices = ( diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 25cb0f3de7..d0f0053a9f 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -74,6 +74,14 @@ const flags = { ), projectMode: parseEnvVarBoolean(process.env.PROJECT_MODE, false), cleanClientApi: parseEnvVarBoolean(process.env.CLEAN_CLIENT_API, false), + optimal304: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_OPTIMAL_304, + false, + ), + optimal304Differ: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_OPTIMAL_304_DIFFER, + false, + ), }; export const defaultExperimentalOptions: IExperimentalOptions = { diff --git a/src/lib/types/stores/event-store.ts b/src/lib/types/stores/event-store.ts index ec3c01cb6a..23392e202d 100644 --- a/src/lib/types/stores/event-store.ts +++ b/src/lib/types/stores/event-store.ts @@ -13,5 +13,6 @@ export interface IEventStore count(): Promise; filteredCount(search: SearchEventsSchema): Promise; searchEvents(search: SearchEventsSchema): Promise; + getMaxRevisionId(currentMax?: number): Promise; query(operations: IQueryOperations[]): Promise; } diff --git a/src/lib/util/isEmpty.ts b/src/lib/util/isEmpty.ts new file mode 100644 index 0000000000..f60409d94a --- /dev/null +++ b/src/lib/util/isEmpty.ts @@ -0,0 +1,3 @@ +export const isEmpty = (object: Object): boolean => { + return Object.keys(object).length === 0; +}; diff --git a/src/server-dev.ts b/src/server-dev.ts index 9e85e8b637..bd525a63ea 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -45,6 +45,8 @@ process.nextTick(async () => { showProjectApiAccess: true, projectScopedSegments: true, projectScopedStickiness: true, + optimal304: true, + optimal304Differ: false, }, }, authentication: { @@ -58,6 +60,12 @@ process.nextTick(async () => { }, ], }, + /* can be tweaked to control configuration caching for /api/client/features + clientFeatureCaching: { + enabled: true, + maxAge: 4000, + }, + */ }), ); } catch (error) { diff --git a/src/test/e2e/api/client/feature.optimal304.e2e.test.ts b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts new file mode 100644 index 0000000000..d459885236 --- /dev/null +++ b/src/test/e2e/api/client/feature.optimal304.e2e.test.ts @@ -0,0 +1,154 @@ +import { + IUnleashTest, + setupAppWithCustomConfig, +} from '../../helpers/test-helper'; +import dbInit, { ITestDb } from '../../helpers/database-init'; +import getLogger from '../../../fixtures/no-logger'; +// import { DEFAULT_ENV } from '../../../../lib/util/constants'; + +let app: IUnleashTest; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('feature_304_api_client', getLogger); + app = await setupAppWithCustomConfig(db.stores, { + experimental: { + flags: { + strictSchemaValidation: true, + optimal304: true, + }, + }, + }); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureX', + description: 'the #1 feature', + impressionData: true, + }, + 'test', + ); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureY', + description: 'soon to be the #1 feature', + }, + 'test', + ); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureZ', + description: 'terrible feature', + }, + 'test', + ); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureArchivedX', + description: 'the #1 feature', + }, + 'test', + ); + + await app.services.featureToggleServiceV2.archiveToggle( + 'featureArchivedX', + 'test', + ); + + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureArchivedY', + description: 'soon to be the #1 feature', + }, + 'test', + ); + + await app.services.featureToggleServiceV2.archiveToggle( + 'featureArchivedY', + 'test', + ); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureArchivedZ', + description: 'terrible feature', + }, + 'test', + ); + await app.services.featureToggleServiceV2.archiveToggle( + 'featureArchivedZ', + 'test', + ); + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'feature.with.variants', + description: 'A feature toggle with variants', + }, + 'test', + ); + await app.services.featureToggleServiceV2.saveVariants( + 'feature.with.variants', + 'default', + [ + { + name: 'control', + weight: 50, + weightType: 'fix', + stickiness: 'default', + }, + { + name: 'new', + weight: 50, + weightType: 'variable', + stickiness: 'default', + }, + ], + 'ivar', + ); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('returns calculated hash', async () => { + const res = await app.request + .get('/api/client/features') + .expect('Content-Type', /json/) + .expect(200); + expect(res.headers.etag).toBe('ae443048:19'); + expect(res.body.meta.etag).toBe('ae443048:19'); +}); + +test('returns 304 for pre-calculated hash', async () => { + return app.request + .get('/api/client/features') + .set('if-none-match', 'ae443048:19') + .expect(304); +}); + +test('returns 200 when content updates and hash does not match anymore', async () => { + await app.services.featureToggleServiceV2.createFeatureToggle( + 'default', + { + name: 'featureNew304', + description: 'the #1 feature', + }, + 'test', + ); + await app.services.eventService.updateMaxRevisionId(); + + const res = await app.request + .get('/api/client/features') + .set('if-none-match', 'ae443048:19') + .expect(200); + + expect(res.headers.etag).toBe('ae443048:20'); + expect(res.body.meta.etag).toBe('ae443048:20'); +}); diff --git a/src/test/fixtures/fake-event-store.ts b/src/test/fixtures/fake-event-store.ts index 5b260440f4..ef21300720 100644 --- a/src/test/fixtures/fake-event-store.ts +++ b/src/test/fixtures/fake-event-store.ts @@ -15,6 +15,10 @@ class FakeEventStore implements IEventStore { this.events = []; } + getMaxRevisionId(): Promise { + throw new Error('Method not implemented.'); + } + store(event: IEvent): Promise { this.events.push(event); this.eventEmitter.emit(event.type, event); diff --git a/yarn.lock b/yarn.lock index e6a1e9b08a..ffb96b46f7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1125,6 +1125,11 @@ dependencies: "@types/node" "*" +"@types/hash-sum@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/hash-sum/-/hash-sum-1.0.0.tgz#838f4e8627887d42b162d05f3d96ca636c2bc504" + integrity sha512-FdLBT93h3kcZ586Aee66HPCVJ6qvxVjBlDWNmxSGSbCZe9hTsjRKdSsl4y1T+3zfujxo9auykQMnFsfyHWD7wg== + "@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0", "@types/istanbul-lib-coverage@^2.0.1": version "2.0.4" resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.4.tgz#8467d4b3c087805d63580480890791277ce35c44" @@ -2430,6 +2435,11 @@ deep-is@^0.1.3: resolved "https://registry.yarnpkg.com/deep-is/-/deep-is-0.1.4.tgz#a6f2dce612fadd2ef1f519b73551f17e85199831" integrity sha512-oIPzksmTg4/MriiaYGO+okXDT7ztn/w3Eptv/+gSIdMdKsJo0u4CfYNFJPy+4SKMuCqGw2wxnA+URMg3t8a/bQ== +deep-object-diff@^1.1.9: + version "1.1.9" + resolved "https://registry.yarnpkg.com/deep-object-diff/-/deep-object-diff-1.1.9.tgz#6df7ef035ad6a0caa44479c536ed7b02570f4595" + integrity sha512-Rn+RuwkmkDwCi2/oXOFS9Gsr5lJZu/yTGpK7wAaAIE75CC+LCGEZHpY6VQJa/RoJcrmaA/docWJZvYohlNkWPA== + deepmerge@^4.2.2: version "4.2.2" resolved "https://registry.yarnpkg.com/deepmerge/-/deepmerge-4.2.2.tgz#44d2ea3679b8f4d4ffba33f03d865fc1e7bf4955" @@ -3602,6 +3612,11 @@ has@^1.0.3: dependencies: function-bind "^1.1.1" +hash-sum@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/hash-sum/-/hash-sum-2.0.0.tgz#81d01bb5de8ea4a214ad5d6ead1b523460b0b45a" + integrity sha512-WdZTbAByD+pHfl/g9QSsBIIwy8IT+EsPiKDs0KNX+zSHhdDLFKdZu0BQHljvO+0QI/BasbMSUa8wYNCZTvhslg== + helmet@^6.0.0: version "6.0.1" resolved "https://registry.yarnpkg.com/helmet/-/helmet-6.0.1.tgz#52ec353638b2e87f14fe079d142b368ac11e79a4"