assignment logging cache prevents duplicate invocations (FF-1069) (#25)

* assignment logging cache prevents duplicate invocations (FF-1069) * add flagKey and remove variationKey from assignment cache key * additional test cases * additional test case. improve naming/clarity * rename hasAssigned to hasLoggedAssignment * add hashed variation value to cache key; handle case of variation changing * add comments to test * handle variation changing * fix comment * refactor into abstract class * instantiate correctly * remove debug * remove import * rename to setLastLoggedAssignment * split into 3 publish methods * fix comment * remove type * remove unused comment * refactor into beforeEach block * require node version 16 * bump to 1.7.0 --------- Co-authored-by: Eric Petzel <[email protected]>
Eppo-exp · Oct 18, 2023 · 71cd364 · 71cd364
1 parent b6f2fe6
commit 71cd364
Show file tree

Hide file tree

Showing 10 changed files with 361 additions and 5 deletions.
diff --git a/.github/workflows/lint-test-sdk.yml b/.github/workflows/lint-test-sdk.yml
@@ -12,7 +12,7 @@ jobs:
       - name: Use Node.js
         uses: actions/setup-node@v1
         with:
-          node-version: '18.x'
+          node-version: '16.x'
       - uses: actions/cache@v2
         with:
           path: './node_modules'
@@ -35,7 +35,7 @@ jobs:
       - name: Use Node.js
         uses: actions/setup-node@v1
         with:
-          node-version: '18.x'
+          node-version: '16.x'
       - uses: actions/cache@v2
         with:
           path: './node_modules'
@@ -45,4 +45,4 @@ jobs:
         working-directory: ./
       - name: Run typecheck
         run: yarn typecheck
-        working-directory: ./
+        working-directory: ./
diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml
@@ -10,7 +10,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/setup-node@v3
         with:
-          node-version: 12
+          node-version: '16.x'
       - run: yarn install
       - run: yarn test
       - uses: JS-DevTools/npm-publish@v1

diff --git a/.npmrc b/.npmrc
@@ -0,0 +1 @@
+engine-strict=true
diff --git a/package.json b/package.json
@@ -1,12 +1,15 @@
 {
   "name": "@eppo/js-client-sdk-common",
-  "version": "1.6.0",
+  "version": "1.7.0",
   "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
   "main": "dist/index.js",
   "files": [
     "/dist"
   ],
   "types": "./dist/index.d.ts",
+  "engines": {
+    "node": ">=16.20"
+  },
   "exports": {
     ".": {
       "types": "./dist/index.d.ts",
@@ -61,6 +64,7 @@
   },
   "dependencies": {
     "axios": "^0.27.2",
+    "lru-cache": "^10.0.1",
     "md5": "^2.3.0"
   }
 }
diff --git a/src/assignment-cache.ts b/src/assignment-cache.ts
@@ -0,0 +1,76 @@
+import { LRUCache } from 'lru-cache';
+
+import { EppoValue } from './eppo_value';
+
+export interface AssignmentCacheKey {
+  subjectKey: string;
+  flagKey: string;
+  allocationKey: string;
+  variationValue: EppoValue;
+}
+
+export interface Cacheable {
+  get(key: string): string | undefined;
+  set(key: string, value: string): void;
+  has(key: string): boolean;
+}
+
+export abstract class AssignmentCache<T extends Cacheable> {
+  // key -> variation value hash
+  protected cache: T;
+
+  constructor(cacheInstance: T) {
+    this.cache = cacheInstance;
+  }
+
+  hasLoggedAssignment(key: AssignmentCacheKey): boolean {
+    // no cache key present
+    if (!this.cache.has(this.getCacheKey(key))) {
+      return false;
+    }
+
+    // the subject has been assigned to a different variation
+    // than was previously logged.
+    // in this case we need to log the assignment again.
+    if (this.cache.get(this.getCacheKey(key)) !== key.variationValue.toHashedString()) {
+      return false;
+    }
+
+    return true;
+  }
+
+  setLastLoggedAssignment(key: AssignmentCacheKey): void {
+    this.cache.set(this.getCacheKey(key), key.variationValue.toHashedString());
+  }
+
+  protected getCacheKey({ subjectKey, flagKey, allocationKey }: AssignmentCacheKey): string {
+    return [`subject:${subjectKey}`, `flag:${flagKey}`, `allocation:${allocationKey}`].join(';');
+  }
+}
+
+/**
+ * A cache that never expires.
+ *
+ * The primary use case is for client-side SDKs, where the cache is only used
+ * for a single user.
+ */
+export class NonExpiringAssignmentCache extends AssignmentCache<Map<string, string>> {
+  constructor() {
+    super(new Map<string, string>());
+  }
+}
+
+/**
+ * A cache that uses the LRU algorithm to evict the least recently used items.
+ *
+ * It is used to limit the size of the cache.
+ *
+ * The primary use case is for server-side SDKs, where the cache is shared across
+ * multiple users. In this case, the cache size should be set to the maximum number
+ * of users that can be active at the same time.
+ */
+export class LRUAssignmentCache extends AssignmentCache<LRUCache<string, string>> {
+  constructor(maxSize: number) {
+    super(new LRUCache<string, string>({ max: maxSize }));
+  }
+}
diff --git a/src/client/eppo-client.spec.ts b/src/client/eppo-client.spec.ts
@@ -369,6 +369,212 @@ describe('EppoClient E2E test', () => {
     expect(assignment).toEqual('control');
   });
 
+  describe('assignment logging deduplication', () => {
+    let client: EppoClient;
+    let mockLogger: IAssignmentLogger;
+
+    beforeEach(() => {
+      mockLogger = td.object<IAssignmentLogger>();
+
+      storage.setEntries({ [flagKey]: mockExperimentConfig });
+      client = new EppoClient(storage);
+      client.setLogger(mockLogger);
+    });
+
+    it('logs duplicate assignments without an assignment cache', () => {
+      client.disableAssignmentCache();
+
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', flagKey);
+
+      // call count should be 2 because there is no cache.
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(2);
+    });
+
+    it('does not log duplicate assignments', () => {
+      client.useNonExpiringAssignmentCache();
+
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', flagKey);
+
+      // call count should be 1 because the second call is a cache hit and not logged.
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(1);
+    });
+
+    it('logs assignment again after the lru cache is full', () => {
+      client.useLRUAssignmentCache(2);
+
+      client.getAssignment('subject-10', flagKey); // logged
+      client.getAssignment('subject-10', flagKey); // cached
+
+      client.getAssignment('subject-11', flagKey); // logged
+      client.getAssignment('subject-11', flagKey); // cached
+
+      client.getAssignment('subject-12', flagKey); // cache evicted subject-10, logged
+      client.getAssignment('subject-10', flagKey); // previously evicted, logged
+      client.getAssignment('subject-12', flagKey); // cached
+
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(4);
+    });
+
+    it('does not cache assignments if the logger had an exception', () => {
+      td.when(mockLogger.logAssignment(td.matchers.anything())).thenThrow(
+        new Error('logging error'),
+      );
+
+      const client = new EppoClient(storage);
+      client.setLogger(mockLogger);
+
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', flagKey);
+
+      // call count should be 2 because the first call had an exception
+      // therefore we are not sure the logger was successful and try again.
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(2);
+    });
+
+    it('logs for each unique flag', () => {
+      storage.setEntries({
+        [flagKey]: mockExperimentConfig,
+        'flag-2': {
+          ...mockExperimentConfig,
+          name: 'flag-2',
+        },
+        'flag-3': {
+          ...mockExperimentConfig,
+          name: 'flag-3',
+        },
+      });
+
+      client.useNonExpiringAssignmentCache();
+
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', 'flag-2');
+      client.getAssignment('subject-10', 'flag-2');
+      client.getAssignment('subject-10', 'flag-3');
+      client.getAssignment('subject-10', 'flag-3');
+      client.getAssignment('subject-10', flagKey);
+      client.getAssignment('subject-10', 'flag-2');
+      client.getAssignment('subject-10', 'flag-3');
+
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(3);
+    });
+
+    it('logs twice for the same flag when rollout increases/flag changes', () => {
+      client.useNonExpiringAssignmentCache();
+
+      storage.setEntries({
+        [flagKey]: {
+          ...mockExperimentConfig,
+          allocations: {
+            allocation1: {
+              percentExposure: 1,
+              variations: [
+                {
+                  name: 'control',
+                  value: 'control',
+                  typedValue: 'control',
+                  shardRange: {
+                    start: 0,
+                    end: 100,
+                  },
+                },
+                {
+                  name: 'treatment',
+                  value: 'treatment',
+                  typedValue: 'treatment',
+                  shardRange: {
+                    start: 0,
+                    end: 0,
+                  },
+                },
+              ],
+            },
+          },
+        },
+      });
+      client.getAssignment('subject-10', flagKey);
+
+      storage.setEntries({
+        [flagKey]: {
+          ...mockExperimentConfig,
+          allocations: {
+            allocation1: {
+              percentExposure: 1,
+              variations: [
+                {
+                  name: 'control',
+                  value: 'control',
+                  typedValue: 'control',
+                  shardRange: {
+                    start: 0,
+                    end: 0,
+                  },
+                },
+                {
+                  name: 'treatment',
+                  value: 'treatment',
+                  typedValue: 'treatment',
+                  shardRange: {
+                    start: 0,
+                    end: 100,
+                  },
+                },
+              ],
+            },
+          },
+        },
+      });
+      client.getAssignment('subject-10', flagKey);
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(2);
+    });
+
+    it('logs the same subject/flag/variation after two changes', () => {
+      client.useNonExpiringAssignmentCache();
+
+      // original configuration version
+      storage.setEntries({ [flagKey]: mockExperimentConfig });
+
+      client.getAssignment('subject-10', flagKey); // log this assignment
+      client.getAssignment('subject-10', flagKey); // cache hit, don't log
+
+      // change the flag
+      storage.setEntries({
+        [flagKey]: {
+          ...mockExperimentConfig,
+          allocations: {
+            allocation1: {
+              percentExposure: 1,
+              variations: [
+                {
+                  name: 'some-new-treatment',
+                  value: 'some-new-treatment',
+                  typedValue: 'some-new-treatment',
+                  shardRange: {
+                    start: 0,
+                    end: 100,
+                  },
+                },
+              ],
+            },
+          },
+        },
+      });
+
+      client.getAssignment('subject-10', flagKey); // log this assignment
+      client.getAssignment('subject-10', flagKey); // cache hit, don't log
+
+      // change the flag again, back to the original
+      storage.setEntries({ [flagKey]: mockExperimentConfig });
+
+      client.getAssignment('subject-10', flagKey); // important: log this assignment
+      client.getAssignment('subject-10', flagKey); // cache hit, don't log
+
+      expect(td.explain(mockLogger.logAssignment).callCount).toEqual(3);
+    });
+  });
+
   it('only returns variation if subject matches rules', () => {
     const entry = {
       ...mockExperimentConfig,