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.

Sign up for GitHub

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

Add support for contextual bandits #64

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa92595
existing tests passing
aarsilv Jul 2, 2024
d5cef16
better test helper
aarsilv Jul 2, 2024
c050f04
tests passing
aarsilv Jul 2, 2024
0230d3e
use latest common SDK
aarsilv Jul 3, 2024
f7db8fb
define mock bandit stores for non bandit tests
aarsilv Jul 3, 2024
5083289
bump versin number
aarsilv Jul 8, 2024
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
2 changes: 2 additions & 0 deletions node-server-sdk.api.md
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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"
}
}
}
154 changes: 110 additions & 44 deletions src/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,21 @@ import {
IConfigurationStore,
Flag,
VariationType,
IBanditEvent,
IBanditLogger,
} from '@eppo/js-client-sdk-common';
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 Down Expand Up @@ -139,45 +145,45 @@ 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),
};

const assignmentFn = typeAssignmentFunctions[variationType];
if (!assignmentFn) {
throw new Error(`Unknown variation type: ${variationType}`);
}
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}`);
}

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, undefined, undefined, requestParamsStub);
const assignment = client.getStringAssignment(flagKey, 'subject-10', {}, 'default-value');
expect(assignment).toEqual('default-value');
});
Expand All @@ -186,9 +192,9 @@ 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, undefined, undefined, requestParamsStub);
const mockLogger = td.object<IAssignmentLogger>();
client.setLogger(mockLogger);
client.setAssignmentLogger(mockLogger);
const assignment = client.getStringAssignment(
flagKey,
'subject-10',
Expand All @@ -211,12 +217,12 @@ 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, undefined, undefined, 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 +233,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 +302,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 +332,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 +364,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
View file
Open in desktop
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
23 changes: 19 additions & 4 deletions test/mockApiServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,30 @@
import * as express from 'express';

import { MOCK_UFC_RESPONSE_FILE, readMockUFCResponse } from './testHelpers';
import {
MOCK_BANDIT_MODELS_RESPONSE_FILE,
MOCK_FLAGS_WITH_BANDITS_RESPONSE_FILE,
MOCK_UFC_RESPONSE_FILE,
readMockResponse,
} from './testHelpers';

const api = express();

export const TEST_SERVER_PORT = 4123;
export const TEST_BANDIT_API_KEY = 'bandit-test-key';
const flagEndpoint = /flag-config\/v1\/config*/;
const banditEndpoint = /flag-config\/v1\/bandits*/;

api.get(flagEndpoint, (_req, res) => {
const mockRacResponse = readMockUFCResponse(MOCK_UFC_RESPONSE_FILE);
res.json(mockRacResponse);
api.get(flagEndpoint, (req, res) => {
const ufcFile = req.url.includes(TEST_BANDIT_API_KEY)
? MOCK_FLAGS_WITH_BANDITS_RESPONSE_FILE
: MOCK_UFC_RESPONSE_FILE;
Comment on lines +18 to +20
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some tests use one file, some use the other; use the API key to decide which to serve

const mockUfcResponse = readMockResponse(ufcFile);
res.json(mockUfcResponse);
});

api.get(banditEndpoint, (req, res) => {
const mockBanditResponse = readMockResponse(MOCK_BANDIT_MODELS_RESPONSE_FILE);
res.json(mockBanditResponse);
});

const server = api.listen(TEST_SERVER_PORT, () => {
Expand Down
Loading
Loading