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

This is very much POC and WIP

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",

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;

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 = (

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 = {

src/lib/types/stores/event-store.ts

@@ -13,5 +13,6 @@ export interface IEventStore
     count(): Promise<number>;
     filteredCount(search: SearchEventsSchema): Promise<number>;
     searchEvents(search: SearchEventsSchema): Promise<IEvent[]>;
+    getMaxRevisionId(currentMax?: number): Promise<number>;
     query(operations: IQueryOperations[]): Promise<IEvent[]>;
 }

src/lib/util/isEmpty.ts

@@ -0,0 +1,3 @@
+export const isEmpty = (object: Object): boolean => {
+    return Object.keys(object).length === 0;
+};

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) {

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

src/test/fixtures/fake-event-store.ts

@@ -15,6 +15,10 @@ class FakeEventStore implements IEventStore {
         this.events = [];
     }
 
+    getMaxRevisionId(): Promise<number> {
+        throw new Error('Method not implemented.');
+    }
+
     store(event: IEvent): Promise<void> {
         this.events.push(event);
         this.eventEmitter.emit(event.type, event);

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"