getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.9k stars 501 forks source link

Performance issue with Sentry 1.29.2 #2303

Closed jtremesay-sereema closed 12 months ago

jtremesay-sereema commented 1 year ago

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.29.2

Steps to Reproduce

Upgrade Sentry from 1.28.1 to 1.29.2

Expected Result

No performance issue when upgrading from Sentry 1.28.1 to 1.29.2.

Actual Result

After upgrading the Sentry SDK from 1.28.1 to 1.29.2 in our ASGI Django app, we started to notice a huge performance isssue:

sentry_perf_issue

Reverting to 1.28.1 fix the issue. I’m too lazy to test 1.29.1.

Unfortunately, we don’t have any logs or errors.

Funny behaviour: when we tests our views on the prod using the request mocker of Django (https://docs.djangoproject.com/en/4.2/topics/testing/advanced/#the-request-factory), the runtime is normal. But when we try with an actual HTTP request, we have the performance issue. So maybe an issue between the Sentry lib and the network/middleware layers of Django?

List and version of our deps :

aiodns==3.0.0
aiohttp==3.8.5
aiosignal==1.3.1
anyio==3.7.1
argon2-cffi==21.3.0
argon2-cffi-bindings==21.2.0
arrow==1.2.3
asgiref==3.7.2
asttokens==2.2.1
async-lru==2.0.4
async-timeout==4.0.2
attrs==23.1.0
Babel==2.12.1
backcall==0.2.0
backoff==2.2.1
beautifulsoup4==4.12.2
bleach==6.0.0
boto3==1.28.22
botocore==1.31.22
Brotli==1.0.9
certifi==2023.7.22
cffi==1.15.1
channels==4.0.0
channels-redis==4.1.0
chardet==5.2.0
charset-normalizer==3.2.0
click==8.1.6
comm==0.1.4
contourpy==1.1.0
cryptography==41.0.3
csscompressor==0.9.5
cycler==0.11.0
debugpy==1.6.7.post1
decorator==5.1.1
defusedxml==0.7.1
dj-database-url==2.0.0
Django==4.2.4
django-admin-sortable2==2.1.9
django-appconf==1.0.5
django-compressor==4.4
django-countries==7.5.1
django-extensions==3.2.3
django-imagekit==4.1.0
django-multiupload==0.6.1
django-phonenumber-field==7.1.0
django-read-only==1.14.0
django-s3-storage==0.14.0
executing==1.2.0
fastjsonschema==2.18.0
fonttools==4.42.0
fqdn==1.5.1
frozenlist==1.4.0
fuzzy-c-means==1.7.0
geomag==0.9.2015
gunicorn==21.2.0
h11==0.14.0
httptools==0.6.0
idna==3.4
inflection==0.5.1
ipykernel==6.25.1
ipython==8.14.0
isoduration==20.11.0
jedi==0.19.0
Jinja2==3.1.2
jmespath==1.0.1
joblib==1.3.2
json5==0.9.14
jsonpointer==2.4
jsonschema==4.19.0
jsonschema-specifications==2023.7.1
jupyter-events==0.7.0
jupyter-lsp==2.2.0
jupyter_client==8.3.0
jupyter_core==5.3.1
jupyter_server==2.7.0
jupyter_server_terminals==0.4.4
jupyterlab==4.0.4
jupyterlab-pygments==0.2.2
jupyterlab_server==2.24.0
kiwisolver==1.4.4
luhn==0.2.0
lxml==4.9.3
markdown2==2.4.10
MarkupSafe==2.1.3
matplotlib==3.7.2
matplotlib-inline==0.1.6
mistune==3.0.1
msgpack==1.0.5
multidict==6.0.4
natsort==8.4.0
nbclient==0.8.0
nbconvert==7.7.3
nbformat==5.9.2
nest-asyncio==1.5.7
notebook_shim==0.2.3
numpy==1.25.2
opencv-contrib-python-headless==4.8.0.74
overrides==7.4.0
packaging==23.1
pandocfilters==1.5.0
parso==0.8.3
pexpect==4.8.0
phonenumberslite==8.13.18
pickleshare==0.7.5
pilkit @ git+https://github.com/sereema/pilkit.git@41f5974a75e6f8fec2516cf34a477cb6d1fd3562
Pillow==10.0.0
platformdirs==3.10.0
prometheus-client==0.17.1
prompt-toolkit==3.0.39
psutil==5.9.5
psycopg==3.1.10
ptyprocess==0.7.0
pure-eval==0.2.2
pycares==4.3.0
pycparser==2.21
pydantic==1.10.12
Pygments==2.16.1
pyparsing==3.0.9
python-dateutil==2.8.2
python-json-logger==2.0.7
pytz==2023.3
PyYAML==6.0.1
pyzmq==25.1.0
rcssmin==1.1.1
redis==4.6.0
referencing==0.30.2
requests==2.31.0
rfc3339-validator==0.1.4
rfc3986-validator==0.1.1
rjsmin==1.2.1
rpds-py==0.9.2
ruptures==1.1.8
s3transfer==0.6.1
scikit-learn==1.3.0
scipy==1.11.1
Send2Trash==1.8.2
sentry-sdk==1.28.1
six==1.16.0
sniffio==1.3.0
soupsieve==2.4.1
sqlparse==0.4.4
stack-data==0.6.2
tabulate==0.8.10
terminado==0.17.1
threadpoolctl==3.2.0
tinycss2==1.2.1
tornado==6.3.2
tqdm==4.66.0
tracking-url==0.0.5
traitlets==5.9.0
typer==0.4.2
typing_extensions==4.7.1
tzdata==2023.3
unittest-xml-reporting==3.2.0
uri-template==1.3.0
urllib3==1.26.16
uvicorn==0.23.2
uvloop==0.17.0
wcwidth==0.2.6
webcolors==1.13
webencodings==0.5.1
websocket-client==1.6.1
websockets==11.0.3
whitenoise==6.5.0
yarl==1.9.2

I will try to gather more useful information.

sentrivana commented 1 year ago

Thanks for the report @jtremesay-sereema! As far as I can tell the only Django-related change we introduced in 1.29.* was adding DB connection data to spans, so that would be the first thing for us to check.

I don't think there's any insight to be gained by testing lower 1.29.* versions as 1.29.2 is just 1.29.0 with a broken feature rolled back.

As far as I know there is no way to opt out of collecting the DB connection data, so the best way to work around this for now is to stay on 1.28.1 while we investigate.

jtremesay-sereema commented 1 year ago

I may have a clue.

In 1.29.X, you do a lot of get_connection_params() (once per sql query).

Unfortunately, we have our own DatabaseWrapper that do some magic stuff in the get_connection_params() method to work with AWS RDS IAM Authentification:

from pathlib import Path

import boto3
from django.db.backends.postgresql import base

SSL_ROOT_CERT = Path(__file__).parent / "rds-ca.pem"

class DatabaseWrapper(base.DatabaseWrapper):
    def get_connection_params(self):
        params = super().get_connection_params()
        params["sslmode"] = "verify-full"
        params["sslrootcert"] = SSL_ROOT_CERT

        aws_region = params.pop("region", None)
        rds_client = boto3.client("rds", region_name=aws_region)
        params["password"] = rds_client.generate_db_auth_token(
            DBHostname=params["host"], DBUsername=params["user"], Port=params.get("port", 5432)
        )

        return params
sentrivana commented 1 year ago

Yes, that definitely looks related. I think we need to reevaluate if calling get_connection_params() every time to get the connection params is the right way to go. I'd hope there is a way to get them from the connection directly -- we need to investigate.

jtremesay-sereema commented 1 year ago

We tried with sentry-sdk 1.30.0 and we still have the performance issue.

sentrivana commented 1 year ago

Apologies @jtremesay-sereema, we missed this! Reopening the issue.

I think I know why the fix didn't work for you -- you're on a recent Django version with psycopg (i.e., psycopg 3). The fix we implemented would've worked for psycopg2 only, which is now deprecated as PG backend in Django. So this will need a different solution.