Skip to content

Commit

Permalink
Do not log exposures when fetching from http API (#92)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tore-statsig
tore-statsig authored Sep 8, 2021
1 parent 116780f commit a155fd2
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 64 deletions.
18 changes: 11 additions & 7 deletions src/__tests__/RulesetsEvalConsistency.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('', () => {
Expand Down Expand Up @@ -62,16 +64,18 @@ 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;
const configs = data.dynamic_configs;
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]);
}
Expand Down
40 changes: 9 additions & 31 deletions src/__tests__/index.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,38 +239,25 @@ 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);

let user = { userID: 123, privateAttributes: { secret: 'do not log' } };
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 () => {
Expand Down Expand Up @@ -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');
Expand All @@ -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);
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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);
Expand Down
46 changes: 20 additions & 26 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
},

/**
Expand All @@ -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);
},

/**
Expand Down Expand Up @@ -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);
}

Expand All @@ -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
Expand Down

0 comments on commit a155fd2

Please sign in to comment.