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

Make sure get_dsn_parameters is an actual function #2441

Merged
merged 11 commits into from
Oct 17, 2023
20 changes: 15 additions & 5 deletions sentry_sdk/integrations/django/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
from __future__ import absolute_import

import inspect
import sys
import threading
import weakref
Expand Down Expand Up @@ -665,12 +666,21 @@
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")
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a world where this could be a coroutine?

Copy link
Contributor Author

@sentrivana sentrivana Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be, that's a psycopg2 sync function.

Btw, sidenote to this. Something I learned while looking at this is that newer Django uses psycopg (i.e., psycopg 3) instead of psycopg2. psycopg doesn't have this function. So this condition will always fall back to using get_connection_params instead. This is relevant because we added the get_dsn_parameters part to bypass calling get_connection_params for postgres.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch was correct 😞 Reopened the original issue: #2303

):
# 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 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()

Check warning on line 679 in sentry_sdk/integrations/django/__init__.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/django/__init__.py#L679

Added line #L679 was not covered by tests

else:
connection_params = db.get_connection_params()

Check warning on line 682 in sentry_sdk/integrations/django/__init__.py

View check run for this annotation

Codecov / codecov/patch

sentry_sdk/integrations/django/__init__.py#L682

Added line #L682 was not covered by tests

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)
Expand Down
21 changes: 20 additions & 1 deletion tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -656,6 +657,24 @@ 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

def get_connection_params(self):
return {}

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",
[
Expand Down
Loading