diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index f74ea4eba4..44715be525 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -114,6 +114,10 @@ class INSTRUMENTER: OTEL = "otel" +class DBOPERATION: + COMMIT = "COMMIT" + + class SPANDATA: """ Additional information describing the type of the span. diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 2041598fa0..fbcfb1638c 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -5,7 +5,7 @@ from importlib import import_module import sentry_sdk -from sentry_sdk.consts import OP, SPANDATA +from sentry_sdk.consts import OP, SPANDATA, DBOPERATION from sentry_sdk.scope import add_global_event_processor, should_send_default_pii from sentry_sdk.serializer import add_global_repr_processor, add_repr_sequence_type from sentry_sdk.tracing import SOURCE_FOR_STYLE, TransactionSource @@ -633,6 +633,7 @@ def install_sql_hook(): real_execute = CursorWrapper.execute real_executemany = CursorWrapper.executemany real_connect = BaseDatabaseWrapper.connect + real_commit = BaseDatabaseWrapper._commit except AttributeError: # This won't work on Django versions < 1.6 return @@ -690,18 +691,33 @@ def connect(self): _set_db_data(span, self) return real_connect(self) + @ensure_integration_enabled(DjangoIntegration, real_commit) + def _commit(self): + # type: (BaseDatabaseWrapper) -> None + with sentry_sdk.start_span( + op=OP.DB, + name=DBOPERATION.COMMIT, + origin=DjangoIntegration.origin_db, + ) as span: + _set_db_data(span, self, DBOPERATION.COMMIT) + return real_commit(self) + CursorWrapper.execute = execute CursorWrapper.executemany = executemany BaseDatabaseWrapper.connect = connect + BaseDatabaseWrapper._commit = _commit ignore_logger("django.db.backends") -def _set_db_data(span, cursor_or_db): - # type: (Span, Any) -> None +def _set_db_data(span, cursor_or_db, db_operation=None): + # type: (Span, Any, Optional[str]) -> None db = cursor_or_db.db if hasattr(cursor_or_db, "db") else cursor_or_db vendor = db.vendor span.set_data(SPANDATA.DB_SYSTEM, vendor) + if db_operation is not None: + span.set_data(SPANDATA.DB_OPERATION, db_operation) + # 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. diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index fbc9e6032e..2d39228524 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -61,6 +61,16 @@ def path(path, *args, **kwargs): path("template-test4", views.template_test4, name="template_test4"), path("postgres-select", views.postgres_select, name="postgres_select"), path("postgres-select-slow", views.postgres_select_orm, name="postgres_select_orm"), + path( + "postgres-insert-no-autocommit", + views.postgres_insert_orm_no_autocommit, + name="postgres_insert_orm_no_autocommit", + ), + path( + "postgres-insert-atomic", + views.postgres_insert_orm_atomic, + name="postgres_insert_orm_atomic", + ), path( "postgres-select-slow-from-supplement", helper_views.postgres_select_orm, diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 9c14bc27d7..e2b14a0def 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -2,6 +2,7 @@ import json import threading +from django.db import transaction from django.contrib.auth import login from django.contrib.auth.models import User from django.core.exceptions import PermissionDenied @@ -246,6 +247,29 @@ def postgres_select_orm(request, *args, **kwargs): return HttpResponse("ok {}".format(user)) +@csrf_exempt +def postgres_insert_orm_no_autocommit(request, *args, **kwargs): + transaction.set_autocommit(False, using="postgres") + try: + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + transaction.commit(using="postgres") + finally: + transaction.set_autocommit(True, using="postgres") + + return HttpResponse("ok {}".format(user)) + + +@csrf_exempt +def postgres_insert_orm_atomic(request, *args, **kwargs): + with transaction.atomic(using="postgres"): + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + return HttpResponse("ok {}".format(user)) + + @csrf_exempt def permission_denied_exc(*args, **kwargs): raise PermissionDenied("bye") diff --git a/tests/integrations/django/test_db_transactions.py b/tests/integrations/django/test_db_transactions.py new file mode 100644 index 0000000000..cd871e94de --- /dev/null +++ b/tests/integrations/django/test_db_transactions.py @@ -0,0 +1,306 @@ +import os +import pytest +from datetime import datetime + +from django.db import connections +from django.contrib.auth.models import User + +try: + from django.urls import reverse +except ImportError: + from django.core.urlresolvers import reverse + +from werkzeug.test import Client + +from sentry_sdk import start_transaction +from sentry_sdk.consts import SPANDATA, DBOPERATION +from sentry_sdk.integrations.django import DjangoIntegration + +from tests.integrations.django.utils import pytest_mark_django_db_decorator +from tests.integrations.django.myapp.wsgi import application + + +@pytest.fixture +def client(): + return Client(application) + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_no_autocommit_execute(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration()], + traces_sample_rate=1.0, + ) + + if "postgres" not in connections: + pytest.skip("postgres tests disabled") + + # trigger Django to open a new connection by marking the existing one as None. + connections["postgres"].connection = None + + events = capture_events() + + client.get(reverse("postgres_insert_orm_no_autocommit")) + + (event,) = events + + # Ensure operation is persisted + assert User.objects.using("postgres").exists() + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + commit_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == DBOPERATION.COMMIT + ] + assert len(commit_spans) == 1 + commit_span = commit_spans[0] + assert commit_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert commit_span["data"].get(SPANDATA.DB_SYSTEM) == "postgresql" + conn_params = connections["postgres"].get_connection_params() + assert commit_span["data"].get(SPANDATA.DB_NAME) is not None + assert commit_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + assert commit_span["data"].get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) + assert commit_span["data"].get(SPANDATA.SERVER_PORT) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_PORT", "5432" + ) + + insert_spans = [ + span for span in event["spans"] if span["description"].startswith("INSERT INTO") + ] + assert len(insert_spans) == 1 + insert_span = insert_spans[0] + + # Verify query and commit statements are siblings + assert commit_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_no_autocommit_executemany(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration()], + traces_sample_rate=1.0, + ) + + events = capture_events() + + with start_transaction(name="test_transaction"): + from django.db import connection, transaction + + cursor = connection.cursor() + + query = """INSERT INTO auth_user ( + password, + is_superuser, + username, + first_name, + last_name, + email, + is_staff, + is_active, + date_joined +) +VALUES ('password', false, %s, %s, %s, %s, false, true, %s);""" + + query_list = ( + ( + "user1", + "John", + "Doe", + "user1@example.com", + datetime(1970, 1, 1), + ), + ( + "user2", + "Max", + "Mustermann", + "user2@example.com", + datetime(1970, 1, 1), + ), + ) + + transaction.set_autocommit(False) + cursor.executemany(query, query_list) + transaction.commit() + transaction.set_autocommit(True) + + (event,) = events + + # Ensure operation is persisted + assert User.objects.exists() + + assert event["contexts"]["trace"]["origin"] == "manual" + assert event["spans"][0]["origin"] == "auto.db.django" + + commit_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == DBOPERATION.COMMIT + ] + assert len(commit_spans) == 1 + commit_span = commit_spans[0] + assert commit_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert commit_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" + conn_params = connection.get_connection_params() + assert commit_span["data"].get(SPANDATA.DB_NAME) is not None + assert commit_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + + insert_spans = [ + span for span in event["spans"] if span["description"].startswith("INSERT INTO") + ] + + # Verify queries and commit statements are siblings + for insert_span in insert_spans: + assert commit_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_execute(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration()], + traces_sample_rate=1.0, + ) + + if "postgres" not in connections: + pytest.skip("postgres tests disabled") + + # trigger Django to open a new connection by marking the existing one as None. + connections["postgres"].connection = None + + events = capture_events() + + client.get(reverse("postgres_insert_orm_atomic")) + + (event,) = events + + # Ensure operation is persisted + assert User.objects.using("postgres").exists() + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + commit_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == DBOPERATION.COMMIT + ] + assert len(commit_spans) == 1 + commit_span = commit_spans[0] + assert commit_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert commit_span["data"].get(SPANDATA.DB_SYSTEM) == "postgresql" + conn_params = connections["postgres"].get_connection_params() + assert commit_span["data"].get(SPANDATA.DB_NAME) is not None + assert commit_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + assert commit_span["data"].get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) + assert commit_span["data"].get(SPANDATA.SERVER_PORT) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_PORT", "5432" + ) + + insert_spans = [ + span for span in event["spans"] if span["description"].startswith("INSERT INTO") + ] + assert len(insert_spans) == 1 + insert_span = insert_spans[0] + + # Verify query and commit statements are siblings + assert commit_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_executemany(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration()], + send_default_pii=True, + traces_sample_rate=1.0, + ) + + events = capture_events() + + with start_transaction(name="test_transaction"): + from django.db import connection, transaction + + with transaction.atomic(): + cursor = connection.cursor() + + query = """INSERT INTO auth_user ( + password, + is_superuser, + username, + first_name, + last_name, + email, + is_staff, + is_active, + date_joined +) +VALUES ('password', false, %s, %s, %s, %s, false, true, %s);""" + + query_list = ( + ( + "user1", + "John", + "Doe", + "user1@example.com", + datetime(1970, 1, 1), + ), + ( + "user2", + "Max", + "Mustermann", + "user2@example.com", + datetime(1970, 1, 1), + ), + ) + cursor.executemany(query, query_list) + + (event,) = events + + # Ensure operation is persisted + assert User.objects.exists() + + assert event["contexts"]["trace"]["origin"] == "manual" + + commit_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == DBOPERATION.COMMIT + ] + assert len(commit_spans) == 1 + commit_span = commit_spans[0] + assert commit_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert commit_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" + conn_params = connection.get_connection_params() + assert commit_span["data"].get(SPANDATA.DB_NAME) is not None + assert commit_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + + insert_spans = [ + span for span in event["spans"] if span["description"].startswith("INSERT INTO") + ] + + # Verify queries and commit statements are siblings + for insert_span in insert_spans: + assert commit_span["parent_span_id"] == insert_span["parent_span_id"]