feat(browser): Add http.response_delivery_type attribute to resource spans
#14056
Conversation
size-limit report 📦
|
| @@ -539,6 +540,10 @@ export function _addResourceSpans( | |||
| setResourceEntrySizeData(attributes, entry, 'encodedBodySize', 'http.response_content_length'); | |||
| setResourceEntrySizeData(attributes, entry, 'decodedBodySize', 'http.decoded_response_content_length'); | |||
|
|
|||
| if (entry.deliveryType != null) { | |||
| attributes['http.response_delivery_type'] = entry.deliveryType || 'default'; | |||
There was a problem hiding this comment.
I'm falling back to "default" here because it seems like we drop attributes with empty string values (which is the actual default of the property) during ingest. Ideally we wouldn't need to do this but I don't see a better way at the moment.
There was a problem hiding this comment.
Maybe add an explaining comment to the code why this is needed
| @@ -539,6 +540,10 @@ export function _addResourceSpans( | |||
| setResourceEntrySizeData(attributes, entry, 'encodedBodySize', 'http.response_content_length'); | |||
| setResourceEntrySizeData(attributes, entry, 'decodedBodySize', 'http.decoded_response_content_length'); | |||
|
|
|||
| if (entry.deliveryType != null) { | |||
| attributes['http.response_delivery_type'] = entry.deliveryType || 'default'; | |||
There was a problem hiding this comment.
Maybe add an explaining comment to the code why this is needed
|
Thx for the reviews! I added a comment in the code as to why I add |
|
Gonna hold off from merging this until getsentry/relay#4174 is out. I think it's way better for us to forward the empty string value instead if |
|
getsentry/relay#4174 is merged and deployed so we can merge this PR now 🎉 |
This PR adds the
deliveryTypefield of aResourcePeformanceEntryto ourresourcespans. This attribute can give a definitive answer if a resource was retrieved from the browser cache or if the request was actually made to the resource src.Unfortunately,
deliveryTypeis not yet supported in all browsers but where it is, it should give us more reliable indicators of browser cache hits/misses.The alternative way of checking for cache hits/misses (which is to check for transfer and decoded body sizes of a response) has a flaw: It only works for requests to 3rd party origins if that origin returns the
Allow-Resource-Timinghttp header with a matching value. Otherwise, we can't determine the cache hit/miss status for 3rd party requests for certain.One important thing: The
deliveryTypevalue indicating that a request was actually made (i.e. cache miss) is the empty string"". We adjusted Relay to forward empty string values (getsentry/relay#4174) to ensure this value is ingested correctly.Idea for Product Improvement
Extracted to getsentry/sentry#79651