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(core): Exclude client reports from offline queuing #7226

Merged

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Feb 19, 2023

If a user sets any transportOptions, the transport is used directly to send client reports rather than beacon which means they can get queued by the offline transport wrapper.

Client reports can be generated every time an event fails to send and these could fill the queue when offline and mean error events get dropped.

This PR modifies forEachEnvelopeItem to allow short-cut out of the for loop if the callback returns true.

@timfish timfish force-pushed the fix/exlcude-client-reports-from-offline branch from 73dc1c2 to 5693635 Compare February 19, 2023 14:10
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.

LGTM! 🚀

@github-actions
Copy link
Contributor

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR fad6c48 94.70 ms 120.99 ms +26.29 ms +27.76 % 135.26 ms +40.56 ms +42.82 %
Previous c46c56c 70.62 ms 92.44 ms +21.82 ms +30.89 % 136.08 ms +65.45 ms +92.68 %
CLS This PR fad6c48 0.06 ms 0.06 ms -0.00 ms -0.07 % 0.06 ms -0.00 ms -0.31 %
Previous c46c56c 0.06 ms 0.06 ms -0.00 ms -0.01 % 0.06 ms -0.00 ms -0.49 %
CPU This PR fad6c48 19.85 % 23.19 % +3.34 pp +16.86 % 29.32 % +9.48 pp +47.76 %
Previous c46c56c 12.93 % 13.56 % +0.63 pp +4.89 % 18.31 % +5.38 pp +41.58 %
JS heap avg This PR fad6c48 1.94 MB 2.05 MB +116.35 kB +6.00 % 2.86 MB +924.32 kB +47.69 %
Previous c46c56c 1.94 MB 1.99 MB +51.45 kB +2.65 % 2.87 MB +930.26 kB +47.99 %
JS heap max This PR fad6c48 2.3 MB 2.59 MB +283.01 kB +12.28 % 3.35 MB +1.05 MB +45.50 %
Previous c46c56c 2.3 MB 2.56 MB +257 kB +11.16 % 3.37 MB +1.07 MB +46.32 %
netTx This PR fad6c48 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
Previous c46c56c 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
netRx This PR fad6c48 0 B 0 B 0 B n/a 41 B +41 B n/a
Previous c46c56c 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR fad6c48 0 0 0 n/a 1 +1 n/a
Previous c46c56c 0 0 0 n/a 1 +1 n/a
netTime This PR fad6c48 0.00 ms 0.00 ms 0.00 ms n/a 107.02 ms +107.02 ms n/a
Previous c46c56c 0.00 ms 0.00 ms 0.00 ms n/a 91.29 ms +91.29 ms n/a

Previous results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
c46c56c+65.45 ms-0.00 ms+5.38 pp+930.26 kB+1.07 MB+2.21 kB+41 B+1+91.29 ms
7f4c4ec+56.64 ms-0.00 ms+5.57 pp+927.42 kB+1.06 MB+2.21 kB+41 B+1+110.83 ms
00d2360+55.18 ms+0.00 ms+2.23 pp+934.14 kB+1.05 MB+2.22 kB+41 B+1+71.65 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Mon, 20 Feb 2023 10:14:54 GMT

@mydea mydea merged commit 12e34d4 into getsentry:develop Feb 20, 2023
@timfish timfish deleted the fix/exlcude-client-reports-from-offline branch February 20, 2023 13:45
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

2 participants