Skip to content

feat(tracing): add long animation frame tracing #12646

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

Merged
merged 15 commits into from
Jul 10, 2024

Conversation

KevinL10
Copy link
Contributor

@KevinL10 KevinL10 commented Jun 25, 2024

Adds an option to trace long animation frames as per #11719. This tracing feature is disabled by default.

Currently, this feature https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/Long_animation_frame_timing is only supported by Chromium browsers.

Usage is opt-in:

Sentry.init({
  dsn: '__PUBLIC_DSN__',
  integrations: [
    Sentry.browserTracingIntegration({
      enableLongAnimationFrame: true,
    }),
  ],
  tracesSampleRate: 1,
});

@KevinL10 KevinL10 changed the title feat(tracing): add long animation frame tracing. feat(tracing): add long animation frame tracing Jun 25, 2024
Copy link
Contributor

github-actions bot commented Jun 27, 2024

size-limit report 📦

Path Size
@sentry/browser 22.29 KB (0%)
@sentry/browser (incl. Tracing) 33.63 KB (+0.58% 🔺)
@sentry/browser (incl. Tracing, Replay) 69.41 KB (+0.32% 🔺)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.74 KB (+0.36% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) 73.46 KB (+0.29% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) 86.12 KB (+0.28% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 87.97 KB (+0.27% 🔺)
@sentry/browser (incl. metrics) 26.58 KB (0%)
@sentry/browser (incl. Feedback) 38.97 KB (0%)
@sentry/browser (incl. sendFeedback) 26.92 KB (0%)
@sentry/browser (incl. FeedbackAsync) 31.53 KB (0%)
@sentry/react 25.04 KB (0%)
@sentry/react (incl. Tracing) 36.68 KB (+0.55% 🔺)
@sentry/vue 26.4 KB (0%)
@sentry/vue (incl. Tracing) 35.52 KB (+0.61% 🔺)
@sentry/svelte 22.43 KB (0%)
CDN Bundle 23.51 KB (0%)
CDN Bundle (incl. Tracing) 35.41 KB (+0.59% 🔺)
CDN Bundle (incl. Tracing, Replay) 69.52 KB (+0.31% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) 74.72 KB (+0.29% 🔺)
CDN Bundle - uncompressed 69 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 104.75 KB (+0.71% 🔺)
CDN Bundle (incl. Tracing, Replay) - uncompressed 215.15 KB (+0.35% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 227.86 KB (+0.33% 🔺)
@sentry/nextjs (client) 36.56 KB (+0.54% 🔺)
@sentry/sveltekit (client) 34.29 KB (+0.59% 🔺)
@sentry/node 130.79 KB (0%)
@sentry/node - without tracing 91.88 KB (0%)
@sentry/aws-serverless 117.05 KB (0%)

@KevinL10 KevinL10 force-pushed the kevinliu/feat/add-long-animation-frame branch from 2083357 to d4ff07d Compare July 5, 2024 18:06
@KevinL10 KevinL10 marked this pull request as ready for review July 5, 2024 20:46
@KevinL10 KevinL10 requested a review from AbhiPrasad July 5, 2024 20:49
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share a link/screenshot of an example trace with these spans?

@AbhiPrasad AbhiPrasad requested review from a team, stephanie-anderson and Lms24 and removed request for a team July 8, 2024 13:35
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to test the scenarios where:

  1. enableLongAnimationFrame is true, but non-chromium browser
  2. enableLongAnimationFrame is false and enableLongTask is true

afterwards we are good to merge. Thanks for sticking with this!

@KevinL10
Copy link
Contributor Author

KevinL10 commented Jul 9, 2024

Example spans from the long-tasks-and-animation-frame-enabled test:
image

image

@KevinL10 KevinL10 requested a review from AbhiPrasad July 9, 2024 19:05
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to adjust https://github.com/getsentry/sentry-javascript/blob/develop/.size-limit.js - you can run yarn size-limit in the root after you run yarn build to see what to change.

I also updated #11719 with a tasklist of follow up items. Do you have the item to do these?

@AbhiPrasad
Copy link
Member

You can ignore Secret Scan as well!

Please make sure to Squash and Merge after the size-limit stuff is resolved.

@KevinL10 KevinL10 enabled auto-merge (squash) July 9, 2024 21:46
@KevinL10 KevinL10 disabled auto-merge July 9, 2024 21:46
@KevinL10 KevinL10 merged commit 2f04224 into develop Jul 10, 2024
120 of 121 checks passed
@KevinL10 KevinL10 deleted the kevinliu/feat/add-long-animation-frame branch July 10, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants