feat: Enhance basic logger destination support. (#650)

kinyoklion · web-flow · commit 21670c4acd62 · 2024-11-01T08:47:14.000-07:00
This adds support to the common basic logger that makes it more suitable
for client SDKs.

Instead of a single destination for all logs you can now control the
destination per log level. This allow you to use all the default
formatting, log level handling, and log tagging while also getting logs
that are nice for use in a browser.
diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts
@@ -1,9 +1,9 @@
 import {
   AutoEnvAttributes,
   base64UrlEncode,
+  BasicLogger,
   LDClient as CommonClient,
   Configuration,
-  createSafeLogger,
   Encoding,
   FlagManager,
   internal,
@@ -98,15 +98,18 @@ export class BrowserClient extends LDClientImpl implements LDClient {
     // Overrides the default logger from the common implementation.
     const logger =
       customLogger ??
-      createSafeLogger({
-        // eslint-disable-next-line no-console
-        debug: debug ? console.debug : () => {},
-        // eslint-disable-next-line no-console
-        info: console.info,
-        // eslint-disable-next-line no-console
-        warn: console.warn,
-        // eslint-disable-next-line no-console
-        error: console.error,
+      new BasicLogger({
+        destination: {
+          // eslint-disable-next-line no-console
+          debug: console.debug,
+          // eslint-disable-next-line no-console
+          info: console.info,
+          // eslint-disable-next-line no-console
+          warn: console.warn,
+          // eslint-disable-next-line no-console
+          error: console.error,
+        },
+        level: debug ? 'debug' : 'info',
       });
 
     // TODO: Use the already-configured baseUri from the SDK config. SDK-560
diff --git a/packages/sdk/browser/src/index.ts b/packages/sdk/browser/src/index.ts
@@ -12,6 +12,8 @@
  */
 import {
   AutoEnvAttributes,
+  BasicLogger,
+  BasicLoggerOptions,
   EvaluationSeriesContext,
   EvaluationSeriesData,
   Hook,
@@ -84,3 +86,54 @@ export function initialize(clientSideId: string, options?: LDOptions): LDClient
   // AutoEnvAttributes are not supported yet in the browser SDK.
   return new BrowserClient(clientSideId, AutoEnvAttributes.Disabled, options);
 }
+
+/**
+ * Provides a simple {@link LDLogger} implementation.
+ *
+ * This logging implementation uses a simple format that includes only the log level
+ * and the message text. By default the output is written to `console.error`.
+ *
+ * To use the logger created by this function, put it into {@link LDOptions.logger}. If
+ * you do not set {@link LDOptions.logger} to anything, the SDK uses a default logger
+ * that will log "info" level and higher priorty messages and it will log messages to
+ * console.info, console.warn, and console.error.
+ *
+ * @param options Configuration for the logger. If no options are specified, the
+ *   logger uses `{ level: 'info' }`.
+ *
+ * @example
+ * This example shows how to use `basicLogger` in your SDK options to enable console
+ * logging only at `warn` and `error` levels.
+ * ```javascript
+ *   const ldOptions = {
+ *     logger: basicLogger({ level: 'warn' }),
+ *   };
+ * ```
+ *
+ * @example
+ * This example shows how to use `basicLogger` in your SDK options to cause all
+ * log output to go to `console.log`
+ * ```javascript
+ *   const ldOptions = {
+ *     logger: basicLogger({ destination: console.log }),
+ *   };
+ * ```
+ *
+ *  * @example
+ * The configuration also allows you to control the destination for each log level.
+ * ```javascript
+ *   const ldOptions = {
+ *     logger: basicLogger({
+ *       destination: {
+ *         debug: console.debug,
+ *         info: console.info,
+ *         warn: console.warn,
+ *         error:console.error
+ *       }
+ *     }),
+ *   };
+ * ```
+ */
+export function basicLogger(options: BasicLoggerOptions): LDLogger {
+  return new BasicLogger(options);
+}
diff --git a/packages/shared/common/__tests__/logging/BasicLogger.test.ts b/packages/shared/common/__tests__/logging/BasicLogger.test.ts
@@ -2,6 +2,10 @@ import { BasicLogger, LDLogLevel } from '../../src';
 
 const spy = jest.spyOn(console, 'error').mockImplementation(() => {});
 
+beforeEach(() => {
+  jest.clearAllMocks();
+});
+
 describe.each<[LDLogLevel, string[]]>([
   [
     'debug',
@@ -64,10 +68,6 @@ describe('given a logger with a custom name', () => {
 describe('given a default logger', () => {
   const logger = new BasicLogger({});
 
-  beforeEach(() => {
-    jest.clearAllMocks();
-  });
-
   it('logs to the console', () => {
     logger.warn('potato', 'bacon');
     expect(spy).toHaveBeenCalledWith('potato', 'bacon');
@@ -81,10 +81,6 @@ describe('given a logger with a destination that throws', () => {
     },
   });
 
-  beforeEach(() => {
-    jest.clearAllMocks();
-  });
-
   it('logs to the console instead of throwing', () => {
     logger.error('a');
     expect(spy).toHaveBeenCalledWith('error: [LaunchDarkly] a');
@@ -94,10 +90,6 @@ describe('given a logger with a destination that throws', () => {
 describe('given a logger with a formatter that throws', () => {
   const strings: string[] = [];
 
-  beforeEach(() => {
-    jest.clearAllMocks();
-  });
-
   const logger = new BasicLogger({
     destination: (...args: any) => {
       strings.push(args.join(' '));
@@ -112,3 +104,102 @@ describe('given a logger with a formatter that throws', () => {
     expect(spy).toHaveBeenCalledTimes(0);
   });
 });
+
+it('dispatches logs correctly with multiple destinations', () => {
+  const debug = jest.fn();
+  const info = jest.fn();
+  const warn = jest.fn();
+  const error = jest.fn();
+
+  const logger = new BasicLogger({
+    destination: {
+      debug,
+      info,
+      warn,
+      error,
+    },
+    level: 'debug',
+  });
+
+  logger.debug('toDebug');
+  logger.info('toInfo');
+  logger.warn('toWarn');
+  logger.error('toError');
+
+  expect(debug).toHaveBeenCalledTimes(1);
+  expect(debug).toHaveBeenCalledWith('debug: [LaunchDarkly] toDebug');
+
+  expect(info).toHaveBeenCalledTimes(1);
+  expect(info).toHaveBeenCalledWith('info: [LaunchDarkly] toInfo');
+
+  expect(warn).toHaveBeenCalledTimes(1);
+  expect(warn).toHaveBeenCalledWith('warn: [LaunchDarkly] toWarn');
+
+  expect(error).toHaveBeenCalledTimes(1);
+  expect(error).toHaveBeenCalledWith('error: [LaunchDarkly] toError');
+});
+
+it('handles destinations which throw', () => {
+  const debug = jest.fn(() => {
+    throw new Error('bad');
+  });
+  const info = jest.fn(() => {
+    throw new Error('bad');
+  });
+  const warn = jest.fn(() => {
+    throw new Error('bad');
+  });
+  const error = jest.fn(() => {
+    throw new Error('bad');
+  });
+
+  const logger = new BasicLogger({
+    destination: {
+      debug,
+      info,
+      warn,
+      error,
+    },
+    level: 'debug',
+  });
+
+  logger.debug('toDebug');
+  logger.info('toInfo');
+  logger.warn('toWarn');
+  logger.error('toError');
+
+  expect(spy).toHaveBeenCalledTimes(4);
+  expect(spy).toHaveBeenCalledWith('debug: [LaunchDarkly] toDebug');
+  expect(spy).toHaveBeenCalledWith('info: [LaunchDarkly] toInfo');
+  expect(spy).toHaveBeenCalledWith('warn: [LaunchDarkly] toWarn');
+  expect(spy).toHaveBeenCalledWith('error: [LaunchDarkly] toError');
+});
+
+it('handles destinations which are not defined', () => {
+  const debug = jest.fn();
+  const info = jest.fn();
+  const logger = new BasicLogger({
+    // @ts-ignore
+    destination: {
+      debug,
+      info,
+    },
+    level: 'debug',
+  });
+
+  logger.debug('toDebug');
+  logger.info('toInfo');
+  logger.warn('toWarn');
+  logger.error('toError');
+
+  expect(debug).toHaveBeenCalledTimes(1);
+  expect(debug).toHaveBeenCalledWith('debug: [LaunchDarkly] toDebug');
+
+  expect(info).toHaveBeenCalledTimes(1);
+  expect(info).toHaveBeenCalledWith('info: [LaunchDarkly] toInfo');
+
+  expect(spy).toHaveBeenCalledTimes(2);
+
+  expect(spy).toHaveBeenCalledWith('toWarn');
+  expect(spy).toHaveBeenCalledWith('toError');
+});
diff --git a/packages/shared/common/src/api/logging/BasicLoggerOptions.ts b/packages/shared/common/src/api/logging/BasicLoggerOptions.ts
@@ -21,18 +21,23 @@ export interface BasicLoggerOptions {
   name?: string;
 
   /**
-   * An optional function to use to print each log line.
+   * An optional function, or map of levels to functions, to use to print each log line.
    *
-   * If this is specified, `basicLogger` calls it to write each line of output. The
+   * If not specified, the default is `console.error`.
+   *
+   * If a function is specified, `basicLogger` calls it to write each line of output. The
    * argument is a fully formatted log line, not including a linefeed. The function
    * is only called for log levels that are enabled.
    *
-   * If not specified, the default is `console.error`.
+   * If a map is specified, then each entry will be used as the destination for the corresponding
+   * log level. Any level that is not specified will use the default of `console.error`.
    *
    * Setting this property to anything other than a function will cause SDK
    * initialization to fail.
    */
-  destination?: (line: string) => void;
+  destination?:
+    | ((line: string) => void)
+    | Record<'debug' | 'info' | 'warn' | 'error', (line: string) => void>;
 
   /**
    * An optional formatter to use. The formatter should be compatible
diff --git a/packages/shared/common/src/logging/BasicLogger.ts b/packages/shared/common/src/logging/BasicLogger.ts
@@ -1,15 +1,15 @@
-import { BasicLoggerOptions, LDLogger } from '../api';
+import { BasicLoggerOptions, LDLogger, LDLogLevel } from '../api';
 import format from './format';
 
-const LogPriority = {
-  debug: 0,
-  info: 1,
-  warn: 2,
-  error: 3,
-  none: 4,
-};
+enum LogPriority {
+  debug = 0,
+  info = 1,
+  warn = 2,
+  error = 3,
+  none = 4,
+}
 
-const LevelNames = ['debug', 'info', 'warn', 'error', 'none'];
+const LEVEL_NAMES: LDLogLevel[] = ['debug', 'info', 'warn', 'error', 'none'];
 
 /**
  * A basic logger which handles filtering by level.
@@ -27,7 +27,7 @@ export default class BasicLogger implements LDLogger {
 
   private _name: string;
 
-  private _destination?: (line: string) => void;
+  private _destinations?: Record<number, (line: string) => void>;
 
   private _formatter?: (...args: any[]) => string;
 
@@ -43,9 +43,23 @@ export default class BasicLogger implements LDLogger {
   constructor(options: BasicLoggerOptions) {
     this._logLevel = LogPriority[options.level ?? 'info'] ?? LogPriority.info;
     this._name = options.name ?? 'LaunchDarkly';
-    // eslint-disable-next-line no-console
-    this._destination = options.destination;
     this._formatter = options.formatter;
+    if (typeof options.destination === 'object') {
+      this._destinations = {
+        [LogPriority.debug]: options.destination.debug,
+        [LogPriority.info]: options.destination.info,
+        [LogPriority.warn]: options.destination.warn,
+        [LogPriority.error]: options.destination.error,
+      };
+    } else if (typeof options.destination === 'function') {
+      const { destination } = options;
+      this._destinations = {
+        [LogPriority.debug]: destination,
+        [LogPriority.info]: destination,
+        [LogPriority.warn]: destination,
+        [LogPriority.error]: destination,
+      };
+    }
   }
 
   private _tryFormat(...args: any[]): string {
@@ -60,9 +74,9 @@ export default class BasicLogger implements LDLogger {
     }
   }
 
-  private _tryWrite(msg: string) {
+  private _tryWrite(destination: (msg: string) => void, msg: string) {
     try {
-      this._destination!(msg);
+      destination(msg);
     } catch {
       // eslint-disable-next-line no-console
       console.error(msg);
@@ -71,10 +85,11 @@ export default class BasicLogger implements LDLogger {
 
   private _log(level: number, args: any[]) {
     if (level >= this._logLevel) {
-      const prefix = `${LevelNames[level]}: [${this._name}]`;
+      const prefix = `${LEVEL_NAMES[level]}: [${this._name}]`;
       try {
-        if (this._destination) {
-          this._tryWrite(`${prefix} ${this._tryFormat(...args)}`);
+        const destination = this._destinations?.[level];
+        if (destination) {
+          this._tryWrite(destination, `${prefix} ${this._tryFormat(...args)}`);
         } else {
           // `console.error` has its own formatter.
           // So we don't need to do anything.
diff --git a/packages/shared/sdk-client/__tests__/configuration/Configuration.test.ts b/packages/shared/sdk-client/__tests__/configuration/Configuration.test.ts
@@ -22,11 +22,7 @@ describe('Configuration', () => {
       withReasons: false,
       eventsUri: 'https://events.launchdarkly.com',
       flushInterval: 30,
-      logger: {
-        _destination: console.error,
-        _logLevel: 1,
-        _name: 'LaunchDarkly',
-      },
+      logger: expect.anything(),
       maxCachedContexts: 5,
       privateAttributes: [],
       sendEvents: true,
diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts
@@ -126,7 +126,10 @@ export interface LDOptions {
    * @remarks
    * Set a custom {@link LDLogger} if you want full control of logging behavior.
    *
-   * @defaultValue A {@link BasicLogger} which outputs to the console at `info` level.
+   * @defaultValue The default logging implementation will varybased on platform. For the browser
+   * the default logger will log "info" level and higher priorty messages and it will log messages to
+   * console.info, console.warn, and console.error. Other platforms may use a `BasicLogger` instance
+   * also defaulted to the "info" level.
    */
   logger?: LDLogger;
 
