diff --git a/CHANGELOG.md b/CHANGELOG.md index e243d3b33..8bc1f26f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## Unreleased + +### Bug Fixes 🐛 + +- Send buffered logs and metrics on the background worker instead of the thread that filled the buffer by @ikraamg in [#3079](https://github.com/getsentry/sentry-ruby/pull/3079) + ## 7.0.0 ### Breaking Changes 🛠 diff --git a/sentry-ruby/lib/sentry/telemetry_event_buffer.rb b/sentry-ruby/lib/sentry/telemetry_event_buffer.rb index 714957943..9adf4a035 100644 --- a/sentry-ruby/lib/sentry/telemetry_event_buffer.rb +++ b/sentry-ruby/lib/sentry/telemetry_event_buffer.rb @@ -125,7 +125,16 @@ def send_items { items: envelope_items } ) - @client.send_envelope(envelope) + # Off the calling thread: send_items runs under @mutex, so a request thread that filled the + # buffer would otherwise hold every other add_item caller behind its HTTP round trip. + queued = Sentry.background_worker.perform do + @client.send_envelope(envelope) + rescue => e + log_error("[#{self.class}] Failed to send #{@event_class}", e, debug: @debug) + end + + # A full worker queue discards the block without raising, the same way it drops an event. + @client.transport.record_lost_event(:queue_overflow, @data_category, num: envelope_items.size) unless queued rescue => e log_error("[#{self.class}] Failed to send #{@event_class}", e, debug: @debug) ensure diff --git a/sentry-ruby/spec/support/shared_examples_for_telemetry_event_buffers.rb b/sentry-ruby/spec/support/shared_examples_for_telemetry_event_buffers.rb index e966f514b..af7b05b3e 100644 --- a/sentry-ruby/spec/support/shared_examples_for_telemetry_event_buffers.rb +++ b/sentry-ruby/spec/support/shared_examples_for_telemetry_event_buffers.rb @@ -58,6 +58,46 @@ end end + describe "sending" do + let(:max_items) { 3 } + + before do + Sentry.configuration.background_worker_threads = 1 + Sentry.background_worker = Sentry::BackgroundWorker.new(Sentry.configuration) + end + + it "sends the envelope on the background worker, not on the thread that filled the buffer" do + sending_thread = nil + allow(client).to receive(:send_envelope) { sending_thread = Thread.current } + + 3.times { subject.add_item(event) } + Sentry.background_worker.shutdown + + expect(sending_thread).not_to be_nil + expect(sending_thread).not_to eq(Thread.current) + end + + it "records the batch as lost when the background worker's queue is full" do + allow(Sentry.background_worker).to receive(:perform).and_return(false) + expect(client.transport).to receive(:record_lost_event).with(:queue_overflow, subject.data_category, num: 3) + + 3.times { subject.add_item(event) } + + expect(subject).to be_empty + end + + it "flushes on the background worker too" do + sending_thread = nil + allow(client).to receive(:send_envelope) { sending_thread = Thread.current } + + subject.add_item(event) + subject.flush + Sentry.background_worker.shutdown + + expect(sending_thread).not_to eq(Thread.current) + end + end + describe "multi-threaded access" do let(:max_items) { 30 }