Add support for contextual bandits (#64)

aarsilv · web-flow · commit c397ca4ab297 · 2024-07-08T12:04:54.000-06:00
* existing tests passing

* better test helper

* tests passing

* use latest common SDK

* define mock bandit stores for non bandit tests

* bump versin number
diff --git a/docs/node-server-sdk.iclientconfig.banditlogger.md b/docs/node-server-sdk.iclientconfig.banditlogger.md
@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [@eppo/node-server-sdk](./node-server-sdk.md) &gt; [IClientConfig](./node-server-sdk.iclientconfig.md) &gt; [banditLogger](./node-server-sdk.iclientconfig.banditlogger.md)
+
+## IClientConfig.banditLogger property
+
+Logging implementation to send bandit actions to your data warehouse
+
+**Signature:**
+
+```typescript
+banditLogger?: IBanditLogger;
+```
diff --git a/docs/node-server-sdk.iclientconfig.md b/docs/node-server-sdk.iclientconfig.md
@@ -18,6 +18,7 @@ export interface IClientConfig
 |  --- | --- | --- | --- |
 |  [apiKey](./node-server-sdk.iclientconfig.apikey.md) |  | string | Eppo API key |
 |  [assignmentLogger](./node-server-sdk.iclientconfig.assignmentlogger.md) |  | IAssignmentLogger | Pass a logging implementation to send variation assignments to your data warehouse. |
+|  [banditLogger?](./node-server-sdk.iclientconfig.banditlogger.md) |  | IBanditLogger | _(Optional)_ Logging implementation to send bandit actions to your data warehouse |
 |  [baseUrl?](./node-server-sdk.iclientconfig.baseurl.md) |  | string | _(Optional)_ Base URL of the Eppo API. Clients should use the default setting in most cases. |
 |  [numInitialRequestRetries?](./node-server-sdk.iclientconfig.numinitialrequestretries.md) |  | number | _(Optional)_ Number of additional times the initial configuration request will be attempted if it fails. This is the request servers typically synchronously wait for completion. A small wait will be done between requests. (Default: 1) |
 |  [numPollRequestRetries?](./node-server-sdk.iclientconfig.numpollrequestretries.md) |  | number | _(Optional)_ Number of additional times polling for updated configurations will be attempted before giving up. Polling is done after a successful initial request. Subsequent attempts are done using an exponential backoff. (Default: 7) |
diff --git a/node-server-sdk.api.md b/node-server-sdk.api.md
@@ -6,6 +6,7 @@
 
 import { IAssignmentEvent } from '@eppo/js-client-sdk-common';
 import { IAssignmentLogger } from '@eppo/js-client-sdk-common';
+import { IBanditLogger } from '@eppo/js-client-sdk-common';
 import { IEppoClient } from '@eppo/js-client-sdk-common';
 
 // @public
@@ -19,6 +20,7 @@ export { IAssignmentLogger }
 export interface IClientConfig {
     apiKey: string;
     assignmentLogger: IAssignmentLogger;
+    banditLogger?: IBanditLogger;
     baseUrl?: string;
     numInitialRequestRetries?: number;
     numPollRequestRetries?: number;
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@eppo/node-server-sdk",
-  "version": "3.0.2",
+  "version": "3.1.0",
   "description": "Eppo node server SDK",
   "main": "dist/index.js",
   "files": [
@@ -29,7 +29,7 @@
   },
   "homepage": "https://github.com/Eppo-exp/node-server-sdk#readme",
   "dependencies": {
-    "@eppo/js-client-sdk-common": "3.0.6",
+    "@eppo/js-client-sdk-common": "3.5.0",
     "lru-cache": "^10.0.1"
   },
   "devDependencies": {
@@ -59,4 +59,4 @@
     "node": ">=18.x",
     "yarn": "1.x"
   }
-}
+}
diff --git a/src/index.spec.ts b/src/index.spec.ts
@@ -6,15 +6,22 @@ import {
   IConfigurationStore,
   Flag,
   VariationType,
+  IBanditEvent,
+  IBanditLogger,
 } 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 * as td from 'testdouble';
 
-import apiServer, { TEST_SERVER_PORT } from '../test/mockApiServer';
+import apiServer, { TEST_BANDIT_API_KEY, TEST_SERVER_PORT } from '../test/mockApiServer';
 import {
+  ASSIGNMENT_TEST_DATA_DIR,
+  BANDIT_TEST_DATA_DIR,
+  BanditTestCase,
   getTestAssignments,
   IAssignmentTestCase,
-  readAssignmentTestData,
   SubjectTestCase,
+  testCasesByFileName,
   validateTestAssignments,
 } from '../test/testHelpers';
 
