Skip to content

fix(v10/core): Don't instrument the SDK's own envelope requests - #24276

Merged
logaretm merged 2 commits into
v10from
awad/fix-client-report-exit-loop
Sep 15, 2026
Merged

logaretm merged 2 commits into
v10from
awad/fix-client-report-exit-loop

Conversation

@logaretm

@logaretm logaretm commented Sep 10, 2026

Copy link
Copy Markdown
Member

Stops the outgoing-http instrumentation from instrumenting Sentry's own envelope sends.

It recognized them only via suppressTracing(), which is unreadable when the SDK doesn't own the OTel setup, since no context manager propagates it. Each send then started a span that got dropped as no_parent_span, and since 10.72 the session sent on beforeExit seeds the first outcome, so the client report flush feeds itself and the process never exits. Matching the ingest URL instead doesn't depend on context propagation.

closes #24262

@logaretm
logaretm marked this pull request as ready for review September 10, 2026 03:09
@logaretm
logaretm requested a review from a team as a code owner September 10, 2026 03:09
@logaretm
logaretm requested review from JPeer264, andreiborza and isaacs and removed request for a team September 10, 2026 03:09

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 06819f1. Configure here.

Comment thread dev-packages/node-integration-tests/suites/client-reports/exit-flush-loop/test.ts Outdated
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size % Change Change
@sentry/browser 27.84 kB added added
@sentry/browser - with treeshaking flags 26.27 kB added added
@sentry/browser (incl. Tracing) 46.7 kB added added
@sentry/browser (incl. Tracing + Span Streaming) 48.48 kB added added
@sentry/browser (incl. Tracing, Profiling) 51.45 kB added added
@sentry/browser (incl. Tracing, Replay) 86 kB added added
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 75.61 kB added added
@sentry/browser (incl. Tracing, Replay with Canvas) 90.73 kB added added
@sentry/browser (incl. Tracing, Replay, Feedback) 103.37 kB added added
@sentry/browser (incl. Feedback) 44.99 kB added added
@sentry/browser (incl. sendFeedback) 32.63 kB added added
@sentry/browser (incl. FeedbackAsync) 37.77 kB added added
@sentry/browser (incl. Metrics) 28.93 kB added added
@sentry/browser (incl. Logs) 29.16 kB added added
@sentry/browser (incl. Metrics & Logs) 29.85 kB added added
@sentry/react 29.63 kB added added
@sentry/react (incl. Tracing) 48.96 kB added added
@sentry/vue 33.25 kB added added
@sentry/vue (incl. Tracing) 48.69 kB added added
@sentry/svelte 27.86 kB added added
CDN Bundle 30.23 kB added added
CDN Bundle (incl. Tracing) 48.66 kB added added
CDN Bundle (incl. Logs, Metrics) 31.81 kB added added
CDN Bundle (incl. Tracing, Logs, Metrics) 49.98 kB added added
CDN Bundle (incl. Replay, Logs, Metrics) 71.16 kB added added
CDN Bundle (incl. Tracing, Replay) 86.24 kB added added
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 87.55 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback) 92.03 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 93.33 kB added added
CDN Bundle - uncompressed 90.04 kB added added
CDN Bundle (incl. Tracing) - uncompressed 146.85 kB added added
CDN Bundle (incl. Logs, Metrics) - uncompressed 94.75 kB added added
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 150.83 kB added added
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 219.71 kB added added
CDN Bundle (incl. Tracing, Replay) - uncompressed 266.28 kB added added
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 270.25 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 279.98 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 283.94 kB added added
@sentry/nextjs (client) 51.52 kB added added
@sentry/sveltekit (client) 47.13 kB added added
@sentry/core/server 80.85 kB added added
@sentry/core/browser 67.02 kB added added
@sentry/node-core 63.53 kB added added
@sentry/node 126.22 kB added added
@sentry/node (incl. diagnostics channel injection) 170.36 kB added added
@sentry/node/import (ESM hook with diagnostics-channel injection) 166 B added added
@sentry/node/light 51.68 kB added added
@sentry/node - without tracing 75.26 kB added added
@sentry/aws-serverless 84.48 kB added added
@sentry/cloudflare (withSentry) - minified 202.95 kB added added
@sentry/cloudflare (withSentry) 499.86 kB added added

@github-actions

Copy link
Copy Markdown
Contributor

👋 @isaacs, @JPeer264, @andreiborza — Please review this PR when you get a chance!

@JPeer264 JPeer264 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. Two questions though:

  1. Don't we have this issue in v11 ? Since this is not a classic backport PR
  2. Do you know which PR introduced this behavior?

@logaretm

logaretm commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

@JPeer264 It looks accidental to me, both that it happens in v10 and that it doesn't in v11

  • v11 doesn't use the OTel context, and with enableOpenTelemetrySetup: true the async context strategy registers the context manager itself so suppressTracing stays working. Not a deliberate fix tho, v11 just never has the broken combo.
  • #23725/#23731 flipped a check that made this reachable for every healthy process, but #20393 is where it loops.

@JPeer264

Copy link
Copy Markdown
Member

Cool thanks.

The outgoing-http subscription skipped Sentry's own transport requests by checking
the scope for an active `suppressTracing()`. That check is unreadable when the SDK
does not own the OpenTelemetry setup (`skipOpenTelemetrySetup: true`, or any other
setup that leaves the OTel API without a registered context manager): the
suppression is written to a context that nothing propagates.

Each envelope send then started an outgoing-request span, and with tracing off that
span was dropped with a `no_parent_span` outcome. Since 10.72 a healthy process also
sends its session on `beforeExit`, which seeded the first outcome, and from there the
client report flush on `beforeExit` fed itself: flush -> instrumented request ->
new outcome -> `beforeExit` again. The process never exited and hammered the DSN
with client reports.

Requests to the ingest endpoint are now recognized by URL, which does not depend on
context propagation.

Fixes #24262
The watchdog marker meant a scenario that died on startup looked the same as a clean
exit, so the test passed with and without the fix when the worktree's node_modules
were broken. Polling for the child's exit with `ensureNoErrorOutput` separates the
two, and it keeps the passing run under a second instead of always sleeping 5s.
@logaretm
logaretm force-pushed the awad/fix-client-report-exit-loop branch from 6c0b949 to 85ce063 Compare September 15, 2026 14:37
@logaretm
logaretm merged commit 738f17e into v10 Sep 15, 2026
617 of 621 checks passed
@logaretm
logaretm deleted the awad/fix-client-report-exit-loop branch September 15, 2026 15:37
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.

2 participants