ref(envelopes): Move rate limiting logic to each item in envelope (#1742

)

* ref(envelopes): Add send_envelope and move rate limiting filtering to items level

* Code style and changelog entry

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Refactoring
 
 - Encapsulate extension helpers [#1725](https://github.com/getsentry/sentry-ruby/pull/1725)
+- Move rate limiting logic to each item in envelope [#1742](https://github.com/getsentry/sentry-ruby/pull/1742)
 
 ## 5.1.0
 

sentry-ruby/lib/sentry/envelope.rb

@@ -3,24 +3,47 @@
 module Sentry
   # @api private
   class Envelope
-    def initialize(headers)
+    class Item
+      attr_accessor :headers, :payload
+
+      def initialize(headers, payload)
+        @headers = headers
+        @payload = payload
+      end
+
+      def type
+        @headers[:type] || 'event'
+      end
+
+      def to_s
+        <<~ITEM
+          #{JSON.generate(@headers)}
+          #{JSON.generate(@payload)}
+        ITEM
+      end
+    end
+
+    attr_accessor :headers, :items
+
+    def initialize(headers = {})
       @headers = headers
       @items = []
     end
 
     def add_item(headers, payload)
-      @items << [headers, payload]
+      @items << Item.new(headers, payload)
     end
 
     def to_s
-      payload = @items.map do |item_headers, item_payload|
-        <<~ENVELOPE
-          #{JSON.generate(item_headers)}
-          #{JSON.generate(item_payload)}
-        ENVELOPE
-      end.join("\n")
-
-      "#{JSON.generate(@headers)}\n#{payload}"
+      [JSON.generate(@headers), *@items.map(&:to_s)].join("\n")
+    end
+
+    def item_types
+      @items.map(&:type)
+    end
+
+    def event_id
+      @headers[:event_id]
     end
   end
 end

sentry-ruby/lib/sentry/transport.rb

@@ -46,23 +46,19 @@ def send_data(data, options = {})
     end
 
     def send_event(event)
-      event_hash = event.to_hash
-      item_type = get_item_type(event_hash)
+      envelope = envelope_from_event(event)
+      send_envelope(envelope)
 
-      if is_rate_limited?(item_type)
-        log_info("Envelope [#{item_type}] not sent: rate limiting")
-        record_lost_event(:ratelimit_backoff, item_type)
-
-        return
-      end
-
-      encoded_data = encode(event)
+      event
+    end
 
-      return nil unless encoded_data
+    def send_envelope(envelope)
+      reject_rate_limited_items(envelope)
 
-      send_data(encoded_data)
+      return if envelope.items.empty?
 
-      event
+      log_info("[Transport] Sending envelope with items [#{envelope.item_types.join(', ')}] #{envelope.event_id} to Sentry")
+      send_data(envelope.to_s)
     end
 
     def is_rate_limited?(item_type)
@@ -106,7 +102,7 @@ def generate_auth_header
       'Sentry ' + fields.map { |key, value| "#{key}=#{value}" }.join(', ')
     end
 
-    def encode(event)
+    def envelope_from_event(event)
       # Convert to hash
       event_payload = event.to_hash
       event_id = event_payload[:event_id] || event_payload["event_id"]
@@ -129,9 +125,8 @@ def encode(event)
       client_report_headers, client_report_payload = fetch_pending_client_report
       envelope.add_item(client_report_headers, client_report_payload) if client_report_headers
 
-      log_info("Sending envelope [#{item_type}] #{event_id} to Sentry")
 
-      envelope.to_s
+      envelope
     end
 
     def record_lost_event(reason, item_type)
@@ -173,6 +168,19 @@ def fetch_pending_client_report
 
       [item_header, item_payload]
     end
+
+    def reject_rate_limited_items(envelope)
+      envelope.items.reject! do |item|
+        if is_rate_limited?(item.type)
+          log_info("[Transport] Envelope item [#{item.type}] not sent: rate limiting")
+          record_lost_event(:ratelimit_backoff, item.type)
+
+          true
+        else
+          false
+        end
+      end
+    end
   end
 end
 

sentry-ruby/spec/sentry/rake_spec.rb

@@ -10,7 +10,7 @@
       message = `cd spec/support && bundle exec rake raise_exception 2>&1`
     end.join
 
-    expect(message).to match(/Sending envelope \[event\] [abcdef0-9]+ to Sentry/)
+    expect(message).to match(/\[Transport\] Sending envelope with items \[event\] [abcdef0-9]+ to Sentry/)
   end
 
   it "skip sending report to Sentry when skip_rake_integration = true" do

sentry-ruby/spec/sentry/transport/http_transport_rate_limiting_spec.rb

@@ -13,7 +13,7 @@
   end
   let(:client) { Sentry.get_current_client }
   let(:data) do
-    subject.encode(client.event_from_message("foobarbaz").to_hash)
+    subject.envelope_from_event(client.event_from_message("foobarbaz").to_hash).to_s
   end
 
   subject { Sentry::HTTPTransport.new(configuration) }

sentry-ruby/spec/sentry/transport/http_transport_spec.rb

@@ -13,7 +13,7 @@
   let(:client) { Sentry::Client.new(configuration) }
   let(:event) { client.event_from_message("foobarbaz") }
   let(:data) do
-    subject.encode(event.to_hash)
+    subject.envelope_from_event(event.to_hash).to_s
   end
 
   subject { client.transport }

sentry-ruby/spec/sentry/transport_spec.rb

@@ -18,7 +18,7 @@
 
   subject { client.transport }
 
-  describe "#encode" do
+  describe "#envelope_from_event" do
 
     before do
       Sentry.init do |config|
@@ -29,7 +29,7 @@
     context "normal event" do
       let(:event) { client.event_from_exception(ZeroDivisionError.new("divided by 0")) }
       it "generates correct envelope content" do
-        result = subject.encode(event.to_hash)
+        result = subject.envelope_from_event(event.to_hash).to_s
 
         envelope_header, item_header, item = result.split("\n")
 
@@ -56,7 +56,7 @@
       end
 
       it "generates correct envelope content" do
-        result = subject.encode(event.to_hash)
+        result = subject.envelope_from_event(event.to_hash).to_s
 
         envelope_header, item_header, item = result.split("\n")
 
@@ -83,7 +83,7 @@
 
       it "incudes client report in envelope" do
         Timecop.travel(Time.now + 90) do
-          result = subject.encode(event.to_hash)
+          result = subject.envelope_from_event(event.to_hash).to_s
 
           client_report_header, client_report_payload = result.split("\n").last(2)
 
@@ -130,7 +130,7 @@
         expect(subject.send_event(event)).to eq(event)
 
         expect(io.string).to match(
-          /INFO -- sentry: Sending envelope \[event\] #{event.event_id} to Sentry/
+          /INFO -- sentry: \[Transport\] Sending envelope with items \[event\] #{event.event_id} to Sentry/
         )
       end
     end

sentry-ruby/spec/sentry_spec.rb

@@ -119,7 +119,7 @@
       it "stops the event and logs correct message" do
         described_class.send(capture_helper, capture_subject)
 
-        expect(string_io.string).to match(/Envelope \[event\] not sent: rate limiting/)
+        expect(string_io.string).to match(/\[Transport\] Envelope item \[event\] not sent: rate limiting/)
       end
     end
   end