Skip to content

fix(core): Exclude client reports from offline queuing - #7226

Merged
mydea merged 3 commits into
getsentry:developfrom
timfish:fix/exlcude-client-reports-from-offline
Feb 20, 2023
Merged

fix(core): Exclude client reports from offline queuing#7226
mydea merged 3 commits into
getsentry:developfrom
timfish:fix/exlcude-client-reports-from-offline

Conversation

@timfish

@timfishtimfish commented Feb 19, 2023

Copy link
Copy Markdown
Collaborator

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
timfishforce-pushed the fix/exlcude-client-reports-from-offline branch from 73dc1c2 to 5693635CompareFebruary 19, 2023 14:10
Comment threadpackages/utils/src/envelope.ts Outdated
Comment threadpackages/utils/src/envelope.ts Outdated

@mydeamydea left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor

Replay SDK metrics 🚀

Plain+Sentry+Replay
RevisionValueValueDiffRatioValueDiffRatio
LCPThis PR fad6c4894.70 ms120.99 ms+26.29 ms+27.76 %135.26 ms+40.56 ms+42.82 %
Previous c46c56c70.62 ms92.44 ms+21.82 ms+30.89 %136.08 ms+65.45 ms+92.68 %
CLSThis PR fad6c480.06 ms0.06 ms-0.00 ms-0.07 %0.06 ms-0.00 ms-0.31 %
Previous c46c56c0.06 ms0.06 ms-0.00 ms-0.01 %0.06 ms-0.00 ms-0.49 %
CPUThis PR fad6c4819.85 %23.19 %+3.34 pp+16.86 %29.32 %+9.48 pp+47.76 %
Previous c46c56c12.93 %13.56 %+0.63 pp+4.89 %18.31 %+5.38 pp+41.58 %
JS heap avgThis PR fad6c481.94 MB2.05 MB+116.35 kB+6.00 %2.86 MB+924.32 kB+47.69 %
Previous c46c56c1.94 MB1.99 MB+51.45 kB+2.65 %2.87 MB+930.26 kB+47.99 %
JS heap maxThis PR fad6c482.3 MB2.59 MB+283.01 kB+12.28 %3.35 MB+1.05 MB+45.50 %
Previous c46c56c2.3 MB2.56 MB+257 kB+11.16 %3.37 MB+1.07 MB+46.32 %
netTxThis PR fad6c480 B0 B0 Bn/a2.21 kB+2.21 kBn/a
Previous c46c56c0 B0 B0 Bn/a2.21 kB+2.21 kBn/a
netRxThis PR fad6c480 B0 B0 Bn/a41 B+41 Bn/a
Previous c46c56c0 B0 B0 Bn/a41 B+41 Bn/a
netCountThis PR fad6c48000n/a1+1n/a
Previous c46c56c000n/a1+1n/a
netTimeThis PR fad6c480.00 ms0.00 ms0.00 msn/a107.02 ms+107.02 msn/a
Previous c46c56c0.00 ms0.00 ms0.00 msn/a91.29 ms+91.29 msn/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:developFeb 20, 2023
@timfish
timfish deleted the fix/exlcude-client-reports-from-offline branch February 20, 2023 13:45
Sign up for freeto 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

@timfish@mydea