Poc: calculate etag based on query and latest revison id (#3062)

This is very much POC and WIP

package.json

@@ -99,13 +99,15 @@
     "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",
     "express-rate-limit": "^6.6.0",
     "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",

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,

src/lib/db/event-store.ts

@@ -149,6 +149,16 @@ class EventStore implements IEventStore {
         }
     }
 
+    async getMaxRevisionId(largerThan: number = 0): Promise<number> {
+        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<void> {
         await this.db(TABLE).where({ id: key }).del();
     }

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);
         });

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,
         },
     );
 

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<ClientFeaturesSchema>,
     ): Promise<void> {
+        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<void> {
+        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: <Empty>');
+                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<IMeta> {
+        // 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<ClientFeaturesSchema>,
+    ): Promise<void> {
+        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<ClientFeatureSchema>,

src/lib/services/event-service.ts

@@ -10,6 +10,8 @@ export default class EventService {
 
     private eventStore: IEventStore;
 
+    private revisionId: number;
+
     constructor(
         { eventStore }: Pick<IUnleashStores, 'eventStore'>,
         { getLogger }: Pick<IUnleashConfig, 'getLogger'>,
@@ -35,6 +37,21 @@ export default class EventService {
             totalEvents,
         };
     }
+
+    async getMaxRevisionId(): Promise<number> {
+        if (this.revisionId) {
+            return this.revisionId;
+        } else {
+            return this.updateMaxRevisionId();
+        }
+    }
+
+    async updateMaxRevisionId(): Promise<number> {
+        this.revisionId = await this.eventStore.getMaxRevisionId(
+            this.revisionId,
+        );
+        return this.revisionId;
+    }
 }
 
 module.exports = EventService;