Skip to content

fix: send buffered telemetry on the background worker - #3079

Open
ikraamg wants to merge 1 commit into
getsentry:masterfrom
ikraamg:buffer-sends-off-thread
Open

fix: send buffered telemetry on the background worker#3079
ikraamg wants to merge 1 commit into
getsentry:masterfrom
ikraamg:buffer-sends-off-thread

Conversation

@ikraamg

@ikraamg ikraamg commented Sep 12, 2026

Copy link
Copy Markdown

Description

TelemetryEventBuffer#send_items runs under @mutex, both from the 5 s timer thread and from the add_item call that reaches max_items. It built the envelope and did the HTTP request to Sentry right there. Two effects:

  • The application thread that added the 100th log event paid a full TCP + TLS + POST round trip inline. With sentry-rails 7.0 turning structured logging on by default with the ActiveRecord subscriber, that is every ~100 SQL statements per process.
  • Every other thread calling add_item (again, every SQL statement) blocked on the mutex behind that send.

This hands the send to Sentry.background_worker, the same path error events and Client#capture_envelope already use. The envelope is still built under the lock; only the network leaves it. The rescue moves inside the posted block so a failed send is logged the way it was, and with background_worker_threads = 0 the behavior is unchanged (immediate executor).

Measured on a Rails API serving ~37 req/s at ~12 queries per request, after upgrading 6.4.1 → 7.0.0 with default settings: the inline flush was 15.8% of request time in an rbspy profile of the Puma workers (Sentry::TelemetryEventBuffer#send_itemsNet::HTTP#connectTCPSocket#initialize), and request p90 stepped from 44 ms to 66 ms in the hour the upgrade deployed while p50 barely moved (28 → 30 ms). Applies to MetricEventBuffer too, since it shares the base class.

Tests: the shared buffer examples get two cases that run a real one-thread worker and assert the envelope is sent off the thread that filled the buffer (both add_item overflow and flush). Full sentry-ruby suite green locally (1520 examples).

🤖 Generated with Claude Code

https://claude.ai/code/session_012QsjWMGPww2cFqGMX71vkB

@ikraamg
ikraamg force-pushed the buffer-sends-off-thread branch 2 times, most recently from 4d9fe38 to f189dda Compare September 12, 2026 20:09
Comment thread sentry-ruby/lib/sentry/telemetry_event_buffer.rb Outdated

@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 1 potential issue.

Fix All in Cursor

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

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f189dda. Configure here.

Comment thread sentry-ruby/lib/sentry/telemetry_event_buffer.rb
TelemetryEventBuffer#send_items runs under @Mutex, from both the timer
thread and the add_item call that reaches max_items. It built the envelope
and did the HTTP request to Sentry right there, so the request thread that
happened to add the 100th log event paid a full TCP + TLS + POST round
trip, and every other thread calling add_item (every SQL statement, with
sentry-rails' ActiveRecord log subscriber) waited on the mutex behind it.

Hand the send to Sentry.background_worker, the same path error events and
Client#capture_envelope already take. The envelope is still built under the
lock; only the network leaves it. The rescue moves inside the posted block
so a failed send is logged the way it was. A full worker queue discards the
block without raising, so that case records the batch as a queue_overflow
client report, as capture_event does for a dropped event.

Measured on a Rails API serving ~37 req/s at ~12 queries each: the inline
flush was 16% of request time and lifted p90 from 44 ms to 66 ms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ikraamg
ikraamg force-pushed the buffer-sends-off-thread branch from f189dda to ea46783 Compare September 12, 2026 20:21
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.

1 participant