From 21670c4acd629f7ccfeb7abbe94fe89723533600 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 1 Nov 2024 08:47:14 -0700 Subject: [PATCH] feat: Enhance basic logger destination support. (#650) 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. --- packages/sdk/browser/src/BrowserClient.ts | 23 ++-- packages/sdk/browser/src/index.ts | 53 ++++++++ .../__tests__/logging/BasicLogger.test.ts | 115 ++++++++++++++++-- .../src/api/logging/BasicLoggerOptions.ts | 13 +- .../shared/common/src/logging/BasicLogger.ts | 49 +++++--- .../configuration/Configuration.test.ts | 6 +- .../shared/sdk-client/src/api/LDOptions.ts | 5 +- 7 files changed, 215 insertions(+), 49 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 892ed688d..0ef3490e9 100644 --- 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 index 6fe3a53a7..0b29b75f3 100644 --- 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 index 971d4a594..426c97c13 100644 --- 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 index 7b983a359..cba75880c 100644 --- 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 index 149aafbdf..ffb6fccb9 100644 --- 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 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 index 7c2c880d7..3f437855a 100644 --- 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 index 27627011a..5ce6b647d 100644 --- 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;