Skip to content

Add impression properties support #865

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

Merged
merged 5 commits into from
Mar 28, 2025
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
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
11.2.0 (March 28, 2025)
- Added new optional argument to the client `getTreatment` methods to allow passing additional evaluation options, such as a map of properties to append to the generated impression object sent to Split's backend. Read more in our docs.

11.1.0 (January 17, 2025)
- Added support for the new impressions tracking toggle available on feature flags, both respecting the setting and including the new field being returned on `SplitView` type objects. Read more in our docs.
- Updated @splitsoftware/splitio-commons package to version 2.1.0.
Expand Down
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@splitsoftware/splitio",
"version": "11.1.0",
"version": "11.1.1-rc.0",
"description": "Split SDK",
"files": [
"README.md",
Expand Down Expand Up @@ -38,7 +38,7 @@
"node": ">=14.0.0"
},
"dependencies": {
"@splitsoftware/splitio-commons": "2.1.0",
"@splitsoftware/splitio-commons": "2.1.1-rc.1",
"bloom-filters": "^3.0.4",
"ioredis": "^4.28.0",
"js-yaml": "^3.13.1",
Expand Down
7 changes: 4 additions & 3 deletions src/__tests__/browserSuites/impressions-listener.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default function (assert) {
const testAttrs = { is_test: true };

// Impression listener is shared across all client instances and does not get affected by configurations.
client.getTreatment('hierarchical_splits_test');
client.getTreatment('hierarchical_splits_test', undefined, { properties: { prop1: 'prop-value' } });
client2.getTreatment('qc_team');
client2.getTreatmentWithConfig('qc_team'); // Validate that the impression is the same.
client3.getTreatment('qc_team', testAttrs);
Expand All @@ -58,7 +58,8 @@ export default function (assert) {
treatment: 'no',
bucketingKey: 'impr_bucketing_2',
label: 'default rule',
pt: undefined
pt: undefined,
properties: undefined
};

assert.equal(listener.logImpression.callCount, 4, 'Impression listener logImpression method should be called after we call client.getTreatment, once per each impression generated.');
Expand All @@ -71,7 +72,7 @@ export default function (assert) {
bucketingKey: undefined,
label: 'expected label',
changeNumber: 2828282828,
pt: undefined
properties: '{"prop1":"prop-value"}'
},
attributes: undefined,
...metaData
Expand Down
14 changes: 14 additions & 0 deletions src/__tests__/browserSuites/impressions.debug.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ export default function (fetchMock, assert) {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[1].m, c: 828282828282, r: 'another expected label', pt: data[0].i[0].m,
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[2].m, c: 828282828282, r: 'another expected label', pt: data[0].i[1].m
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[3].m, c: 828282828282, r: 'another expected label', pt: data[0].i[2].m, properties: '{"prop1":"value1"}'
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[4].m, c: 828282828282, r: 'another expected label', pt: data[0].i[3].m, properties: '{"prop1":"value2"}'
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[5].m, c: 828282828282, r: 'another expected label', pt: data[0].i[4].m, properties: '{"prop1":"value3"}'
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[6].m, c: 828282828282, r: 'another expected label', pt: data[0].i[5].m, properties: '{"prop1":"value4"}'
}]
}]);

