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

Truncate long strings in large payloads to respect 128kb limit #38

Closed
brianr opened this issue Nov 6, 2014 · 6 comments
Closed

Truncate long strings in large payloads to respect 128kb limit #38

brianr opened this issue Nov 6, 2014 · 6 comments
Assignees

Comments

@brianr
Copy link
Member

brianr commented Nov 6, 2014

rollbar-gem does this: https://github.com/rollbar/rollbar-gem/blob/master/lib/rollbar.rb#L400-L410 . We should do something similar here.

@coryvirok coryvirok added this to the v1.0.0 Release milestone Apr 17, 2017
@ArturMoczulski
Copy link
Contributor

@brianr I need a bit more explanation on what needs to be done here:

  • What parts of the payload should be truncated?
  • The 128kb limit is a limit for the whole payload or single field of the payload?
  • Can you provide an example of when such truncating should be done?

@ArturMoczulski
Copy link
Contributor

Also, the rollbar-gem example you provided seems to not be up to date (lines 400-410 are not there anymore). Can you point me to where this resides now?

@coryvirok
Copy link
Contributor

Here's a more up-to-date link - https://github.com/rollbar/rollbar-gem/tree/master/lib/rollbar/truncation

cc @rokob for direction on how to proceed with truncation.

@rokob
Copy link
Contributor

rokob commented Apr 18, 2017

The API will reject requests that are too large: https://rollbar.com/docs/api/items_post/#response-codes

413 | Request entity too large | Max payload size is 128kb. Try removing or truncating unnecessary large data included in the payload, like whole binary files or long strings.

The docs say 128kb, the reality is that the entire entity-body of the post request must be below 1MB. That is, the Content-Length as defined by: "Content-Length entity-header field indicates the size of the entity-body, in decimal number of OCTETs" must be less than 1MB.

What this means in practice is that if the payload itself is too large, we need to truncate some things to get it below the desired threshold. We want to truncate strings that are values in the request body (i.e. not keys in a JSON object).

The way the gem decides to do this, is to pick a max payload size of 0.5 MB (https://github.com/rollbar/rollbar-gem/blob/bf6d439be15455b557886fc73304ba00303aa278/lib/rollbar/truncation.rb#L12), and then to apply strategies in order until the payload satisfies this size limit: https://github.com/rollbar/rollbar-gem/blob/bf6d439be15455b557886fc73304ba00303aa278/lib/rollbar/truncation.rb#L22-L25

Those strategies https://github.com/rollbar/rollbar-gem/tree/bf6d439be15455b557886fc73304ba00303aa278/lib/rollbar/truncation start with some conservative truncations such as removing the middle of the stack trace if there are an excessive number of frames, and get more aggressive as the payload continues to be too large.

There is a bit of flexibility in how to do the truncation, but the main rules are:

  1. Must be less than a given size (1MB for the whole entity-body, but we could be safe and make it 0.5 MB the way the gem does)
  2. We want to remove as little information as possible, and the information we do remove should be non-essential for debugging purposes.

The strategies in the gem try to only remove data for very long strings or excessively large stack traces, and if that doesn't work, they truncate further.

@ArturMoczulski
Copy link
Contributor

This sounds like it's going to take much longer than six hours I provided in the initial estimate

@rokob rokob modified the milestones: v1.1.0 Release, v1.0.0 Release Apr 25, 2017
@ArturMoczulski ArturMoczulski self-assigned this May 16, 2017
rokob added a commit that referenced this issue May 16, 2017
@rokob
Copy link
Contributor

rokob commented May 16, 2017

Merged

@rokob rokob closed this as completed May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants