Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PERF: Make truncating backtraces more memory-efficient #108

Merged
merged 1 commit into from Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/logster.rb
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require 'logster/version'
require 'logster/logger'
require 'logster/message'
require 'logster/configuration'
Expand Down
25 changes: 11 additions & 14 deletions lib/logster/message.rb
Expand Up @@ -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

Expand Down
24 changes: 23 additions & 1 deletion test/logster/test_message.rb
Expand Up @@ -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
Expand Down