From 92cd45ab92eaa94f4b4f1bc57a274409c34542ac Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 8 Nov 2024 18:20:02 +0000 Subject: [PATCH 1/4] Switched to Expiring LRU cache for bandit actions --- src/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.ts b/src/index.ts index 8bd4194..1c42ee7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -125,7 +125,9 @@ export async function init(config: IClientConfig): Promise { // we estimate this will use no more than 10 MB of memory // and should be appropriate for most server-side use cases. clientInstance.useLRUInMemoryAssignmentCache(50_000); - clientInstance.useLRUInMemoryBanditAssignmentCache(50_000); + // for bandits variant of LRU cache is use, that will + // expire on its own after set time. Defaults to 10 minutes + clientInstance.useExpiringInMemoryBanditAssignmentCache(50_000); // Fetch configurations (which will also start regular polling per requestConfiguration) await clientInstance.fetchFlagConfigurations(); From b947919360d85c83e6958aee6434d0aa79855a20 Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 29 Nov 2024 14:42:20 +0000 Subject: [PATCH 2/4] Expiring bandit cache --- src/index.spec.ts | 164 +++++++++++++++++++++++++++++++++++++++++----- src/index.ts | 8 +-- 2 files changed, 150 insertions(+), 22 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index 2dec475..ee405ed 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -11,6 +11,7 @@ import { } from '@eppo/js-client-sdk-common'; import { BanditParameters, BanditVariation } from '@eppo/js-client-sdk-common/dist/interfaces'; import { ContextAttributes } from '@eppo/js-client-sdk-common/dist/types'; +import {Attributes} from "@eppo/js-client-sdk-common/src/types"; import * as td from 'testdouble'; import apiServer, { TEST_BANDIT_API_KEY, TEST_SERVER_PORT } from '../test/mockApiServer'; @@ -27,6 +28,8 @@ import { import { getInstance, IAssignmentEvent, IAssignmentLogger, init } from '.'; +import SpyInstance = jest.SpyInstance; + const { DEFAULT_POLL_INTERVAL_MS, POLL_JITTER_PCT } = constants; describe('EppoClient E2E test', () => { @@ -189,12 +192,12 @@ describe('EppoClient E2E test', () => { it('returns the default value when ufc config is absent', () => { const mockConfigStore = td.object>(); td.when(mockConfigStore.get(flagKey)).thenReturn(null); - const client = new EppoClient( - mockConfigStore, - mockBanditVariationStore, - mockBanditModelStore, - requestParamsStub, - ); + const client = new EppoClient({ + flagConfigurationStore: mockConfigStore, + banditVariationConfigurationStore: mockBanditVariationStore, + banditModelConfigurationStore: mockBanditModelStore, + configurationRequestParameters: requestParamsStub, + }); const assignment = client.getStringAssignment(flagKey, 'subject-10', {}, 'default-value'); expect(assignment).toEqual('default-value'); }); @@ -203,12 +206,12 @@ describe('EppoClient E2E test', () => { const mockConfigStore = td.object>(); td.when(mockConfigStore.get(flagKey)).thenReturn(mockUfcFlagConfig); const subjectAttributes = { foo: 3 }; - const client = new EppoClient( - mockConfigStore, - mockBanditVariationStore, - mockBanditModelStore, - requestParamsStub, - ); + const client = new EppoClient({ + flagConfigurationStore: mockConfigStore, + banditVariationConfigurationStore: mockBanditVariationStore, + banditModelConfigurationStore: mockBanditModelStore, + configurationRequestParameters: requestParamsStub, + }); const mockLogger = td.object(); client.setAssignmentLogger(mockLogger); const assignment = client.getStringAssignment( @@ -233,12 +236,12 @@ describe('EppoClient E2E test', () => { const mockConfigStore = td.object>(); td.when(mockConfigStore.get(flagKey)).thenReturn(mockUfcFlagConfig); const subjectAttributes = { foo: 3 }; - const client = new EppoClient( - mockConfigStore, - mockBanditVariationStore, - mockBanditModelStore, - requestParamsStub, - ); + const client = new EppoClient({ + flagConfigurationStore: mockConfigStore, + banditVariationConfigurationStore: mockBanditVariationStore, + banditModelConfigurationStore: mockBanditModelStore, + configurationRequestParameters: requestParamsStub, + }); const mockLogger = td.object(); td.when(mockLogger.logAssignment(td.matchers.anything())).thenThrow( new Error('logging error'), @@ -312,6 +315,131 @@ describe('EppoClient E2E test', () => { // Ensure that this test case correctly checked some test assignments expect(numAssignmentsChecked).toBeGreaterThan(0); }); + + + describe('Bandit assignment cache', () => { + const flagKey = 'banner_bandit_flag'; // piggyback off a shared test data flag + const bobKey = 'bob'; + const bobAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' }; + const bobActions: Record = { + nike: { brand_affinity: 1.5, loyalty_tier: 'silver' }, + adidas: { brand_affinity: -1.0, loyalty_tier: 'bronze' }, + reebok: { brand_affinity: 0.5, loyalty_tier: 'gold' }, + }; + + const aliceKey = 'alice'; + const aliceAttributes: Attributes = {age: 25, country: 'USA', gender_identity: 'female' }; + const aliceActions: Record = { + nike: { brand_affinity: 1.5, loyalty_tier: 'silver' }, + adidas: {brand_affinity: -1.0, loyalty_tier: 'bronze' }, + reebok: {brand_affinity: 0.5, loyalty_tier: 'gold' }, + } + const charlieKey = 'charlie'; + const charlieAttributes: Attributes = {age: 25, country: 'USA', gender_identity: 'female' }; + const charlieActions: Record = { + nike: { brand_affinity: 1.0, loyalty_tier: 'gold' }, + adidas: {brand_affinity: 1.0, loyalty_tier: 'silver' }, + puma: {}, + } + + let banditLoggerSpy: SpyInstance; + const defaultBanditCacheTTL = 600_000 + + beforeAll(async () => { + const dummyBanditLogger: IBanditLogger = { + logBanditAction(banditEvent: IBanditEvent) { + console.log( + `Bandit ${banditEvent.bandit} assigned ${banditEvent.subject} the action ${banditEvent.action}`, + ); + }, + }; + banditLoggerSpy = jest.spyOn(dummyBanditLogger, 'logBanditAction'); + await init({ + apiKey: TEST_BANDIT_API_KEY, // Flag to dummy test server we want bandit-related files + baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`, + assignmentLogger: mockLogger, + banditLogger: dummyBanditLogger, + }); + }); + + it('Should not log bandit assignment if cached version is still valid', async () => { + const client = getInstance(); + client.useExpiringInMemoryBanditAssignmentCache(2); + + // Let's say someone is rage refreshing - we want to log assignment only once + for (const _ of Array(3).keys()) { + client.getBanditAction( + flagKey, + bobKey, + bobAttributes, + bobActions, + 'default', + ); + } + expect(banditLoggerSpy).toHaveBeenCalledTimes(1) + }); + it('Should log bandit assignment if cached entry is expired', async () => { + jest.useFakeTimers(); + + const client = getInstance(); + client.useExpiringInMemoryBanditAssignmentCache(2); + + client.getBanditAction( + flagKey, + bobKey, + bobAttributes, + bobActions, + 'default', + ); + jest.advanceTimersByTime(defaultBanditCacheTTL); + client.getBanditAction( + flagKey, + bobKey, + bobAttributes, + bobActions, + 'default', + ); + expect(banditLoggerSpy).toHaveBeenCalledTimes(2) + }) + + it('Should invalidate least used cache entry if cache reaches max size', async () => { + const client = getInstance(); + client.useExpiringInMemoryBanditAssignmentCache(2); + + client.getBanditAction( + flagKey, + bobKey, + bobAttributes, + bobActions, + 'default', + ); + client.getBanditAction( + flagKey, + aliceKey, + aliceAttributes, + aliceActions, + 'default', + ); + client.getBanditAction( + flagKey, + charlieKey, + charlieAttributes, + charlieActions, + 'default' + ); + // even though bob was called 2nd time within cache validity time + // we expect assignment to be logged because max cache size is 2 + // and currently storing alice and charlie + client.getBanditAction( + flagKey, + bobKey, + bobAttributes, + bobActions, + 'default', + ); + expect(banditLoggerSpy).toHaveBeenCalledTimes(4); + }); + }); }); describe('initialization errors', () => { diff --git a/src/index.ts b/src/index.ts index 1c42ee7..13cd6be 100644 --- a/src/index.ts +++ b/src/index.ts @@ -92,7 +92,7 @@ let clientInstance: EppoClient; export async function init(config: IClientConfig): Promise { validation.validateNotBlank(config.apiKey, 'API key required'); - const requestConfiguration: FlagConfigurationRequestParameters = { + const configurationRequestParameters: FlagConfigurationRequestParameters = { apiKey: config.apiKey, sdkName, sdkVersion, @@ -110,12 +110,12 @@ export async function init(config: IClientConfig): Promise { const banditVariationConfigurationStore = new MemoryOnlyConfigurationStore(); const banditModelConfigurationStore = new MemoryOnlyConfigurationStore(); - clientInstance = new EppoClient( + clientInstance = new EppoClient({ flagConfigurationStore, banditVariationConfigurationStore, banditModelConfigurationStore, - requestConfiguration, - ); + configurationRequestParameters, + }); clientInstance.setAssignmentLogger(config.assignmentLogger); if (config.banditLogger) { clientInstance.setBanditLogger(config.banditLogger); From 6ad02cfefd5c81b7bb53eb060d2f7a297298ddcb Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 29 Nov 2024 16:00:27 +0000 Subject: [PATCH 3/4] linter --- src/index.spec.ts | 81 +++++++++++------------------------------------ 1 file changed, 19 insertions(+), 62 deletions(-) diff --git a/src/index.spec.ts b/src/index.spec.ts index 938ac31..b1f952c 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -11,7 +11,7 @@ import { } from '@eppo/js-client-sdk-common'; import { BanditParameters, BanditVariation } from '@eppo/js-client-sdk-common/dist/interfaces'; import { ContextAttributes } from '@eppo/js-client-sdk-common/dist/types'; -import {Attributes} from "@eppo/js-client-sdk-common/src/types"; +import { Attributes } from '@eppo/js-client-sdk-common/src/types'; import * as td from 'testdouble'; import apiServer, { TEST_BANDIT_API_KEY, TEST_SERVER_PORT } from '../test/mockApiServer'; @@ -317,7 +317,6 @@ describe('EppoClient E2E test', () => { expect(numAssignmentsChecked).toBeGreaterThan(0); }); - describe('Bandit assignment cache', () => { const flagKey = 'banner_bandit_flag'; // piggyback off a shared test data flag const bobKey = 'bob'; @@ -329,22 +328,22 @@ describe('EppoClient E2E test', () => { }; const aliceKey = 'alice'; - const aliceAttributes: Attributes = {age: 25, country: 'USA', gender_identity: 'female' }; + const aliceAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' }; const aliceActions: Record = { nike: { brand_affinity: 1.5, loyalty_tier: 'silver' }, - adidas: {brand_affinity: -1.0, loyalty_tier: 'bronze' }, - reebok: {brand_affinity: 0.5, loyalty_tier: 'gold' }, - } + adidas: { brand_affinity: -1.0, loyalty_tier: 'bronze' }, + reebok: { brand_affinity: 0.5, loyalty_tier: 'gold' }, + }; const charlieKey = 'charlie'; - const charlieAttributes: Attributes = {age: 25, country: 'USA', gender_identity: 'female' }; + const charlieAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' }; const charlieActions: Record = { nike: { brand_affinity: 1.0, loyalty_tier: 'gold' }, - adidas: {brand_affinity: 1.0, loyalty_tier: 'silver' }, + adidas: { brand_affinity: 1.0, loyalty_tier: 'silver' }, puma: {}, - } + }; let banditLoggerSpy: SpyInstance; - const defaultBanditCacheTTL = 600_000 + const defaultBanditCacheTTL = 600_000; beforeAll(async () => { const dummyBanditLogger: IBanditLogger = { @@ -369,15 +368,9 @@ describe('EppoClient E2E test', () => { // Let's say someone is rage refreshing - we want to log assignment only once for (const _ of Array(3).keys()) { - client.getBanditAction( - flagKey, - bobKey, - bobAttributes, - bobActions, - 'default', - ); + client.getBanditAction(flagKey, bobKey, bobAttributes, bobActions, 'default'); } - expect(banditLoggerSpy).toHaveBeenCalledTimes(1) + expect(banditLoggerSpy).toHaveBeenCalledTimes(1); }); it('Should log bandit assignment if cached entry is expired', async () => { jest.useFakeTimers(); @@ -385,59 +378,23 @@ describe('EppoClient E2E test', () => { const client = getInstance(); client.useExpiringInMemoryBanditAssignmentCache(2); - client.getBanditAction( - flagKey, - bobKey, - bobAttributes, - bobActions, - 'default', - ); + client.getBanditAction(flagKey, bobKey, bobAttributes, bobActions, 'default'); jest.advanceTimersByTime(defaultBanditCacheTTL); - client.getBanditAction( - flagKey, - bobKey, - bobAttributes, - bobActions, - 'default', - ); - expect(banditLoggerSpy).toHaveBeenCalledTimes(2) - }) + client.getBanditAction(flagKey, bobKey, bobAttributes, bobActions, 'default'); + expect(banditLoggerSpy).toHaveBeenCalledTimes(2); + }); it('Should invalidate least used cache entry if cache reaches max size', async () => { const client = getInstance(); client.useExpiringInMemoryBanditAssignmentCache(2); - client.getBanditAction( - flagKey, - bobKey, - bobAttributes, - bobActions, - 'default', - ); - client.getBanditAction( - flagKey, - aliceKey, - aliceAttributes, - aliceActions, - 'default', - ); - client.getBanditAction( - flagKey, - charlieKey, - charlieAttributes, - charlieActions, - 'default' - ); + client.getBanditAction(flagKey, bobKey, bobAttributes, bobActions, 'default'); + client.getBanditAction(flagKey, aliceKey, aliceAttributes, aliceActions, 'default'); + client.getBanditAction(flagKey, charlieKey, charlieAttributes, charlieActions, 'default'); // even though bob was called 2nd time within cache validity time // we expect assignment to be logged because max cache size is 2 // and currently storing alice and charlie - client.getBanditAction( - flagKey, - bobKey, - bobAttributes, - bobActions, - 'default', - ); + client.getBanditAction(flagKey, bobKey, bobAttributes, bobActions, 'default'); expect(banditLoggerSpy).toHaveBeenCalledTimes(4); }); }); From 9e0ed56ec25c3964bf1cd055eab1da392b29620b Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 29 Nov 2024 16:03:55 +0000 Subject: [PATCH 4/4] bandit cache tests fix --- src/index.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.spec.ts b/src/index.spec.ts index b1f952c..afa3a82 100644 --- a/src/index.spec.ts +++ b/src/index.spec.ts @@ -374,6 +374,7 @@ describe('EppoClient E2E test', () => { }); it('Should log bandit assignment if cached entry is expired', async () => { jest.useFakeTimers(); + banditLoggerSpy.mockReset(); const client = getInstance(); client.useExpiringInMemoryBanditAssignmentCache(2); @@ -385,6 +386,7 @@ describe('EppoClient E2E test', () => { }); it('Should invalidate least used cache entry if cache reaches max size', async () => { + banditLoggerSpy.mockReset(); const client = getInstance(); client.useExpiringInMemoryBanditAssignmentCache(2);