feat(sveltekit): Add partial instrumentation for client-side fetch#7626
Merged
feat(sveltekit): Add partial instrumentation for client-side fetch#7626
fetch#7626Conversation
fetch
Contributor
size-limit report 📦
|
AbhiPrasad
reviewed
Mar 27, 2023
51eca27 to
ab20d0b
Compare
ea8a521 to
df10ac4
Compare
Lms24
commented
Apr 3, 2023
| ...(urlObject.hash && { 'http.hash': urlObject.hash.substring(1) }), | ||
| }; | ||
|
|
||
| // TODO: extract this to a util function (and use it in breadcrumbs integration as well) |
Member
Author
There was a problem hiding this comment.
Will do this in a separate PR as this spans multiple packages
Lms24
commented
Apr 3, 2023
| (host && | ||
| host | ||
| // Always filter out authority | ||
| .replace(/^.*@/, '[filtered]:[filtered]@') |
Member
Author
There was a problem hiding this comment.
This ain't pretty, I know... turns out finding a proper authority regex that isn't ReDos vulnerable is very hard. I think we can leave this as is for now and maybe revisit whenever we sanitize URLs everywhere on the browser side.
(I'd argue that this function should nevertheless be a good base we can reuse later on).
mydea
reviewed
Apr 4, 2023
mydea
reviewed
Apr 4, 2023
mydea
reviewed
Apr 4, 2023
AbhiPrasad
reviewed
Apr 4, 2023
mydea
reviewed
Apr 4, 2023
7cafcd7 to
5f05517
Compare
mydea
approved these changes
Apr 5, 2023
properly sanitize url, add url data apply data to breadcrumbs cleanup set span status add tests sorry fix authority url "fix" authority regex again cleanup move addTracingHeadersToFetchRequest back to tracing-internal cleanup apply suggestions re-activate request object test adjust types after exporting
b7ce245 to
0314949
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds partial instrumentation to the client-side
fetchpassed to the universalloadfunctions. It enables distributed traces of fetch calls happening inside aloadfunction.Limitation:
fetchrequests made by SvelteKit (e.g. to call server-only load functions) are not touched by this instrumentation because we cannot access the Kit-internal fetch function at this time. Opened an issue on the SvelteKit repo to find a solution for this.Anyway, here's a fancy screenshot of a successful trace:
My guess is that sveltejs/kit#9542 and sveltejs/kit#9530 are gonna take a little while longer to be resolved. I'd say, we add this in for the moment so that we at least get these traces connected. We should take it out/adjust whenever we have
handleLoadorhandleFetchavailable.ref: #7413