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: Add basic logging support for browser-telemetry. #736

Merged
merged 10 commits into from
Jan 16, 2025
52 changes: 52 additions & 0 deletions packages/telemetry/browser-telemetry/__tests__/logging.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { MinLogger } from '../src/api';
import { fallbackLogger, prefixLog, safeMinLogger } from '../src/logging';

afterEach(() => {
jest.resetAllMocks();
});

it('prefixes the message with the telemetry prefix', () => {
const message = 'test message';
const prefixed = prefixLog(message);
expect(prefixed).toBe('LaunchDarkly - Browser Telemetry: test message');
});

it('uses fallback logger when no logger provided', () => {
const spy = jest.spyOn(fallbackLogger, 'warn');
const logger = safeMinLogger(undefined);

logger.warn('test message');

expect(spy).toHaveBeenCalledWith('test message');
spy.mockRestore();
});

it('uses provided logger when it works correctly', () => {
const mockWarn = jest.fn();
const testLogger: MinLogger = {
warn: mockWarn,
};

const logger = safeMinLogger(testLogger);
logger.warn('test message');

expect(mockWarn).toHaveBeenCalledWith('test message');
});

it('falls back to fallback logger when provided logger throws', () => {
const spy = jest.spyOn(fallbackLogger, 'warn');
const testLogger: MinLogger = {
warn: () => {
throw new Error('logger error');
},
};

const logger = safeMinLogger(testLogger);
logger.warn('test message');

expect(spy).toHaveBeenCalledWith('test message');
expect(spy).toHaveBeenCalledWith(
'LaunchDarkly - Browser Telemetry: The provided logger threw an exception, using fallback logger.',
);
spy.mockRestore();
});
26 changes: 13 additions & 13 deletions packages/telemetry/browser-telemetry/__tests__/options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ it('warns when maxPendingEvents is not a number', () => {

expect(outOptions.maxPendingEvents).toEqual(defaultOptions().maxPendingEvents);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "maxPendingEvents" should be of type number, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "maxPendingEvents" should be of type number, got string, using default value',
);
});

Expand All @@ -88,7 +88,7 @@ it('warns when breadcrumbs config is not an object', () => {

expect(outOptions.breadcrumbs).toEqual(defaultOptions().breadcrumbs);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs" should be of type object, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs" should be of type object, got string, using default value',
);
});

Expand All @@ -103,7 +103,7 @@ it('warns when collectors is not an array', () => {

expect(outOptions.collectors).toEqual(defaultOptions().collectors);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "collectors" should be of type Collector[], got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "collectors" should be of type Collector[], got string, using default value',
);
});

Expand Down Expand Up @@ -131,7 +131,7 @@ it('warns when stack config is not an object', () => {

expect(outOptions.stack).toEqual(defaultOptions().stack);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "stack" should be of type object, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "stack" should be of type object, got string, using default value',
);
});

Expand All @@ -150,7 +150,7 @@ it('warns when breadcrumbs.maxBreadcrumbs is not a number', () => {
defaultOptions().breadcrumbs.maxBreadcrumbs,
);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.maxBreadcrumbs" should be of type number, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.maxBreadcrumbs" should be of type number, got string, using default value',
);
});

Expand Down Expand Up @@ -181,7 +181,7 @@ it('warns when breadcrumbs.click is not boolean', () => {

expect(outOptions.breadcrumbs.click).toEqual(defaultOptions().breadcrumbs.click);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.click" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.click" should be of type boolean, got string, using default value',
);
});

Expand All @@ -198,7 +198,7 @@ it('warns when breadcrumbs.evaluations is not boolean', () => {

expect(outOptions.breadcrumbs.evaluations).toEqual(defaultOptions().breadcrumbs.evaluations);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.evaluations" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.evaluations" should be of type boolean, got string, using default value',
);
});

Expand All @@ -215,7 +215,7 @@ it('warns when breadcrumbs.flagChange is not boolean', () => {

expect(outOptions.breadcrumbs.flagChange).toEqual(defaultOptions().breadcrumbs.flagChange);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.flagChange" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.flagChange" should be of type boolean, got string, using default value',
);
});

Expand All @@ -232,7 +232,7 @@ it('warns when breadcrumbs.keyboardInput is not boolean', () => {

expect(outOptions.breadcrumbs.keyboardInput).toEqual(defaultOptions().breadcrumbs.keyboardInput);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.keyboardInput" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.keyboardInput" should be of type boolean, got string, using default value',
);
});

Expand Down Expand Up @@ -305,7 +305,7 @@ it('warns when breadcrumbs.http is not an object', () => {

expect(outOptions.breadcrumbs.http).toEqual(defaultOptions().breadcrumbs.http);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.http" should be of type HttpBreadCrumbOptions | false, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.http" should be of type HttpBreadCrumbOptions | false, got string, using default value',
);
});

