Skip to content
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

ci(replay): Overhead measurement #6611

Merged
merged 55 commits into from
Jan 16, 2023
Merged

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Dec 22, 2022

implements #6503

image

Progress:

  • metrics collection (Vitals, CPU, JS Heap)
  • retry & multi-run logic
  • current run analysis
  • baseline run analysis
  • implement replay test app
  • run in CI
  • add PR comments using GH APIs
  • download baseline data from main branch using GH APIs

Outstanding/known issues

  • A comment is not produced for PRs running from forks - this is due to action security policies. There is a non-trivial workaround so if it's necessary to have these metrics from fork-originating PRs, I can look into that too (as a follow up).
  • I've seen (multiple times) that the app that includes replay somehow blocks UI and browser during shutdown - maybe this needs more investigation or is it a known limitation?

@vaind vaind changed the title chore: Replay SDK overhead measurement chore: Replay SDK overhead measurement [WIP] Dec 22, 2022
@vaind vaind changed the title chore: Replay SDK overhead measurement [WIP] ci(replay): Overhead measurement [WIP] Dec 22, 2022
@vaind vaind force-pushed the chore/replay-metrics branch 9 times, most recently from 2d7bad6 to 88ae1ec Compare January 2, 2023 19:33
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Hi @vaind! Great to see progress on the overhead measurements!

I haven't looked too closely over this PR but I was just wondering if there's a specific reason you chose puppeteer over e.g. playwright? The reason I'm asking is that long-term, we'd like to remove Puppeteer in favour of Playwright as it's causing quite a few issues for us and we'Ve had good experiences with Playwright so far. Therefore I'd like to avoid using it in new projects we add to the repo.

I know that you're not yet finished but feel free to tag me or anyone from the team if you have questions. Cheers!

@vaind
Copy link
Contributor Author

vaind commented Jan 3, 2023

Thanks for having a first look @Lms24. As for Puppeteer - no specific reason, I've seen it used somewhere in the repo so went for it. Switched to playwright now 👍

@vaind vaind force-pushed the chore/replay-metrics branch 13 times, most recently from 05c34af to a6869aa Compare January 6, 2023 12:18
@vaind vaind marked this pull request as ready for review January 6, 2023 13:18
@vaind vaind changed the title ci(replay): Overhead measurement [WIP] ci(replay): Overhead measurement Jan 6, 2023
@vaind vaind force-pushed the chore/replay-metrics branch 2 times, most recently from af04359 to 7a4e90f Compare January 9, 2023 15:00
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Thanks for addressing our feedback! Will merge it soon

@Lms24 Lms24 merged commit 1e72c0c into getsentry:master Jan 16, 2023
@vaind vaind deleted the chore/replay-metrics branch January 16, 2023 14:09
billyvg added a commit to getsentry/web-benchmark that referenced this pull request Jun 22, 2023
Thanks to @vaind for their initial work on this at getsentry/sentry-javascript#6611. We wanted to extract this to a separate tool so it can be run outside of the `sentry-javascript` repo.
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.

None yet

4 participants