Merge pull request #850 from splitio/impressions_toggle

Impressions toggle

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,7 @@
+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.
+
 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).
 

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio",
-  "version": "11.0.4",
+  "version": "11.1.0",
   "description": "Split SDK",
   "files": [
     "README.md",
@@ -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",

package.json

@@ -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

src/__tests__/browserSuites/impressions-listener.spec.js

@@ -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 = {
@@ -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 });
@@ -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();
@@ -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');
   });
 }

src/__tests__/browserSuites/impressions.debug.spec.js

@@ -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;
@@ -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.');
@@ -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();

src/__tests__/browserSuites/impressions.none.spec.js

@@ -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,
@@ -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;
   });
@@ -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');
   });
 }

src/__tests__/browserSuites/impressions.spec.js

@@ -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.');

src/__tests__/browserSuites/manager.spec.js

@@ -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();
@@ -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;
     });