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

Sameeran/ff 1130 sdk assigns subjects to a holdout js common #27

Merged
merged 16 commits into from
Nov 18, 2023
Merged

Sameeran/ff 1130 sdk assigns subjects to a holdout js common #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
54f3e77
Add holdout test and update internal get assignment
sameerank Nov 8, 2023
e4095ce
Add holdoutKey and holdoutVariation to the output
sameerank Nov 8, 2023
9fdaab4
Update test to check that get assignment returns control variation an…
sameerank Nov 8, 2023
438fcec
Test that get assignment returns the shipped variation and logs all_s…
sameerank Nov 9, 2023
1e51348
Make holdout field nullable
sameerank Nov 9, 2023
ee0478e
Make toBeNull consistent
sameerank Nov 9, 2023
02bc5bd
Restructure holdouts in mock config for test
sameerank Nov 15, 2023
370ffda
Update get assignment code to match new format
sameerank Nov 16, 2023
ed0f1d9
Remove console log
sameerank Nov 16, 2023
b233553
Change subject shard range to 10000 in tests
sameerank Nov 16, 2023
ddd7130
Rename HoldoutVariationType to NullableHoldoutVariationType
sameerank Nov 16, 2023
375b13d
Update makeful to include holdout test data
sameerank Nov 18, 2023
b837c3f
Try removing dot
sameerank Nov 18, 2023
b4a85fe
Fix spelling of directory
sameerank Nov 18, 2023
87cfce9
Bump axios version and use nullish coalesce
sameerank Nov 18, 2023
c02aa8d
Bump version number of sdk
sameerank Nov 18, 2023
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
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ test-data:
cp ${gitDataDir}rac-experiments-v3.json ${testDataDir}
cp ${gitDataDir}rac-experiments-v3-obfuscated.json ${testDataDir}
cp -r ${gitDataDir}assignment-v2 ${testDataDir}
cp -r ${gitDataDir}assignment-v2-holdouts/. ${testDataDir}assignment-v2
rm -rf ${tempDir}

## prepare
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
@@ -1,6 +1,6 @@
{
"name": "@eppo/js-client-sdk-common",
"version": "1.7.0",
"version": "1.8.0",
"description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)",
"main": "dist/index.js",
"files": [
Expand Down Expand Up @@ -63,7 +63,7 @@
"xhr-mock": "^2.5.1"
},
"dependencies": {
"axios": "^0.27.2",
"axios": "^1.6.0",
"lru-cache": "^10.0.1",
"md5": "^2.3.0"
}
Expand Down
17 changes: 17 additions & 0 deletions src/assignment-logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
export enum HoldoutVariationEnum {
STATUS_QUO = 'status_quo',
ALL_SHIPPED = 'all_shipped_variants',
}

export type NullableHoldoutVariationType = HoldoutVariationEnum | null;

/**
* Holds data about the variation a subject was assigned to.
* @public
Expand Down Expand Up @@ -33,6 +40,16 @@ export interface IAssignmentEvent {
*/
timestamp: string;

/**
* An Eppo holdout key
*/
holdout: string | null;

/**
* The Eppo holdout variation for the assigned variation
*/
holdoutVariation: NullableHoldoutVariationType;
Copy link
Member

Choose a reason for hiding this comment

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

nice


// eslint-disable-next-line @typescript-eslint/no-explicit-any
subjectAttributes: Record<string, any>;
}
Expand Down
244 changes: 232 additions & 12 deletions src/client/eppo-client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('EppoClient E2E test', () => {
const mockExperimentConfig = {
name: flagKey,
enabled: true,
subjectShards: 100,
subjectShards: 10000,
overrides: {},
typedOverrides: {},
rules: [
Expand All @@ -99,32 +99,35 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 34,
end: 3333,
},
},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 34,
end: 67,
start: 3333,
end: 6667,
},
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 67,
end: 100,
start: 6667,
end: 10000,
},
},
],
Expand Down Expand Up @@ -470,14 +473,17 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
{
Expand All @@ -502,6 +508,9 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'control',
Expand All @@ -518,7 +527,7 @@ describe('EppoClient E2E test', () => {
typedValue: 'treatment',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
],
Expand Down Expand Up @@ -546,14 +555,17 @@ describe('EppoClient E2E test', () => {
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: null,
shippedVariationKey: null,
holdouts: [],
variations: [
{
name: 'some-new-treatment',
value: 'some-new-treatment',
typedValue: 'some-new-treatment',
shardRange: {
start: 0,
end: 100,
end: 10000,
},
},
],
Expand Down Expand Up @@ -596,13 +608,221 @@ describe('EppoClient E2E test', () => {

const client = new EppoClient(storage);
let assignment = client.getAssignment('subject-10', flagKey, { appVersion: 9 });
expect(assignment).toEqual(null);
expect(assignment).toBeNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice being more explicit

assignment = client.getAssignment('subject-10', flagKey);
expect(assignment).toEqual(null);
expect(assignment).toBeNull();
assignment = client.getAssignment('subject-10', flagKey, { appVersion: 11 });
expect(assignment).toEqual('control');
});

it('returns control variation and logs holdout key if subject is in holdout in an experiment allocation', () => {
const entry = {
...mockExperimentConfig,
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: 'variation-7',
shippedVariationKey: null,
holdouts: [
{
holdoutKey: 'holdout-2',
statusQuoShardRange: {
start: 0,
end: 200,
},
shippedShardRange: null, // this is an experiment allocation because shippedShardRange is null
},
{
holdoutKey: 'holdout-3',
statusQuoShardRange: {
start: 200,
end: 400,
},
shippedShardRange: null,
},
],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 3333,
},
variationKey: 'variation-7',
Copy link
Contributor

Choose a reason for hiding this comment

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

Was key introduced as an immutable identifier for a variation? Is it basically variation-<id>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup. I think in could be helpful to have variation keys in case we need to add more methods for working with variations

},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 3333,
end: 6667,
},
variationKey: 'variation-8',
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 6667,
end: 10000,
},
variationKey: 'variation-9',
},
],
},
},
};

