diff --git a/evaluate_flag.js b/evaluate_flag.js index 34bcbbf..2eb6e0b 100644 --- a/evaluate_flag.js +++ b/evaluate_flag.js @@ -288,11 +288,15 @@ function getResultForVariationOrRollout(r, user, flag, reason, cb) { if (!r) { cb(new Error('Fallthrough variation undefined'), errorResult('MALFORMED_FLAG')); } else { - const index = variationForUser(r, user, flag); + const [index, inExperiment] = variationForUser(r, user, flag); if (index === null || index === undefined) { cb(new Error('Variation/rollout object with no variation or rollout'), errorResult('MALFORMED_FLAG')); } else { - getVariation(flag, index, reason, cb); + const transformedReason = reason; + if (inExperiment) { + transformedReason.inExperiment = true; + } + getVariation(flag, index, transformedReason, cb); } } } @@ -301,27 +305,28 @@ function errorResult(errorKind) { return { value: null, variationIndex: null, reason: { kind: 'ERROR', errorKind: errorKind } }; } -// Given a variation or rollout 'r', select -// the variation for the given user +// Given a variation or rollout 'r', select the variation for the given user. +// Returns an array of the form [variationIndex, inExperiment]. function variationForUser(r, user, flag) { if (r.variation !== null && r.variation !== undefined) { // This represets a fixed variation; return it - return r.variation; + return [r.variation, false]; } const rollout = r.rollout; if (rollout) { + const isExperiment = rollout.kind === 'experiment'; const variations = rollout.variations; if (variations && variations.length > 0) { // This represents a percentage rollout. Assume // we're rolling out by key const bucketBy = rollout.bucketBy || 'key'; - const bucket = bucketUser(user, flag.key, bucketBy, flag.salt); + const bucket = bucketUser(user, flag.key, bucketBy, flag.salt, rollout.seed); let sum = 0; for (let i = 0; i < variations.length; i++) { const variate = variations[i]; sum += variate.weight / 100000.0; if (bucket < sum) { - return variate.variation; + return [variate.variation, isExperiment && !variate.untracked]; } } @@ -330,11 +335,12 @@ function variationForUser(r, user, flag) { // data could contain buckets that don't actually add up to 100000. Rather than returning an error in // this case (or changing the scaling, which would potentially change the results for *all* users), we // will simply put the user in the last bucket. - return variations[variations.length - 1].variation; + const lastVariate = variations[variations.length - 1]; + return [lastVariate.variation, isExperiment && !lastVariate.untracked]; } } - return null; + return [null, false]; } // Fetch an attribute value from a user object. Automatically @@ -350,7 +356,7 @@ function userValue(user, attr) { } // Compute a percentile for a user -function bucketUser(user, key, attr, salt) { +function bucketUser(user, key, attr, salt, seed) { let idHash = bucketableStringValue(userValue(user, attr)); if (idHash === null) { @@ -361,7 +367,8 @@ function bucketUser(user, key, attr, salt) { idHash += '.' + user.secondary; } - const hashKey = util.format('%s.%s.%s', key, salt, idHash); + const prefix = seed ? util.format('%d.', seed) : util.format('%s.%s.', key, salt); + const hashKey = prefix + idHash; const hashVal = parseInt(sha1Hex(hashKey).substring(0, 15), 16); return hashVal / 0xfffffffffffffff; diff --git a/event_factory.js b/event_factory.js index 7369507..6e28a50 100644 --- a/event_factory.js +++ b/event_factory.js @@ -3,6 +3,11 @@ function EventFactory(withReasons) { function isExperiment(flag, reason) { if (reason) { + // If the reason says we're in an experiment, we are. Otherwise, apply + // the legacy rule exclusion logic. + if (reason.inExperiment) { + return true; + } switch (reason.kind) { case 'RULE_MATCH': { const index = reason.ruleIndex; diff --git a/test/LDClient-events-test.js b/test/LDClient-events-test.js index 6a96ec8..6941e75 100644 --- a/test/LDClient-events-test.js +++ b/test/LDClient-events-test.js @@ -196,7 +196,87 @@ describe('LDClient - analytics events', () => { value: 'b', default: 'c', trackEvents: true, - reason: { kind: 'FALLTHROUGH' } + reason: { kind: 'FALLTHROUGH' }, + }); + }); + + it('forces tracking when an evaluation is in the tracked portion of an experiment rollout', async () => { + var flag = { + key: 'flagkey', + version: 1, + on: true, + targets: [], + rules: [], + fallthrough: { + rollout: { + kind: 'experiment', + variations: [ + { + weight: 100000, + variation: 1, + }, + ], + }, + }, + variations: ['a', 'b'], + }; + var client = stubs.createClient({ eventProcessor: eventProcessor }, { flagkey: flag }); + await client.waitForInitialization(); + await client.variation(flag.key, defaultUser, 'c'); + + expect(eventProcessor.events).toHaveLength(1); + var e = eventProcessor.events[0]; + expect(e).toEqual({ + kind: 'feature', + creationDate: e.creationDate, + key: 'flagkey', + version: 1, + user: defaultUser, + variation: 1, + value: 'b', + default: 'c', + trackEvents: true, + reason: { kind: 'FALLTHROUGH', inExperiment: true }, + }); + }); + + it('does not force tracking when an evaluation is in the untracked portion of an experiment rollout', async () => { + var flag = { + key: 'flagkey', + version: 1, + on: true, + targets: [], + rules: [], + fallthrough: { + rollout: { + kind: 'experiment', + variations: [ + { + weight: 100000, + variation: 1, + untracked: true, + }, + ], + }, + }, + variations: ['a', 'b'], + }; + var client = stubs.createClient({ eventProcessor: eventProcessor }, { flagkey: flag }); + await client.waitForInitialization(); + debugger; + await client.variation(flag.key, defaultUser, 'c'); + + expect(eventProcessor.events).toHaveLength(1); + var e = eventProcessor.events[0]; + expect(e).toEqual({ + kind: 'feature', + creationDate: e.creationDate, + key: 'flagkey', + version: 1, + user: defaultUser, + variation: 1, + value: 'b', + default: 'c', }); }); diff --git a/test/evaluate_flag-test.js b/test/evaluate_flag-test.js index 06220ab..7b43d8e 100644 --- a/test/evaluate_flag-test.js +++ b/test/evaluate_flag-test.js @@ -754,6 +754,58 @@ describe('rollout', () => { done(); }); }); + + describe('with seed', () => { + const seed = 61; + const flagKey = 'flagkey'; + const salt = 'salt'; + const rollout = { + kind: 'experiment', + seed, + variations: [ + { variation: 0, weight: 10000 }, + { variation: 1, weight: 20000 }, + { variation: 0, weight: 70000, untracked: true }, + ], + }; + const flag = { + key: flagKey, + salt: salt, + on: true, + fallthrough: { rollout: rollout }, + variations: [null, null, null], + }; + + it('buckets user into first variant of the experiment', done => { + var user = { key: 'userKeyA' }; + evaluate.evaluate(flag, user, featureStore, eventFactory, (err, detail) => { + expect(err).toEqual(null); + expect(detail.variationIndex).toEqual(0); + expect(detail.reason.inExperiment).toBe(true); + done(); + }); + }); + + it('uses seed to bucket user into second variant of the experiment', done => { + var user = { key: 'userKeyB' }; + evaluate.evaluate(flag, user, featureStore, eventFactory, (err, detail) => { + expect(err).toEqual(null); + expect(detail.variationIndex).toEqual(1); + expect(detail.reason.inExperiment).toBe(true); + done(); + }); + }); + + it('buckets user outside of the experiment', done => { + var user = { key: 'userKeyC' }; + evaluate.evaluate(flag, user, featureStore, eventFactory, (err, detail) => { + expect(err).toEqual(null); + expect(detail.variationIndex).toEqual(0); + expect(detail.reason.inExperiment).toBe(undefined); + done(); + }); + }); + }); }); describe('bucketUser', () => { @@ -795,4 +847,38 @@ describe('bucketUser', () => { var bucket = evaluate.bucketUser(user, 'hashKey', 'floatAttr', 'saltyA'); expect(bucket).toBe(0); }); + + describe('when seed is present', () => { + const seed = 61; + it('gets expected bucket values for specific keys', () => { + var user = { key: 'userKeyA' }; + var bucket = evaluate.bucketUser(user, 'hashKey', 'key', 'saltyA', seed); + expect(bucket).toBeCloseTo(0.09801207, 7); + + user = { key: 'userKeyB' }; + bucket = evaluate.bucketUser(user, 'hashKey', 'key', 'saltyA', seed); + expect(bucket).toBeCloseTo(0.14483777, 7); + + user = { key: 'userKeyC' }; + bucket = evaluate.bucketUser(user, 'hashKey', 'key', 'saltyA', seed); + expect(bucket).toBeCloseTo(0.9242641, 7); + }); + + it('should not generate a different bucket when hashKey or salt are changed', () => { + let user = { key: 'userKeyA' }; + let bucket = evaluate.bucketUser(user, 'hashKey', 'key', 'saltyA', seed); + let bucketDifferentHashKey = evaluate.bucketUser(user, 'otherHashKey', 'key', 'saltyA', seed); + let bucketDifferentSalt = evaluate.bucketUser(user, 'hashKey', 'key', 'otherSaltyA', seed); + + expect(bucketDifferentHashKey).toBeCloseTo(bucket, 7); + expect(bucketDifferentSalt).toBeCloseTo(bucket, 7); + }); + + it('should generate a new bucket if the seed changes', () => { + const otherSeed = 60; + var user = { key: 'userKeyA' }; + var bucket = evaluate.bucketUser(user, 'hashKey', 'key', 'saltyA', otherSeed); + expect(bucket).toBeCloseTo(0.7008816, 7); + }); + }); });