pyrollbar icon indicating copy to clipboard operation
pyrollbar copied to clipboard

Limit number of elements processed by `traverse`

Open mikhail-iurkov opened this issue 3 years ago • 4 comments

Description of the change

When rollbar is called, transformations are applied to arguments and local variables, which can contain large structures, leading to cpu and memory consumption (for example multi-gigabyte memoryview in locals).

This PR adds limit to maximum number of elements processed - so structures are effectively cropped past that limit.

Type of change

  • [X] Bug fix (non-breaking change that fixes an issue)
  • [ ] New feature (non-breaking change that adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] Maintenance
  • [ ] New release

Related issues

Checklists

Development

  • [ ] Lint rules pass locally
  • [ ] The code changed/added as part of this pull request has been covered with tests
  • [ ] All tests related to the changed code pass in development

Code review

  • [ ] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • [ ] "Ready for review" label attached to the PR and reviewers assigned
  • [ ] Issue from task tracker has a link to this pull request
  • [ ] Changes have been reviewed by at least one other engineer

mikhail-iurkov avatar Feb 21 '22 11:02 mikhail-iurkov

Hi @mikhail-iurkov! My apologies that this has sat dormant as long as it has. I like the idea. I think this is a good idea. However, this could be a breaking change for anyone who wants those larger structures included.

What do you think about making this a configuration option with a default that does not deviate from the current behavior?

danielmorell avatar Aug 18 '23 21:08 danielmorell

a default that does not deviate from the current behavior?

Current behavior is sending multi-gigabyte memoryview from locals (with no success). I doubt anyone relies on this.

homm avatar Aug 21 '23 19:08 homm

@homm You are totally correct. However, I can also see a senario where a user may need to reduce the maximum even further than 100. Let's say to 20. This is probably more likely than someone wanting to send a 100+ element list.

danielmorell avatar Sep 01 '23 17:09 danielmorell

So your code suggestions are? (except that it's obviously should be rebased)

homm avatar Sep 04 '23 08:09 homm