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

Impressions toggle #850

Merged
merged 11 commits into from
Jan 17, 2025
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
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.

11.0.4 (January 9, 2025)
- Bugfixing - Updated @splitsoftware/splitio-commons package to version 2.0.3, which properly handles rejected promises when using targeting rules with segment matchers in consumer modes (e.g., Redis and Pluggable storages).

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.0.4",
"version": "11.1.0",
"description": "Split SDK",
"files": [
"README.md",
Expand Down Expand Up @@ -38,7 +38,7 @@
"node": ">=14.0.0"
},
"dependencies": {
"@splitsoftware/splitio-commons": "2.0.3",
"@splitsoftware/splitio-commons": "2.1.0",
"bloom-filters": "^3.0.4",
"ioredis": "^4.28.0",
"js-yaml": "^3.13.1",
Expand Down
8 changes: 6 additions & 2 deletions src/__tests__/browserSuites/impressions-listener.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,21 @@ export default function (assert) {
keyName: 'marcio@split.io',
treatment: 'no',
bucketingKey: 'impr_bucketing_2',
label: 'default rule'
label: 'default rule',
pt: undefined
};

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).calledWithMatch({
assert.true(listener.logImpression.getCall(0).calledWithExactly({
impression: {
feature: 'hierarchical_splits_test',
keyName: 'nicolas@split.io',
treatment: 'on',
time: listener.logImpression.getCall(0).args[0].impression.time,
bucketingKey: undefined,
label: 'expected label',
changeNumber: 2828282828,
pt: undefined
},
attributes: undefined,
...metaData
Expand Down
66 changes: 34 additions & 32 deletions src/__tests__/browserSuites/impressions.debug.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import splitChangesMock1 from '../mocks/splitchanges.since.-1.json';
import splitChangesMock2 from '../mocks/splitchanges.since.1457552620999.json';
import membershipsFacundo from '../mocks/memberships.facundo@split.io.json';
import { DEBUG } from '@splitsoftware/splitio-commons/src/utils/constants';
import { truncateTimeFrame } from '@splitsoftware/splitio-commons/src/utils/time';
import { url } from '../testUtils';

const baseUrls = {
Expand All @@ -19,6 +20,8 @@ const settings = settingsFactory({
streamingEnabled: false
});

let truncatedTimeFrame;

export default function (fetchMock, assert) {
// Mocking this specific route to make sure we only get the items we want to test from the handlers.
fetchMock.getOnce(url(settings, '/splitChanges?s=1.2&since=-1'), { status: 200, body: splitChangesMock1 });
Expand Down Expand Up @@ -47,41 +50,21 @@ export default function (fetchMock, assert) {
});

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

assert.equal(resp.length, 1, 'We performed three evaluations so we should have 1 impressions type');

const alwaysOnWithConfigImpr = resp.filter(e => e.f === 'split_with_config')[0];

assert.equal(alwaysOnWithConfigImpr.i.length, 3);

function validateImpressionData(output, expected) {
assert.equal(output.k, expected.keyName, 'Present impressions should have the correct key.');
assert.equal(output.b, expected.bucketingKey, 'Present impressions should have the correct bucketingKey.');
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.pt, expected.pt, 'Present impressions should have the correct previousTime.');
}

validateImpressionData(alwaysOnWithConfigImpr.i[0], {
keyName: 'facundo@split.io', label: 'another expected label', treatment: 'o.n',
bucketingKey: undefined, changeNumber: 828282828282, pt: undefined
});
validateImpressionData(alwaysOnWithConfigImpr.i[1], {
keyName: 'facundo@split.io', label: 'another expected label', treatment: 'o.n',
bucketingKey: undefined, changeNumber: 828282828282, pt: alwaysOnWithConfigImpr.i[0].m
});
validateImpressionData(alwaysOnWithConfigImpr.i[2], {
keyName: 'facundo@split.io', label: 'another expected label', treatment: 'o.n',
bucketingKey: undefined, changeNumber: 828282828282, pt: alwaysOnWithConfigImpr.i[1].m
});
};

fetchMock.postOnce(url(settings, '/testImpressions/bulk'), (url, req) => {
assert.equal(req.headers.SplitSDKImpressionsMode, DEBUG);
assertPayload(req);
const data = JSON.parse(req.body);

assert.deepEqual(data, [{
f: 'split_with_config',
i: [{
k: 'facundo@split.io', t: 'o.n', m: data[0].i[0].m, c: 828282828282, r: 'another expected label'
}, {
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
}]
}]);

client.destroy().then(() => {
assert.end();
Expand All @@ -90,9 +73,28 @@ export default function (fetchMock, assert) {
return 200;
});

fetchMock.postOnce(url(settings, '/testImpressions/count'), (url, opts) => {
assert.deepEqual(JSON.parse(opts.body), {
pf: [{ f: 'always_on_track_impressions_false', m: truncatedTimeFrame, rc: 1 }]
}, 'We should generate impression count for the feature with track impressions disabled.');

return 200;
});

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

return 200;
});

client.ready().then(() => {
truncatedTimeFrame = truncateTimeFrame(Date.now());

client.getTreatment('split_with_config');
client.getTreatment('split_with_config');
client.getTreatment('split_with_config');
assert.equal(client.getTreatment('always_on_track_impressions_false'), 'on');
});
}
6 changes: 4 additions & 2 deletions src/__tests__/browserSuites/impressions.none.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ export default async function (fetchMock, assert) {
pf: [
{ f: 'split_with_config', m: truncatedTimeFrame, rc: 2 },
{ f: 'always_off', m: truncatedTimeFrame, rc: 4 },
{ f: 'always_on', m: truncatedTimeFrame, rc: 2 }
{ f: 'always_on', m: truncatedTimeFrame, rc: 2 },
{ f: 'always_on_track_impressions_false', m: truncatedTimeFrame, rc: 1 }
]
});
return 200;
Expand All @@ -75,7 +76,7 @@ export default async function (fetchMock, assert) {
},
{
k: 'emma@split.io',
fs: ['always_off', 'always_on']
fs: ['always_off', 'always_on', 'always_on_track_impressions_false']
}
]
}, 'We performed evaluations for two keys, so we should have 2 item total.');
Expand All @@ -93,6 +94,7 @@ export default async function (fetchMock, assert) {
client.getTreatment('always_on');
client.getTreatment('always_off');
client.getTreatment('split_with_config');
sharedClient.getTreatment('always_on_track_impressions_false');

client.destroy().then(() => {
assert.end();
Expand Down
41 changes: 27 additions & 14 deletions src/__tests__/browserSuites/impressions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,19 @@ export default function (fetchMock, assert) {
const assertPayload = req => {
const resp = JSON.parse(req.body);

assert.equal(resp.length, 2, 'We performed three evaluations so we should have 2 impressions type');
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 alwaysOnWithConfigImpr = resp.filter(e => e.f === 'split_with_config')[0];
const splitWithConfigImpr = resp.filter(e => e.f === 'split_with_config')[0];
const alwaysOnWithTrackImpressionsFalse = resp.filter(e => e.f === 'always_on_track_impressions_false');

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(alwaysOnWithConfigImpr, 'Split evaluated with config should have generated an impression too.');
assert.false(Object.prototype.hasOwnProperty.call(alwaysOnWithConfigImpr.i[0], 'configuration'), 'Impressions do not change with configuration evaluations.');
assert.false(Object.prototype.hasOwnProperty.call(alwaysOnWithConfigImpr.i[0], 'config'), 'Impressions do not change with configuration evaluations.');
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(alwaysOnWithTrackImpressionsFalse.length, 0);

const {
k,
Expand Down Expand Up @@ -94,18 +96,26 @@ export default function (fetchMock, assert) {
fetchMock.postOnce(url(settings, '/testImpressions/count'), (url, opts) => {
const data = JSON.parse(opts.body);

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

// finding these validate the feature names collection too
const dependencyChildImpr = data.pf.filter(e => e.f === 'hierarchical_splits_test')[0];
const alwaysOnWithConfigImpr = data.pf.filter(e => e.f === 'split_with_config')[0];
const splitWithConfigImpr = data.pf.filter(e => e.f === 'split_with_config')[0];
const alwaysOnWithTrackImpressionsFalse = data.pf.filter(e => e.f === 'always_on_track_impressions_false')[0];

assert.equal(dependencyChildImpr.rc, 1);
assert.equal(typeof dependencyChildImpr.m, 'number');
assert.equal(dependencyChildImpr.m, truncatedTimeFrame);
assert.equal(alwaysOnWithConfigImpr.rc, 3);
assert.equal(typeof alwaysOnWithConfigImpr.m, 'number');
assert.equal(alwaysOnWithConfigImpr.m, truncatedTimeFrame);
assert.equal(splitWithConfigImpr.rc, 2);
assert.equal(typeof splitWithConfigImpr.m, 'number');
assert.equal(splitWithConfigImpr.m, truncatedTimeFrame);
assert.equal(alwaysOnWithTrackImpressionsFalse.rc, 1);
assert.equal(typeof alwaysOnWithTrackImpressionsFalse.m, 'number');
assert.equal(alwaysOnWithTrackImpressionsFalse.m, truncatedTimeFrame);

return 200;
});

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

return 200;
});
Expand All @@ -120,5 +130,8 @@ export default function (fetchMock, assert) {
}, 'We should get an evaluation as always.');
client.getTreatmentWithConfig('split_with_config');
client.getTreatmentWithConfig('split_with_config');

// Impression should not be tracked
assert.equal(client.getTreatment('always_on_track_impressions_false'), 'on');
});
}
3 changes: 2 additions & 1 deletion src/__tests__/browserSuites/manager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export default async function (settings, fetchMock, assert) {
'treatments': map(mockSplits.splits[index].conditions[0].partitions, partition => partition.treatment),
'configs': mockSplits.splits[index].configurations || {},
'sets': mockSplits.splits[index].sets || [],
'defaultTreatment': mockSplits.splits[index].defaultTreatment
'defaultTreatment': mockSplits.splits[index].defaultTreatment,
'impressionsDisabled': false
});

assert.equal(manager.split('non_existent'), null, 'Trying to get a manager.split() of a Split that does not exist returns null.');
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/browserSuites/telemetry.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default async function telemetryBrowserSuite(fetchMock, t) {

// @TODO check if iDe value is correct
assert.deepEqual(data, {
mE: {}, hE: { sp: { 500: 1 }, ms: { 500: 1 } }, tR: 0, aR: 0, iQ: 4, iDe: 1, iDr: 0, spC: 32, seC: 1, skC: 1, eQ: 1, eD: 0, sE: [], t: [], ufs: {}
mE: {}, hE: { sp: { 500: 1 }, ms: { 500: 1 } }, tR: 0, aR: 0, iQ: 4, iDe: 1, iDr: 0, spC: 33, seC: 1, skC: 1, eQ: 1, eD: 0, sE: [], t: [], ufs: {}
}, 'metrics/usage JSON payload should be the expected');

finish.next();
Expand All @@ -96,7 +96,7 @@ export default async function telemetryBrowserSuite(fetchMock, t) {
// @TODO check if iDe value is correct
assert.deepEqual(data, {
mL: {}, mE: {}, hE: {}, hL: {}, // errors and latencies were popped
tR: 0, aR: 0, iQ: 4, iDe: 1, iDr: 0, spC: 32, seC: 1, skC: 1, eQ: 1, eD: 0, sE: [], t: [], ufs: {}
tR: 0, aR: 0, iQ: 4, iDe: 1, iDr: 0, spC: 33, seC: 1, skC: 1, eQ: 1, eD: 0, sE: [], t: [], ufs: {}
}, '2nd metrics/usage JSON payload should be the expected');
return 200;
});
Expand Down
Loading