From a155fd2dd3756a363ad9e142af8f1d0fefd67c7c Mon Sep 17 00:00:00 2001 From: tore-statsig <74584483+tore-statsig@users.noreply.github.com> Date: Wed, 8 Sep 2021 15:43:43 -0700 Subject: [PATCH] Do not log exposures when fetching from http API (#92) --- src/__tests__/RulesetsEvalConsistency.test.js | 18 +++++--- src/__tests__/index.test.js | 40 ++++------------ src/index.js | 46 ++++++++----------- 3 files changed, 40 insertions(+), 64 deletions(-) diff --git a/src/__tests__/RulesetsEvalConsistency.test.js b/src/__tests__/RulesetsEvalConsistency.test.js index 7b8d430..564c6b1 100644 --- a/src/__tests__/RulesetsEvalConsistency.test.js +++ b/src/__tests__/RulesetsEvalConsistency.test.js @@ -27,10 +27,12 @@ if (secret) { 'https://us-west-2.api.statsig.com/v1', 'https://us-east-2.api.statsig.com/v1', 'https://ap-south-1.api.statsig.com/v1', - 'https://latest.api.statsig.com/v1' - ].map(url => test(`server and SDK evaluates gates to the same results on ${url}`, async () => { - await _validateServerSDKConsistency(url); - })); + 'https://latest.api.statsig.com/v1', + ].map((url) => + test(`server and SDK evaluates gates to the same results on ${url}`, async () => { + await _validateServerSDKConsistency(url); + }), + ); }); } else { describe('', () => { @@ -62,8 +64,8 @@ async function _validateServerSDKConsistency(api) { expect.assertions(totalChecks); const statsig = require('../index'); - await statsig.initialize(secret, {api: api}); - + await statsig.initialize(secret, { api: api }); + const promises = testData.map(async (data) => { const user = data.user; const gates = data.feature_gates; @@ -71,7 +73,9 @@ async function _validateServerSDKConsistency(api) { for (const name in gates) { const sdkValue = await statsig.checkGate(user, name); if (sdkValue !== gates[name]) { - console.log(`Test failed for gate ${name}. Server got ${gates[name]}, SDK got ${sdkValue}`); + console.log( + `Test failed for gate ${name}. Server got ${gates[name]}, SDK got ${sdkValue}`, + ); } expect(sdkValue).toEqual(gates[name]); } diff --git a/src/__tests__/index.test.js b/src/__tests__/index.test.js index 6c0c388..936790e 100644 --- a/src/__tests__/index.test.js +++ b/src/__tests__/index.test.js @@ -239,16 +239,13 @@ describe('Verify behavior of top level index functions', () => { }); }); - test('Verify when Evaluator fails, checkGate() returns correct value and logs an exposure correctly from server', async () => { - expect.assertions(3); + test('Verify when Evaluator fails, checkGate() returns correct value and does not lot an exposure', async () => { + expect.assertions(2); const statsig = require('../index'); const Evaluator = require('../Evaluator'); jest.spyOn(Evaluator, 'checkGate').mockImplementation((user, gateName) => { - if (gateName === 'gate_pass') - return { value: true, rule_id: 'rule_id_pass' }; - if (gateName === 'gate_server') return FETCH_FROM_SERVER; - return { value: false, rule_id: 'rule_id_fail' }; + return FETCH_FROM_SERVER; }); await statsig.initialize(secretKey); @@ -256,21 +253,11 @@ describe('Verify behavior of top level index functions', () => { let gateName = 'gate_server'; const spy = jest.spyOn(statsig._logger, 'log'); - const gateExposure = new LogEvent('statsig::gate_exposure'); - gateExposure.setUser({ - userID: 123, - }); - gateExposure.setMetadata({ - gate: gateName, - gateValue: String(true), - ruleID: 'rule_id_gate_server', - }); await expect(statsig.checkGate(user, gateName)).resolves.toStrictEqual( true, ); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith(gateExposure); + expect(spy).toHaveBeenCalledTimes(0); }); test('Verify Evaluator returns correct value for checkGate() and logs an exposure correctly', async () => { @@ -346,8 +333,8 @@ describe('Verify behavior of top level index functions', () => { expect(spy).toHaveBeenCalledWith(gateExposure); }); - test('Verify when Evaluator fails to evaluate, getConfig() and getExperiment() return correct value and logs an exposure correctly after fetching from server', async () => { - expect.assertions(6); + test('Verify when Evaluator fails to evaluate, getConfig() and getExperiment() return correct value and do not log exposures', async () => { + expect.assertions(5); const statsig = require('../index'); const Evaluator = require('../Evaluator'); @@ -361,14 +348,6 @@ describe('Verify behavior of top level index functions', () => { let configName = 'config_server'; const spy = jest.spyOn(statsig._logger, 'log'); - const configExposure = new LogEvent('statsig::config_exposure'); - configExposure.setUser({ - userID: 123, - }); - configExposure.setMetadata({ - config: configName, - ruleID: 'rule_id_config_server', - }); await statsig.getConfig(user, configName).then((data) => { expect(data.getValue('number')).toStrictEqual(123); @@ -380,8 +359,7 @@ describe('Verify behavior of top level index functions', () => { expect(data.getValue('string')).toStrictEqual('123'); }); - expect(spy).toHaveBeenCalledTimes(2); - expect(spy).toHaveBeenCalledWith(configExposure); + expect(spy).toHaveBeenCalledTimes(0); }); test('Verify when Evaluator evaluates successfully, getConfig() and getExperiment() return correct value and logs an exposure', async () => { @@ -494,12 +472,12 @@ describe('Verify behavior of top level index functions', () => { statsig.logEventObject({ eventName: 'event', time: 123, - user: {userID: '123', privateAttributes: { secret: 'do not log' }} + user: { userID: '123', privateAttributes: { secret: 'do not log' } }, }); const logEvent = new LogEvent('event'); logEvent.setMetadata(null); - logEvent.setUser({userID: '123'}); + logEvent.setUser({ userID: '123' }); logEvent.setValue(null); logEvent.setTime(123); expect(spy).toBeCalledWith(logEvent); diff --git a/src/index.js b/src/index.js index 1af35a8..5fa77d8 100644 --- a/src/index.js +++ b/src/index.js @@ -82,16 +82,9 @@ const statsig = { ); } user = normalizeUser(user); - return this._getGateValue(user, gateName) - .then((gate) => { - const value = gate?.value ?? false; - statsig._logger.logGateExposure(user, gateName, value, gate.rule_id); - return Promise.resolve(value); - }) - .catch(() => { - statsig._logger.logGateExposure(user, gateName, false); - return Promise.resolve(false); - }); + return this._getGateValue(user, gateName).then((gate) => { + return gate.value; + }); }, /** @@ -118,18 +111,7 @@ const statsig = { } user = normalizeUser(user); - return this._getConfigValue(user, configName) - .then((config) => { - if (config == null) { - config = new DynamicConfig(configName); - } - statsig._logger.logConfigExposure(user, configName, config.getRuleID()); - return Promise.resolve(config); - }) - .catch(() => { - statsig._logger.logConfigExposure(user, configName); - return Promise.resolve(new DynamicConfig(configName)); - }); + return this._getConfigValue(user, configName); }, /** @@ -242,8 +224,16 @@ const statsig = { }, _getGateValue(user, gateName) { - const ret = Evaluator.checkGate(user, gateName); + let ret = Evaluator.checkGate(user, gateName); + if (ret == null) { + ret = { + value: false, + rule_id: '', + }; + } if (ret !== FETCH_FROM_SERVER) { + const value = ret.value; + statsig._logger.logGateExposure(user, gateName, value, ret.rule_id); return Promise.resolve(ret); } @@ -264,9 +254,13 @@ const statsig = { }, _getConfigValue(user, configName) { - const ret = Evaluator.getConfig(user, configName); - if (ret !== FETCH_FROM_SERVER) { - return Promise.resolve(ret); + let config = Evaluator.getConfig(user, configName); + if (config == null) { + config = new DynamicConfig(configName); + } + if (config !== FETCH_FROM_SERVER) { + statsig._logger.logConfigExposure(user, configName, config.getRuleID()); + return Promise.resolve(config); } return fetcher