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

fix(replay): Fix missing fetch/xhr requests #7134

Merged
merged 1 commit into from Feb 10, 2023

Conversation

billyvg
Copy link
Member

@billyvg billyvg commented Feb 10, 2023

Logic was flipped for the filter function so fetch and xhr requests were not being recorded at all.

Closes https://github.com/getsentry/team-replay/issues/9
Closes #7078

Logic was flipped for the filter function so fetch and xhr requests were not being recorded at all.
@billyvg
Copy link
Member Author

billyvg commented Feb 10, 2023

We should add an integration test for this.

@billyvg billyvg requested review from mydea and Lms24 February 10, 2023 03:57
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.06 KB (-0.02% 🔽)
@sentry/browser - ES5 CDN Bundle (minified) 62.19 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.69 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified) 55.33 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 20.42 KB (0%)
@sentry/browser - Webpack (minified) 66.77 KB (0%)
@sentry/react - Webpack (gzipped + minified) 20.45 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 47.85 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.97 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 25.23 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 42.27 KB (-0.01% 🔽)
@sentry/replay - Webpack (gzipped + minified) 36.7 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 59.93 KB (-0.01% 🔽)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 53.48 KB (-0.01% 🔽)

Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

Damn, good catch! filter boolean responses are always a bit ambiguous 😬

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.

Good catch!

I added a point to our list of integration test scenarios: #7044

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR c3806eb 70.01 ms 98.07 ms +28.06 ms +40.07 % 149.86 ms +79.85 ms +114.05 %
Previous b49d9f7 96.68 ms 129.08 ms +32.40 ms +33.52 % 149.47 ms +52.79 ms +54.61 %
CLS This PR c3806eb 0.06 ms 0.06 ms -0.00 ms -0.12 % 0.06 ms -0.00 ms -0.16 %
Previous b49d9f7 0.06 ms 0.06 ms -0.00 ms -0.38 % 0.06 ms -0.00 ms -0.27 %
CPU This PR c3806eb 16.26 % 14.68 % -1.57 pp -9.67 % 28.36 % +12.10 pp +74.45 %
Previous b49d9f7 22.38 % 21.54 % -0.84 pp -3.76 % 37.84 % +15.46 pp +69.08 %
JS heap avg This PR c3806eb 1.94 MB 1.99 MB +44.35 kB +2.28 % 2.99 MB +1.05 MB +53.81 %
Previous b49d9f7 1.94 MB 1.99 MB +47.17 kB +2.43 % 3.01 MB +1.07 MB +54.98 %
JS heap max This PR c3806eb 2.3 MB 2.56 MB +252.17 kB +10.95 % 4.46 MB +2.16 MB +93.75 %
Previous b49d9f7 2.32 MB 2.56 MB +234.93 kB +10.12 % 4.53 MB +2.21 MB +95.07 %
netTx This PR c3806eb 0 B 0 B 0 B n/a 2.54 kB +2.54 kB n/a
Previous b49d9f7 0 B 0 B 0 B n/a 2.59 kB +2.59 kB n/a
netRx This PR c3806eb 0 B 0 B 0 B n/a 41 B +41 B n/a
Previous b49d9f7 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR c3806eb 0 0 0 n/a 1 +1 n/a
Previous b49d9f7 0 0 0 n/a 1 +1 n/a
netTime This PR c3806eb 0.00 ms 0.00 ms 0.00 ms n/a 93.58 ms +93.58 ms n/a
Previous b49d9f7 0.00 ms 0.00 ms 0.00 ms n/a 70.48 ms +70.48 ms n/a

Previous results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
b49d9f7+52.79 ms-0.00 ms+15.46 pp+1.07 MB+2.21 MB+2.59 kB+41 B+1+70.48 ms
7e57cb7+47.89 ms+0.00 ms+10.59 pp+1.09 MB+2.21 MB+2.59 kB+41 B+1+72.93 ms
538c3a6+63.63 ms+0.00 ms+14.14 pp+1.07 MB+2.16 MB+2.6 kB+41 B+1+112.08 ms
fc7b716+52.44 ms+0.00 ms+14.74 pp+1.09 MB+2.21 MB+2.68 kB+41 B+1+89.64 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Fri, 10 Feb 2023 10:44:36 GMT

@billyvg billyvg marked this pull request as ready for review February 10, 2023 13:56
@billyvg billyvg merged commit 4827b60 into develop Feb 10, 2023
@billyvg billyvg deleted the fix-replay-fix-missing-fetch-xhr-requests branch February 10, 2023 13:57
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.

Replay: Missing fetch/xhr breadcrumbs
3 participants