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

lib/common_exports.ts

@@ -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';

lib/core/audience_evaluator/index.tests.js

@@ -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 || {})}),
@@ -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() {
@@ -170,7 +182,6 @@ describe('lib/core/audience_evaluator', function() {
 
           beforeEach(function() {
             sandbox.stub(conditionTreeEvaluator, 'evaluate');
-            sandbox.stub(customAttributeConditionEvaluator, 'evaluate');
           });
 
           afterEach(function() {
@@ -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() {
@@ -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'
+            )
           });
         });
       });

lib/core/audience_evaluator/index.ts

@@ -13,9 +13,6 @@
  * 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,
 } from '../../utils/enums';
@@ -25,11 +22,13 @@ import * as odpSegmentsConditionEvaluator from './odp_segment_condition_evaluato
 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';
 
 export class AudienceEvaluator {
+  private logger?: LoggerFacade;
+
   private typeToEvaluatorMap: {
     [key: string]: {
       [key: string]: (condition: Condition, user: OptimizelyUserContext) => boolean | null
@@ -43,11 +42,12 @@ export class AudienceEvaluator {
    *                                                   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,
-      custom_attribute: customAttributeConditionEvaluator,
-      third_party_dimension: odpSegmentsConditionEvaluator,
+      custom_attribute: customAttributeConditionEvaluator.getEvaluator(this.logger),
+      third_party_dimension: odpSegmentsConditionEvaluator.getEvaluator(this.logger),
     };
   }
 
@@ -77,16 +77,15 @@ export class AudienceEvaluator {
     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;
@@ -105,15 +104,14 @@ export class AudienceEvaluator {
   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) {
-      logger.log(
-        LOG_LEVEL.ERROR,
-        CONDITION_EVALUATOR_ERROR, MODULE_NAME, condition.type, err.message
+      this.logger?.error(
+        CONDITION_EVALUATOR_ERROR, condition.type, err.message
       );
     }
 
@@ -123,6 +121,6 @@ export class AudienceEvaluator {
 
 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);
 };

lib/core/audience_evaluator/odp_segment_condition_evaluator/index.tests.js

@@ -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';
 
@@ -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));
   });
 });