Skip to content

[FSSDK-11035] refactor logger and error handler #982

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 26 commits into from
Jan 14, 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
2 changes: 0 additions & 2 deletions lib/common_exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* limitations under the License.
*/

export { LogLevel, LogHandler, getLogger, setLogHandler } from './modules/logging';
export { LOG_LEVEL } from './utils/enums';
export { createLogger } from './plugins/logger';
export { createStaticProjectConfigManager } from './project_config/config_manager_factory';
export { PollingConfigManagerConfig } from './project_config/config_manager_factory';
138 changes: 102 additions & 36 deletions lib/core/audience_evaluator/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,20 @@
import sinon from 'sinon';
import { assert } from 'chai';
import { sprintf } from '../../utils/fns';
import { getLogger } from '../../modules/logging';

import AudienceEvaluator, { createAudienceEvaluator } from './index';
import * as conditionTreeEvaluator from '../condition_tree_evaluator';
import * as customAttributeConditionEvaluator from '../custom_attribute_condition_evaluator';
import { AUDIENCE_EVALUATION_RESULT, EVALUATING_AUDIENCE } from '../../log_messages';
// import { getEvaluator } from '../custom_attribute_condition_evaluator';

var buildLogMessageFromArgs = args => sprintf(args[1], ...args.splice(2));
var mockLogger = getLogger();
var mockLogger = {
debug: () => {},
info: () => {},
warn: () => {},
error: () => {},
}

