Skip to content

feat(replay): Migrate sentry-replay into monorepo #6270

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 355 commits into from
Nov 24, 2022
Merged

Conversation

mydea
Copy link
Member

@mydea mydea commented Nov 23, 2022

This PR migrates the whole code from https://github.com/getsentry/sentry-replay into the monorepo.

Note:

  • Build, lint and tests work, but will need adjustments
  • It is excluded from releasing in craft
  • Some eslint rules etc. need to be fixed in later steps

Closes #6257

billyvg and others added 30 commits June 1, 2022 07:10
…sentry/sentry-replay#75)

* Remove `@sentry/tracing`
* Clean up peer deps
* Upgrade `rollup-plugin-resolve-node` (deprecated) to `@rollup/plugin-resolve-node`
We were still looking for transactions for the root event, but they are normal events now.
…sentry-replay#82)

This moves the core SDK instrumentation handlers into the main plugin class as the typings are more accurate and allows better testing of the handler data transforms as well.
Refactor this handler so that we can re-use `addReplayEvent`. This will help us apply the same logic for our core SDK instrumentation handlers.
…ntry-replay#83)

Previously we did not create replay events after receiving new updates from core SDK (e.g. breadcrumbs) -- they only get flushed after an rrweb event.

This causes breadcrumbs to be attached to a new session instead of the previous session.

e.g. old session -> breadcrumbs -> <idle> (no upload happens)
 --> new session -> rrweb checkout -> upload (with old breadcrumbs)

 Fixes getsentry/sentry-replay#79
Adds an explicit breadcrumb for when a page is unloaded
Add an EventBuffer that compresses events in a webworker.
)

Move typescript and rollup configuration files to `config/`. The root `tsconfig` is required for dev builds and for tooling (e.g. IDEs) to work. It also includes typechecking for tests, which are ignored in production builds.
This would provide a better interface for the session. This fixes/simplifies updating the session directly where previously it would not save to storage, only the session "instance" (object).
…ION` (getsentry/sentry-replay#98)

We are noticing a lot of short sessions that I suspect is related to the visibility change timeout. Keeping the code around and changing the constants in case we decide to tweak this later.
…ry-replay#102)

We're going to axe the "visible" breadcrumb, as it is a point of confusion for users (i.e. what is the difference between blur/focus and visibility?) We will be able to get the information we need with focus/blur alone.
…y-replay#108)

* Move `@sentry/browser` to devDeps as it is only used in tests.
* Import from `@sentry/core` instead of `@sentry/browser`
* Clean up type imports from `@sentry/hub`
…ay#106)

I suspect we have a lot of "empty" replays due to this issue where we are immediately creating the root replay event. This had the possibility of having a replay w/ no recording events. Unfortunately, after some testing, a lot of short replays still get through because we immediately flush updates after a full DOM checkout. However, this is still needed in order to implement getsentry/sentry-replay#101.
…rong (getsentry/sentry-replay#110)

* `captureReplay` needs to happen before `sendReplay`
* `captureEvent` should only happen after a *successful* `sendReplay`
* fix `sendReplay` promises where it now awaits the retries before resolving
Change the logic for when to flush after a full snapshot checkout. Add
option `initialFlushDelay` to control when to flush.

Do NOT flush on checkout after a session has expired. It will need to
wait for additional user actions before recording the new sessions
replay.
@mydea mydea added the Package: replay Issues related to the Sentry Replay SDK label Nov 23, 2022
@mydea mydea assigned mydea and Lms24 Nov 23, 2022
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.

🚀 🚀 🚀

@AbhiPrasad
Copy link
Member

So we are going to rebase and merge onto the repo right?

}

set previousSessionId(id: string | undefined) {
this._previousSessionId = id;

Check failure

Code scanning / CodeQL

Insecure randomness

This security context depends on a cryptographically insecure random number at [Math.random()](1).
}

if (session.id !== this.session?.id) {
session.previousSessionId = this.session?.id;

Check failure

Code scanning / CodeQL

Insecure randomness

This security context depends on a cryptographically insecure random number at [Math.random()](1).
@mydea
Copy link
Member Author

mydea commented Nov 23, 2022

To log this here, we explored the different options, and basically merging via merge commit is the only really feasible one.

Squashing would lose all git history of replay.
Rebasing is not possible, as basically all "early" commits of replay have conflicts (e.g. package.json, ... all files in the root of the project).

So we will merge in from an orphaned branch. This is what this will look like:

Screenshot 2022-11-23 at 15 43 15

(not perfect, declaration files are stil not in the right spot)
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.5 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 60.29 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.16 KB (-0.01% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 53.63 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 19.9 KB (0%)
@sentry/browser - Webpack (minified) 65.12 KB (0%)
@sentry/react - Webpack (gzipped + minified) 19.92 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 45.93 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.37 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.78 KB (+0.01% 🔺)

@Lms24 Lms24 merged commit 6de1dd8 into master Nov 24, 2022
@Lms24 Lms24 deleted the replay-migration branch November 24, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate sentry-replay into sentry-javascript
9 participants