Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(core): Capture # of dropped spans through beforeSendTransaction #13022

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Jul 23, 2024

Fixes #12849.

This is a bit tricky because beforeSendTransaction can return a promise, so in order to avoid dealing with this I put the # of spans on the sdk metadata, and then compare that afterwards.

@mydea mydea requested review from cleptric, Lms24 and s1gr1d July 23, 2024 15:18
@mydea mydea self-assigned this Jul 23, 2024
const spans = event.spans || [];
// the transaction itself counts as one span, plus all the child spans that are added
const spanCount = 1 + spans.length;
this._outcomes['span'] = (this._outcomes['span'] || 0) + spanCount;
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed that this was actually buggy, because the key is built from reason + category 😬 now this is unified and should work consistently.

Copy link
Contributor

github-actions bot commented Jul 23, 2024

size-limit report 📦

Path Size
@sentry/browser 22.4 KB (+0.43% 🔺)
@sentry/browser (incl. Tracing) 33.85 KB (+0.37% 🔺)
@sentry/browser (incl. Tracing, Replay) 69.91 KB (+0.14% 🔺)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.25 KB (+0.15% 🔺)
@sentry/browser (incl. Tracing, Replay with Canvas) 74.31 KB (+0.13% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback) 86.68 KB (+0.12% 🔺)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 88.56 KB (+0.14% 🔺)
@sentry/browser (incl. metrics) 26.71 KB (+0.34% 🔺)
@sentry/browser (incl. Feedback) 39.11 KB (+0.23% 🔺)
@sentry/browser (incl. sendFeedback) 27.02 KB (+0.33% 🔺)
@sentry/browser (incl. FeedbackAsync) 31.66 KB (+0.27% 🔺)
@sentry/react 25.17 KB (+0.37% 🔺)
@sentry/react (incl. Tracing) 36.9 KB (+0.27% 🔺)
@sentry/vue 26.55 KB (+0.39% 🔺)
@sentry/vue (incl. Tracing) 35.7 KB (+0.25% 🔺)
@sentry/svelte 22.54 KB (+0.42% 🔺)
CDN Bundle 23.59 KB (+0.32% 🔺)
CDN Bundle (incl. Tracing) 35.58 KB (+0.22% 🔺)
CDN Bundle (incl. Tracing, Replay) 69.95 KB (+0.13% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) 75.22 KB (+0.1% 🔺)
CDN Bundle - uncompressed 69.32 KB (+0.47% 🔺)
CDN Bundle (incl. Tracing) - uncompressed 105.42 KB (+0.31% 🔺)
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.06 KB (+0.15% 🔺)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 229.89 KB (+0.14% 🔺)
@sentry/nextjs (client) 36.75 KB (+0.26% 🔺)
@sentry/sveltekit (client) 34.49 KB (+0.29% 🔺)
@sentry/node 111.86 KB (+0.09% 🔺)
@sentry/node - without tracing 89.3 KB (+0.1% 🔺)
@sentry/aws-serverless 98.44 KB (+0.1% 🔺)

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.

l: no particular strong feelings given these are client reports but should we add a test for them?

@mydea
Copy link
Member Author

mydea commented Jul 24, 2024

l: no particular strong feelings given these are client reports but should we add a test for them?

Yeah I thought about this too, I'll add some I think!

@mydea mydea force-pushed the fn/dropped-spans-beforeSendTransaction branch from c67e225 to aeefa66 Compare July 24, 2024 08:20
@mydea mydea force-pushed the fn/dropped-spans-beforeSendTransaction branch from 15d9059 to f6ddc1c Compare July 25, 2024 12:13
@mydea mydea merged commit df45d65 into develop Jul 29, 2024
127 checks passed
@mydea mydea deleted the fn/dropped-spans-beforeSendTransaction branch July 29, 2024 07:00
krystofwoldrich pushed a commit that referenced this pull request Aug 13, 2024
#13022)

Fixes #12849.

This is a bit tricky because `beforeSendTransaction` can return a
promise, so in order to avoid dealing with this I put the # of spans on
the sdk metadata, and then compare that afterwards.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Record spans dropped via beforeSendTransaction
2 participants