feat: Add basic logging support for browser-telemetry. (launchdarkly#736)

Adds the ability to specify a logger and to also use the base SDK logger
when possible. The base SDK of a 3.x version does not expose the logger,
but 4.x will.

There is a minimal replication of the warning level of the logging
interface to allow compatibility with both the 3.x SDK and the 4.x SDK.

Prefixing is done at message time instead of as part of the logger.
Potentially it could be ideal to do it in the logger, but this approach
makes it clear that the interpolation supported by most browser loggers
will not come into play. (Where if the logger does this prefixing it is
either complex or you lose the sprintf style formatting support.)

Some earlier log messages has been added, but a logger instance was not
yet available.

Author: kinyoklion

packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts

@@ -1,3 +1,4 @@
+import { LDClientLogging } from '../src/api';
 import { LDClientTracking } from '../src/api/client/LDClientTracking';
 import BrowserTelemetryImpl from '../src/BrowserTelemetryImpl';
 import { ParsedOptions } from '../src/options';
@@ -210,6 +211,30 @@ it('unregisters collectors on close', () => {
   expect(mockCollector.unregister).toHaveBeenCalled();
 });
 
+it('logs event dropped message when maxPendingEvents is reached', () => {
+  const mockLogger = {
+    warn: jest.fn(),
+  };
+  const telemetry = new BrowserTelemetryImpl({
+    ...defaultOptions,
+    maxPendingEvents: 2,
+    logger: mockLogger,
+  });
+  telemetry.captureError(new Error('Test error'));
+  expect(mockLogger.warn).not.toHaveBeenCalled();
+  telemetry.captureError(new Error('Test error 2'));
+  expect(mockLogger.warn).not.toHaveBeenCalled();
+
+  telemetry.captureError(new Error('Test error 3'));
+  expect(mockLogger.warn).toHaveBeenCalledWith(
+    'LaunchDarkly - Browser Telemetry: Maximum pending events reached. Old events will be dropped until the SDK' +
+      ' client is registered.',
+  );
+
+  telemetry.captureError(new Error('Test error 4'));
+  expect(mockLogger.warn).toHaveBeenCalledTimes(1);
+});
+
 it('filters breadcrumbs using provided filters', () => {
   const options: ParsedOptions = {
     ...defaultOptions,
@@ -359,3 +384,141 @@ it('omits breadcrumbs when a filter is not a function', () => {
     }),
   );
 });
+
+it('warns when a breadcrumb filter is not a function', () => {
+  const mockLogger = {
+    warn: jest.fn(),
+  };
+  const options: ParsedOptions = {
+    ...defaultOptions,
+    // @ts-ignore
+    breadcrumbs: { ...defaultOptions.breadcrumbs, filters: ['potato'] },
+    logger: mockLogger,
+  };
+
+  const telemetry = new BrowserTelemetryImpl(options);
+  telemetry.addBreadcrumb({
+    type: 'custom',
+    data: { id: 1 },
+    timestamp: Date.now(),
+    class: 'custom',
+    level: 'info',
+  });
+
+  expect(mockLogger.warn).toHaveBeenCalledWith(
+    'LaunchDarkly - Browser Telemetry: Error applying breadcrumb filters: TypeError: filter is not a function',
+  );
+});
+
+it('warns when a breadcrumb filter throws an exception', () => {
+  const mockLogger = {
+    warn: jest.fn(),
+  };
+  const options: ParsedOptions = {
+    ...defaultOptions,
+    breadcrumbs: {
+      ...defaultOptions.breadcrumbs,
+      filters: [
+        () => {
+          throw new Error('Filter error');
+        },
+      ],
+    },
+    logger: mockLogger,
+  };
+
+  const telemetry = new BrowserTelemetryImpl(options);
+  telemetry.addBreadcrumb({
+    type: 'custom',
+    data: { id: 1 },
+    timestamp: Date.now(),
+    class: 'custom',
+    level: 'info',
+  });
+
+  expect(mockLogger.warn).toHaveBeenCalledWith(
+    'LaunchDarkly - Browser Telemetry: Error applying breadcrumb filters: Error: Filter error',
+  );
+});
+
+it('only logs breadcrumb filter error once', () => {
+  const mockLogger = {
+    warn: jest.fn(),
+  };
+  const options: ParsedOptions = {
+    ...defaultOptions,
+    breadcrumbs: {
+      ...defaultOptions.breadcrumbs,
+      filters: [
+        () => {
+          throw new Error('Filter error');
+        },
+      ],
+    },
+    logger: mockLogger,
+  };
+
+  const telemetry = new BrowserTelemetryImpl(options);
+
+  // Add multiple breadcrumbs that will trigger the filter error
+  telemetry.addBreadcrumb({
+    type: 'custom',
+    data: { id: 1 },
+    timestamp: Date.now(),
+    class: 'custom',
+    level: 'info',
+  });
+
+  telemetry.addBreadcrumb({
+    type: 'custom',
+    data: { id: 2 },
+    timestamp: Date.now(),
+    class: 'custom',
+    level: 'info',
+  });
+
+  // Verify warning was only logged once
+  expect(mockLogger.warn).toHaveBeenCalledTimes(1);
+  expect(mockLogger.warn).toHaveBeenCalledWith(
+    'LaunchDarkly - Browser Telemetry: Error applying breadcrumb filters: Error: Filter error',
+  );
+});
+
+it('uses the client logger when no logger is provided', () => {
+  const options: ParsedOptions = {
+    ...defaultOptions,
+    breadcrumbs: {
+      ...defaultOptions.breadcrumbs,
+      filters: [
+        () => {
+          throw new Error('Filter error');
+        },
+      ],
+    },
+  };
+
+  const telemetry = new BrowserTelemetryImpl(options);
+
+  const mockClientWithLogging: jest.Mocked<LDClientLogging & LDClientTracking> = {
+    logger: {
+      warn: jest.fn(),
+    },
+    track: jest.fn(),
+  };
+
+  telemetry.register(mockClientWithLogging);
+
+  // Add multiple breadcrumbs that will trigger the filter error
+  telemetry.addBreadcrumb({
+    type: 'custom',
+    data: { id: 1 },
+    timestamp: Date.now(),
+    class: 'custom',
+    level: 'info',
+  });
+
+  expect(mockClientWithLogging.logger.warn).toHaveBeenCalledTimes(1);
+  expect(mockClientWithLogging.logger.warn).toHaveBeenCalledWith(
+    'LaunchDarkly - Browser Telemetry: Error applying breadcrumb filters: Error: Filter error',
+  );
+});

packages/telemetry/browser-telemetry/__tests__/logging.test.ts

@@ -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();
+});

packages/telemetry/browser-telemetry/__tests__/options.test.ts

@@ -63,7 +63,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',
   );
 });
 
@@ -90,7 +90,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',
   );
 });
 
@@ -105,7 +105,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',
   );
 });
 
@@ -133,7 +133,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',
   );
 });
 
@@ -152,7 +152,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',
   );
 });
 
@@ -183,7 +183,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',
   );
 });
 
@@ -200,7 +200,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',
   );
 });
 
@@ -217,7 +217,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',
   );
 });
 
@@ -234,7 +234,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',
   );
 });
 
@@ -307,7 +307,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',
   );
 });
 
@@ -328,7 +328,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',
   );
 });
 
@@ -349,7 +349,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',
   );
 });
 
@@ -419,7 +419,7 @@ 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',
   );
 });
 
@@ -435,6 +435,6 @@ it('warns when filters is not an array', () => {
   );
   expect(outOptions.breadcrumbs.filters).toEqual([]);
   expect(mockLogger.warn).toHaveBeenCalledWith(
-    'Config option "breadcrumbs.filters" should be of type array, got string, using default value',
+    'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.filters" should be of type array, got string, using default value',
   );
 });