feat: Add http collectors. #673
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
There are a few things duplicated in this PR and the DOM collector PR. They are a baseline to running tests. So there will be some merge conflicts to resolve. |
| */ | ||
| export default function decorateFetch(callback: (breadcrumb: HttpBreadcrumb) => void) { | ||
| // TODO: Check if already wrapped? | ||
| // TODO: Centralized mechanism to wrapping? |
There was a problem hiding this comment.
Are TODOs for this PR?
There was a problem hiding this comment.
Made a ticket as this is probably work that should be done as its own thing.
| * | ||
| * @param url URL to filter. | ||
| * @returns A filtered URL. | ||
| */ |
There was a problem hiding this comment.
This seems like it could be overlooked as endpoints evolve. Is it possible to make a test that will fail if this function doesn't filter the "set of all endpoints cases"? Not a trivial problem to solve.
There was a problem hiding this comment.
Perhaps an option is to define this closer to the endpoint urls?
There was a problem hiding this comment.
We cannot really define them closer, as this is a different package and the endpoints are defined in the SDK package.
We could add documentation in the SDK package. In a different PR, that indicates the telemetry package needs updated with the new paths.
Best reviewed after: #672