PERF: Make truncating backtraces more memory-efficient (#108)

lib/logster.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require 'logster/version'
 require 'logster/logger'
 require 'logster/message'
 require 'logster/configuration'

lib/logster/message.rb

@@ -260,22 +260,19 @@ def apply_env_size_limit(size_limit)
     def apply_message_size_limit(limit, gems_dir: nil)
       size = self.to_json(exclude_env: true).bytesize
       if size > limit && @backtrace
-        backtrace_limit = limit - (size - @backtrace.bytesize)
         @backtrace.gsub!(gems_dir, "") if gems_dir
         @backtrace.strip!
-        stop = false
-        while @backtrace.bytesize > backtrace_limit && backtrace_limit > 0 && !stop
-          orig = @backtrace.dup
-          lines = @backtrace.lines
-          if lines.size > 1
-            lines.pop
-            @backtrace = lines.join
-          else
-            @backtrace.slice!(-1)
-          end
-          # protection to ensure we never get stuck
-          stop = orig == @backtrace
-        end
+        size = self.to_json(exclude_env: true).bytesize
+        backtrace_limit = limit - (size - @backtrace.bytesize)
+        return if backtrace_limit <= 0 || size <= limit
+        truncate_backtrace(backtrace_limit)
+      end
+    end
+
+    def truncate_backtrace(bytes_limit)
+      @backtrace = @backtrace.byteslice(0...bytes_limit)
+      while !@backtrace[-1].valid_encoding? && @backtrace.size > 1
+        @backtrace.slice!(-1)
       end
     end
 

test/logster/test_message.rb

@@ -217,12 +217,34 @@ def test_apply_message_size_limit_removes_lines_from_backtrace_to_keep_total_siz
       /var/www/discourse/lib/scheduler/defer.rb:89:in `do_work'
       /var/www/discourse/lib/scheduler/defer.rb:79:in `block (2 levels) in start_thread'
     TEXT
+
+    expected = <<~TEXT
+      rails_multisite-2.0.7/lib/rails_multisite/connection_management.rb:220:in `with_connection'
+      rails_multisite-2.0.7/lib/rails_multisite/connection_management.rb:60:in `with_connection'
+      /var/www/discourse
+    TEXT
     message = Logster::Message.new(Logger::WARN, '', 'message', 1)
     message.backtrace = backtrace.dup
     assert_operator(message.to_json(exclude_env: true).bytesize, :>=, 350)
     message.apply_message_size_limit(350)
     assert_operator(message.to_json(exclude_env: true).bytesize, :<=, 350)
-    assert_equal(backtrace.lines.first(2).join.strip, message.backtrace.strip)
+    assert_equal(expected.strip, message.backtrace.strip)
+  end
+
+  def test_truncate_backtrace_shouldnt_corrupt_backtrace_when_it_contains_multibytes_characters
+    backtrace = "aहa"
+    message = Logster::Message.new(Logger::WARN, '', 'message', 1)
+    message.backtrace = backtrace.dup
+    message.truncate_backtrace(3)
+    assert_equal("a", message.backtrace)
+
+    message.backtrace = backtrace.dup
+    message.truncate_backtrace(4)
+    assert_equal("aह", message.backtrace)
+
+    message.backtrace = backtrace.dup
+    message.truncate_backtrace(5)
+    assert_equal(backtrace, message.backtrace)
   end
 
   def test_apply_message_size_limit_doesnt_remove_backtrace_entirely