From e551293794a26a7ed0712a67928c216f7afa07bb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:11:23 -0700 Subject: [PATCH 1/5] feat: Add better common logger support. --- .../__tests__/logging/BasicLogger.test.ts | 115 ++++++++++++++++-- .../src/api/logging/BasicLoggerOptions.ts | 13 +- .../shared/common/src/logging/BasicLogger.ts | 49 +++++--- 3 files changed, 144 insertions(+), 33 deletions(-) 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..d4be31aa7 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 collection of 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. From f762aea85686bbd11f51a66dfd5f82fb4a31f30c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:12:44 -0700 Subject: [PATCH 2/5] feat: Use enhanced logger support for browser SDK. --- packages/sdk/browser/src/BrowserClient.ts | 22 ++++---- packages/sdk/browser/src/index.ts | 53 +++++++++++++++++++ .../shared/sdk-client/src/api/LDOptions.ts | 5 +- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 892ed688d..898c0f306 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -1,6 +1,7 @@ import { AutoEnvAttributes, base64UrlEncode, + BasicLogger, LDClient as CommonClient, Configuration, createSafeLogger, @@ -98,15 +99,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..7c02521cb 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: ld.basicLogger({ destination: console.log }), + * }; + * ``` + * + * * @example + * The configuration also allows you to control the destination for each log level. + * ```javascript + * const ldOptions = { + * logger: ld.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/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; From ccfad0abe8173f4f1e7e7e5a4178b540d0178068 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:16:51 -0700 Subject: [PATCH 3/5] Lint and test fix. --- packages/sdk/browser/src/BrowserClient.ts | 1 - .../__tests__/configuration/Configuration.test.ts | 6 +----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 898c0f306..0ef3490e9 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -4,7 +4,6 @@ import { BasicLogger, LDClient as CommonClient, Configuration, - createSafeLogger, Encoding, FlagManager, internal, 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, From c584389f3ff711ae4c776c359fdbcd700be7a6fc Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:19:08 -0700 Subject: [PATCH 4/5] Cleanup examples. --- packages/sdk/browser/src/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sdk/browser/src/index.ts b/packages/sdk/browser/src/index.ts index 7c02521cb..0b29b75f3 100644 --- a/packages/sdk/browser/src/index.ts +++ b/packages/sdk/browser/src/index.ts @@ -115,7 +115,7 @@ export function initialize(clientSideId: string, options?: LDOptions): LDClient * log output to go to `console.log` * ```javascript * const ldOptions = { - * logger: ld.basicLogger({ destination: console.log }), + * logger: basicLogger({ destination: console.log }), * }; * ``` * @@ -123,7 +123,7 @@ export function initialize(clientSideId: string, options?: LDOptions): LDClient * The configuration also allows you to control the destination for each log level. * ```javascript * const ldOptions = { - * logger: ld.basicLogger({ + * logger: basicLogger({ * destination: { * debug: console.debug, * info: console.info, From 0a1bc37aced39584e11c403b894570dfcd2a2157 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:20:40 -0700 Subject: [PATCH 5/5] Improve comment. --- packages/shared/common/src/api/logging/BasicLoggerOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/api/logging/BasicLoggerOptions.ts b/packages/shared/common/src/api/logging/BasicLoggerOptions.ts index d4be31aa7..cba75880c 100644 --- a/packages/shared/common/src/api/logging/BasicLoggerOptions.ts +++ b/packages/shared/common/src/api/logging/BasicLoggerOptions.ts @@ -21,7 +21,7 @@ export interface BasicLoggerOptions { name?: string; /** - * An optional function, or collection of functions to use to print each log line. + * An optional function, or map of levels to functions, to use to print each log line. * * If not specified, the default is `console.error`. *