Skip to content

Conversation

@kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Jan 22, 2025

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.

},
},
collectors: [new ErrorCollector(), new ErrorCollector()],
errorFilters: expect.arrayContaining([errorFilter]),
Copy link
Member Author

Choose a reason for hiding this comment

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

The breadcrumbs filter is scoped to breadcrumbs, so it can just be called filters, but this is top level configuration so I think it needs error prepended.

@github-actions
Copy link
Contributor

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 19014 bytes
Size limit: 21000

@github-actions
Copy link
Contributor

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Size: 15354 bytes
Size limit: 20000

@github-actions
Copy link
Contributor

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Size: 19826 bytes
Size limit: 21000

@kinyoklion kinyoklion marked this pull request as ready for review January 22, 2025 00:28
@kinyoklion kinyoklion requested a review from a team as a code owner January 22, 2025 00:28
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}`));
Copy link
Contributor

Choose a reason for hiding this comment

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

Does e need to be json stringified in order to print the value instead of [Object object] or the type?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see this code was just moved from a catch to here. I'm still curious if this logs helpful information, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Generally no, as long as it is actually an error or a string. The default formatting for the error type will use the name and message (https://tc39.es/ecma262/multipage/fundamental-objects.html#sec-error.prototype.tostring).

Sometimes it is good to have custom error formatting when you aren't certain it will be an error.

There is some additional complexity generally with errors as they tend to either be self-referential, or have serialization which results in strange field output.

Just logging an error will typically use the message.

> console.log(`Error ${new Error("potato")}`)
Error Error: potato

JSON.stringify an error.

JSON.stringify(new Error("potato"))
'{}'

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! Gotcha, thanks for explaining.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will make a task to see if I can safely add some meta-data about the specific filter. If nothing else potentially the index.

Copy link
Contributor

Choose a reason for hiding this comment

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

is e always unknown here or can it be typed to an Error if it's always called from a try-catch?

Copy link
Member Author

Choose a reason for hiding this comment

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

I could add a type guard using instanceof, but the typing of the catch is any/unknown depending on ts version. You technically can do really annoying things like throw 10.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh dang, TIL. Makes sense why you've implemented it as an unknown type.

@kinyoklion kinyoklion merged commit 5cffb2b into main Jan 22, 2025
23 checks passed
@kinyoklion kinyoklion deleted the rlamb/emsr-31/error-filters branch January 22, 2025 16:53
@github-actions github-actions bot mentioned this pull request Jan 22, 2025
kinyoklion pushed a commit that referenced this pull request Jan 22, 2025
🤖 I have created a release *beep* *boop*
---


<details><summary>browser-telemetry: 0.1.0</summary>

##
[0.1.0](browser-telemetry-v0.0.9...browser-telemetry-v0.1.0)
(2025-01-22)


### ⚠ BREAKING CHANGES

* Updated AI config interface.
([#697](#697))

### Features

* Add basic logging support for browser-telemetry.
([#736](#736))
([2ef1486](2ef1486))
* Add browser telemetry options.
([#675](#675))
([c8352b2](c8352b2))
* Add browser-telemetry API types.
([#669](#669))
([89967ee](89967ee))
* Add DOM collectors.
([#672](#672))
([4473a06](4473a06))
* Add http collectors.
([#673](#673))
([6e60ddc](6e60ddc))
* Add singleton support for browser-telemetry.
([#739](#739))
([68a3b87](68a3b87))
* Add stack trace parsing.
([#676](#676))
([ca1dd49](ca1dd49))
* Add support for breadcrumb filtering.
([#733](#733))
([5c327a1](5c327a1))
* Add support for the session init event.
([320c07d](320c07d))
* Add the ability to filter errors.
([#743](#743))
([5cffb2b](5cffb2b))
* Export browser-telemetry initialization method.
([d1b364e](d1b364e))
* Implement browser telemetry client.
([#691](#691))
([db74a99](db74a99))
* Make browser-telemetry specific inspector type.
([#741](#741))
([14ecdb3](14ecdb3))
* Random uuid for telemetry package.
([#689](#689))
([4cf34f9](4cf34f9))
* Rename initializeTelemetryInstance to initTelemetryInstance for
consistency with initTelemetry.
([257734f](257734f))
* Source maps with inline sources for browser-telemetry.
([#735](#735))
([1656a85](1656a85))
* Updated AI config interface.
([#697](#697))
([cd72ea8](cd72ea8))
* Vendor TraceKit
([d1b364e](d1b364e))


### Bug Fixes

* Clear pending events buffer when registered.
([#727](#727))
([b6ad7df](b6ad7df))
* Export BrowserTelemetry, BrowserTelemetryInspector, and
ImplementsCrumb.
([257734f](257734f))
* Fix breadcrumb filter option parsing.
([#742](#742))
([833f4ce](833f4ce))
* Remove BrowserTelemetry until more types are available.
([#671](#671))
([796b8a3](796b8a3))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants