Skip to content

[FSSDK-10935] Refactor log object export #976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions lib/core/audience_evaluator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@
*/
import { getLogger } from '../../modules/logging';

import fns from '../../utils/fns';

Check warning on line 18 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

'fns' is defined but never used

Check warning on line 18 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

'fns' is defined but never used

Check warning on line 18 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

'fns' is defined but never used

Check warning on line 18 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (16)

'fns' is defined but never used

Check warning on line 18 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / lint

'fns' is defined but never used
import {
LOG_LEVEL,
LOG_MESSAGES,
ERROR_MESSAGES,
} from '../../utils/enums';
import * as conditionTreeEvaluator from '../condition_tree_evaluator';
import * as customAttributeConditionEvaluator from '../custom_attribute_condition_evaluator';
import * as odpSegmentsConditionEvaluator from './odp_segment_condition_evaluator';
import { Audience, Condition, OptimizelyUserContext } from '../../shared_types';
import { CONDITION_EVALUATOR_ERROR, UNKNOWN_CONDITION_TYPE } from '../../error_messages';
import { AUDIENCE_EVALUATION_RESULT, EVALUATING_AUDIENCE} from '../../log_messages';

const logger = getLogger();
const MODULE_NAME = 'AUDIENCE_EVALUATOR';
Expand All @@ -45,7 +45,7 @@
*/
constructor(UNSTABLE_conditionEvaluators: unknown) {
this.typeToEvaluatorMap = {
...UNSTABLE_conditionEvaluators as any,

Check warning on line 48 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 48 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type

Check warning on line 48 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 48 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (16)

Unexpected any. Specify a different type

Check warning on line 48 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
custom_attribute: customAttributeConditionEvaluator,
third_party_dimension: odpSegmentsConditionEvaluator,
};
Expand Down Expand Up @@ -79,14 +79,14 @@
if (audience) {
logger.log(
LOG_LEVEL.DEBUG,
LOG_MESSAGES.EVALUATING_AUDIENCE, MODULE_NAME, audienceId, JSON.stringify(audience.conditions)
EVALUATING_AUDIENCE, MODULE_NAME, audienceId, JSON.stringify(audience.conditions)
);
const result = conditionTreeEvaluator.evaluate(
audience.conditions as unknown[] ,
this.evaluateConditionWithUserAttributes.bind(this, user)
);
const resultText = result === null ? 'UNKNOWN' : result.toString().toUpperCase();
logger.log(LOG_LEVEL.DEBUG, LOG_MESSAGES.AUDIENCE_EVALUATION_RESULT, MODULE_NAME, audienceId, resultText);
logger.log(LOG_LEVEL.DEBUG, AUDIENCE_EVALUATION_RESULT, MODULE_NAME, audienceId, resultText);
return result;
}
return null;
Expand All @@ -105,15 +105,15 @@
evaluateConditionWithUserAttributes(user: OptimizelyUserContext, condition: Condition): boolean | null {
const evaluator = this.typeToEvaluatorMap[condition.type];
if (!evaluator) {
logger.log(LOG_LEVEL.WARNING, LOG_MESSAGES.UNKNOWN_CONDITION_TYPE, MODULE_NAME, JSON.stringify(condition));
logger.log(LOG_LEVEL.WARNING, UNKNOWN_CONDITION_TYPE, MODULE_NAME, JSON.stringify(condition));
return null;
}
try {
return evaluator.evaluate(condition, user);
} catch (err: any) {

Check warning on line 113 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 113 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type

Check warning on line 113 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 113 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (16)

Unexpected any. Specify a different type

Check warning on line 113 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
logger.log(
LOG_LEVEL.ERROR,
ERROR_MESSAGES.CONDITION_EVALUATOR_ERROR, MODULE_NAME, condition.type, err.message
CONDITION_EVALUATOR_ERROR, MODULE_NAME, condition.type, err.message
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ import sinon from 'sinon';
import { assert } from 'chai';
import { sprintf } from '../../../utils/fns';

import {
LOG_LEVEL,
LOG_MESSAGES,
} from '../../../utils/enums';
import { LOG_LEVEL } from '../../../utils/enums';
import * as logging from '../../../modules/logging';
import * as odpSegmentEvalutor from './';
import { UNKNOWN_MATCH_TYPE } from '../../../error_messages';

var odpSegment1Condition = {
"value": "odp-segment-1",
Expand Down Expand Up @@ -68,6 +66,6 @@ describe('lib/core/audience_evaluator/odp_segment_condition_evaluator', function
sinon.assert.calledOnce(stubLogHandler.log);
assert.strictEqual(stubLogHandler.log.args[0][0], LOG_LEVEL.WARNING);
var logMessage = stubLogHandler.log.args[0][1];
assert.strictEqual(logMessage, sprintf(LOG_MESSAGES.UNKNOWN_MATCH_TYPE, 'ODP_SEGMENT_CONDITION_EVALUATOR', JSON.stringify(invalidOdpMatchCondition)));
assert.strictEqual(logMessage, sprintf(UNKNOWN_MATCH_TYPE, 'ODP_SEGMENT_CONDITION_EVALUATOR', JSON.stringify(invalidOdpMatchCondition)));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/
import { UNKNOWN_MATCH_TYPE } from '../../../error_messages';
import { getLogger } from '../../../modules/logging';
import { Condition, OptimizelyUserContext } from '../../../shared_types';

import { LOG_MESSAGES } from '../../../utils/enums';

const MODULE_NAME = 'ODP_SEGMENT_CONDITION_EVALUATOR';

const logger = getLogger();
Expand Down Expand Up @@ -45,7 +44,7 @@ EVALUATORS_BY_MATCH_TYPE[QUALIFIED_MATCH_TYPE] = qualifiedEvaluator;
export function evaluate(condition: Condition, user: OptimizelyUserContext): boolean | null {
const conditionMatch = condition.match;
if (typeof conditionMatch !== 'undefined' && MATCH_TYPES.indexOf(conditionMatch) === -1) {
logger.warn(LOG_MESSAGES.UNKNOWN_MATCH_TYPE, MODULE_NAME, JSON.stringify(condition));
logger.warn(UNKNOWN_MATCH_TYPE, MODULE_NAME, JSON.stringify(condition));
return null;
}

Expand Down
43 changes: 23 additions & 20 deletions lib/core/bucketer/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@ import { cloneDeep } from 'lodash';
import { sprintf } from '../../utils/fns';

import * as bucketer from './';
import {
ERROR_MESSAGES,
LOG_MESSAGES,
LOG_LEVEL,
} from '../../utils/enums';
import { LOG_LEVEL } from '../../utils/enums';
import { createLogger } from '../../plugins/logger';
import projectConfig from '../../project_config/project_config';
import { getTestProjectConfig } from '../../tests/test_data';
import { INVALID_BUCKETING_ID, INVALID_GROUP_ID } from '../../error_messages';
import {
USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_NOT_IN_ANY_EXPERIMENT,
USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
} from '.';

var buildLogMessageFromArgs = args => sprintf(args[1], ...args.splice(2));
var testData = getTestProjectConfig();
Expand Down Expand Up @@ -78,7 +81,7 @@ describe('lib/core/bucketer', function () {

var bucketedUser_log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(bucketedUser_log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'ppid1')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'ppid1')
);

var bucketerParamsTest2 = cloneDeep(bucketerParams);
Expand All @@ -88,7 +91,7 @@ describe('lib/core/bucketer', function () {
var notBucketedUser_log1 = buildLogMessageFromArgs(createdLogger.log.args[1]);

expect(notBucketedUser_log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'ppid2')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'ppid2')
);
});
});
Expand Down Expand Up @@ -140,13 +143,13 @@ describe('lib/core/bucketer', function () {

var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
);

var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
expect(log2).to.equal(
sprintf(
LOG_MESSAGES.USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
'BUCKETER',
'testUser',
'groupExperiment1',
Expand All @@ -156,7 +159,7 @@ describe('lib/core/bucketer', function () {

var log3 = buildLogMessageFromArgs(createdLogger.log.args[2]);
expect(log3).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50', 'testUser')
);
});

Expand All @@ -171,12 +174,12 @@ describe('lib/core/bucketer', function () {

var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '5000', 'testUser')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '5000', 'testUser')
);
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
expect(log2).to.equal(
sprintf(
LOG_MESSAGES.USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
'BUCKETER',
'testUser',
'groupExperiment1',
Expand All @@ -196,10 +199,10 @@ describe('lib/core/bucketer', function () {

var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'testUser')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '50000', 'testUser')
);
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
expect(log2).to.equal(sprintf(LOG_MESSAGES.USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
expect(log2).to.equal(sprintf(USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
});

it('should return decision response with variation null when a user is bucketed into traffic space of deleted experiment within a random group', function () {
Expand All @@ -213,10 +216,10 @@ describe('lib/core/bucketer', function () {

var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(log1).to.equal(
sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '9000', 'testUser')
sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '9000', 'testUser')
);
var log2 = buildLogMessageFromArgs(createdLogger.log.args[1]);
expect(log2).to.equal(sprintf(LOG_MESSAGES.USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
expect(log2).to.equal(sprintf(USER_NOT_IN_ANY_EXPERIMENT, 'BUCKETER', 'testUser', '666'));
});

it('should throw an error if group ID is not in the datafile', function () {
Expand All @@ -225,7 +228,7 @@ describe('lib/core/bucketer', function () {

assert.throws(function () {
bucketer.bucket(bucketerParamsWithInvalidGroupId);
}, sprintf(ERROR_MESSAGES.INVALID_GROUP_ID, 'BUCKETER', '6969'));
}, sprintf(INVALID_GROUP_ID, 'BUCKETER', '6969'));
});
});

Expand Down Expand Up @@ -254,7 +257,7 @@ describe('lib/core/bucketer', function () {
sinon.assert.calledOnce(createdLogger.log);

var log1 = buildLogMessageFromArgs(createdLogger.log.args[0]);
expect(log1).to.equal(sprintf(LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '0', 'testUser'));
expect(log1).to.equal(sprintf(USER_ASSIGNED_TO_EXPERIMENT_BUCKET, 'BUCKETER', '0', 'testUser'));
});

it('should return decision response with variation null when a user does not fall into an experiment within an overlapping group', function () {
Expand Down Expand Up @@ -357,8 +360,8 @@ describe('lib/core/bucketer', function () {
bucketer._generateBucketValue(null);
} );
expect([
sprintf(ERROR_MESSAGES.INVALID_BUCKETING_ID, 'BUCKETER', null, "Cannot read property 'length' of null"), // node v14
sprintf(ERROR_MESSAGES.INVALID_BUCKETING_ID, 'BUCKETER', null, "Cannot read properties of null (reading \'length\')") // node v16
sprintf(INVALID_BUCKETING_ID, 'BUCKETER', null, "Cannot read property 'length' of null"), // node v14
sprintf(INVALID_BUCKETING_ID, 'BUCKETER', null, "Cannot read properties of null (reading \'length\')") // node v16
]).contain(response.message);
});
});
Expand Down
39 changes: 21 additions & 18 deletions lib/core/bucketer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@
Group,
} from '../../shared_types';

import {
ERROR_MESSAGES,
LOG_LEVEL,
LOG_MESSAGES,
} from '../../utils/enums';
import { LOG_LEVEL } from '../../utils/enums';
import { INVALID_BUCKETING_ID, INVALID_GROUP_ID } from '../../error_messages';

export const USER_NOT_IN_ANY_EXPERIMENT = '%s: User %s is not in any experiment of group %s.';
export const USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP = '%s: User %s is not in experiment %s of group %s.';
export const USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP = '%s: User %s is in experiment %s of group %s.';
export const USER_ASSIGNED_TO_EXPERIMENT_BUCKET = '%s: Assigned bucket %s to user with bucketing ID %s.';
export const INVALID_VARIATION_ID = '%s: Bucketed into an invalid variation ID. Returning null.';

const HASH_SEED = 1;
const MAX_HASH_VALUE = Math.pow(2, 32);
Expand Down Expand Up @@ -63,7 +66,7 @@
if (groupId) {
const group = bucketerParams.groupIdMap[groupId];
if (!group) {
throw new Error(sprintf(ERROR_MESSAGES.INVALID_GROUP_ID, MODULE_NAME, groupId));
throw new Error(sprintf(INVALID_GROUP_ID, MODULE_NAME, groupId));
}
if (group.policy === RANDOM_POLICY) {
const bucketedExperimentId = bucketUserIntoExperiment(
Expand All @@ -77,13 +80,13 @@
if (bucketedExperimentId === null) {
bucketerParams.logger.log(
LOG_LEVEL.INFO,
LOG_MESSAGES.USER_NOT_IN_ANY_EXPERIMENT,
USER_NOT_IN_ANY_EXPERIMENT,
MODULE_NAME,
bucketerParams.userId,
groupId,
);
decideReasons.push([
LOG_MESSAGES.USER_NOT_IN_ANY_EXPERIMENT,
USER_NOT_IN_ANY_EXPERIMENT,
MODULE_NAME,
bucketerParams.userId,
groupId,
Expand All @@ -98,14 +101,14 @@
if (bucketedExperimentId !== bucketerParams.experimentId) {
bucketerParams.logger.log(
LOG_LEVEL.INFO,
LOG_MESSAGES.USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
MODULE_NAME,
bucketerParams.userId,
bucketerParams.experimentKey,
groupId,
);
decideReasons.push([
LOG_MESSAGES.USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_NOT_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
MODULE_NAME,
bucketerParams.userId,
bucketerParams.experimentKey,
Expand All @@ -120,14 +123,14 @@
// Continue bucketing if user is bucketed into specified experiment
bucketerParams.logger.log(
LOG_LEVEL.INFO,
LOG_MESSAGES.USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
MODULE_NAME,
bucketerParams.userId,
bucketerParams.experimentKey,
groupId,
);
decideReasons.push([
LOG_MESSAGES.USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP,
MODULE_NAME,
bucketerParams.userId,
bucketerParams.experimentKey,
Expand All @@ -140,13 +143,13 @@

bucketerParams.logger.log(
LOG_LEVEL.DEBUG,
LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
MODULE_NAME,
bucketValue,
bucketerParams.userId,
);
decideReasons.push([
LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
MODULE_NAME,
bucketValue,
bucketerParams.userId,
Expand All @@ -156,8 +159,8 @@
if (entityId !== null) {
if (!bucketerParams.variationIdMap[entityId]) {
if (entityId) {
bucketerParams.logger.log(LOG_LEVEL.WARNING, LOG_MESSAGES.INVALID_VARIATION_ID, MODULE_NAME);
decideReasons.push([LOG_MESSAGES.INVALID_VARIATION_ID, MODULE_NAME]);
bucketerParams.logger.log(LOG_LEVEL.WARNING, INVALID_VARIATION_ID, MODULE_NAME);
decideReasons.push([INVALID_VARIATION_ID, MODULE_NAME]);
}
return {
result: null,
Expand Down Expand Up @@ -190,7 +193,7 @@
const bucketValue = _generateBucketValue(bucketingKey);
logger.log(
LOG_LEVEL.DEBUG,
LOG_MESSAGES.USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
USER_ASSIGNED_TO_EXPERIMENT_BUCKET,
MODULE_NAME,
bucketValue,
userId,
Expand Down Expand Up @@ -234,8 +237,8 @@
const hashValue = murmurhash.v3(bucketingKey, HASH_SEED);
const ratio = hashValue / MAX_HASH_VALUE;
return Math.floor(ratio * MAX_TRAFFIC_VALUE);
} catch (ex: any) {

Check warning on line 240 in lib/core/bucketer/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 240 in lib/core/bucketer/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type

Check warning on line 240 in lib/core/bucketer/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 240 in lib/core/bucketer/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (16)

Unexpected any. Specify a different type

Check warning on line 240 in lib/core/bucketer/index.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
throw new Error(sprintf(ERROR_MESSAGES.INVALID_BUCKETING_ID, MODULE_NAME, bucketingKey, ex.message));
throw new Error(sprintf(INVALID_BUCKETING_ID, MODULE_NAME, bucketingKey, ex.message));
}
};

Expand Down
Loading
Loading