Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for contextual bandits #64

Merged
merged 6 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/node-server-sdk.iclientconfig.banditlogger.md
Original file line number Diff line number Diff line change
@@ -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;
```
1 change: 1 addition & 0 deletions docs/node-server-sdk.iclientconfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand Down
2 changes: 2 additions & 0 deletions node-server-sdk.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -19,6 +20,7 @@ export { IAssignmentLogger }
export interface IClientConfig {
apiKey: string;
assignmentLogger: IAssignmentLogger;
banditLogger?: IBanditLogger;
baseUrl?: string;
numInitialRequestRetries?: number;
numPollRequestRetries?: number;
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📈

"lru-cache": "^10.0.1"
},
"devDependencies": {
Expand Down Expand Up @@ -59,4 +59,4 @@
"node": ">=18.x",
"yarn": "1.x"
}
}
}
175 changes: 131 additions & 44 deletions src/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test actually spins up a server vs mocking fetch responses. Since different tests need different UFC files served, I'm switching off the provided SDK key.

Any client initialized with this key will be served the UFC file with bandit flags.

import {
ASSIGNMENT_TEST_DATA_DIR,
BANDIT_TEST_DATA_DIR,
BanditTestCase,
getTestAssignments,
IAssignmentTestCase,
readAssignmentTestData,
SubjectTestCase,
testCasesByFileName,
validateTestAssignments,
} from '../test/testHelpers';

Expand All @@ -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.
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes for nice, unique test case names / output
image


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');
});
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -227,6 +254,66 @@ describe('EppoClient E2E test', () => {
});
});

describe('Shared Bandit Test Cases', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bandits in the Node SDK!

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}`);
}
Comment on lines +299 to +306
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️


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 = {
Expand All @@ -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');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our http client interface now has two methods, one for the UFC and one for bandit model parameters

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');
Expand Down Expand Up @@ -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');
});
Expand Down Expand Up @@ -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');
Expand Down
28 changes: 22 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
*/
Expand Down Expand Up @@ -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,
Expand All @@ -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[]>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This store is needed to easily know if a flag has any bandits, which we want for handling an edge case of empty actions being passed to getBanditAction()

const banditModelConfigurationStore = new MemoryOnlyConfigurationStore<BanditParameters>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This store holds the nitty gritty model parameters (coefficients, gamma, etc.)


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
Expand Down
Loading
Loading