Merge pull request #865 from splitio/impression_properties

Add impression properties support

Author: EmilianoSanchez

CHANGES.txt

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

package-lock.json

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

package.json

@@ -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);
@@ -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.');
@@ -71,7 +72,7 @@ export default function (assert) {
           bucketingKey: undefined,
           label: 'expected label',
           changeNumber: 2828282828,
-          pt: undefined
+          properties: '{"prop1":"prop-value"}'
         },
         attributes: undefined,
         ...metaData

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

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

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

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

src/__tests__/browserSuites/impressions.spec.js

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

src/__tests__/consumer/node_redis.spec.js

@@ -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',
@@ -55,7 +55,7 @@ export default function (assert) {
           bucketingKey: undefined,
           label: 'expected label',
           changeNumber: 2828282828,
-          pt: undefined
+          properties: '{"prop1":"prop-value"}'
         },
         attributes: undefined,
         ...metaData

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

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

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

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