@@ -29,6 +36,11 @@ describe('EppoClient E2E test', () => {
     },
   };
 
+  // These two stores should not be used as this file doesn't test bandits, but we want them to be defined so bandit
+  // functionality is still "on" for the client when we explicitly instantiate the client (vs. using init())
+  const mockBanditVariationStore = td.object<IConfigurationStore<BanditVariation[]>>();
+  const mockBanditModelStore = td.object<IConfigurationStore<BanditParameters>>();
+
   const flagKey = 'mock-experiment';
 
   // Configuration for a single flag within the UFC.
@@ -139,45 +151,50 @@ describe('EppoClient E2E test', () => {
       });
     });
 
-    describe('UFC General Test Cases', () => {
-      it.each(readAssignmentTestData())(
-        'test variation assignment splits',
-        async ({ flag, variationType, defaultValue, subjects }: IAssignmentTestCase) => {
-          const client = getInstance();
-
-          let assignments: {
-            subject: SubjectTestCase;
-            assignment: string | boolean | number | object;
-          }[] = [];
-
-          const typeAssignmentFunctions = {
-            [VariationType.BOOLEAN]: client.getBoolAssignment.bind(client),
-            [VariationType.NUMERIC]: client.getNumericAssignment.bind(client),
-            [VariationType.INTEGER]: client.getIntegerAssignment.bind(client),
-            [VariationType.STRING]: client.getStringAssignment.bind(client),
-            [VariationType.JSON]: client.getJSONAssignment.bind(client),
-          };
+    describe('Shared UFC General Test Cases', () => {
+      const testCases = testCasesByFileName<IAssignmentTestCase>(ASSIGNMENT_TEST_DATA_DIR);
+
+      it.each(Object.keys(testCases))('test variation assignment splits - %s', async (fileName) => {
+        const { flag, variationType, defaultValue, subjects } = testCases[fileName];
+        const client = getInstance();
+
+        let assignments: {
+          subject: SubjectTestCase;
+          assignment: string | boolean | number | object;
+        }[] = [];
+
+        const typeAssignmentFunctions = {
+          [VariationType.BOOLEAN]: client.getBooleanAssignment.bind(client),
+          [VariationType.NUMERIC]: client.getNumericAssignment.bind(client),
+          [VariationType.INTEGER]: client.getIntegerAssignment.bind(client),
+          [VariationType.STRING]: client.getStringAssignment.bind(client),
+          [VariationType.JSON]: client.getJSONAssignment.bind(client),
+        };
+
+        const assignmentFn = typeAssignmentFunctions[variationType];
+        if (!assignmentFn) {
+          throw new Error(`Unknown variation type: ${variationType}`);
+        }
 
-          const assignmentFn = typeAssignmentFunctions[variationType];
-          if (!assignmentFn) {
-            throw new Error(`Unknown variation type: ${variationType}`);
-          }
+        assignments = getTestAssignments(
+          { flag, variationType, defaultValue, subjects },
+          assignmentFn,
+          false,
+        );
 
-          assignments = getTestAssignments(
-            { flag, variationType, defaultValue, subjects },
-            assignmentFn,
-            false,
-          );
-
-          validateTestAssignments(assignments, flag);
-        },
-      );
+        validateTestAssignments(assignments, flag);
+      });
     });
 
     it('returns the default value when ufc config is absent', () => {
       const mockConfigStore = td.object<IConfigurationStore<Flag>>();
       td.when(mockConfigStore.get(flagKey)).thenReturn(null);
-      const client = new EppoClient(mockConfigStore, requestParamsStub);
+      const client = new EppoClient(
+        mockConfigStore,
+        mockBanditVariationStore,
+        mockBanditModelStore,
+        requestParamsStub,
+      );
       const assignment = client.getStringAssignment(flagKey, 'subject-10', {}, 'default-value');
       expect(assignment).toEqual('default-value');
     });
@@ -186,9 +203,14 @@ describe('EppoClient E2E test', () => {
       const mockConfigStore = td.object<IConfigurationStore<Flag>>();
       td.when(mockConfigStore.get(flagKey)).thenReturn(mockUfcFlagConfig);
       const subjectAttributes = { foo: 3 };
-      const client = new EppoClient(mockConfigStore, requestParamsStub);
+      const client = new EppoClient(
+        mockConfigStore,
+        mockBanditVariationStore,
+        mockBanditModelStore,
+        requestParamsStub,
+      );
       const mockLogger = td.object<IAssignmentLogger>();
-      client.setLogger(mockLogger);
+      client.setAssignmentLogger(mockLogger);
       const assignment = client.getStringAssignment(
         flagKey,
         'subject-10',
@@ -211,12 +233,17 @@ describe('EppoClient E2E test', () => {
       const mockConfigStore = td.object<IConfigurationStore<Flag>>();
       td.when(mockConfigStore.get(flagKey)).thenReturn(mockUfcFlagConfig);
       const subjectAttributes = { foo: 3 };
-      const client = new EppoClient(mockConfigStore, requestParamsStub);
+      const client = new EppoClient(
+        mockConfigStore,
+        mockBanditVariationStore,
+        mockBanditModelStore,
+        requestParamsStub,
+      );
       const mockLogger = td.object<IAssignmentLogger>();
       td.when(mockLogger.logAssignment(td.matchers.anything())).thenThrow(
         new Error('logging error'),
       );
-      client.setLogger(mockLogger);
+      client.setAssignmentLogger(mockLogger);
       const assignment = client.getStringAssignment(
         flagKey,
         'subject-10',
@@ -227,6 +254,66 @@ describe('EppoClient E2E test', () => {
     });
   });
 
+  describe('Shared Bandit Test Cases', () => {
+    beforeAll(async () => {
+      const dummyBanditLogger: IBanditLogger = {
+        logBanditAction(banditEvent: IBanditEvent) {
+          console.log(
+            `Bandit ${banditEvent.bandit} assigned ${banditEvent.subject} the action ${banditEvent.action}`,
+          );
+        },
+      };
+
+      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,
+      });
+    });
+
+    const testCases = testCasesByFileName<BanditTestCase>(BANDIT_TEST_DATA_DIR);
+
+    it.each(Object.keys(testCases))('Shared bandit test case - %s', async (fileName: string) => {
+      const { flag: flagKey, defaultValue, subjects } = testCases[fileName];
+      let numAssignmentsChecked = 0;
+      subjects.forEach((subject) => {
+        // test files have actions as an array, so we convert them to a map as expected by the client
+        const actions: Record<string, ContextAttributes> = {};
+        subject.actions.forEach((action) => {
+          actions[action.actionKey] = {
+            numericAttributes: action.numericAttributes,
+            categoricalAttributes: action.categoricalAttributes,
+          };
+        });
+
+        // get the bandit assignment for the test case
+        const banditAssignment = getInstance().getBanditAction(
+          flagKey,
+          subject.subjectKey,
+          subject.subjectAttributes,
+          actions,
+          defaultValue,
+        );
+
+        // Do this check in addition to assertions to provide helpful information on exactly which
+        // evaluation failed to produce an expected result
+        if (
+          banditAssignment.variation !== subject.assignment.variation ||
+          banditAssignment.action !== subject.assignment.action
+        ) {
+          console.error(`Unexpected result for flag ${flagKey} and subject ${subject.subjectKey}`);
+        }
+
+        expect(banditAssignment.variation).toBe(subject.assignment.variation);
+        expect(banditAssignment.action).toBe(subject.assignment.action);
+        numAssignmentsChecked += 1;
+      });
+      // Ensure that this test case correctly checked some test assignments
+      expect(numAssignmentsChecked).toBeGreaterThan(0);
+    });
+  });
+
   describe('initialization errors', () => {
     const maxRetryDelay = POLL_INTERVAL_MS * POLL_JITTER_PCT;
     const mockConfigResponse = {
@@ -236,9 +323,9 @@ describe('EppoClient E2E test', () => {
     };
 
     it('retries initial configuration request before resolving', async () => {
-      td.replace(HttpClient.prototype, 'get');
+      td.replace(HttpClient.prototype, 'getUniversalFlagConfiguration');
       let callCount = 0;
-      td.when(HttpClient.prototype.get(td.matchers.anything())).thenDo(() => {
+      td.when(HttpClient.prototype.getUniversalFlagConfiguration()).thenDo(() => {
         if (++callCount === 1) {
           // Throw an error for the first call
           throw new Error('Intentional Thrown Error For Test');
@@ -266,9 +353,9 @@ describe('EppoClient E2E test', () => {
     });
 
     it('gives up initial request and throws error after hitting max retries', async () => {
-      td.replace(HttpClient.prototype, 'get');
+      td.replace(HttpClient.prototype, 'getUniversalFlagConfiguration');
       let callCount = 0;
-      td.when(HttpClient.prototype.get(td.matchers.anything())).thenDo(async () => {
+      td.when(HttpClient.prototype.getUniversalFlagConfiguration()).thenDo(async () => {
         callCount += 1;
         throw new Error('Intentional Thrown Error For Test');
       });
@@ -298,9 +385,9 @@ describe('EppoClient E2E test', () => {
     });
 
     it('gives up initial request but still polls later if configured to do so', async () => {
-      td.replace(HttpClient.prototype, 'get');
+      td.replace(HttpClient.prototype, 'getUniversalFlagConfiguration');
       let callCount = 0;
-      td.when(HttpClient.prototype.get(td.matchers.anything())).thenDo(() => {
+      td.when(HttpClient.prototype.getUniversalFlagConfiguration()).thenDo(() => {
         if (++callCount <= 2) {
           // Throw an error for the first call
           throw new Error('Intentional Thrown Error For Test');
diff --git a/src/index.ts b/src/index.ts
@@ -6,12 +6,12 @@ import {
   FlagConfigurationRequestParameters,
   MemoryOnlyConfigurationStore,
   Flag,
+  IBanditLogger,
 } from '@eppo/js-client-sdk-common';
-import { ObfuscatedFlag } from '@eppo/js-client-sdk-common/dist/interfaces';
+import { BanditParameters, BanditVariation } from '@eppo/js-client-sdk-common/dist/interfaces';
 
 import { sdkName, sdkVersion } from './sdk-data';
 
-
 /**
  * Configuration used for initializing the Eppo client
  * @public
@@ -33,6 +33,11 @@ export interface IClientConfig {
    */
   assignmentLogger: IAssignmentLogger;
 
+  /**
+   * Logging implementation to send bandit actions to your data warehouse
+   */
+  banditLogger?: IBanditLogger;
+
   /***
    * Timeout in milliseconds for the HTTPS request for the experiment configuration. (Default: 5000)
    */
@@ -76,7 +81,6 @@ let clientInstance: IEppoClient;
  */
 export async function init(config: IClientConfig): Promise<IEppoClient> {
   validation.validateNotBlank(config.apiKey, 'API key required');
-  const configurationStore = new MemoryOnlyConfigurationStore<Flag | ObfuscatedFlag>();
 
   const requestConfiguration: FlagConfigurationRequestParameters = {
     apiKey: config.apiKey,
@@ -86,13 +90,25 @@ export async function init(config: IClientConfig): Promise<IEppoClient> {
     requestTimeoutMs: config.requestTimeoutMs ?? undefined,
     numInitialRequestRetries: config.numInitialRequestRetries ?? undefined,
     numPollRequestRetries: config.numPollRequestRetries ?? undefined,
-    pollAfterSuccessfulInitialization: true, // For servers we always want to keep polling for the life of the server
+    pollAfterSuccessfulInitialization: true, // For servers, we always want to keep polling for the life of the server
     pollAfterFailedInitialization: config.pollAfterFailedInitialization ?? false,
     throwOnFailedInitialization: config.throwOnFailedInitialization ?? true,
   };
 
-  clientInstance = new EppoClient(configurationStore, requestConfiguration);
-  clientInstance.setLogger(config.assignmentLogger);
+  const flagConfigurationStore = new MemoryOnlyConfigurationStore<Flag>();
+  const banditVariationConfigurationStore = new MemoryOnlyConfigurationStore<BanditVariation[]>();
+  const banditModelConfigurationStore = new MemoryOnlyConfigurationStore<BanditParameters>();
+
+  clientInstance = new EppoClient(
+    flagConfigurationStore,
+    banditVariationConfigurationStore,
+    banditModelConfigurationStore,
+    requestConfiguration,
+  );
+  clientInstance.setAssignmentLogger(config.assignmentLogger);
+  if (config.banditLogger) {
+    clientInstance.setBanditLogger(config.banditLogger);
+  }
 
   // default to LRU cache with 50_000 entries.
   // we estimate this will use no more than 10 MB of memory
diff --git a/test/mockApiServer.ts b/test/mockApiServer.ts
diff --git a/test/testHelpers.ts b/test/testHelpers.ts
diff --git a/yarn.lock b/yarn.lock
