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
fix: Do not capture SQL params for now #503
Conversation
07da5ac
to
cdb71e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt about having _experiements.record_params
Ture | False
And leave the code that captures them?
Default should be False
Greetings @untitaker and thank you for taking care of python Sentry SDK! Could we elaborate a bit on the issue? Is it written anywhere with some details? One of my team's big Django applications is using now So I am wondering if our issue is related to what is described here and might be solved by this PR. |
@amureki I have no clue... please create a new issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:glock:
@untitaker I would create a new issue when I would be able to gather more details on our case. |
@amureki Without disclosing too much customer information, the issue that triggered this was a long running celery task that fired a lot of DB queries with massive params list each. In the specific case this created an OOM Theoretically you can have a long-running web request that does the same thing but this seems unrelated. I assume this will generally improve memory usage though, but it doesn't seem like that's your problem. I would create a new issue anyway. having a version range where it works and doesn't work might be good enough. Thanks |
We have memory usage issues for very large queries, because large params are held in memory (spans or breadcrumb buffer). Do not capture SQL params for now.