var getMockUserContext = (attributes, segments) => ({
getAttributes: () => ({ ... (attributes || {})}),
Expand Down Expand Up @@ -82,11 +88,17 @@ describe('lib/core/audience_evaluator', function() {
var audienceEvaluator;

beforeEach(function() {
sinon.stub(mockLogger, 'log');
sinon.stub(mockLogger, 'info');
sinon.stub(mockLogger, 'debug');
sinon.stub(mockLogger, 'warn');
sinon.stub(mockLogger, 'error');
});

afterEach(function() {
mockLogger.log.restore();
mockLogger.info.restore();
mockLogger.debug.restore();
mockLogger.warn.restore();
mockLogger.error.restore();
});

describe('APIs', function() {
Expand Down Expand Up @@ -170,7 +182,6 @@ describe('lib/core/audience_evaluator', function() {

beforeEach(function() {
sandbox.stub(conditionTreeEvaluator, 'evaluate');
sandbox.stub(customAttributeConditionEvaluator, 'evaluate');
});

afterEach(function() {
Expand Down Expand Up @@ -199,26 +210,40 @@ describe('lib/core/audience_evaluator', function() {
conditionTreeEvaluator.evaluate.callsFake(function(conditions, leafEvaluator) {
return leafEvaluator(conditions[1]);
});
customAttributeConditionEvaluator.evaluate.returns(false);

const mockCustomAttributeConditionEvaluator = sinon.stub().returns(false);

sinon.stub(customAttributeConditionEvaluator, 'getEvaluator').returns({
evaluate: mockCustomAttributeConditionEvaluator,
});

const audienceEvaluator = createAudienceEvaluator();

var userAttributes = { device_model: 'android' };
var user = getMockUserContext(userAttributes);
var result = audienceEvaluator.evaluate(['or', '1'], audiencesById, user);
sinon.assert.calledOnce(customAttributeConditionEvaluator.evaluate);
sinon.assert.calledOnce(mockCustomAttributeConditionEvaluator);
sinon.assert.calledWithExactly(
customAttributeConditionEvaluator.evaluate,
mockCustomAttributeConditionEvaluator,
iphoneUserAudience.conditions[1],
user,
);
assert.isFalse(result);

customAttributeConditionEvaluator.getEvaluator.restore();
});
});

describe('Audience evaluation logging', function() {
var sandbox = sinon.sandbox.create();
var mockCustomAttributeConditionEvaluator;

beforeEach(function() {
mockCustomAttributeConditionEvaluator = sinon.stub();
sandbox.stub(conditionTreeEvaluator, 'evaluate');
sandbox.stub(customAttributeConditionEvaluator, 'evaluate');
sandbox.stub(customAttributeConditionEvaluator, 'getEvaluator').returns({
evaluate: mockCustomAttributeConditionEvaluator,
});
});

afterEach(function() {
Expand All @@ -229,69 +254,110 @@ describe('lib/core/audience_evaluator', function() {
conditionTreeEvaluator.evaluate.callsFake(function(conditions, leafEvaluator) {
return leafEvaluator(conditions[1]);
});
customAttributeConditionEvaluator.evaluate.returns(null);

mockCustomAttributeConditionEvaluator.returns(null);
var userAttributes = { device_model: 5.5 };
var user = getMockUserContext(userAttributes);

const audienceEvaluator = createAudienceEvaluator({}, mockLogger);

var result = audienceEvaluator.evaluate(['or', '1'], audiencesById, user);
sinon.assert.calledOnce(customAttributeConditionEvaluator.evaluate);

sinon.assert.calledOnce(mockCustomAttributeConditionEvaluator);
sinon.assert.calledWithExactly(
customAttributeConditionEvaluator.evaluate,
mockCustomAttributeConditionEvaluator,
iphoneUserAudience.conditions[1],
user
);
assert.isFalse(result);
assert.strictEqual(2, mockLogger.log.callCount);
assert.strictEqual(
buildLogMessageFromArgs(mockLogger.log.args[0]),
'AUDIENCE_EVALUATOR: Starting to evaluate audience "1" with conditions: ["and",{"name":"device_model","value":"iphone","type":"custom_attribute"}].'
);
assert.strictEqual(buildLogMessageFromArgs(mockLogger.log.args[1]), 'AUDIENCE_EVALUATOR: Audience "1" evaluated to UNKNOWN.');
assert.strictEqual(2, mockLogger.debug.callCount);

sinon.assert.calledWithExactly(
mockLogger.debug,
EVALUATING_AUDIENCE,
'1',
JSON.stringify(['and', iphoneUserAudience.conditions[1]])
)

sinon.assert.calledWithExactly(
mockLogger.debug,
AUDIENCE_EVALUATION_RESULT,
'1',
'UNKNOWN'
)
});

it('logs correctly when conditionTreeEvaluator.evaluate returns true', function() {
conditionTreeEvaluator.evaluate.callsFake(function(conditions, leafEvaluator) {
return leafEvaluator(conditions[1]);
});
customAttributeConditionEvaluator.evaluate.returns(true);

mockCustomAttributeConditionEvaluator.returns(true);

var userAttributes = { device_model: 'iphone' };
var user = getMockUserContext(userAttributes);

const audienceEvaluator = createAudienceEvaluator({}, mockLogger);

var result = audienceEvaluator.evaluate(['or', '1'], audiencesById, user);
sinon.assert.calledOnce(customAttributeConditionEvaluator.evaluate);
sinon.assert.calledOnce(mockCustomAttributeConditionEvaluator);
sinon.assert.calledWithExactly(
customAttributeConditionEvaluator.evaluate,
mockCustomAttributeConditionEvaluator,
iphoneUserAudience.conditions[1],
user,
);
assert.isTrue(result);
assert.strictEqual(2, mockLogger.log.callCount);
assert.strictEqual(
buildLogMessageFromArgs(mockLogger.log.args[0]),
'AUDIENCE_EVALUATOR: Starting to evaluate audience "1" with conditions: ["and",{"name":"device_model","value":"iphone","type":"custom_attribute"}].'
);
assert.strictEqual(buildLogMessageFromArgs(mockLogger.log.args[1]), 'AUDIENCE_EVALUATOR: Audience "1" evaluated to TRUE.');
assert.strictEqual(2, mockLogger.debug.callCount);
sinon.assert.calledWithExactly(
mockLogger.debug,
EVALUATING_AUDIENCE,
'1',
JSON.stringify(['and', iphoneUserAudience.conditions[1]])
)

sinon.assert.calledWithExactly(
mockLogger.debug,
AUDIENCE_EVALUATION_RESULT,
'1',
'TRUE'
)
});

it('logs correctly when conditionTreeEvaluator.evaluate returns false', function() {
conditionTreeEvaluator.evaluate.callsFake(function(conditions, leafEvaluator) {
return leafEvaluator(conditions[1]);
});
customAttributeConditionEvaluator.evaluate.returns(false);

mockCustomAttributeConditionEvaluator.returns(false);

var userAttributes = { device_model: 'android' };
var user = getMockUserContext(userAttributes);

const audienceEvaluator = createAudienceEvaluator({}, mockLogger);

var result = audienceEvaluator.evaluate(['or', '1'], audiencesById, user);
sinon.assert.calledOnce(customAttributeConditionEvaluator.evaluate);
sinon.assert.calledOnce(mockCustomAttributeConditionEvaluator);
sinon.assert.calledWithExactly(
customAttributeConditionEvaluator.evaluate,
mockCustomAttributeConditionEvaluator,
iphoneUserAudience.conditions[1],
user,
);
assert.isFalse(result);
assert.strictEqual(2, mockLogger.log.callCount);
assert.strictEqual(
buildLogMessageFromArgs(mockLogger.log.args[0]),
'AUDIENCE_EVALUATOR: Starting to evaluate audience "1" with conditions: ["and",{"name":"device_model","value":"iphone","type":"custom_attribute"}].'
);
assert.strictEqual(buildLogMessageFromArgs(mockLogger.log.args[1]), 'AUDIENCE_EVALUATOR: Audience "1" evaluated to FALSE.');
assert.strictEqual(2, mockLogger.debug.callCount);

sinon.assert.calledWithExactly(
mockLogger.debug,
EVALUATING_AUDIENCE,
'1',
JSON.stringify(['and', iphoneUserAudience.conditions[1]])
)

sinon.assert.calledWithExactly(
mockLogger.debug,
AUDIENCE_EVALUATION_RESULT,
'1',
'FALSE'
)
});
});
});
Expand Down
32 changes: 15 additions & 17 deletions lib/core/audience_evaluator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { getLogger } from '../../modules/logging';

import fns from '../../utils/fns';
import {
LOG_LEVEL,

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

View workflow job for this annotation

GitHub Actions / lint

'LOG_LEVEL' is defined but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (20)

'LOG_LEVEL' is defined but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (18)

'LOG_LEVEL' is defined but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (22)

'LOG_LEVEL' is defined but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (16)

'LOG_LEVEL' is defined but never used
} from '../../utils/enums';
import * as conditionTreeEvaluator from '../condition_tree_evaluator';
import * as customAttributeConditionEvaluator from '../custom_attribute_condition_evaluator';
Expand All @@ -25,11 +22,13 @@
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';
import { LoggerFacade } from '../../logging/logger';

const logger = getLogger();
const MODULE_NAME = 'AUDIENCE_EVALUATOR';

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

View workflow job for this annotation

GitHub Actions / lint

'MODULE_NAME' is assigned a value but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (20)

'MODULE_NAME' is assigned a value but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (18)

'MODULE_NAME' is assigned a value but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (22)

'MODULE_NAME' is assigned a value but never used

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

View workflow job for this annotation

GitHub Actions / unit_tests (16)

'MODULE_NAME' is assigned a value but never used

export class AudienceEvaluator {
private logger?: LoggerFacade;

private typeToEvaluatorMap: {
[key: string]: {
[key: string]: (condition: Condition, user: OptimizelyUserContext) => boolean | null
Expand All @@ -43,11 +42,12 @@
* Optimizely evaluators cannot be overridden.
* @constructor
*/
constructor(UNSTABLE_conditionEvaluators: unknown) {
constructor(UNSTABLE_conditionEvaluators: unknown, logger?: LoggerFacade) {
this.logger = logger;
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 / lint

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 (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 (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 (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 (16)

Unexpected any. Specify a different type
custom_attribute: customAttributeConditionEvaluator,
third_party_dimension: odpSegmentsConditionEvaluator,
custom_attribute: customAttributeConditionEvaluator.getEvaluator(this.logger),
third_party_dimension: odpSegmentsConditionEvaluator.getEvaluator(this.logger),
};
}

Expand Down Expand Up @@ -77,16 +77,15 @@
const evaluateAudience = (audienceId: string) => {
const audience = audiencesById[audienceId];
if (audience) {
logger.log(
LOG_LEVEL.DEBUG,
EVALUATING_AUDIENCE, MODULE_NAME, audienceId, JSON.stringify(audience.conditions)
this.logger?.debug(
EVALUATING_AUDIENCE, 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, AUDIENCE_EVALUATION_RESULT, MODULE_NAME, audienceId, resultText);
this.logger?.debug(AUDIENCE_EVALUATION_RESULT, audienceId, resultText);
return result;
}
return null;
Expand All @@ -105,15 +104,14 @@
evaluateConditionWithUserAttributes(user: OptimizelyUserContext, condition: Condition): boolean | null {
const evaluator = this.typeToEvaluatorMap[condition.type];
if (!evaluator) {
logger.log(LOG_LEVEL.WARNING, UNKNOWN_CONDITION_TYPE, MODULE_NAME, JSON.stringify(condition));
this.logger?.warn(UNKNOWN_CONDITION_TYPE, JSON.stringify(condition));
return null;
}
try {
return evaluator.evaluate(condition, user);
} catch (err: any) {

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

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 112 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 112 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 112 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 112 in lib/core/audience_evaluator/index.ts

View workflow job for this annotation

GitHub Actions / unit_tests (16)

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

Expand All @@ -123,6 +121,6 @@

export default AudienceEvaluator;

export const createAudienceEvaluator = function(UNSTABLE_conditionEvaluators: unknown): AudienceEvaluator {
return new AudienceEvaluator(UNSTABLE_conditionEvaluators);
export const createAudienceEvaluator = function(UNSTABLE_conditionEvaluators: unknown, logger?: LoggerFacade): AudienceEvaluator {
return new AudienceEvaluator(UNSTABLE_conditionEvaluators, logger);
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { assert } from 'chai';
import { sprintf } from '../../../utils/fns';

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

Expand All @@ -34,38 +33,44 @@ var getMockUserContext = (attributes, segments) => ({
isQualifiedFor: segment => segments.indexOf(segment) > -1
});

var createLogger = () => ({
debug: () => {},
info: () => {},
warn: () => {},
error: () => {},
child: () => createLogger(),
})

describe('lib/core/audience_evaluator/odp_segment_condition_evaluator', function() {
var stubLogHandler;
const mockLogger = createLogger();
const { evaluate } = odpSegmentEvalutor.getEvaluator(mockLogger);

beforeEach(function() {
stubLogHandler = {
log: sinon.stub(),
};
logging.setLogLevel('notset');
logging.setLogHandler(stubLogHandler);
sinon.stub(mockLogger, 'warn');
sinon.stub(mockLogger, 'error');
});

afterEach(function() {
logging.resetLogger();
mockLogger.warn.restore();
mockLogger.error.restore();
});

it('should return true when segment qualifies and known match type is provided', () => {
assert.isTrue(odpSegmentEvalutor.evaluate(odpSegment1Condition, getMockUserContext({}, ['odp-segment-1'])));
assert.isTrue(evaluate(odpSegment1Condition, getMockUserContext({}, ['odp-segment-1'])));
});

it('should return false when segment does not qualify and known match type is provided', () => {
assert.isFalse(odpSegmentEvalutor.evaluate(odpSegment1Condition, getMockUserContext({}, ['odp-segment-2'])));
assert.isFalse(evaluate(odpSegment1Condition, getMockUserContext({}, ['odp-segment-2'])));
})

it('should return null when segment qualifies but unknown match type is provided', () => {
const invalidOdpMatchCondition = {
... odpSegment1Condition,
"match": 'unknown',
};
assert.isNull(odpSegmentEvalutor.evaluate(invalidOdpMatchCondition, getMockUserContext({}, ['odp-segment-1'])));
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(UNKNOWN_MATCH_TYPE, 'ODP_SEGMENT_CONDITION_EVALUATOR', JSON.stringify(invalidOdpMatchCondition)));
assert.isNull(evaluate(invalidOdpMatchCondition, getMockUserContext({}, ['odp-segment-1'])));
sinon.assert.calledOnce(mockLogger.warn);
assert.strictEqual(mockLogger.warn.args[0][0], UNKNOWN_MATCH_TYPE);
assert.strictEqual(mockLogger.warn.args[0][1], JSON.stringify(invalidOdpMatchCondition));
});
});
Loading
Loading