Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Enhance basic logger destination support. #650

Merged
merged 6 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions packages/sdk/browser/src/BrowserClient.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {
AutoEnvAttributes,
base64UrlEncode,
BasicLogger,
LDClient as CommonClient,
Configuration,
createSafeLogger,
Encoding,
FlagManager,
internal,
Expand Down Expand Up @@ -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
Expand Down
53 changes: 53 additions & 0 deletions packages/sdk/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
*/
import {
AutoEnvAttributes,
BasicLogger,
BasicLoggerOptions,
EvaluationSeriesContext,
EvaluationSeriesData,
Hook,
Expand Down Expand Up @@ -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);
}
115 changes: 103 additions & 12 deletions packages/shared/common/__tests__/logging/BasicLogger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { BasicLogger, LDLogLevel } from '../../src';

const spy = jest.spyOn(console, 'error').mockImplementation(() => {});

beforeEach(() => {
jest.clearAllMocks();
});

describe.each<[LDLogLevel, string[]]>([
[
'debug',
Expand Down Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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(' '));
Expand All @@ -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');
});
13 changes: 9 additions & 4 deletions packages/shared/common/src/api/logging/BasicLoggerOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 32 additions & 17 deletions packages/shared/common/src/logging/BasicLogger.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;

Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/shared/sdk-client/src/api/LDOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading