From 6e650289c88c617d1e44cfe21271d89a5ee154ba Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Tue, 20 Dec 2022 15:02:07 +0100 Subject: [PATCH] do etag calculation when memoizing feature toggles To prevent express from having to do etag calculation of all feature toggle responses even if hitting a memoized feature toggle this PR adds etag calculation to the memoized function. This will cost us an extra etag calculation for client SDK requests without ETag headers, but for SDK requests with ETag headers, it should reduce the number of etag calculations we do down to 1 per 0.6 seconds per query Co-authored-by: Gard Rimestad Co-authored-by: Nuno Gois --- package.json | 1 + src/lib/routes/client-api/feature.ts | 88 +++++++++++++++++++++++++--- src/lib/types/experimental.ts | 4 ++ yarn.lock | 5 ++ 4 files changed, 90 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index b17ef6cb1a..897bca1e47 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,7 @@ "db-migrate-shared": "1.2.0", "deepmerge": "^4.2.2", "errorhandler": "^1.5.1", + "etag": "^1.8.1", "express": "^4.18.2", "express-rate-limit": "^6.6.0", "express-session": "^1.17.1", diff --git a/src/lib/routes/client-api/feature.ts b/src/lib/routes/client-api/feature.ts index 04594e682c..e3306dc83f 100644 --- a/src/lib/routes/client-api/feature.ts +++ b/src/lib/routes/client-api/feature.ts @@ -1,7 +1,7 @@ import memoizee from 'memoizee'; import { Response } from 'express'; 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,14 +25,14 @@ import { clientFeaturesSchema, ClientFeaturesSchema, } from '../../openapi/spec/client-features-schema'; - +import etag from 'etag'; const version = 2; interface QueryOverride { project?: string[]; environment?: string; } - +const FEATURE_TOGGLE_MEMOIZED_ETAGS = 'clientFeaturesMemoizedEtags'; export default class FeatureController extends Controller { private readonly logger: Logger; @@ -48,6 +48,8 @@ export default class FeatureController extends Controller { private cachedFeatures: any; + private seenEtags: Map; + constructor( { featureToggleServiceV2, @@ -64,12 +66,13 @@ export default class FeatureController extends Controller { config: IUnleashConfig, ) { super(config); - const { clientFeatureCaching } = config; + const { clientFeatureCaching, flagResolver } = config; this.featureToggleServiceV2 = featureToggleServiceV2; this.segmentService = segmentService; this.clientSpecService = clientSpecService; this.openApiService = openApiService; this.logger = config.getLogger('client-api/feature.js'); + this.seenEtags = new Map(); this.route({ method: 'get', @@ -106,7 +109,7 @@ export default class FeatureController extends Controller { if (clientFeatureCaching?.enabled) { this.cache = true; this.cachedFeatures = memoizee( - (query) => this.resolveFeaturesAndSegments(query), + (query) => this.resolveFeaturesAndSegments(query, flagResolver), { promise: true, maxAge: clientFeatureCaching.maxAge, @@ -116,16 +119,53 @@ export default class FeatureController extends Controller { }, }, ); + this.logger.info('Cached features was enabled'); + if (flagResolver.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS)) { + this.logger.info('Memoized etags was enabled'); + this.route({ + method: 'get', + path: '', + handler: this.getAllCached, + permission: NONE, + middleware: [ + openApiService.validPath({ + operationId: 'getAllClientFeatures', + tags: ['Client'], + responses: { + 200: createResponseSchema( + 'clientFeaturesSchema', + ), + }, + }), + ], + }); + } } } private async resolveFeaturesAndSegments( query?: IFeatureToggleQuery, + flagResolver?: IFlagResolver, ): Promise<[FeatureConfigurationClient[], ISegment[]]> { return Promise.all([ this.featureToggleServiceV2.getClientFeatures(query), this.segmentService.getActive(), - ]); + ]).then((data) => { + if (flagResolver?.isEnabled(FEATURE_TOGGLE_MEMOIZED_ETAGS)) { + this.seenEtags.set( + JSON.stringify(query), + etag( + JSON.stringify({ + version, + features: data[0], + query: { ...query }, + segments: data[1], + }), + ), + ); + } + return data; + }); } private async resolveQuery( @@ -138,7 +178,7 @@ export default class FeatureController extends Controller { if (!isAllProjects(user.projects)) { override.project = user.projects; } - if (user.environment !== ALL) { + if (user.environment != ALL) { override.environment = user.environment; } } @@ -195,12 +235,44 @@ export default class FeatureController extends Controller { return query; } + async getAllCached( + req: IAuthRequest, + res: Response, + ): Promise { + const query = await this.resolveQuery(req); + const [features, segments] = await this.cachedFeatures(query); + const modifiedSince = req.header('If-None-Match').substring(2); + this.logger.debug(`ETag header from Client: ${modifiedSince}`); + const cached = this.seenEtags.get(JSON.stringify(query)); + this.logger.debug(`ETag header from memoizee ${cached}`); + if (modifiedSince !== undefined && modifiedSince === cached) { + // We have a match. Return 304 + res.status(304); + res.end(); + return; + } + if (this.clientSpecService.requestSupportsSpec(req, 'segments')) { + this.openApiService.respondWithValidation( + 200, + res, + clientFeaturesSchema.$id, + { version, features, query: { ...query }, segments }, + ); + } else { + this.openApiService.respondWithValidation( + 200, + res, + clientFeaturesSchema.$id, + { version, features, query }, + ); + } + } + async getAll( req: IAuthRequest, res: Response, ): Promise { const query = await this.resolveQuery(req); - const [features, segments] = this.cache ? await this.cachedFeatures(query) : await this.resolveFeaturesAndSegments(query); diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 94b49f5af7..926bbb7b1d 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -47,6 +47,10 @@ const flags = { process.env.UNLEASH_EXPERIMENTAL_MAINTENANCE, false, ), + clientFeaturesMemoizedEtags: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_CLIENT_FEATURES_MEMOIZED_ETAGS, + false, + ), }; export const defaultExperimentalOptions: IExperimentalOptions = { diff --git a/yarn.lock b/yarn.lock index f5e9eba441..ad16ca914a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3345,6 +3345,11 @@ esutils@^2.0.2: resolved "https://registry.npmjs.org/esutils/-/esutils-2.0.3.tgz" integrity sha512-kVscqXk4OCp68SZ0dkgEKVi6/8ij300KBWTJq32P/dYeWTSwK41WyTxalN1eRmA5Z9UU/LX9D7FWSmV9SAYx6g== +etag@^1.8.1: + version "1.8.1" + resolved "https://registry.yarnpkg.com/etag/-/etag-1.8.1.tgz#41ae2eeb65efa62268aebfea83ac7d79299b0887" + integrity sha512-aIL5Fx7mawVa300al2BnEE4iNvo1qETxLrPI/o05L7z6go7fCw1J6EQmbK4FmJ2AS7kgVF/KEZWufBfdClMcPg== + etag@~1.8.1: version "1.8.1" resolved "https://registry.npmjs.org/etag/-/etag-1.8.1.tgz"