From d2552ec15f152185ddd9579bc2851df9a9bc1818 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Thu, 13 Nov 2025 11:13:21 +0100 Subject: [PATCH 01/10] feat(django): Instrument database commits --- sentry_sdk/consts.py | 4 + sentry_sdk/integrations/django/__init__.py | 24 ++- .../integrations/django/test_db_query_data.py | 192 +++++++++++++++++- 3 files changed, 214 insertions(+), 6 deletions(-) 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..47dd2854df 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,14 +691,26 @@ def connect(self): _set_db_data(span, self) return real_connect(self) + @ensure_integration_enabled(DjangoIntegration, real_commit) + def commit(self): + # type: (BaseDatabaseWrapper) -> None + print("commiting") + with sentry_sdk.start_span( + op=OP.DB, + name="commit", # 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 - ignore_logger("django.db.backends") + BaseDatabaseWrapper.commit = commit -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) @@ -735,6 +748,9 @@ def _set_db_data(span, cursor_or_db): if db_name is not None: span.set_data(SPANDATA.DB_NAME, db_name) + if db_operation is not None: + span.set_data(SPANDATA.DB_OPERATION, db_operation) + server_address = connection_params.get("host") if server_address is not None: span.set_data(SPANDATA.SERVER_ADDRESS, server_address) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 41ad9d5e1c..d224e1275a 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -5,7 +5,7 @@ from unittest import mock from django import VERSION as DJANGO_VERSION -from django.db import connections +from django.db import connection, connections, transaction try: from django.urls import reverse @@ -15,7 +15,7 @@ from werkzeug.test import Client from sentry_sdk import start_transaction -from sentry_sdk.consts import SPANDATA +from sentry_sdk.consts import SPANDATA, DBOPERATION from sentry_sdk.integrations.django import DjangoIntegration from sentry_sdk.tracing_utils import record_sql_queries @@ -481,6 +481,7 @@ def test_db_span_origin_execute(sentry_init, client, capture_events): assert event["contexts"]["trace"]["origin"] == "auto.http.django" for span in event["spans"]: + print("span is", span["op"], span["description"]) if span["op"] == "db": assert span["origin"] == "auto.db.django" else: @@ -524,3 +525,190 @@ def test_db_span_origin_executemany(sentry_init, client, capture_events): 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" + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_no_autocommit_execute(sentry_init, client, capture_events): + """ + Verify we record a breadcrumb when opening a new database. + """ + 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_select_orm_no_autocommit")) + + (event,) = events + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + for span in event["spans"]: + if span["op"] == "db": + assert span["origin"] == "auto.db.django" + else: + assert span["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" + + +@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() + + if "postgres" not in connections: + pytest.skip("postgres tests disabled") + + with start_transaction(name="test_transaction"): + from django.db import connection, transaction + + cursor = connection.cursor() + + query = """UPDATE auth_user SET username = %s where id = %s;""" + query_list = ( + ( + "test1", + 1, + ), + ( + "test2", + 2, + ), + ) + cursor.executemany(query, query_list) + + transaction.commit() + + (event,) = events + + 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" + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_execute(sentry_init, client, capture_events): + """ + Verify we record a breadcrumb when opening a new database. + """ + sentry_init( + integrations=[DjangoIntegration()], + send_default_pii=True, + 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() + + with transaction.atomic(): + client.get(reverse("postgres_select_orm_atomic")) + connections["postgres"].commit() + + (event,) = events + + 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" + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_executemany(sentry_init, client, capture_events): + """ + Verify we record a breadcrumb when opening a new database. + """ + sentry_init( + integrations=[DjangoIntegration()], + send_default_pii=True, + 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() + + with start_transaction(name="test_transaction"): + with transaction.atomic(): + cursor = connection.cursor() + + query = """UPDATE auth_user SET username = %s where id = %s;""" + query_list = ( + ( + "test1", + 1, + ), + ( + "test2", + 2, + ), + ) + cursor.executemany(query, query_list) + + (event,) = events + + 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" From 58ecb5bfd1870f9bf41835ff3e62f86728d23b25 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Thu, 13 Nov 2025 11:26:27 +0100 Subject: [PATCH 02/10] . --- tests/integrations/django/myapp/urls.py | 10 ++++++++++ tests/integrations/django/myapp/views.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index fbc9e6032e..5b2cb5a337 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-select-no-autocommit", + views.postgres_select_orm_no_autocommit, + name="postgres_select_orm_no_autocommit", + ), + path( + "postgres-select-atomic", + views.postgres_select_orm_atomic, + name="postgres_select_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..0a4b40e933 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,21 @@ def postgres_select_orm(request, *args, **kwargs): return HttpResponse("ok {}".format(user)) +@csrf_exempt +def postgres_select_orm_no_autocommit(request, *args, **kwargs): + transaction.set_autocommit(False) + user = User.objects.using("postgres").all().first() + transaction.commit() + return HttpResponse("ok {}".format(user)) + + +@csrf_exempt +def postgres_select_orm_atomic(request, *args, **kwargs): + with transaction.atomic(): + user = User.objects.using("postgres").all().first() + return HttpResponse("ok {}".format(user)) + + @csrf_exempt def permission_denied_exc(*args, **kwargs): raise PermissionDenied("bye") From ad5ef1da79d00eed4b2076a863a6f4b567d489e9 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Tue, 18 Nov 2025 16:26:31 +0100 Subject: [PATCH 03/10] tests --- .../integrations/django/test_db_query_data.py | 115 +++++++++++------- 1 file changed, 73 insertions(+), 42 deletions(-) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index d224e1275a..6c7b2de196 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -6,6 +6,7 @@ from django import VERSION as DJANGO_VERSION from django.db import connection, connections, transaction +from django.contrib.auth.models import User try: from django.urls import reverse @@ -526,22 +527,10 @@ def test_db_span_origin_executemany(sentry_init, client, capture_events): 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" - @pytest.mark.forked -@pytest_mark_django_db_decorator(transaction=True) +@pytest_mark_django_db_decorator(transaction=True, databases=["postgres"]) def test_db_no_autocommit_execute(sentry_init, client, capture_events): - """ - Verify we record a breadcrumb when opening a new database. - """ sentry_init( integrations=[DjangoIntegration()], traces_sample_rate=1.0, @@ -555,10 +544,13 @@ def test_db_no_autocommit_execute(sentry_init, client, capture_events): events = capture_events() - client.get(reverse("postgres_select_orm_no_autocommit")) + 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" for span in event["spans"]: @@ -595,23 +587,46 @@ def test_db_no_autocommit_executemany(sentry_init, client, capture_events): cursor = connection.cursor() - query = """UPDATE auth_user SET username = %s where id = %s;""" + 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 = ( ( - "test1", - 1, + "user1", + "John", + "Doe", + "user1@example.com", + datetime(1970, 1, 1), ), ( - "test2", - 2, + "user2", + "Max", + "Mustermann", + "user2@example.com", + datetime(1970, 1, 1), ), ) - cursor.executemany(query, query_list) + 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" @@ -626,11 +641,8 @@ def test_db_no_autocommit_executemany(sentry_init, client, capture_events): @pytest.mark.forked -@pytest_mark_django_db_decorator(transaction=True) +@pytest_mark_django_db_decorator(transaction=True, databases=["postgres"]) def test_db_atomic_execute(sentry_init, client, capture_events): - """ - Verify we record a breadcrumb when opening a new database. - """ sentry_init( integrations=[DjangoIntegration()], send_default_pii=True, @@ -645,12 +657,13 @@ def test_db_atomic_execute(sentry_init, client, capture_events): events = capture_events() - with transaction.atomic(): - client.get(reverse("postgres_select_orm_atomic")) - connections["postgres"].commit() + 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 = [ @@ -666,9 +679,6 @@ def test_db_atomic_execute(sentry_init, client, capture_events): @pytest.mark.forked @pytest_mark_django_db_decorator(transaction=True) def test_db_atomic_executemany(sentry_init, client, capture_events): - """ - Verify we record a breadcrumb when opening a new database. - """ sentry_init( integrations=[DjangoIntegration()], send_default_pii=True, @@ -687,21 +697,42 @@ def test_db_atomic_executemany(sentry_init, client, capture_events): with transaction.atomic(): cursor = connection.cursor() - query = """UPDATE auth_user SET username = %s where id = %s;""" - query_list = ( - ( - "test1", - 1, - ), - ( - "test2", - 2, - ), - ) - cursor.executemany(query, query_list) + 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 = [ From 7a82862b334b2f78f604d12431d3350ac61f7dd1 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Tue, 18 Nov 2025 16:28:04 +0100 Subject: [PATCH 04/10] . --- sentry_sdk/integrations/django/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 47dd2854df..0cffebd69c 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -634,6 +634,7 @@ def install_sql_hook(): real_executemany = CursorWrapper.executemany real_connect = BaseDatabaseWrapper.connect real_commit = BaseDatabaseWrapper.commit + ignore_logger("django.db.backends") except AttributeError: # This won't work on Django versions < 1.6 return @@ -694,10 +695,9 @@ def connect(self): @ensure_integration_enabled(DjangoIntegration, real_commit) def commit(self): # type: (BaseDatabaseWrapper) -> None - print("commiting") with sentry_sdk.start_span( op=OP.DB, - name="commit", # DBOPERATION.COMMIT, + name=DBOPERATION.COMMIT, origin=DjangoIntegration.origin_db, ) as span: _set_db_data(span, self, DBOPERATION.COMMIT) From e8f664d3acd3c52c083eb28e2ceb22ece9cb3240 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Tue, 18 Nov 2025 16:29:25 +0100 Subject: [PATCH 05/10] . --- 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 0cffebd69c..69bab50719 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -634,7 +634,6 @@ def install_sql_hook(): real_executemany = CursorWrapper.executemany real_connect = BaseDatabaseWrapper.connect real_commit = BaseDatabaseWrapper.commit - ignore_logger("django.db.backends") except AttributeError: # This won't work on Django versions < 1.6 return @@ -707,6 +706,7 @@ def commit(self): CursorWrapper.executemany = executemany BaseDatabaseWrapper.connect = connect BaseDatabaseWrapper.commit = commit + ignore_logger("django.db.backends") def _set_db_data(span, cursor_or_db, db_operation=None): From 943e3bbf35c78ba43ad9c0823eff0152b3df5753 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Tue, 18 Nov 2025 16:31:14 +0100 Subject: [PATCH 06/10] . --- tests/integrations/django/myapp/urls.py | 12 ++++++------ tests/integrations/django/myapp/views.py | 20 +++++++++++++------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 5b2cb5a337..2d39228524 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -62,14 +62,14 @@ def path(path, *args, **kwargs): path("postgres-select", views.postgres_select, name="postgres_select"), path("postgres-select-slow", views.postgres_select_orm, name="postgres_select_orm"), path( - "postgres-select-no-autocommit", - views.postgres_select_orm_no_autocommit, - name="postgres_select_orm_no_autocommit", + "postgres-insert-no-autocommit", + views.postgres_insert_orm_no_autocommit, + name="postgres_insert_orm_no_autocommit", ), path( - "postgres-select-atomic", - views.postgres_select_orm_atomic, - name="postgres_select_orm_atomic", + "postgres-insert-atomic", + views.postgres_insert_orm_atomic, + name="postgres_insert_orm_atomic", ), path( "postgres-select-slow-from-supplement", diff --git a/tests/integrations/django/myapp/views.py b/tests/integrations/django/myapp/views.py index 0a4b40e933..9b6e6f2253 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -248,17 +248,23 @@ def postgres_select_orm(request, *args, **kwargs): @csrf_exempt -def postgres_select_orm_no_autocommit(request, *args, **kwargs): - transaction.set_autocommit(False) - user = User.objects.using("postgres").all().first() - transaction.commit() +def postgres_insert_orm_no_autocommit(request, *args, **kwargs): + transaction.set_autocommit(False, using="postgres") + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + transaction.commit(using="postgres") + transaction.set_autocommit(True, using="postgres") + return HttpResponse("ok {}".format(user)) @csrf_exempt -def postgres_select_orm_atomic(request, *args, **kwargs): - with transaction.atomic(): - user = User.objects.using("postgres").all().first() +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)) From e397f65083aa7355bcd07401bc6208a9ed122821 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Tue, 18 Nov 2025 16:37:43 +0100 Subject: [PATCH 07/10] . --- tests/integrations/django/test_db_query_data.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 6c7b2de196..56b835ee95 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -482,7 +482,6 @@ def test_db_span_origin_execute(sentry_init, client, capture_events): assert event["contexts"]["trace"]["origin"] == "auto.http.django" for span in event["spans"]: - print("span is", span["op"], span["description"]) if span["op"] == "db": assert span["origin"] == "auto.db.django" else: @@ -529,7 +528,7 @@ def test_db_span_origin_executemany(sentry_init, client, capture_events): @pytest.mark.forked -@pytest_mark_django_db_decorator(transaction=True, databases=["postgres"]) +@pytest_mark_django_db_decorator(transaction=True) def test_db_no_autocommit_execute(sentry_init, client, capture_events): sentry_init( integrations=[DjangoIntegration()], @@ -641,7 +640,7 @@ def test_db_no_autocommit_executemany(sentry_init, client, capture_events): @pytest.mark.forked -@pytest_mark_django_db_decorator(transaction=True, databases=["postgres"]) +@pytest_mark_django_db_decorator(transaction=True) def test_db_atomic_execute(sentry_init, client, capture_events): sentry_init( integrations=[DjangoIntegration()], From 3c4127cbe55aadc81400c52035c09c7385b63e1c Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Wed, 19 Nov 2025 13:35:49 +0100 Subject: [PATCH 08/10] . --- sentry_sdk/integrations/django/__init__.py | 6 +- .../integrations/django/test_db_query_data.py | 222 +------------- .../django/test_db_transactions.py | 284 ++++++++++++++++++ 3 files changed, 289 insertions(+), 223 deletions(-) create mode 100644 tests/integrations/django/test_db_transactions.py diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 69bab50719..ee91733da6 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -715,6 +715,9 @@ def _set_db_data(span, cursor_or_db, db_operation=None): 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. @@ -748,9 +751,6 @@ def _set_db_data(span, cursor_or_db, db_operation=None): if db_name is not None: span.set_data(SPANDATA.DB_NAME, db_name) - if db_operation is not None: - span.set_data(SPANDATA.DB_OPERATION, db_operation) - server_address = connection_params.get("host") if server_address is not None: span.set_data(SPANDATA.SERVER_ADDRESS, server_address) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 56b835ee95..41ad9d5e1c 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -5,8 +5,7 @@ from unittest import mock from django import VERSION as DJANGO_VERSION -from django.db import connection, connections, transaction -from django.contrib.auth.models import User +from django.db import connections try: from django.urls import reverse @@ -16,7 +15,7 @@ from werkzeug.test import Client from sentry_sdk import start_transaction -from sentry_sdk.consts import SPANDATA, DBOPERATION +from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.django import DjangoIntegration from sentry_sdk.tracing_utils import record_sql_queries @@ -525,220 +524,3 @@ def test_db_span_origin_executemany(sentry_init, client, capture_events): assert event["contexts"]["trace"]["origin"] == "manual" assert event["spans"][0]["origin"] == "auto.db.django" - - -@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" - - for span in event["spans"]: - if span["op"] == "db": - assert span["origin"] == "auto.db.django" - else: - assert span["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" - - -@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() - - if "postgres" not in connections: - pytest.skip("postgres tests disabled") - - 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" - - -@pytest.mark.forked -@pytest_mark_django_db_decorator(transaction=True) -def test_db_atomic_execute(sentry_init, client, capture_events): - sentry_init( - integrations=[DjangoIntegration()], - send_default_pii=True, - 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" - - -@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, - ) - - 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() - - with start_transaction(name="test_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" diff --git a/tests/integrations/django/test_db_transactions.py b/tests/integrations/django/test_db_transactions.py new file mode 100644 index 0000000000..c6d8f8b9df --- /dev/null +++ b/tests/integrations/django/test_db_transactions.py @@ -0,0 +1,284 @@ +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" + ) + + +@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) == "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" + ) + + +@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" + ) + + +@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 = 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" + ) From 7972103c53f4f3305621da17c8f65f0e1059a05e Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Wed, 19 Nov 2025 13:48:51 +0100 Subject: [PATCH 09/10] . --- .../django/test_db_transactions.py | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/tests/integrations/django/test_db_transactions.py b/tests/integrations/django/test_db_transactions.py index c6d8f8b9df..b69a756dde 100644 --- a/tests/integrations/django/test_db_transactions.py +++ b/tests/integrations/django/test_db_transactions.py @@ -142,18 +142,12 @@ def test_db_no_autocommit_executemany(sentry_init, client, capture_events): 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_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") - 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" - ) @pytest.mark.forked @@ -271,14 +265,8 @@ def test_db_atomic_executemany(sentry_init, client, capture_events): # Verify other database attributes assert commit_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" - conn_params = connections["postgres"].get_connection_params() + 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") - 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" - ) From 639182d225314e2c0a4cd04db3520df56e2acac0 Mon Sep 17 00:00:00 2001 From: Alexander Alderman Webb Date: Thu, 20 Nov 2025 09:23:06 +0100 Subject: [PATCH 10/10] wrap _commit instead of commit --- sentry_sdk/integrations/django/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index ee91733da6..fbcfb1638c 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -633,7 +633,7 @@ def install_sql_hook(): real_execute = CursorWrapper.execute real_executemany = CursorWrapper.executemany real_connect = BaseDatabaseWrapper.connect - real_commit = BaseDatabaseWrapper.commit + real_commit = BaseDatabaseWrapper._commit except AttributeError: # This won't work on Django versions < 1.6 return @@ -692,7 +692,7 @@ def connect(self): return real_connect(self) @ensure_integration_enabled(DjangoIntegration, real_commit) - def commit(self): + def _commit(self): # type: (BaseDatabaseWrapper) -> None with sentry_sdk.start_span( op=OP.DB, @@ -705,7 +705,7 @@ def commit(self): CursorWrapper.execute = execute CursorWrapper.executemany = executemany BaseDatabaseWrapper.connect = connect - BaseDatabaseWrapper.commit = commit + BaseDatabaseWrapper._commit = _commit ignore_logger("django.db.backends")