Merge pull request #6 from edx/robrap/ARCH-687-logging-interface

ARCH-687: feat: provide wrapping interface for logging service

.gitignore

@@ -1,3 +1,4 @@
+.idea
 coverage
 dist
 node_modules

README.rst

@@ -12,26 +12,38 @@ To install frontend-logging into your project::
 
     npm i --save @edx/frontend-logging
 
-To use the logging service, use::
+To configure the logging service::
 
-    import LoggingService from '@edx/frontend-logging';
+    import { configureLoggingService, NewRelicLoggingService } from '@edx/frontend-logging';
 
-    LoggingService.logAPIErrorResponse(e);
+    // configure with any concrete implementation that implements the expected interface
+    configureLoggingService(NewRelicLoggingService);
 
-New Relic Browser and Insights
-------------------------------
+To use the configured logging service::
+
+    import { logAPIErrorResponse, logInfo, logError } from '@edx/frontend-logging';
+};
+
+    logInfo(message);
+    logAPIErrorResponse(e);  // handles errors or axios error responses
+    logError(e);
+
+NewRelicLoggingService
+----------------------
+
+The NewRelicLoggingService is a concrete implementation of the logging service interface that sends messages to NewRelic that can be seen in NewRelic Browser and NewRelic Insights. When in development mode, all messages will instead be sent to the console.
 
 When you use ``logError`` or ``logAPIErrorResponse``, your errors will appear under "JS errors" for your Browser application.
 
-Additionally, when you use `logAPIErrorResponse`, you get some additional custom metrics available you can use in a New Relic Insights query like the following::
+Additionally, when you use `logAPIErrorResponse`, you get some additional custom metrics for Axios error responses. To see those details, you can use a New Relic Insights query like the following::
 
     SELECT * from JavaScriptError WHERE errorStatus is not null SINCE 10 days ago
 
 When using ``logInfo``, these only appear in New Relic Insights when querying for page actions as follows::
 
     SELECT * from PageAction WHERE actionName = 'INFO' SINCE 1 hour ago
 
-You can also add your own custom metrics, or see the code to find other standard custom attributes.
+You can also add your own custom metrics as an additional argument, or see the code to find other standard custom attributes.
 
 
 .. |Build Status| image:: https://api.travis-ci.org/edx/frontend-logging.svg?branch=master

src/LoggingService.js → src/NewRelicLoggingService.js

@@ -16,7 +16,7 @@ function fixErrorLength(error) {
   return error;
 }
 
-class LoggingService {
+class NewRelicLoggingService {
   static logInfo(message) {
     if (process.env.NODE_ENV === 'development') {
       console.log(message); // eslint-disable-line
@@ -74,4 +74,4 @@ class LoggingService {
   }
 }
 
-export default LoggingService;
+export default NewRelicLoggingService;

src/LoggingService.test.js → src/NewRelicLoggingService.test.js

@@ -1,5 +1,5 @@
 import MAX_ERROR_LENGTH from './constants';
-import LoggingService from './LoggingService';
+import NewRelicLoggingService from './NewRelicLoggingService';
 
 global.newrelic = {
   addPageAction: jest.fn(),
@@ -13,7 +13,7 @@ describe('logInfo', () => {
 
   it('calls New Relic client to log message if the client is available', () => {
     const message = 'Test log';
-    LoggingService.logInfo(message);
+    NewRelicLoggingService.logInfo(message);
     expect(global.newrelic.addPageAction).toHaveBeenCalledWith('INFO', { message });
   });
 });
@@ -25,20 +25,20 @@ describe('logError', () => {
 
   it('calls New Relic client to log error if the client is available', () => {
     const error = new Error('Failed!');
-    LoggingService.logError(error);
+    NewRelicLoggingService.logError(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(error, undefined);
   });
 
   it('calls New Relic client to log error if the client is available', () => {
     const error = new Error('Failed!');
-    LoggingService.logError(error);
+    NewRelicLoggingService.logError(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(error, undefined);
   });
 
   it('calls New Relic client with truncated error string', () => {
     const error = new Array(MAX_ERROR_LENGTH + 500 + 1).join('0');
     const expectedError = new Array(MAX_ERROR_LENGTH + 1).join('0');
-    LoggingService.logError(error);
+    NewRelicLoggingService.logError(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, undefined);
   });
 
@@ -49,7 +49,7 @@ describe('logError', () => {
     const expectedError = {
       message: new Array(MAX_ERROR_LENGTH + 1).join('0'),
     };
-    LoggingService.logError(error);
+    NewRelicLoggingService.logError(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, undefined);
   });
 });
@@ -79,7 +79,7 @@ describe('logAPIErrorResponse', () => {
       errorUrl: error.request.responseURL,
       errorData: error.request.responseText,
     };
-    LoggingService.logAPIErrorResponse(error);
+    NewRelicLoggingService.logAPIErrorResponse(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes);
   });
 
@@ -104,7 +104,7 @@ describe('logAPIErrorResponse', () => {
       errorData: JSON.stringify(error.response.data),
       test: 'custom',
     };
-    LoggingService.logAPIErrorResponse(error, { test: 'custom' });
+    NewRelicLoggingService.logAPIErrorResponse(error, { test: 'custom' });
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes);
   });
 
@@ -123,7 +123,7 @@ describe('logAPIErrorResponse', () => {
       errorUrl: '',
       errorData: '',
     };
-    LoggingService.logAPIErrorResponse(error);
+    NewRelicLoggingService.logAPIErrorResponse(error);
     expect(global.newrelic.noticeError).toHaveBeenCalledWith(expectedError, expectedAttributes);
   });
 });

src/index.js

@@ -1,3 +1,15 @@
-import LoggingService from './LoggingService';
+import NewRelicLoggingService from './NewRelicLoggingService';
+import {
+  configureLoggingService,
+  logAPIErrorResponse,
+  logInfo,
+  logError,
+} from './logging';
 
-export default LoggingService;
+export {
+  configureLoggingService,
+  logAPIErrorResponse,
+  logInfo,
+  logError,
+  NewRelicLoggingService,
+};

src/logging.js

@@ -0,0 +1,57 @@
+/**
+ * Provides a wrapper for any logging service implementation of the expected
+ * logging service interface.
+ *
+ * This enables shared libraries or applications that want to support multiple
+ * logging service implementations to be coded against these wrapping functions,
+ * and allows any concrete implementation to be injected.
+ */
+
+let loggingService = null;
+
+function ensureLoggingServiceAPI(newLoggingService, functionName) {
+  if (typeof newLoggingService[functionName] !== 'function') {
+    throw Error(`The loggingService API must have a ${functionName} function.`);
+  }
+}
+
+function configureLoggingService(newLoggingService) {
+  if (!newLoggingService) {
+    throw Error('The loggingService is required.');
+  }
+  ensureLoggingServiceAPI(newLoggingService, 'logAPIErrorResponse');
+  ensureLoggingServiceAPI(newLoggingService, 'logInfo');
+  ensureLoggingServiceAPI(newLoggingService, 'logError');
+  loggingService = newLoggingService;
+}
+
+function resetLoggingService() {
+  loggingService = null;
+}
+
+function getLoggingService() {
+  if (!loggingService) {
+    throw Error('You must first configure the loggingService.');
+  }
+  return loggingService;
+}
+
+function logInfo(message) {
+  return getLoggingService().logInfo(message);
+}
+
+function logError(error, customAttributes) {
+  return getLoggingService().logError(error, customAttributes);
+}
+
+function logAPIErrorResponse(error, customAttributes) {
+  return getLoggingService().logAPIErrorResponse(error, customAttributes);
+}
+
+export {
+  configureLoggingService,
+  logAPIErrorResponse,
+  logInfo,
+  logError,
+  resetLoggingService,
+};

src/logging.test.js

@@ -0,0 +1,90 @@
+import NewRelicLoggingService from './NewRelicLoggingService';
+
+import {
+  configureLoggingService,
+  logAPIErrorResponse,
+  logInfo,
+  logError,
+  resetLoggingService,
+} from './logging';
+
+jest.mock('./NewRelicLoggingService');
+
+const arg1 = 'argument one';
+const arg2 = 'argument two';
+
+describe('configureLoggingService', () => {
+  it('fails when loggingService is not supplied', () => {
+    expect(configureLoggingService)
+      .toThrowError(new Error('The loggingService is required.'));
+  });
+
+  it('fails when loggingService has invalid API', () => {
+    expect(() => configureLoggingService({}))
+      .toThrowError(new Error('The loggingService API must have a logAPIErrorResponse function.'));
+  });
+});
+
+describe('configured logging service', () => {
+  beforeEach(() => {
+    // uses NewRelicLoggingService as any example of a concrete implementation
+    configureLoggingService(NewRelicLoggingService);
+  });
+
+  describe('logInfo', () => {
+    it('passes call through to NewRelicLoggingService', () => {
+      const mockStatic = jest.fn();
+      NewRelicLoggingService.logInfo = mockStatic.bind(NewRelicLoggingService);
+
+      logInfo(arg1);
+      expect(mockStatic).toHaveBeenCalledWith(arg1);
+    });
+  });
+
+  describe('logError', () => {
+    it('passes call through to NewRelicLoggingService', () => {
+      const mockStatic = jest.fn();
+      NewRelicLoggingService.logError = mockStatic.bind(NewRelicLoggingService);
+
+      logError(arg1, arg2);
+      expect(mockStatic).toHaveBeenCalledWith(arg1, arg2);
+    });
+  });
+
+  describe('logAPIErrorResponse', () => {
+    it('passes call through to NewRelicLoggingService', () => {
+      const mockStatic = jest.fn();
+      NewRelicLoggingService.logAPIErrorResponse = mockStatic.bind(NewRelicLoggingService);
+
+      logAPIErrorResponse(arg1, arg2);
+      expect(mockStatic).toHaveBeenCalledWith(arg1, arg2);
+    });
+  });
+});
+
+describe('test failures when logging service is not configured', () => {
+  beforeAll(() => {
+    resetLoggingService();
+  });
+
+  describe('logInfo', () => {
+    it('throws an error', () => {
+      expect(() => logInfo(arg1))
+        .toThrowError(new Error('You must first configure the loggingService.'));
+    });
+  });
+
+  describe('logError', () => {
+    it('throws an error', () => {
+      expect(() => logError(arg1, arg2))
+        .toThrowError(new Error('You must first configure the loggingService.'));
+    });
+  });
+
+  describe('logAPIErrorResponse', () => {
+    it('throws an error', () => {
+      expect(() => logAPIErrorResponse(arg1, arg2))
+        .toThrowError(new Error('You must first configure the loggingService.'));
+    });
+  });
+});