Skip to content

Commit

Permalink
fix: truncation improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
waltjones committed May 30, 2019
1 parent ee507a1 commit a5f7e02
Show file tree
Hide file tree
Showing 9 changed files with 182 additions and 16 deletions.
9 changes: 3 additions & 6 deletions lib/rollbar/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,20 +121,17 @@ def handle_too_large_payload(stringified_payload, final_payload, attempts)
host = stringified_payload['data'].fetch('server', {})['host']

notifier.send_failsafe(
too_large_payload_string(stringified_payload, final_payload, attempts),
too_large_payload_string(attempts),
nil,
uuid,
host
)
logger.error("[Rollbar] Payload too large to be sent for UUID #{uuid}: #{Rollbar::JSON.dump(payload)}")
end

def too_large_payload_string(stringified_payload, final_payload, attempts)
original_size = Rollbar::JSON.dump(stringified_payload).bytesize
final_size = final_payload.bytesize

def too_large_payload_string(attempts)
'Could not send payload due to it being too large after truncating attempts. ' \
"Original size: #{original_size} Attempts: #{attempts.join(', ')} Final size: #{final_size}"
"Original size: #{attempts.first} Attempts: #{attempts.join(', ')} Final size: #{attempts.last}"
end

def ignored?
Expand Down
6 changes: 5 additions & 1 deletion lib/rollbar/truncation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
require 'rollbar/truncation/frames_strategy'
require 'rollbar/truncation/strings_strategy'
require 'rollbar/truncation/min_body_strategy'
require 'rollbar/truncation/remove_request_strategy'
require 'rollbar/truncation/remove_extra_strategy'

module Rollbar
module Truncation
Expand All @@ -13,7 +15,9 @@ module Truncation
STRATEGIES = [RawStrategy,
FramesStrategy,
StringsStrategy,
MinBodyStrategy].freeze
MinBodyStrategy,
RemoveRequestStrategy,
RemoveExtraStrategy].freeze

def self.truncate(payload, attempts = [])
result = nil
Expand Down
5 changes: 2 additions & 3 deletions lib/rollbar/truncation/min_body_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ def self.call(payload)
end

def call(payload)
new_payload = Rollbar::Util.deep_copy(payload)
body = new_payload['data']['body']
body = payload['data']['body']

if body['trace_chain']
body['trace_chain'] = body['trace_chain'].map do |trace_data|
Expand All @@ -22,7 +21,7 @@ def call(payload)
body['trace'] = truncate_trace_data(body['trace'])
end

dump(new_payload)
dump(payload)
end

def truncate_trace_data(trace_data)
Expand Down
35 changes: 35 additions & 0 deletions lib/rollbar/truncation/remove_extra_strategy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
require 'rollbar/util'

module Rollbar
module Truncation
class RemoveExtraStrategy
include ::Rollbar::Truncation::Mixin

def self.call(payload)
new.call(payload)
end

def call(payload)
body = payload['data']['body']

delete_message_extra(body)
delete_trace_chain_extra(body)
delete_trace_extra(body)

dump(payload)
end

def delete_message_extra(body)
body['message'].delete('extra') if body['message'] && body['message']['extra']
end

def delete_trace_chain_extra(body)
body['trace_chain'][0].delete('extra') if body['trace_chain'] && body['trace_chain'][0]['extra']
end

def delete_trace_extra(body)
body['trace'].delete('extra') if body['trace'] && body['trace']['extra']
end
end
end
end
21 changes: 21 additions & 0 deletions lib/rollbar/truncation/remove_request_strategy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require 'rollbar/util'

module Rollbar
module Truncation
class RemoveRequestStrategy
include ::Rollbar::Truncation::Mixin

def self.call(payload)
new.call(payload)
end

def call(payload)
data = payload['data']

data.delete('request') if data['request']

dump(payload)
end
end
end
end
7 changes: 3 additions & 4 deletions lib/rollbar/truncation/strings_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,20 @@ module Truncation
class StringsStrategy
include ::Rollbar::Truncation::Mixin

STRING_THRESHOLDS = [1024, 512, 256, 128, 64].freeze
STRING_THRESHOLDS = [1024, 512, 256].freeze

def self.call(payload)
new.call(payload)
end

def call(payload)
result = nil
new_payload = Rollbar::Util.deep_copy(payload)

STRING_THRESHOLDS.each do |threshold|
truncate_proc = truncate_strings_proc(threshold)

