feat(browser): Add `http.response_delivery_type` attribute to resource spans #14056

Lms24 · 2024-10-23T07:53:00Z

This PR adds the deliveryType field of a ResourcePeformanceEntry to our resource spans. 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, deliveryType is 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-Timing http 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 deliveryType value 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

…e spans

github-actions · 2024-10-23T07:59:13Z

size-limit report 📦

Path	Size	% Change	Change
@sentry/browser	22.74 KB	-	-
@sentry/browser - with treeshaking flags	21.53 KB	-	-
@sentry/browser (incl. Tracing)	35.14 KB	+0.05%	+17 B 🔺
@sentry/browser (incl. Tracing, Replay)	71.86 KB	+0.03%	+20 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	62.29 KB	+0.03%	+16 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas)	76.17 KB	+0.03%	+19 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback)	88.97 KB	+0.03%	+21 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback, metrics)	90.81 KB	+0.02%	+15 B 🔺
@sentry/browser (incl. metrics)	26.99 KB	-	-
@sentry/browser (incl. Feedback)	39.89 KB	-	-
@sentry/browser (incl. sendFeedback)	27.39 KB	-	-
@sentry/browser (incl. FeedbackAsync)	32.18 KB	-	-
@sentry/react	25.5 KB	-	-
@sentry/react (incl. Tracing)	38.1 KB	+0.05%	+16 B 🔺
@sentry/vue	26.89 KB	-	-
@sentry/vue (incl. Tracing)	37.02 KB	+0.05%	+16 B 🔺
@sentry/svelte	22.88 KB	-	-
CDN Bundle	24.1 KB	-	-
CDN Bundle (incl. Tracing)	36.96 KB	+0.06%	+21 B 🔺
CDN Bundle (incl. Tracing, Replay)	71.62 KB	+0.03%	+21 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback)	76.96 KB	+0.03%	+21 B 🔺
CDN Bundle - uncompressed	70.64 KB	-	-
CDN Bundle (incl. Tracing) - uncompressed	109.71 KB	+0.07%	+72 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed	222.23 KB	+0.04%	+72 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	235.45 KB	+0.03%	+72 B 🔺
@sentry/nextjs (client)	38.18 KB	+0.06%	+21 B 🔺
@sentry/sveltekit (client)	35.75 KB	+0.05%	+16 B 🔺
@sentry/node	129.6 KB	-	-
@sentry/node - without tracing	94.3 KB	-	-
@sentry/aws-serverless	105.17 KB	-	-

View base workflow run

Lms24 · 2024-10-23T08:47:22Z

packages/browser-utils/src/metrics/browserMetrics.ts

@@ -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';


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.

Maybe add an explaining comment to the code why this is needed

s1gr1d

Nice addition!

s1gr1d · 2024-10-23T12:37:56Z

packages/browser-utils/src/metrics/browserMetrics.ts

@@ -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';


Maybe add an explaining comment to the code why this is needed

Lms24 · 2024-10-24T07:22:23Z

Thx for the reviews! I added a comment in the code as to why I add default as a fallback value. However, I'm gonna hold off from merging this PR because it seems like empty string values should actually be allowed (at least during ingest). If it turns out we can fix this product-side, I'd like to avoid adding "default" but simply forward the empty string to stay consistent with what the browser hands to us.

Lms24 · 2024-10-28T09:27:43Z

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 "default", so it makes sense to wait on this.

Lms24 · 2024-10-31T11:33:53Z

