Move envelope item processing/trimming logic to the Item class (#1824)

* Extract item trimming logic from Transport Item should take care of this check/trimming process and return the best result it can provide, instead of going back-and-forth with the Transport class. * Move MAX_SERIALIZED_PAYLOAD_SIZE to Item class The size limitation is Item's concern instead of Event's, so it makes more sense to define it in the Item class. * Item should encapsulate all the size-related logic * Change log level for dropping oversized items We use info level for other event related actions (dropping/sending). So we should also use the same level when an event is being dropped due to oversize. * Update changelog
getsentry · May 20, 2022 · 4ba0ca2 · 4ba0ca2
1 parent cb8c015
commit 4ba0ca2
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 51 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@
 
 - Handle exception with large stacktrace without dropping entire item [#1807](https://github.com/getsentry/sentry-ruby/pull/1807)
 
+### Refactoring
+
+- Move envelope item processing/trimming logic to the Item class [#1824](https://github.com/getsentry/sentry-ruby/pull/1824)
+
 ## 5.3.1
 
 ### Bug Fixes

diff --git a/sentry-ruby/lib/sentry/envelope.rb b/sentry-ruby/lib/sentry/envelope.rb
@@ -4,6 +4,9 @@ module Sentry
   # @api private
   class Envelope
     class Item
+      STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD = 500
+      MAX_SERIALIZED_PAYLOAD_SIZE = 1024 * 200
+
       attr_accessor :headers, :payload
 
       def initialize(headers, payload)
@@ -21,6 +24,54 @@ def to_s
           #{JSON.generate(@payload)}
         ITEM
       end
+
+      def serialize
+        result = to_s
+
+        if result.bytesize > MAX_SERIALIZED_PAYLOAD_SIZE
+          remove_breadcrumbs!
+          result = to_s
+        end
+
+        if result.bytesize > MAX_SERIALIZED_PAYLOAD_SIZE
+          reduce_stacktrace!
+          result = to_s
+        end
+
+        [result, result.bytesize > MAX_SERIALIZED_PAYLOAD_SIZE]
+      end
+
+      def size_breakdown
+        payload.map do |key, value|
+          "#{key}: #{JSON.generate(value).bytesize}"
+        end.join(", ")
+      end
+
+      private
+
+      def remove_breadcrumbs!
+        if payload.key?(:breadcrumbs)
+          payload.delete(:breadcrumbs)
+        elsif payload.key?("breadcrumbs")
+          payload.delete("breadcrumbs")
+        end
+      end
+
+      def reduce_stacktrace!
+        if exceptions = payload.dig(:exception, :values) || payload.dig("exception", "values")
+          exceptions.each do |exception|
+            # in most cases there is only one exception (2 or 3 when have multiple causes), so we won't loop through this double condition much
+            traces = exception.dig(:stacktrace, :frames) || exception.dig("stacktrace", "frames")
+
+            if traces && traces.size > STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD
+              size_on_both_ends = STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD / 2
+              traces.replace(
+                traces[0..(size_on_both_ends - 1)] + traces[-size_on_both_ends..-1],
+              )
+            end
+          end
+        end
+      end
     end
 
     attr_accessor :headers, :items

diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb
@@ -26,7 +26,6 @@ class Event
     WRITER_ATTRIBUTES = SERIALIZEABLE_ATTRIBUTES - %i(type timestamp level)
 
     MAX_MESSAGE_SIZE_IN_BYTES = 1024 * 8
-    MAX_SERIALIZED_PAYLOAD_SIZE = 1024 * 200
 
     SKIP_INSPECTION_ATTRIBUTES = [:@modules, :@stacktrace_builder, :@send_default_pii, :@trusted_proxies, :@rack_env_whitelist]
 

diff --git a/sentry-ruby/lib/sentry/transport.rb b/sentry-ruby/lib/sentry/transport.rb
@@ -9,7 +9,6 @@ class Transport
     PROTOCOL_VERSION = '7'
     USER_AGENT = "sentry-ruby/#{Sentry::VERSION}"
     CLIENT_REPORT_INTERVAL = 30
-    STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD = 500
 
     # https://develop.sentry.dev/sdk/client-reports/#envelope-item-payload
     CLIENT_REPORT_REASONS = [
@@ -71,50 +70,10 @@ def serialize_envelope(envelope)
       serialized_results = []
 
       envelope.items.each do |item|
-        result = item.to_s
+        result, oversized = item.serialize
 
-        if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
-          if item.payload.key?(:breadcrumbs)
-            item.payload.delete(:breadcrumbs)
-          elsif item.payload.key?("breadcrumbs")
-            item.payload.delete("breadcrumbs")
-          end
-
-          result = item.to_s
-        end
-
-        if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
-          if single_exceptions = item.payload.dig(:exception, :values)
-            single_exceptions.each do |single_exception|
-              traces = single_exception.dig(:stacktrace, :frames)
-              if traces && traces.size > STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD
-                size_on_both_ends = STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD / 2
-                traces.replace(
-                  traces[0..(size_on_both_ends - 1)] + traces[-size_on_both_ends..-1],
-                )
-              end
-            end
-          elsif single_exceptions = item.payload.dig("exception", "values")
-            single_exceptions.each do |single_exception|
-              traces = single_exception.dig("stacktrace", "frames")
-              if traces && traces.size > STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD
-                size_on_both_ends = STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD / 2
-                traces.replace(
-                  traces[0..(size_on_both_ends - 1)] + traces[-size_on_both_ends..-1],
-                )
-              end
-            end
-          end
-
-          result = item.to_s
-        end
-
-        if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
-          size_breakdown = item.payload.map do |key, value|
-            "#{key}: #{JSON.generate(value).bytesize}"
-          end.join(", ")
-
-          log_debug("Envelope item [#{item.type}] is still oversized without breadcrumbs: {#{size_breakdown}}")
+        if oversized
+          log_info("Envelope item [#{item.type}] is still oversized after size reduction: {#{item.size_breakdown}}")
 
           next
         end

diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb
@@ -110,12 +110,12 @@
             event.breadcrumbs.record Sentry::Breadcrumb.new(category: i.to_s, message: "x" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES)
           end
           serialized_result = JSON.generate(event.to_hash)
-          expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
+          expect(serialized_result.bytesize).to be > Sentry::Envelope::Item::MAX_SERIALIZED_PAYLOAD_SIZE
         end
 
         it "removes breadcrumbs and carry on" do
           data, _ = subject.serialize_envelope(envelope)
-          expect(data.bytesize).to be < Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
+          expect(data.bytesize).to be < Sentry::Envelope::Item::MAX_SERIALIZED_PAYLOAD_SIZE
 
           expect(envelope.items.count).to eq(1)
 
@@ -147,7 +147,7 @@
           project_root = "/fake/project_root"
           Regexp.new("^(#{project_root}/)?#{Sentry::Backtrace::APP_DIRS_PATTERN}")
         end
-        let(:frame_list_limit) { Sentry::Transport::STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD }
+        let(:frame_list_limit) { 500 }
         let(:frame_list_size) { frame_list_limit * 4 }
 
         before do
@@ -163,12 +163,12 @@
           single_exception.instance_variable_set(:@stacktrace, new_stacktrace)
 
           serialized_result = JSON.generate(event.to_hash)
-          expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
+          expect(serialized_result.bytesize).to be > Sentry::Envelope::Item::MAX_SERIALIZED_PAYLOAD_SIZE
         end
 
         it "keeps some stacktrace frames and carry on" do
           data, _ = subject.serialize_envelope(envelope)
-          expect(data.bytesize).to be < Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
+          expect(data.bytesize).to be < Sentry::Envelope::Item::MAX_SERIALIZED_PAYLOAD_SIZE
 
           expect(envelope.items.count).to eq(1)
 
@@ -273,7 +273,7 @@
           event.breadcrumbs.record Sentry::Breadcrumb.new(category: i.to_s, message: "x" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES)
         end
         serialized_result = JSON.generate(event.to_hash)
-        expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
+        expect(serialized_result.bytesize).to be > Sentry::Envelope::Item::MAX_SERIALIZED_PAYLOAD_SIZE
       end
 
       it "deletes the event's breadcrumbs and sends it" do