From b4e037bc52859ecde1397a004748d9752f67bf0a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 13:31:08 +0200 Subject: [PATCH 1/7] Make sure get_dsn_params is a function Some non-standard DB backends have their own __getattr__, which renders our check for attributes useless. --- sentry_sdk/integrations/django/__init__.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 03d0545b1d..7645568e22 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +import inspect import sys import threading import weakref @@ -665,12 +666,18 @@ def _set_db_data(span, cursor_or_db): vendor = db.vendor span.set_data(SPANDATA.DB_SYSTEM, vendor) - connection_params = ( - cursor_or_db.connection.get_dsn_parameters() - if hasattr(cursor_or_db, "connection") + connection_params = {} + + if ( + hasattr(cursor_or_db, "connection") and hasattr(cursor_or_db.connection, "get_dsn_parameters") - else db.get_connection_params() - ) + and inspect.isfunction(cursor_or_db.connection.get_dsn_parameters) + ): + connection_params = cursor_or_db.connection.get_dsn_parameters() + + elif hasattr(db, "get_connection_params"): + connection_params = db.get_connection_params() + db_name = connection_params.get("dbname") or connection_params.get("database") if db_name is not None: span.set_data(SPANDATA.DB_NAME, db_name) From 4683486c3762cb8ce610fa7217e82c4ecf6cdf41 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 13:54:10 +0200 Subject: [PATCH 2/7] add comment --- sentry_sdk/integrations/django/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 7645568e22..7f372c86de 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -673,6 +673,11 @@ def _set_db_data(span, cursor_or_db): and hasattr(cursor_or_db.connection, "get_dsn_parameters") and inspect.isfunction(cursor_or_db.connection.get_dsn_parameters) ): + # Some custom backends override `__getattr__`, making it look like `cursor_or_db` + # actually has a `connection` and the `connection` has a `get_dsn_parameters` + # attribute, only to explode once you actually want to call the latter. + # Hence the `inspect` check whether `get_dsn_parameters` is an actual callable + # function. connection_params = cursor_or_db.connection.get_dsn_parameters() elif hasattr(db, "get_connection_params"): From 1ff5303994e9c800a934201f6969916f3a095244 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 13:55:06 +0200 Subject: [PATCH 3/7] wording --- sentry_sdk/integrations/django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 7f372c86de..b51673678c 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -675,7 +675,7 @@ def _set_db_data(span, cursor_or_db): ): # Some custom backends override `__getattr__`, making it look like `cursor_or_db` # actually has a `connection` and the `connection` has a `get_dsn_parameters` - # attribute, only to explode once you actually want to call the latter. + # attribute, only to throw an error once you actually want to call it. # Hence the `inspect` check whether `get_dsn_parameters` is an actual callable # function. connection_params = cursor_or_db.connection.get_dsn_parameters() From 6e4416c1b884acaf8b7005bd70e776d150bdba44 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 14:50:53 +0200 Subject: [PATCH 4/7] a "test" --- tests/integrations/django/test_basic.py | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 379c4d9614..dc0904c797 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -22,10 +22,11 @@ from sentry_sdk._compat import PY2, PY310 from sentry_sdk import capture_message, capture_exception, configure_scope from sentry_sdk.consts import SPANDATA -from sentry_sdk.integrations.django import DjangoIntegration +from sentry_sdk.integrations.django import DjangoIntegration, _set_db_data from sentry_sdk.integrations.django.signals_handlers import _get_receiver_name from sentry_sdk.integrations.django.caching import _get_span_description from sentry_sdk.integrations.executing import ExecutingIntegration +from sentry_sdk.tracing import Span from tests.integrations.django.myapp.wsgi import application from tests.integrations.django.utils import pytest_mark_django_db_decorator @@ -648,7 +649,7 @@ def test_db_connection_span_data(sentry_init, client, capture_events): assert data.get(SPANDATA.DB_SYSTEM) == "postgresql" assert ( data.get(SPANDATA.DB_NAME) - == connections["postgres"].get_connection_params()["database"] + == connections["postgres"].get_dsn_parameters()["database"] ) assert data.get(SPANDATA.SERVER_ADDRESS) == os.environ.get( "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" @@ -656,6 +657,22 @@ def test_db_connection_span_data(sentry_init, client, capture_events): assert data.get(SPANDATA.SERVER_PORT) == "5432" +def test_set_db_data_custom_backend(): + class DummyBackend(object): + # https://github.com/mongodb/mongo-python-driver/blob/6ffae5522c960252b8c9adfe2a19b29ff28187cb/pymongo/collection.py#L126 + def __getattr__(self, attr): + return self + + def __call__(self): + raise TypeError + + span = Span() + try: + _set_db_data(span, DummyBackend()) + except TypeError: + pytest.fail("A TypeError was raised") + + @pytest.mark.parametrize( "transaction_style,client_url,expected_transaction,expected_source,expected_response", [ From b453b18402660de51dd1ad147a9e2870d8eb0b77 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 14:59:59 +0200 Subject: [PATCH 5/7] fix old test --- tests/integrations/django/test_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index dc0904c797..795b8c5d23 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -649,7 +649,7 @@ def test_db_connection_span_data(sentry_init, client, capture_events): assert data.get(SPANDATA.DB_SYSTEM) == "postgresql" assert ( data.get(SPANDATA.DB_NAME) - == connections["postgres"].get_dsn_parameters()["database"] + == connections["postgres"].get_connection_params()["database"] ) assert data.get(SPANDATA.SERVER_ADDRESS) == os.environ.get( "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" From b214eb47fa2de08d3df204751e610d0e7bead722 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 15:49:28 +0200 Subject: [PATCH 6/7] fix test --- sentry_sdk/integrations/django/__init__.py | 4 +++- tests/integrations/django/test_basic.py | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index b51673678c..e5558d0745 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -680,7 +680,9 @@ def _set_db_data(span, cursor_or_db): # function. connection_params = cursor_or_db.connection.get_dsn_parameters() - elif hasattr(db, "get_connection_params"): + elif hasattr(db, "get_connection_params") and inspect.isfunction( + db.get_connection_params + ): connection_params = db.get_connection_params() db_name = connection_params.get("dbname") or connection_params.get("database") diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 795b8c5d23..e599c78843 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -666,9 +666,11 @@ def __getattr__(self, attr): def __call__(self): raise TypeError - span = Span() + def get_connection_params(self): + return {} + try: - _set_db_data(span, DummyBackend()) + _set_db_data(Span(), DummyBackend()) except TypeError: pytest.fail("A TypeError was raised") From 42fccef609e234b8d8979c20a03400275ee007fd Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 12 Oct 2023 16:05:39 +0200 Subject: [PATCH 7/7] keep previous logic --- sentry_sdk/integrations/django/__init__.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index e5558d0745..c82ef4f148 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -666,8 +666,6 @@ def _set_db_data(span, cursor_or_db): vendor = db.vendor span.set_data(SPANDATA.DB_SYSTEM, vendor) - connection_params = {} - if ( hasattr(cursor_or_db, "connection") and hasattr(cursor_or_db.connection, "get_dsn_parameters") @@ -680,9 +678,7 @@ def _set_db_data(span, cursor_or_db): # function. connection_params = cursor_or_db.connection.get_dsn_parameters() - elif hasattr(db, "get_connection_params") and inspect.isfunction( - db.get_connection_params - ): + else: connection_params = db.get_connection_params() db_name = connection_params.get("dbname") or connection_params.get("database")