getsentry/relay#4174 is merged and deployed so we can merge this PR now 🎉

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@sentry/node](https://github.com/getsentry/sentry-javascript/tree/master/packages/node) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.36.0` -> `8.37.1`](https://renovatebot.com/diffs/npm/@sentry%2fnode/8.36.0/8.37.1) | | [@sentry/react](https://github.com/getsentry/sentry-javascript/tree/master/packages/react) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.36.0` -> `8.37.1`](https://renovatebot.com/diffs/npm/@sentry%2freact/8.36.0/8.37.1) | --- ### Release Notes <details> <summary>getsentry/sentry-javascript (@sentry/node)</summary> ### [`v8.37.1`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8371) [Compare Source](getsentry/sentry-javascript@8.37.0...8.37.1) - feat(deps): Bump [@opentelemetry/instrumentation](https://github.com/opentelemetry/instrumentation) from 0.53.0 to 0.54.0 for [@sentry/opentelemetry](https://github.com/sentry/opentelemetry) ([#14187](getsentry/sentry-javascript#14187)) ### [`v8.37.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8370) [Compare Source](getsentry/sentry-javascript@8.36.0...8.37.0) ##### Important Changes - **feat(nuxt): Add `piniaIntegration` ([#14138](getsentry/sentry-javascript#14138 The Nuxt SDK now allows you to track Pinia state for captured errors. To enable the Pinia plugin, add the `piniaIntegration` to your client config: ```ts // sentry.client.config.ts import { usePinia } from '#imports'; Sentry.init({ integrations: [ Sentry.piniaIntegration(usePinia(), { /* optional Pinia plugin options */ }), ], }); ``` - **feat: Deprecate metrics API ([#14157](getsentry/sentry-javascript#14157 The Sentry Metrics beta has ended in favour of revisiting metrics in another form at a later date. This new approach will include different APIs, making the current metrics API unnecessary. This release deprecates the metrics API with the plan to remove in the next SDK major version. If you currently use the metrics API in your code, you can safely continue to do so but sent data will no longer be processed by Sentry. [Learn more](https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Metrics-Beta-Ended-on-October-7th) about the end of the Metrics beta. ##### Other Changes - feat(browser): Add `http.response_delivery_type` attribute to resource spans ([#14056](getsentry/sentry-javascript#14056)) - feat(browser): Add `skipBrowserExtensionCheck` escape hatch option ([#14147](getsentry/sentry-javascript#14147)) - feat(deps): Bump [@opentelemetry/instrumentation](https://github.com/opentelemetry/instrumentation) from 0.53.0 to 0.54.0 ([#14174](getsentry/sentry-javascript#14174)) - feat(deps): Bump [@opentelemetry/instrumentation-fastify](https://github.com/opentelemetry/instrumentation-fastify) from 0.40.0 to 0.41.0 ([#14175](getsentry/sentry-javascript#14175)) - feat(deps): Bump [@opentelemetry/instrumentation-graphql](https://github.com/opentelemetry/instrumentation-graphql) from 0.43.0 to 0.44.0 ([#14173](getsentry/sentry-javascript#14173)) - feat(deps): Bump [@opentelemetry/instrumentation-mongodb](https://github.com/opentelemetry/instrumentation-mongodb) from 0.47.0 to 0.48.0 ([#14171](getsentry/sentry-javascript#14171)) - feat(deps): Bump [@opentelemetry/propagator-aws-xray](https://github.com/opentelemetry/propagator-aws-xray) from 1.25.1 to 1.26.0 ([#14172](getsentry/sentry-javascript#14172)) - feat(nuxt): Add `asyncFunctionReExports` to define re-exported server functions ([#14104](getsentry/sentry-javascript#14104)) - feat(nuxt): Add `piniaIntegration` ([#14138](getsentry/sentry-javascript#14138)) - fix(browser): Avoid recording long task spans starting before their parent span ([#14183](getsentry/sentry-javascript#14183)) - fix(core): Ensure errors thrown in async cron jobs bubble up ([#14182](getsentry/sentry-javascript#14182)) - fix(core): Silently fail `maybeInstrument` ([#14140](getsentry/sentry-javascript#14140)) - fix(nextjs): Resolve path for dynamic webpack import ([#13751](getsentry/sentry-javascript#13751)) - fix(node): Make sure `modulesIntegration` does not crash esm apps ([#14169](getsentry/sentry-javascript#14169)) Work in this release was contributed by [@rexxars](https://github.com/rexxars). Thank you for your contribution! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/299 Reviewed-by: Alexandre Soro <code@soro.dev> Co-authored-by: renovate <renovate@git.tristess.app> Co-committed-by: renovate <renovate@git.tristess.app>

feat(browser): Add http.response_delivery_type attribute to resourc…

16b21ce

…e spans

Lms24 self-assigned this Oct 23, 2024

Lms24 commented Oct 23, 2024

View reviewed changes

add integration test for empty string attribute

b176f1d

Lms24 requested review from AbhiPrasad and s1gr1d October 23, 2024 08:51

fix tests

9620908

s1gr1d approved these changes Oct 23, 2024

View reviewed changes

AbhiPrasad approved these changes Oct 23, 2024

View reviewed changes

Dhrumil-Sentry mentioned this pull request Oct 23, 2024

Helps users quickly understand if a resource script was loaded from a cache or not getsentry/sentry#79651

Open

add comment

d8feca0

switch to empty string as default value

595f2d3

Lms24 enabled auto-merge (squash) October 31, 2024 11:37

Lms24 merged commit d2a9826 into develop Oct 31, 2024
148 checks passed

Lms24 deleted the lms/feat-resource-deliverytype branch October 31, 2024 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(browser): Add `http.response_delivery_type` attribute to resource spans #14056

feat(browser): Add `http.response_delivery_type` attribute to resource spans #14056

Lms24 commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

Lms24 Oct 23, 2024 •

edited

Loading

s1gr1d Oct 23, 2024

s1gr1d left a comment

s1gr1d Oct 23, 2024

Lms24 commented Oct 24, 2024

Lms24 commented Oct 28, 2024

Lms24 commented Oct 31, 2024

feat(browser): Add http.response_delivery_type attribute to resource spans #14056

feat(browser): Add http.response_delivery_type attribute to resource spans #14056

Conversation

Lms24 commented Oct 23, 2024 • edited Loading

Idea for Product Improvement

github-actions bot commented Oct 23, 2024 • edited Loading

size-limit report 📦

Lms24 Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

s1gr1d Oct 23, 2024

Choose a reason for hiding this comment

s1gr1d left a comment

Choose a reason for hiding this comment

s1gr1d Oct 23, 2024

Choose a reason for hiding this comment

Lms24 commented Oct 24, 2024

Lms24 commented Oct 28, 2024

Lms24 commented Oct 31, 2024

feat(browser): Add `http.response_delivery_type` attribute to resource spans #14056

feat(browser): Add `http.response_delivery_type` attribute to resource spans #14056

Lms24 commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024 •

edited

Loading

Lms24 Oct 23, 2024 •

edited

Loading