-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracing): Change resource span op name and add data #2816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
size-limit report
|
b8a4d95 to
9881251
Compare
| if (entry.decodedBodySize) { | ||
| data.decodedBodySize = entry.decodedBodySize; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need a timing (such as responseStart) to exclude cross-origin resources that aren't setup to allow timing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also could include some sort of comparison (eg. entry.name.startsWith(window.location.origin)) to determine whether the resource is cross-origin explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave this for the future, merging this in for now.
Opening a ticket.
dashed
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
9881251 to
a68731c
Compare
a68731c to
8bb9c42
Compare
8bb9c42 to
49a5d6e
Compare
|
|
||
| const data: Record<string, any> = {}; | ||
| if ('transferSize' in entry) { | ||
| data.transferSize = entry.transferSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that arbitrary data fields are not special-cased in the UI to make them read nice, would it be desirable here to use a "pretty" format like data["Transfer Size"]? cc @dashed
Specifically:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that should work fine.

Add
transferSize,encodedBodySize, anddecodedBodySizemetrics to resource spans and adjust resource op name to account forinitiatorType(ex.resource.css,resource.img)This ones for 🇨🇦