-
-
Notifications
You must be signed in to change notification settings - Fork 221
feat(perf-issues): Convert Body Size params into snake_case forms #920
feat(perf-issues): Convert Body Size params into snake_case forms #920
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I'd appreciate some feedback on how to handle differentiating encoded/decoded here. The OTel spec linked in the description mentions:
which I'm interpreting as: the field can support both encoded and decoded values, but we need both at the same time for our detectors. |
| `url` | string | The URL of the resource that was fetched. | `https://example.com` | | ||
| `method` | string | The HTTP method used. | `GET` | | ||
| `type` | string | The type of the resource that was fetched. | `xhr` | | ||
| `http.encoded_response_content_length` | number | The encoded body size of the request. | `123` | |
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.
resource.encoded_body_size
etc. yeah @AbhiPrasad ? You'll also want to change that content length header from encoded_body_size to http.response_content_length
as well I imagine.
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 think we can actually just use http.X
for everything here - since it is just http information. We can tell the difference by the operation (resource.X
vs. http.X
) if we want more granular analysis.
To match otel I think we use http.request_content_length
to represent the encoded body size, and introduce http.decoded_request_content_length
to represent the decoded body size.
We should still indicate that what these fields used to be called for reference (since older SDKs will keep sending this info).
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.
Okay, I've called out the original names in the descriptions and used http.request_content_length
, http.decoded_request_body_length
, and http.transfer_size
.
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.
Thanks for going the extra mile @narsaynorath!
We're proposing to change the names of these non-standard keys ("Encoded Body Size", "Decoded Body Size", and "Transfer Size") to be more aligned with OTel's standards. A spec doc here references
http.response_content_length
, which I am adding for the Giant Payload detector, but I am usingresource.encoded_body_size
andresource.decoded_body_size
for the old values.