From 5cffb2b5216f94941498ebb6bb783d0a8841d566 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 22 Jan 2025 08:53:41 -0800 Subject: [PATCH] feat: Add the ability to filter errors. (#743) This PR adds support for error level filtering. This is the highest level filter type capable of filtering on any data. Other filters are provided to simplify the implementation of filtering across all error types. Breadcrumb filters could be implemented in terms of an error filter, but they would have higher complexity and different performance characteristics. For example with a breadcrumb filter we filter that breadcrumb regardless of how many events it may appear in, versus having to filter that same breadcrumb each time an event is captures. Custom url filters operate at the HTTP capture level similarly reducing the frequency of redaction. --- .../__tests__/BrowserTelemetryImpl.test.ts | 123 ++++++++++++++++++ .../__tests__/options.test.ts | 38 +++++- .../src/BrowserTelemetryImpl.ts | 60 +++++---- .../browser-telemetry/src/api/Options.ts | 23 +++- .../browser-telemetry/src/options.ts | 14 ++ 5 files changed, 231 insertions(+), 27 deletions(-) diff --git a/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts b/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts index 1b0c866e9..3a50c40dd 100644 --- a/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts +++ b/packages/telemetry/browser-telemetry/__tests__/BrowserTelemetryImpl.test.ts @@ -33,6 +33,7 @@ const defaultOptions: ParsedOptions = { }, }, collectors: [], + errorFilters: [], }; it('sends buffered events when client is registered', () => { @@ -534,3 +535,125 @@ it('sends session init event when client is registered', () => { }), ); }); + +it('applies error filters to captured errors', () => { + const options: ParsedOptions = { + ...defaultOptions, + errorFilters: [ + (error) => ({ + ...error, + message: error.message.replace('secret', 'redacted'), + }), + ], + }; + const telemetry = new BrowserTelemetryImpl(options); + + telemetry.captureError(new Error('Error with secret info')); + telemetry.register(mockClient); + + expect(mockClient.track).toHaveBeenCalledWith( + '$ld:telemetry:error', + expect.objectContaining({ + message: 'Error with redacted info', + }), + ); +}); + +it('filters out errors when filter returns undefined', () => { + const options: ParsedOptions = { + ...defaultOptions, + errorFilters: [() => undefined], + }; + const telemetry = new BrowserTelemetryImpl(options); + + telemetry.captureError(new Error('Test error')); + telemetry.register(mockClient); + + // Verify only session init event was tracked + expect(mockClient.track).toHaveBeenCalledTimes(1); + expect(mockClient.track).toHaveBeenCalledWith( + '$ld:telemetry:session:init', + expect.objectContaining({ + sessionId: expect.any(String), + }), + ); +}); + +it('applies multiple error filters in sequence', () => { + const options: ParsedOptions = { + ...defaultOptions, + errorFilters: [ + (error) => ({ + ...error, + message: error.message.replace('secret', 'redacted'), + }), + (error) => ({ + ...error, + message: error.message.replace('redacted', 'sneaky'), + }), + ], + }; + const telemetry = new BrowserTelemetryImpl(options); + + telemetry.captureError(new Error('Error with secret info')); + telemetry.register(mockClient); + + expect(mockClient.track).toHaveBeenCalledWith( + '$ld:telemetry:error', + expect.objectContaining({ + message: 'Error with sneaky info', + }), + ); +}); + +it('handles error filter throwing an exception', () => { + const mockLogger = { + warn: jest.fn(), + }; + const options: ParsedOptions = { + ...defaultOptions, + errorFilters: [ + () => { + throw new Error('Filter error'); + }, + ], + logger: mockLogger, + }; + const telemetry = new BrowserTelemetryImpl(options); + + telemetry.captureError(new Error('Test error')); + telemetry.register(mockClient); + + expect(mockLogger.warn).toHaveBeenCalledWith( + 'LaunchDarkly - Browser Telemetry: Error applying error filters: Error: Filter error', + ); + // Verify only session init event was tracked + expect(mockClient.track).toHaveBeenCalledTimes(1); + expect(mockClient.track).toHaveBeenCalledWith( + '$ld:telemetry:session:init', + expect.objectContaining({ + sessionId: expect.any(String), + }), + ); +}); + +it('only logs error filter error once', () => { + const mockLogger = { + warn: jest.fn(), + }; + const options: ParsedOptions = { + ...defaultOptions, + errorFilters: [ + () => { + throw new Error('Filter error'); + }, + ], + logger: mockLogger, + }; + const telemetry = new BrowserTelemetryImpl(options); + + telemetry.captureError(new Error('Error 1')); + telemetry.captureError(new Error('Error 2')); + + expect(mockLogger.warn).toHaveBeenCalledTimes(1); +}); diff --git a/packages/telemetry/browser-telemetry/__tests__/options.test.ts b/packages/telemetry/browser-telemetry/__tests__/options.test.ts index db6bc15a3..84cd6afa3 100644 --- a/packages/telemetry/browser-telemetry/__tests__/options.test.ts +++ b/packages/telemetry/browser-telemetry/__tests__/options.test.ts @@ -1,4 +1,5 @@ import { Breadcrumb } from '../src/api/Breadcrumb'; +import { ErrorData } from '../src/api/ErrorData'; import ErrorCollector from '../src/collectors/error'; import parse, { defaultOptions } from '../src/options'; @@ -16,7 +17,8 @@ it('handles an empty configuration', () => { }); it('can set all options at once', () => { - const filter = (breadcrumb: Breadcrumb) => breadcrumb; + const breadcrumbFilter = (breadcrumb: Breadcrumb) => breadcrumb; + const errorFilter = (error: ErrorData) => error; const outOptions = parse({ maxPendingEvents: 1, breadcrumbs: { @@ -24,9 +26,10 @@ it('can set all options at once', () => { click: false, evaluations: false, flagChange: false, - filters: [filter], + filters: [breadcrumbFilter], }, collectors: [new ErrorCollector(), new ErrorCollector()], + errorFilters: [errorFilter], }); expect(outOptions).toEqual({ maxPendingEvents: 1, @@ -41,7 +44,7 @@ it('can set all options at once', () => { instrumentFetch: true, instrumentXhr: true, }, - filters: expect.arrayContaining([filter]), + filters: expect.arrayContaining([breadcrumbFilter]), }, stack: { source: { @@ -51,6 +54,7 @@ it('can set all options at once', () => { }, }, collectors: [new ErrorCollector(), new ErrorCollector()], + errorFilters: expect.arrayContaining([errorFilter]), }); expect(mockLogger.warn).not.toHaveBeenCalled(); }); @@ -441,3 +445,31 @@ it('warns when filters is not an array', () => { 'LaunchDarkly - Browser Telemetry: Config option "breadcrumbs.filters" should be of type BreadcrumbFilter[], got string, using default value', ); }); + +it('warns when errorFilters is not an array', () => { + const outOptions = parse( + { + // @ts-ignore + errorFilters: 'not an array', + }, + mockLogger, + ); + + expect(outOptions.errorFilters).toEqual([]); + expect(mockLogger.warn).toHaveBeenCalledWith( + 'LaunchDarkly - Browser Telemetry: Config option "errorFilters" should be of type ErrorDataFilter[], got string, using default value', + ); +}); + +it('accepts valid error filters array', () => { + const errorFilters = [(error: any) => error]; + const outOptions = parse( + { + errorFilters, + }, + mockLogger, + ); + + expect(outOptions.errorFilters).toEqual(errorFilters); + expect(mockLogger.warn).not.toHaveBeenCalled(); +}); diff --git a/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts b/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts index fb931596b..1b08d881b 100644 --- a/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts +++ b/packages/telemetry/browser-telemetry/src/BrowserTelemetryImpl.ts @@ -5,7 +5,7 @@ */ import type { LDContext, LDEvaluationDetail } from '@launchdarkly/js-client-sdk'; -import { BreadcrumbFilter, LDClientLogging, LDClientTracking, MinLogger } from './api'; +import { LDClientLogging, LDClientTracking, MinLogger } from './api'; import { Breadcrumb, FeatureManagementBreadcrumb } from './api/Breadcrumb'; import { BrowserTelemetry } from './api/BrowserTelemetry'; import { BrowserTelemetryInspector } from './api/client/BrowserTelemetryInspector'; @@ -54,11 +54,8 @@ function safeValue(u: unknown): string | boolean | number | undefined { } } -function applyBreadcrumbFilter( - breadcrumb: Breadcrumb | undefined, - filter: BreadcrumbFilter, -): Breadcrumb | undefined { - return breadcrumb === undefined ? undefined : filter(breadcrumb); +function applyFilter(item: T | undefined, filter: (item: T) => T | undefined): T | undefined { + return item === undefined ? undefined : filter(item); } function configureTraceKit(options: ParsedStackOptions) { @@ -69,7 +66,7 @@ function configureTraceKit(options: ParsedStackOptions) { // from the before context. // The typing for this is a bool, but it accepts a number. const beforeAfterMax = Math.max(options.source.afterLines, options.source.beforeLines); - // The assignment here has bene split to prevent esbuild from complaining about an assigment to + // The assignment here has bene split to prevent esbuild from complaining about an assignment to // an import. TraceKit exports a single object and the interface requires modifying an exported // var. const anyObj = TraceKit as any; @@ -105,6 +102,8 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { private _eventsDropped: boolean = false; // Used to ensure we only log the breadcrumb filter error once. private _breadcrumbFilterError: boolean = false; + // Used to ensure we only log the error filter error once. + private _errorFilterError: boolean = false; constructor(private _options: ParsedOptions) { configureTraceKit(_options.stack); @@ -198,8 +197,18 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { * @param event The event data. */ private _capture(type: string, event: EventData) { + const filteredEvent = this._applyFilters(event, this._options.errorFilters, (e: unknown) => { + if (!this._errorFilterError) { + this._errorFilterError = true; + this._logger.warn(prefixLog(`Error applying error filters: ${e}`)); + } + }); + if (filteredEvent === undefined) { + return; + } + if (this._client === undefined) { - this._pendingEvents.push({ type, data: event }); + this._pendingEvents.push({ type, data: filteredEvent }); if (this._pendingEvents.length > this._maxPendingEvents) { if (!this._eventsDropped) { this._eventsDropped = true; @@ -212,7 +221,7 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { this._pendingEvents.shift(); } } - this._client?.track(type, event); + this._client?.track(type, filteredEvent); } captureError(exception: Error): void { @@ -241,27 +250,34 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { this.captureError(errorEvent.error); } - private _applyBreadcrumbFilters( - breadcrumb: Breadcrumb, - filters: BreadcrumbFilter[], - ): Breadcrumb | undefined { + private _applyFilters( + item: T, + filters: ((item: T) => T | undefined)[], + handleError: (e: unknown) => void, + ): T | undefined { try { return filters.reduce( - (breadcrumbToFilter: Breadcrumb | undefined, filter: BreadcrumbFilter) => - applyBreadcrumbFilter(breadcrumbToFilter, filter), - breadcrumb, + (itemToFilter: T | undefined, filter: (item: T) => T | undefined) => + applyFilter(itemToFilter, filter), + item, ); } catch (e) { - if (!this._breadcrumbFilterError) { - this._breadcrumbFilterError = true; - this._logger.warn(prefixLog(`Error applying breadcrumb filters: ${e}`)); - } + handleError(e); return undefined; } } addBreadcrumb(breadcrumb: Breadcrumb): void { - const filtered = this._applyBreadcrumbFilters(breadcrumb, this._options.breadcrumbs.filters); + const filtered = this._applyFilters( + breadcrumb, + this._options.breadcrumbs.filters, + (e: unknown) => { + if (!this._breadcrumbFilterError) { + this._breadcrumbFilterError = true; + this._logger.warn(prefixLog(`Error applying breadcrumb filters: ${e}`)); + } + }, + ); if (filtered !== undefined) { this._breadcrumbs.push(filtered); if (this._breadcrumbs.length > this._maxBreadcrumbs) { @@ -275,7 +291,7 @@ export default class BrowserTelemetryImpl implements BrowserTelemetry { } /** - * Used to automatically collect flag usage for breacrumbs. + * Used to automatically collect flag usage for breadcrumbs. * * When session replay is in use the data is also forwarded to the session * replay collector. diff --git a/packages/telemetry/browser-telemetry/src/api/Options.ts b/packages/telemetry/browser-telemetry/src/api/Options.ts index 32137ea71..e0f206e49 100644 --- a/packages/telemetry/browser-telemetry/src/api/Options.ts +++ b/packages/telemetry/browser-telemetry/src/api/Options.ts @@ -1,5 +1,6 @@ import { Breadcrumb } from './Breadcrumb'; import { Collector } from './Collector'; +import { ErrorData } from './ErrorData'; import { MinLogger } from './MinLogger'; /** @@ -27,13 +28,21 @@ export interface UrlFilter { /** * Interface for breadcrumb filters. * - * Given a breadcrumb the filter may return a modified breadcrumb or undefined to - * exclude the breadcrumb. + * Given a breadcrumb the filter may return a modified breadcrumb or undefined to exclude the breadcrumb. */ export interface BreadcrumbFilter { (breadcrumb: Breadcrumb): Breadcrumb | undefined; } +/** + * Interface for filtering error data before it is sent to LaunchDarkly. + * + * Given {@link ErrorData} the filter may return modified data or undefined to exclude the breadcrumb. + */ +export interface ErrorDataFilter { + (event: ErrorData): ErrorData | undefined; +} + export interface HttpBreadcrumbOptions { /** * If fetch should be instrumented and breadcrumbs included for fetch requests. @@ -197,4 +206,14 @@ export interface Options { * logger. The 3.x SDKs do not expose their logger. */ logger?: MinLogger; + + /** + * Custom error data filters. + * + * Can be used to redact or modify error data. + * + * For filtering breadcrumbs or URLs in error data, see {@link breadcrumbs.filters} and + * {@link breadcrumbs.http.customUrlFilter}. + */ + errorFilters?: ErrorDataFilter[]; } diff --git a/packages/telemetry/browser-telemetry/src/options.ts b/packages/telemetry/browser-telemetry/src/options.ts index 0b14615dd..ce6ab21e1 100644 --- a/packages/telemetry/browser-telemetry/src/options.ts +++ b/packages/telemetry/browser-telemetry/src/options.ts @@ -2,6 +2,7 @@ import { Collector } from './api/Collector'; import { MinLogger } from './api/MinLogger'; import { BreadcrumbFilter, + ErrorDataFilter, HttpBreadcrumbOptions, Options, StackOptions, @@ -32,6 +33,7 @@ export function defaultOptions(): ParsedOptions { }, maxPendingEvents: 100, collectors: [], + errorFilters: [], }; } @@ -211,6 +213,13 @@ export default function parse(options: Options, logger?: MinLogger): ParsedOptio }), ], logger: parseLogger(options), + errorFilters: itemOrDefault(options.errorFilters, defaults.errorFilters, (item) => { + if (Array.isArray(item)) { + return true; + } + logger?.warn(wrongOptionType('errorFilters', 'ErrorDataFilter[]', typeof item)); + return false; + }), }; } @@ -324,4 +333,9 @@ export interface ParsedOptions { * Logger to use for warnings. */ logger?: MinLogger; + + /** + * Custom error data filters. + */ + errorFilters: ErrorDataFilter[]; }