search

getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
8.02k stars 1.59k forks source link

feat(opentelemetry): Ignore propagation context when not continuing trace #14409

Open mydea opened 4 days ago

mydea commented 4 days ago

Let's see how that would even work...

Implements changes on top of https://github.com/getsentry/sentry-javascript/pull/14426

github-actions[bot] commented 4 days ago

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results. Path Size % Change Change
@sentry/browser 23.01 KB - -
@sentry/browser - with treeshaking flags 21.72 KB - -
@sentry/browser (incl. Tracing) 35.62 KB - -
@sentry/browser (incl. Tracing, Replay) 72.4 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.69 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.7 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 89.17 KB - -
@sentry/browser (incl. Feedback) 39.75 KB - -
@sentry/browser (incl. sendFeedback) 27.64 KB - -
@sentry/browser (incl. FeedbackAsync) 32.43 KB - -
@sentry/react 25.71 KB - -
@sentry/react (incl. Tracing) 38.48 KB - -
@sentry/vue 27.2 KB - -
@sentry/vue (incl. Tracing) 37.43 KB - -
@sentry/svelte 23.18 KB - -
CDN Bundle 24.18 KB - -
CDN Bundle (incl. Tracing) 37.19 KB - -
CDN Bundle (incl. Tracing, Replay) 71.95 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 77.3 KB - -
CDN Bundle - uncompressed 71.15 KB - -
CDN Bundle (incl. Tracing) - uncompressed 110.55 KB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 223.35 KB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 236.57 KB - -
@sentry/nextjs (client) 38.7 KB - -
@sentry/sveltekit (client) 36.13 KB - -
@sentry/node 134.62 KB -0.01% -12 B 🔽
@sentry/node - without tracing 96.45 KB -0.02% -15 B 🔽
@sentry/aws-serverless 106.68 KB -0.02% -13 B 🔽

View base workflow run

codecov[bot] commented 4 days ago

:x: 1 Tests Failed:

Tests completed Failed Passed Skipped
207 1 206 6
View the top 1 failed tests by shortest run time > > ```python > transactions.test.ts Sends an API route transaction to OTLP > ``` > >
Stack Traces | 4.85s run time > > > > > ```python > > transactions.test.ts:4:5 Sends an API route transaction to OTLP > > ``` > >

To view more test analytics, go to the Test Analytics Dashboard Got feedback? Let us know on Github

mydea commented 13 hours ago

Something I noticed but not 100% sure if problematic:

Our startNewTrace API recycles the propagation context and doesn't set a parent span. So I guess in that case, we'd not take the traceId created from startNewTrace but create yet another one? Probably that's even fine but it kinda questions the existance of startNewTrace, right? 😅

Oh I guess it still makes sense when you want to actively start a new trace while there is an active span 🤔

hmm yeah true! But I guess this is still fine in so far as generatePropagationContext() would basically just mean "ignore this propagation context", kind of 🤔 Since the traceId and spanId in there are random anyhow, it should not really matter if we use this or another one, but.... 🤔