storage.setEntries({ [flagKey]: entry });

const mockLogger = td.object<IAssignmentLogger>();
const client = new EppoClient(storage);
client.setLogger(mockLogger);
td.reset();

// subject-79 --> holdout shard is 186
let assignment = client.getAssignment('subject-79', flagKey);
expect(assignment).toEqual('control');
// Only log holdout key (not variation) if this is an experiment allocation
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdout).toEqual('holdout-2');

// subject-8 --> holdout shard is 201
assignment = client.getAssignment('subject-8', flagKey);
expect(assignment).toEqual('control');
// Only log holdout key (not variation) if this is an experiment allocation
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdout).toEqual('holdout-3');

// subject-11 --> holdout shard is 9137 (outside holdout), non-holdout assignment shard is 8414
assignment = client.getAssignment('subject-11', flagKey);
expect(assignment).toEqual('variant-2');
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdout).toBeNull();
Comment on lines +704 to +705
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

});

it('returns the shipped variation and logs holdout key and variation if subject is in holdout in a rollout allocation', () => {
const entry = {
...mockExperimentConfig,
allocations: {
allocation1: {
percentExposure: 1,
statusQuoVariationKey: 'variation-7',
shippedVariationKey: 'variation-8',
holdouts: [
{
holdoutKey: 'holdout-2',
statusQuoShardRange: {
start: 0,
end: 100,
},
shippedShardRange: {
start: 100,
end: 200,
},
},
{
holdoutKey: 'holdout-3',
statusQuoShardRange: {
start: 200,
end: 300,
},
shippedShardRange: {
start: 300,
end: 400,
},
},
],
variations: [
{
name: 'control',
value: 'control',
typedValue: 'control',
shardRange: {
start: 0,
end: 0,
},
variationKey: 'variation-7',
},
{
name: 'variant-1',
value: 'variant-1',
typedValue: 'variant-1',
shardRange: {
start: 0,
end: 0,
},
variationKey: 'variation-8',
},
{
name: 'variant-2',
value: 'variant-2',
typedValue: 'variant-2',
shardRange: {
start: 0,
end: 10000,
},
variationKey: 'variation-9',
},
],
},
},
};

storage.setEntries({ [flagKey]: entry });

const mockLogger = td.object<IAssignmentLogger>();
const client = new EppoClient(storage);
client.setLogger(mockLogger);
td.reset();

// subject-227 --> holdout shard is 57
let assignment = client.getAssignment('subject-227', flagKey);
expect(assignment).toEqual('control');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdoutVariation).toEqual(
'status_quo',
);
expect(td.explain(mockLogger.logAssignment).calls[0].args[0].holdout).toEqual('holdout-2');

// subject-79 --> holdout shard is 186
assignment = client.getAssignment('subject-79', flagKey);
expect(assignment).toEqual('variant-1');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdoutVariation).toEqual(
'all_shipped_variants',
);
expect(td.explain(mockLogger.logAssignment).calls[1].args[0].holdout).toEqual('holdout-2');

// subject-8 --> holdout shard is 201
assignment = client.getAssignment('subject-8', flagKey);
expect(assignment).toEqual('control');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdoutVariation).toEqual(
'status_quo',
);
expect(td.explain(mockLogger.logAssignment).calls[2].args[0].holdout).toEqual('holdout-3');

// subject-50 --> holdout shard is 347
assignment = client.getAssignment('subject-50', flagKey);
expect(assignment).toEqual('variant-1');
// Log both holdout key and variation if this is a rollout allocation
expect(td.explain(mockLogger.logAssignment).calls[3].args[0].holdoutVariation).toEqual(
'all_shipped_variants',
);
expect(td.explain(mockLogger.logAssignment).calls[3].args[0].holdout).toEqual('holdout-3');

// subject-7 --> holdout shard is 9483 (outside holdout), non-holdout assignment shard is 8673
assignment = client.getAssignment('subject-7', flagKey);
expect(assignment).toEqual('variant-2');
expect(td.explain(mockLogger.logAssignment).calls[4].args[0].holdoutVariation).toBeNull();
expect(td.explain(mockLogger.logAssignment).calls[4].args[0].holdout).toBeNull();
});

function getAssignmentsWithSubjectAttributes(
subjectsWithAttributes: {
subjectKey: string;
Expand Down Expand Up @@ -732,7 +952,7 @@ describe('EppoClient E2E test', () => {
},
);

expect(variation).not.toEqual(null);
expect(variation).not.toBeNull();
expect(td.explain(mockLogger.logAssignment).callCount).toEqual(1);
});
});
Expand Down
Loading