Expand All @@ -326,7 +326,7 @@ it('warns when breadcrumbs.http.instrumentFetch is not boolean', () => {
defaultOptions().breadcrumbs.http.instrumentFetch,
);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.http.instrumentFetch" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.http.instrumentFetch" should be of type boolean, got string, using default value',
);
});

Expand All @@ -347,7 +347,7 @@ it('warns when breadcrumbs.http.instrumentXhr is not boolean', () => {
defaultOptions().breadcrumbs.http.instrumentXhr,
);
expect(mockLogger.warn).toHaveBeenCalledWith(
'Config option "breadcrumbs.http.instrumentXhr" should be of type boolean, got string, using default value',
'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.http.instrumentXhr" should be of type boolean, got string, using default value',
);
});

Expand Down Expand Up @@ -417,6 +417,6 @@ it('warns when breadcrumbs.http.customUrlFilter is not a function', () => {

expect(outOptions.breadcrumbs.http.customUrlFilter).toBeUndefined();
expect(mockLogger.warn).toHaveBeenCalledWith(
'The "breadcrumbs.http.customUrlFilter" must be a function. Received string',
'LaunchDarkly - Browser Telemetry: The "breadcrumbs.http.customUrlFilter" must be a function. Received string',
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
import type { LDContext, LDEvaluationDetail, LDInspection } from '@launchdarkly/js-client-sdk';

import { LDClientTracking } from './api';
import { LDClientTracking, MinLogger } from './api';
import { Breadcrumb, FeatureManagementBreadcrumb } from './api/Breadcrumb';
import { BrowserTelemetry } from './api/BrowserTelemetry';
import { Collector } from './api/Collector';
Expand All @@ -18,6 +18,7 @@ import FetchCollector from './collectors/http/fetch';
import XhrCollector from './collectors/http/xhr';
import defaultUrlFilter from './filters/defaultUrlFilter';
import makeInspectors from './inspectors';
import { fallbackLogger, prefixLog } from './logging';
import { ParsedOptions, ParsedStackOptions } from './options';
import randomUuidV4 from './randomUuidV4';
import parse from './stack/StackParser';
Expand Down Expand Up @@ -80,6 +81,8 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry {
private _collectors: Collector[] = [];
private _sessionId: string = randomUuidV4();

private _logger: MinLogger;

constructor(private _options: ParsedOptions) {
configureTraceKit(_options.stack);

Expand Down Expand Up @@ -127,16 +130,28 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry {
const inspectors: LDInspection[] = [];
makeInspectors(_options, inspectors, impl);
this._inspectorInstances.push(...inspectors);

// Set the initial logger, it may be replaced when the client is registered.
// For typescript purposes, we need the logger to be directly set in the constructor.
this._logger = this._options.logger ?? fallbackLogger;
}

register(client: LDClientTracking): void {
this._client = client;
// When the client is registered, we need to set the logger again, because we may be able to use the client's
// logger.
this._setLogger();
this._pendingEvents.forEach((event) => {
this._client?.track(event.type, event.data);
});
this._pendingEvents = [];
}

private _setLogger() {
this._logger =
this._options.logger ?? ((this._client as any)?.logger as MinLogger) ?? fallbackLogger;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, basically in the 4.x SDK there is a logger property, but in 3.x there is not.

Additionally error monitoring should always be initialized as early as possible, which is before the SDK (there are also some inter-dependencies that require this order). As a result we need some intermediate logging.

}

inspectors(): LDInspection[] {
return this._inspectorInstances;
}
Expand All @@ -153,7 +168,11 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry {
if (this._client === undefined) {
this._pendingEvents.push({ type, data: event });
if (this._pendingEvents.length > this._maxPendingEvents) {
// TODO: Log when pending events must be dropped. (SDK-915)
this._logger.warn(
prefixLog(
`Dropping ${this._pendingEvents.length - this._maxPendingEvents} pending events.`,
),
);
this._pendingEvents.shift();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,11 @@
* This allows usage with multiple SDK versions.
*/
export interface MinLogger {
/**
* The warning logger.
*
* @param args
* A sequence of any JavaScript values.
*/
warn(...args: any[]): void;
}
12 changes: 12 additions & 0 deletions packages/telemetry/browser-telemetry/src/api/Options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Collector } from './Collector';
import { MinLogger } from './MinLogger';

/**
* Interface for URL filters.
Expand Down Expand Up @@ -143,4 +144,15 @@ export interface Options {
* Configuration that controls the capture of the stack trace.
*/
stack?: StackOptions;

/**
* Logger to use for warnings.
*
* This option is compatible with the `LDLogger` interface used by the LaunchDarkly SDK.
*
* If this option is not provided, the logs will be written to console.log unless the LaunchDarkly SDK is registered,
* and the registered SDK instance exposes its logger. In which case, the logs will be written to the registered SDK's
* logger. The 3.x SDKs do not expose their logger.
*/
logger?: MinLogger;
}
1 change: 1 addition & 0 deletions packages/telemetry/browser-telemetry/src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ export * from './Options';
export * from './Recorder';
export * from './stack';
export * from './client';
export * from './MinLogger';
3 changes: 2 additions & 1 deletion packages/telemetry/browser-telemetry/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { BrowserTelemetry } from './api/BrowserTelemetry';
import { Options } from './api/Options';
import BrowserTelemetryImpl from './BrowserTelemetryImpl';
import { safeMinLogger } from './logging';
import parse from './options';

export * from './api';

export function initializeTelemetry(options?: Options): BrowserTelemetry {
const parsedOptions = parse(options || {});
const parsedOptions = parse(options || {}, safeMinLogger(options?.logger));
return new BrowserTelemetryImpl(parsedOptions);
}
33 changes: 33 additions & 0 deletions packages/telemetry/browser-telemetry/src/logging.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { MinLogger } from './api';

export const fallbackLogger: MinLogger = {
// Intentionally using console.warn as a fallback logger.
// eslint-disable-next-line no-console
warn: console.warn,
};

const loggingPrefix = 'LaunchDarkly - Browser Telemetry:';

export function prefixLog(message: string) {
return `${loggingPrefix} ${message}`;
}

export function safeMinLogger(logger: MinLogger | undefined): MinLogger {
return {
warn: (...args: any[]) => {
if (!logger) {
fallbackLogger.warn(...args);
return;
}

try {
logger.warn(...args);
} catch {
fallbackLogger.warn(...args);
fallbackLogger.warn(
prefixLog('The provided logger threw an exception, using fallback logger.'),
);
}
},
};
}
31 changes: 27 additions & 4 deletions packages/telemetry/browser-telemetry/src/options.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Collector } from './api/Collector';
import { MinLogger } from './api/MinLogger';
import { HttpBreadCrumbOptions, Options, StackOptions, UrlFilter } from './api/Options';
import { MinLogger } from './MinLogger';
import { fallbackLogger, prefixLog, safeMinLogger } from './logging';

export function defaultOptions(): ParsedOptions {
return {
Expand Down Expand Up @@ -28,7 +29,9 @@ export function defaultOptions(): ParsedOptions {
}

function wrongOptionType(name: string, expectedType: string, actualType: string): string {
return `Config option "${name}" should be of type ${expectedType}, got ${actualType}, using default value`;
return prefixLog(
`Config option "${name}" should be of type ${expectedType}, got ${actualType}, using default value`,
);
}

function checkBasic<T>(type: string, name: string, logger?: MinLogger): (item: T) => boolean {
Expand Down Expand Up @@ -77,7 +80,9 @@ function parseHttp(
if (options?.customUrlFilter) {
if (typeof options.customUrlFilter !== 'function') {
logger?.warn(
`The "breadcrumbs.http.customUrlFilter" must be a function. Received ${typeof options.customUrlFilter}`,
prefixLog(
`The "breadcrumbs.http.customUrlFilter" must be a function. Received ${typeof options.customUrlFilter}`,
),
);
}
}
Expand All @@ -101,6 +106,18 @@ function parseHttp(
};
}

function parseLogger(options: Options): MinLogger | undefined {
if (options.logger) {
const { logger } = options;
if (typeof logger === 'object' && logger !== null && 'warn' in logger) {
return safeMinLogger(logger);
}
// Using console.warn here because the logger is not suitable to log with.
fallbackLogger.warn(wrongOptionType('logger', 'MinLogger or LDLogger', typeof logger));
}
return undefined;
}

function parseStack(
options: StackOptions | undefined,
defaults: ParsedStackOptions,
Expand Down Expand Up @@ -175,10 +192,11 @@ export default function parse(options: Options, logger?: MinLogger): ParsedOptio
if (Array.isArray(item)) {
return true;
}
logger?.warn(logger?.warn(wrongOptionType('collectors', 'Collector[]', typeof item)));
logger?.warn(wrongOptionType('collectors', 'Collector[]', typeof item));
return false;
}),
],
logger: parseLogger(options),
};
}

Expand Down Expand Up @@ -282,4 +300,9 @@ export interface ParsedOptions {
* Additional, or custom, collectors.
*/
collectors: Collector[];

/**
* Logger to use for warnings.
*/
logger?: MinLogger;
}
Loading