::Rollbar::Util.iterate_and_update(new_payload, truncate_proc)
result = dump(new_payload)
::Rollbar::Util.iterate_and_update(payload, truncate_proc)
result = dump(payload)

break unless truncate?(result)
end
Expand Down
4 changes: 2 additions & 2 deletions spec/rollbar/item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ def as_json(*)
it 'calls Notifier#send_failsafe and logs the error' do
original_size = Rollbar::JSON.dump(payload).bytesize
attempts = []
final_size = Rollbar::Truncation.truncate(payload.clone, attempts).bytesize
final_size = Rollbar::Truncation.truncate(Rollbar::Util.deep_copy(payload), attempts).bytesize
# final_size = original_size
rollbar_message = "Could not send payload due to it being too large after truncating attempts. Original size: #{original_size} Attempts: #{attempts.join(', ')} Final size: #{final_size}"
uuid = payload['data']['uuid']
Expand All @@ -769,7 +769,7 @@ def as_json(*)
payload['data'].delete('server')
original_size = Rollbar::JSON.dump(payload).bytesize
attempts = []
final_size = Rollbar::Truncation.truncate(payload.clone, attempts).bytesize
final_size = Rollbar::Truncation.truncate(Rollbar::Util.deep_copy(payload), attempts).bytesize
# final_size = original_size
rollbar_message = "Could not send payload due to it being too large after truncating attempts. Original size: #{original_size} Attempts: #{attempts.join(', ')} Final size: #{final_size}"
uuid = payload['data']['uuid']
Expand Down
87 changes: 87 additions & 0 deletions spec/rollbar/truncation/remove_extra_strategy_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
require 'spec_helper'
require 'rollbar/truncation/remove_extra_strategy'

describe Rollbar::Truncation::RemoveExtraStrategy do
describe '.call' do
let(:extra) { { 'bar' => 'baz' } }
let(:request) { { 'baz' => 'fizz' } }

context 'with message payload type' do
let(:message_body) { { 'foo' => 'bar' } }

let(:payload) do
{
'data' => {
'body' => {
'message' => {
'body' => message_body,
'extra' => extra
}
},
'request' => request
}
}
end

it 'should truncate the extra data in the payload' do
result = Rollbar::JSON.load(described_class.call(Rollbar::Util.deep_copy(payload)))

expect(result['data']['body']['message']['extra']).to be_nil
expect(result['data']['body']['message']['body']).to be_eql(message_body)
expect(result['data']['request']).to be_eql(request)
end
end

context 'with trace payload type' do
let(:trace_frames) { { 'foo' => 'bar' } }

let(:payload) do
{
'data' => {
'body' => {
'trace' => {
'frames' => trace_frames,
'extra' => extra
}
},
'request' => request
}
}
end

it 'should truncate the extra data in the payload' do
result = Rollbar::JSON.load(described_class.call(Rollbar::Util.deep_copy(payload)))

expect(result['data']['body']['trace']['extra']).to be_nil
expect(result['data']['body']['trace']['frames']).to be_eql(trace_frames)
expect(result['data']['request']).to be_eql(request)
end
end

context 'with trace_chain payload type' do
let(:trace_frames) { { 'foo' => 'bar' } }

let(:payload) do
{
'data' => {
'body' => {
'trace_chain' => [{
'frames' => trace_frames,
'extra' => extra
}]
},
'request' => request
}
}
end

it 'should truncate the extra data in the payload' do
result = Rollbar::JSON.load(described_class.call(Rollbar::Util.deep_copy(payload)))

expect(result['data']['body']['trace_chain'][0]['extra']).to be_nil
expect(result['data']['body']['trace_chain'][0]['frames']).to be_eql(trace_frames)
expect(result['data']['request']).to be_eql(request)
end
end
end
end
24 changes: 24 additions & 0 deletions spec/rollbar/truncation/remove_request_strategy_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'spec_helper'
require 'rollbar/truncation/remove_request_strategy'

describe Rollbar::Truncation::RemoveRequestStrategy do
describe '.call' do
let(:body) { { 'foo' => 'bar' } }
let(:request) { { 'bar' => 'baz' } }
let(:payload) do
{
'data' => {
'body' => body,
'request' => request
}
}
end

it 'should truncate the request in the payload' do
result = Rollbar::JSON.load(described_class.call(Rollbar::Util.deep_copy(payload)))

expect(result['data']['body']).to be_eql(body)
expect(result['data']['request']).to be_nil
end
end
end

0 comments on commit a5f7e02

Please sign in to comment.