Skip to content

fix(node): Don't leak __span property into breadcrumbs #14798

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 3 commits into from
Dec 19, 2024

Conversation

lforst
Copy link
Member

@lforst lforst commented Dec 19, 2024

It seems like the fact that fetch breadcrumbs had the __span property was an accident since like forever. This PR makes it more explicit what we put in the breadcrumb data.

Copy link
Contributor

github-actions bot commented Dec 19, 2024

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.16 KB +0.13% +29 B 🔺
@sentry/browser - with treeshaking flags 21.83 KB +0.12% +25 B 🔺
@sentry/browser (incl. Tracing) 35.66 KB +0.14% +48 B 🔺
@sentry/browser (incl. Tracing, Replay) 72.92 KB +0.05% +30 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 63.31 KB +0.04% +22 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 77.32 KB +0.04% +29 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 89.72 KB +0.04% +32 B 🔺
@sentry/browser (incl. Feedback) 39.91 KB +0.08% +30 B 🔺
@sentry/browser (incl. sendFeedback) 27.77 KB +0.1% +28 B 🔺
@sentry/browser (incl. FeedbackAsync) 32.53 KB +0.09% +28 B 🔺
@sentry/react 25.88 KB +0.09% +23 B 🔺
@sentry/react (incl. Tracing) 38.52 KB +0.09% +33 B 🔺
@sentry/vue 27.38 KB +0.09% +24 B 🔺
@sentry/vue (incl. Tracing) 37.52 KB +0.07% +24 B 🔺
@sentry/svelte 23.33 KB +0.11% +24 B 🔺
CDN Bundle 24.24 KB +0.11% +26 B 🔺
CDN Bundle (incl. Tracing) 35.79 KB +0.07% +23 B 🔺
CDN Bundle (incl. Tracing, Replay) 70.9 KB +0.04% +24 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 76.2 KB +0.04% +27 B 🔺
CDN Bundle - uncompressed 71.24 KB +0.21% +151 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 106.75 KB +0.14% +151 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 220.82 KB +0.07% +151 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 234.02 KB +0.07% +151 B 🔺
@sentry/nextjs (client) 38.78 KB +0.07% +24 B 🔺
@sentry/sveltekit (client) 36.2 KB +0.08% +28 B 🔺
@sentry/node 157.95 KB - -
@sentry/node - without tracing 98.84 KB +0.01% +2 B 🔺
@sentry/aws-serverless 126.57 KB - -

View base workflow run

Copy link

codecov bot commented Dec 19, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
622 1 621 297
View the top 1 failed tests by shortest run time
middleware.test.ts Should trace outgoing fetch requests inside middleware and create breadcrumbs for it
Stack Traces | 0.087s run time
middleware.test.ts:55:5 Should trace outgoing fetch requests inside middleware and create breadcrumbs for it

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@lforst lforst requested review from mydea and AbhiPrasad December 19, 2024 12:05
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

There's an argument that this is valuable because it allows people to relate breadcrumbs to spans in a trace.

I'm fine to remove this for now though.

@lforst lforst merged commit 9308877 into develop Dec 19, 2024
151 checks passed
@lforst lforst deleted the lforst-dont-leak-underscoreunderscorespan branch December 19, 2024 14:12
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