Expand Down Expand Up @@ -96,5 +104,11 @@ export default function (fetchMock, assert) {
client.getTreatment('split_with_config');
client.getTreatment('split_with_config');
assert.equal(client.getTreatment('always_on_impressions_disabled_true'), 'on');

// with properties
assert.equal(client.getTreatment('split_with_config', undefined, { properties: { prop1: 'value1' } }), 'o.n');
assert.equal(client.getTreatments(['split_with_config'], undefined, { properties: { prop1: 'value2' } }).split_with_config, 'o.n');
assert.equal(client.getTreatmentWithConfig('split_with_config', undefined, { properties: { prop1: 'value3' } }).treatment, 'o.n');
assert.equal(client.getTreatmentsWithConfig(['split_with_config'], undefined, { properties: { prop1: 'value4' } }).split_with_config.treatment, 'o.n');
});
}
75 changes: 31 additions & 44 deletions src/__tests__/browserSuites/impressions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,21 @@ export default function (fetchMock, assert) {

const client = splitio.client();
const assertPayload = req => {
const resp = JSON.parse(req.body);

assert.equal(resp.length, 2, 'We performed evaluations for 3 features, but one with `impressionsDisabled` true, so we should have 2 items total');

const dependencyChildImpr = resp.filter(e => e.f === 'hierarchical_splits_test')[0];
const splitWithConfigImpr = resp.filter(e => e.f === 'split_with_config')[0];
const alwaysOnWithImpressionsDisabledTrue = resp.filter(e => e.f === 'always_on_impressions_disabled_true');

assert.true(dependencyChildImpr, 'Split we wanted to evaluate should be present on the impressions.');
assert.false(resp.some(e => e.f === 'hierarchical_dep_always_on'), 'Parent split evaluations should not result in impressions.');
assert.false(resp.some(e => e.f === 'hierarchical_dep_hierarchical'), 'No matter how deep is the chain.');
assert.true(splitWithConfigImpr, 'Split evaluated with config should have generated an impression too.');
assert.false(Object.prototype.hasOwnProperty.call(splitWithConfigImpr.i[0], 'configuration'), 'Impressions do not change with configuration evaluations.');
assert.false(Object.prototype.hasOwnProperty.call(splitWithConfigImpr.i[0], 'config'), 'Impressions do not change with configuration evaluations.');
assert.equal(alwaysOnWithImpressionsDisabledTrue.length, 0);

const {
k,
r,
t
} = dependencyChildImpr.i[0];

assert.equal(k, 'facundo@split.io', 'Present impression should have the correct key.');
// The label present on the mock.
assert.equal(r, 'expected label', 'Present impression should have the correct label.');
assert.equal(t, 'on', 'Present impression should have the correct treatment.');
const reqBody = JSON.parse(req.body);

assert.deepEqual(reqBody, [{
f: 'hierarchical_splits_test',
i: [{
k: 'facundo@split.io', t: 'on', m: reqBody[0].i[0].m, c: 2828282828, r: 'expected label'
}]
}, {
f: 'split_with_config',
i: [{
k: 'facundo@split.io', t: 'o.n', m: reqBody[1].i[0].m, c: 828282828282, r: 'another expected label'
}, {
k: 'facundo@split.io', t: 'o.n', m: reqBody[1].i[1].m, c: 828282828282, r: 'another expected label', properties: '{"some":"value2"}'
}]
}], 'We performed evaluations for 3 features, but one with `impressionsDisabled` true, so we should have 2 items total');
};

fetchMock.postOnce(url(settings, '/testImpressions/bulk'), (url, req) => {
Expand All @@ -93,28 +82,23 @@ export default function (fetchMock, assert) {
return 200;
});

fetchMock.postOnce(url(settings, '/testImpressions/count'), (url, opts) => {
const data = JSON.parse(opts.body);
fetchMock.postOnce(url(settings, '/testImpressions/count'), (url, req) => {
const reqBody = JSON.parse(req.body);

assert.equal(data.pf.length, 2, 'We should generate impressions count for 2 features.');

// finding these validate the feature names collection too
const splitWithConfigImpr = data.pf.filter(e => e.f === 'split_with_config')[0];
const alwaysOnWithImpressionsDisabledTrue = data.pf.filter(e => e.f === 'always_on_impressions_disabled_true')[0];

assert.equal(splitWithConfigImpr.rc, 2);
assert.equal(typeof splitWithConfigImpr.m, 'number');
assert.equal(splitWithConfigImpr.m, truncatedTimeFrame);
assert.equal(alwaysOnWithImpressionsDisabledTrue.rc, 1);
assert.equal(typeof alwaysOnWithImpressionsDisabledTrue.m, 'number');
assert.equal(alwaysOnWithImpressionsDisabledTrue.m, truncatedTimeFrame);
assert.deepEqual(reqBody, {
pf: [{
f: 'split_with_config', m: truncatedTimeFrame, rc: 2
}, {
f: 'always_on_impressions_disabled_true', m: truncatedTimeFrame, rc: 1
}]
}, 'We should generate impressions count for 2 features.');

return 200;
});

fetchMock.postOnce(url(settings, '/v1/keys/cs'), (url, opts) => {
assert.deepEqual(JSON.parse(opts.body), {
keys: [{ fs: [ 'always_on_impressions_disabled_true' ], k: 'facundo@split.io' }]
keys: [{ fs: ['always_on_impressions_disabled_true'], k: 'facundo@split.io' }]
}, 'We should only track unique keys for features flags with track impressions disabled.');

return 200;
Expand All @@ -129,9 +113,12 @@ export default function (fetchMock, assert) {
config: '{"color":"brown","dimensions":{"height":12,"width":14},"text":{"inner":"click me"}}'
}, 'We should get an evaluation as always.');
client.getTreatmentWithConfig('split_with_config');
client.getTreatmentWithConfig('split_with_config');
client.getTreatmentWithConfig('split_with_config', undefined, { properties: { /* empty properties are ignored */ } });

// Impression should not be tracked (passed properties will not be submitted)
assert.equal(client.getTreatment('always_on_impressions_disabled_true'), 'on', undefined, { properties: { some: 'value1' } });

// Impression should not be tracked
assert.equal(client.getTreatment('always_on_impressions_disabled_true'), 'on');
// Tracked impression with properties should be handled in DEBUG mode (doesn't increase `rc` count but adds an impression)
assert.equal(client.getTreatment('split_with_config', undefined, { properties: { some: 'value2' } }), 'o.n');
});
}
9 changes: 6 additions & 3 deletions src/__tests__/consumer/node_redis.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const NA = 'NA';

const redisPort = '6385';

const TOTAL_RAW_IMPRESSIONS = 16;
const TOTAL_RAW_IMPRESSIONS = 17;
const TOTAL_EVENTS = 2;
const DEDUPED_IMPRESSIONS = 3;

Expand Down Expand Up @@ -118,7 +118,8 @@ tape('Node.js Redis', function (t) {

/** Evaluation, track and manager methods on SDK_READY */

assert.equal(await client.getTreatment('UT_Segment_member', 'UT_IN_SEGMENT'), 'on', 'Evaluations using Redis storage should be correct.');
assert.equal(await client.getTreatment('UT_Segment_member', 'UT_IN_SEGMENT', undefined, { properties: { /* empty properties are ignored */ } }), 'on', 'Evaluations using Redis storage should be correct.');
assert.equal(await client.getTreatment('UT_Segment_member', 'UT_IN_SEGMENT', undefined, { properties: { some: 'value1' } }), 'on', 'Evaluations using Redis storage should be correct.');
assert.equal(await client.getTreatment('other', 'UT_IN_SEGMENT'), 'off', 'Evaluations using Redis storage should be correct.');

assert.equal(await client.getTreatment('UT_Segment_member', 'UT_NOT_IN_SEGMENT'), 'off', 'Evaluations using Redis storage should be correct.');
Expand Down Expand Up @@ -223,7 +224,9 @@ tape('Node.js Redis', function (t) {
// this should be deduped
assert.equal(await client.getTreatment('other', 'UT_IN_SEGMENT'), 'off', 'Evaluations using Redis storage should be correct.');
// this should be deduped
assert.equal(await client.getTreatment('other', 'UT_IN_SEGMENT'), 'off', 'Evaluations using Redis storage should be correct.');
assert.equal(await client.getTreatment('other', 'UT_IN_SEGMENT', undefined, { properties: { /* empty properties are ignored */ } }), 'off', 'Evaluations using Redis storage should be correct.');
// this should not be deduped because of properties
assert.equal(await client.getTreatment('other', 'UT_IN_SEGMENT', undefined, { properties: { some: 'value1' } }), 'off', 'Evaluations using Redis storage should be correct.');

assert.equal(await client.getTreatment('UT_Segment_member', 'UT_NOT_IN_SEGMENT'), 'off', 'Evaluations using Redis storage should be correct.');
assert.equal(await client.getTreatment('other', 'UT_NOT_IN_SEGMENT'), 'on', 'Evaluations using Redis storage should be correct.');
Expand Down
6 changes: 3 additions & 3 deletions src/__tests__/nodeSuites/impressions-listener.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ export default function (assert) {
const testAttrs = { is_test: true };

// Generate one impression, depends on hierarchical_dep_hierarchical which depends on hierarchical_dep_always_on
client.getTreatment('nicolas@split.io', 'hierarchical_splits_test');
client.getTreatment('nicolas@split.io', 'hierarchical_splits_test', undefined, { properties: { prop1: 'prop-value' } });
client.getTreatment({ matchingKey: 'marcio@split.io', bucketingKey: 'impr_bucketing_2' }, 'qc_team');
client.getTreatment('facundo@split.io', 'qc_team', testAttrs);
client.getTreatment('facundo@split.io', 'qc_team', testAttrs);

setTimeout(() => {
assert.true(listener.logImpression.callCount, 4, 'Impression listener logImpression method should be called after we call client.getTreatment, once per each impression generated.');
assert.equal(listener.logImpression.callCount, 4, 'Impression listener logImpression method should be called after we call client.getTreatment, once per each impression generated.');
assert.true(listener.logImpression.getCall(0).calledWithExactly({
impression: {
feature: 'hierarchical_splits_test',
Expand All @@ -55,7 +55,7 @@ export default function (assert) {
bucketingKey: undefined,
label: 'expected label',
changeNumber: 2828282828,
pt: undefined
properties: '{"prop1":"prop-value"}'
},
attributes: undefined,
...metaData
Expand Down
13 changes: 13 additions & 0 deletions src/__tests__/nodeSuites/impressions.debug.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ export default async function (key, fetchMock, assert) {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[1].m, c: 828282828282, r: 'another expected label', b: 'test_buck_key', pt: data[0].i[0].m
}, {
k: 'facundo@split.io', t: 'o.n', m: data[0].i[2].m, c: 828282828282, r: 'another expected label', b: 'test_buck_key', pt: data[0].i[1].m
}, {
k: 'emi@split.io', t: 'o.n', m: data[0].i[3].m, c: 828282828282, r: 'another expected label', properties: '{"prop1":"value1"}'
}, {
k: 'emi@split.io', t: 'o.n', m: data[0].i[4].m, c: 828282828282, r: 'another expected label', pt: data[0].i[3].m, properties: '{"prop1":"value2"}'
}, {
k: 'emi@split.io', t: 'o.n', m: data[0].i[5].m, c: 828282828282, r: 'another expected label', pt: data[0].i[4].m, properties: '{"prop1":"value3"}'
}, {
k: 'emi@split.io', t: 'o.n', m: data[0].i[6].m, c: 828282828282, r: 'another expected label', pt: data[0].i[5].m, properties: '{"prop1":"value4"}'
}]
}], 'We performed evaluations for one split, so we should have 1 item total.');

Expand Down Expand Up @@ -98,4 +106,9 @@ export default async function (key, fetchMock, assert) {
assert.equal(client.getTreatment({ matchingKey: key, bucketingKey: 'test_buck_key' }, 'split_with_config'), 'o.n');
assert.equal(client.getTreatment({ matchingKey: key, bucketingKey: 'test_buck_key' }, 'split_with_config'), 'o.n');
assert.equal(client.getTreatment({ matchingKey: 'other_key', bucketingKey: 'test_buck_key' }, 'always_on_impressions_disabled_true'), 'on');

assert.equal(client.getTreatment('emi@split.io', 'split_with_config', undefined, { properties: { prop1: 'value1' } }), 'o.n');
assert.equal(client.getTreatments('emi@split.io', ['split_with_config'], undefined, { properties: { prop1: 'value2' } }).split_with_config, 'o.n');
assert.equal(client.getTreatmentWithConfig('emi@split.io', 'split_with_config', undefined, { properties: { prop1: 'value3' } }).treatment, 'o.n');
assert.equal(client.getTreatmentsWithConfig('emi@split.io', ['split_with_config'], undefined, { properties: { prop1: 'value4' } }).split_with_config.treatment, 'o.n');
}
14 changes: 11 additions & 3 deletions src/__tests__/nodeSuites/impressions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default async function (key, fetchMock, assert) {
const alwaysOnWithImpressionsDisabledTrue = data.filter(e => e.f === 'always_on_impressions_disabled_true');

assert.equal(notExistentSplitImpr.i.length, 1); // Only one, the split not found is filtered by the non existent Split check.
assert.equal(splitWithConfigImpr.i.length, 2);
assert.equal(splitWithConfigImpr.i.length, 3);
assert.equal(dependencyChildImpr.i.length, 1);
assert.equal(alwaysOnWithImpressionsDisabledTrue.length, 0);

Expand All @@ -75,6 +75,7 @@ export default async function (key, fetchMock, assert) {
assert.equal(output.t, expected.treatment, 'Present impressions should have the correct treatment.');
assert.equal(output.r, expected.label, 'Present impressions should have the correct label.');
assert.equal(output.c, expected.changeNumber, 'Present impressions should have the correct changeNumber.');
assert.equal(output.properties, expected.properties, 'Present impressions should have the correct properties.');
assert.true(output.m >= (performedWhenReady ? readyEvaluationsStart : evaluationsStart) && output.m <= evaluationsEnd, 'Present impressions should have the correct timestamp (test with error margin).');
}

Expand All @@ -90,6 +91,10 @@ export default async function (key, fetchMock, assert) {
keyName: 'facundo@split.io', label: 'another expected label', treatment: 'o.n',
bucketingKey: 'test_buck_key', changeNumber: 828282828282
});
validateImpressionData(splitWithConfigImpr.i[2], {
keyName: 'other_key', label: 'another expected label', treatment: 'o.n',
changeNumber: 828282828282, properties: '{"some":"value2"}'
});

// Not push impressions with a invalid key (aka matching key)
assert.true(
Expand Down Expand Up @@ -153,8 +158,11 @@ export default async function (key, fetchMock, assert) {
client.getTreatmentWithConfig({ matchingKey: key, bucketingKey: 'test_buck_key' }, 'split_with_config');
client.getTreatmentWithConfig({ matchingKey: 'different', bucketingKey: 'test_buck_key' }, 'split_with_config');

// Impression should not be tracked
assert.equal(client.getTreatment('other_key', 'always_on_impressions_disabled_true'), 'on');
// Impression should not be tracked (passed properties will not be submitted)
assert.equal(client.getTreatment('other_key', 'always_on_impressions_disabled_true'), 'on', undefined, { properties: { some: 'value1' } });

// Tracked impression with properties should be handled in DEBUG mode
assert.equal(client.getTreatment('other_key', 'split_with_config', undefined, { properties: { some: 'value2' } }), 'o.n');

evaluationsEnd = Date.now();
}
Loading