diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 339c516508..6faff402d2 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -116,6 +116,7 @@ class INSTRUMENTER: class SPANNAME: DB_COMMIT = "COMMIT" + DB_ROLLBACK = "ROLLBACK" class SPANDATA: diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 95a4fbae9f..c18a03a38c 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -636,6 +636,7 @@ def install_sql_hook(): real_executemany = CursorWrapper.executemany real_connect = BaseDatabaseWrapper.connect real_commit = BaseDatabaseWrapper._commit + real_rollback = BaseDatabaseWrapper._rollback except AttributeError: # This won't work on Django versions < 1.6 return @@ -708,10 +709,26 @@ def _commit(self): _set_db_data(span, self, SPANNAME.DB_COMMIT) return real_commit(self) + def _rollback(self): + # type: (BaseDatabaseWrapper) -> None + integration = sentry_sdk.get_client().get_integration(DjangoIntegration) + + if integration is None or not integration.db_transaction_spans: + return real_rollback(self) + + with sentry_sdk.start_span( + op=OP.DB, + name=SPANNAME.DB_ROLLBACK, + origin=DjangoIntegration.origin_db, + ) as span: + _set_db_data(span, self, SPANNAME.DB_ROLLBACK) + return real_rollback(self) + CursorWrapper.execute = execute CursorWrapper.executemany = executemany BaseDatabaseWrapper.connect = connect BaseDatabaseWrapper._commit = _commit + BaseDatabaseWrapper._rollback = _rollback ignore_logger("django.db.backends") diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 2d39228524..26d5a1bf2c 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -66,11 +66,26 @@ def path(path, *args, **kwargs): views.postgres_insert_orm_no_autocommit, name="postgres_insert_orm_no_autocommit", ), + path( + "postgres-insert-no-autocommit-rollback", + views.postgres_insert_orm_no_autocommit_rollback, + name="postgres_insert_orm_no_autocommit_rollback", + ), path( "postgres-insert-atomic", views.postgres_insert_orm_atomic, name="postgres_insert_orm_atomic", ), + path( + "postgres-insert-atomic-rollback", + views.postgres_insert_orm_atomic_rollback, + name="postgres_insert_orm_atomic_rollback", + ), + path( + "postgres-insert-atomic-exception", + views.postgres_insert_orm_atomic_exception, + name="postgres_insert_orm_atomic_exception", + ), 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 4787e631f3..6d199a3740 100644 --- a/tests/integrations/django/myapp/views.py +++ b/tests/integrations/django/myapp/views.py @@ -264,6 +264,23 @@ def postgres_insert_orm_no_autocommit(request, *args, **kwargs): return HttpResponse("ok {}".format(user)) +@csrf_exempt +def postgres_insert_orm_no_autocommit_rollback(request, *args, **kwargs): + transaction.set_autocommit(False, using="postgres") + try: + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + transaction.rollback(using="postgres") + except Exception: + transaction.rollback(using="postgres") + transaction.set_autocommit(True, using="postgres") + raise + + 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"): @@ -273,6 +290,30 @@ def postgres_insert_orm_atomic(request, *args, **kwargs): return HttpResponse("ok {}".format(user)) +@csrf_exempt +def postgres_insert_orm_atomic_rollback(request, *args, **kwargs): + with transaction.atomic(using="postgres"): + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + transaction.set_rollback(True, using="postgres") + return HttpResponse("ok {}".format(user)) + + +@csrf_exempt +def postgres_insert_orm_atomic_exception(request, *args, **kwargs): + try: + with transaction.atomic(using="postgres"): + user = User.objects.db_manager("postgres").create_user( + username="user1", + ) + transaction.set_rollback(True, using="postgres") + 1 / 0 + except ZeroDivisionError: + pass + 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 index f2a1495b43..2750397b0e 100644 --- a/tests/integrations/django/test_db_transactions.py +++ b/tests/integrations/django/test_db_transactions.py @@ -44,6 +44,7 @@ def test_db_transaction_spans_disabled_no_autocommit( events = capture_events() + client.get(reverse("postgres_insert_orm_no_autocommit_rollback")) client.get(reverse("postgres_insert_orm_no_autocommit")) with start_transaction(name="test_transaction"): @@ -81,23 +82,71 @@ def test_db_transaction_spans_disabled_no_autocommit( ), ) + transaction.set_autocommit(False) + cursor.executemany(query, query_list) + transaction.rollback() + transaction.set_autocommit(True) + + 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) - (postgres_spans, sqlite_spans) = events + (postgres_rollback, postgres_commit, sqlite_rollback, sqlite_commit) = events # Ensure operation is persisted assert User.objects.using("postgres").exists() - assert postgres_spans["contexts"]["trace"]["origin"] == "auto.http.django" - assert sqlite_spans["contexts"]["trace"]["origin"] == "manual" + assert postgres_rollback["contexts"]["trace"]["origin"] == "auto.http.django" + assert postgres_commit["contexts"]["trace"]["origin"] == "auto.http.django" + assert sqlite_rollback["contexts"]["trace"]["origin"] == "manual" + assert sqlite_commit["contexts"]["trace"]["origin"] == "manual" commit_spans = [ span - for span in itertools.chain(postgres_spans["spans"], sqlite_spans["spans"]) + for span in itertools.chain( + postgres_rollback["spans"], + postgres_commit["spans"], + sqlite_rollback["spans"], + sqlite_commit["spans"], + ) if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_COMMIT + or span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK ] assert len(commit_spans) == 0 @@ -118,6 +167,7 @@ def test_db_transaction_spans_disabled_atomic(sentry_init, client, capture_event events = capture_events() + client.get(reverse("postgres_insert_orm_atomic_rollback")) client.get(reverse("postgres_insert_orm_atomic")) with start_transaction(name="test_transaction"): @@ -137,6 +187,44 @@ def test_db_transaction_spans_disabled_atomic(sentry_init, client, capture_event 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) + transaction.set_rollback(True) + + 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 = ( @@ -157,18 +245,26 @@ def test_db_transaction_spans_disabled_atomic(sentry_init, client, capture_event ) cursor.executemany(query, query_list) - (postgres_spans, sqlite_spans) = events + (postgres_rollback, postgres_commit, sqlite_rollback, sqlite_commit) = events # Ensure operation is persisted assert User.objects.using("postgres").exists() - assert postgres_spans["contexts"]["trace"]["origin"] == "auto.http.django" - assert sqlite_spans["contexts"]["trace"]["origin"] == "manual" + assert postgres_rollback["contexts"]["trace"]["origin"] == "auto.http.django" + assert postgres_commit["contexts"]["trace"]["origin"] == "auto.http.django" + assert sqlite_rollback["contexts"]["trace"]["origin"] == "manual" + assert sqlite_commit["contexts"]["trace"]["origin"] == "manual" commit_spans = [ span - for span in itertools.chain(postgres_spans["spans"], sqlite_spans["spans"]) + for span in itertools.chain( + postgres_rollback["spans"], + postgres_commit["spans"], + sqlite_rollback["spans"], + sqlite_commit["spans"], + ) if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_COMMIT + or span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK ] assert len(commit_spans) == 0 @@ -315,6 +411,148 @@ def test_db_no_autocommit_executemany(sentry_init, client, capture_events): 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_rollback_execute(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=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_no_autocommit_rollback")) + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.using("postgres").exists() + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "postgresql" + conn_params = connections["postgres"].get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + assert rollback_span["data"].get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) + assert rollback_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 rollback statements are siblings + assert rollback_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_no_autocommit_rollback_executemany(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=True)], + 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.rollback() + transaction.set_autocommit(True) + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.exists() + + assert event["contexts"]["trace"]["origin"] == "manual" + assert event["spans"][0]["origin"] == "auto.db.django" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" + conn_params = connection.get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_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 rollback statements are siblings + for insert_span in insert_spans: + assert rollback_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): @@ -452,3 +690,288 @@ def test_db_atomic_executemany(sentry_init, client, capture_events): # 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_rollback_execute(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=True)], + 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_rollback")) + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.using("postgres").exists() + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "postgresql" + conn_params = connections["postgres"].get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + assert rollback_span["data"].get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) + assert rollback_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 rollback statements are siblings + assert rollback_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_rollback_executemany(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=True)], + 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) + transaction.set_rollback(True) + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.exists() + + assert event["contexts"]["trace"]["origin"] == "manual" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" + conn_params = connection.get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_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 rollback statements are siblings + for insert_span in insert_spans: + assert rollback_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_execute_exception(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=True)], + 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_exception")) + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.using("postgres").exists() + + assert event["contexts"]["trace"]["origin"] == "auto.http.django" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "postgresql" + conn_params = connections["postgres"].get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_span["data"].get(SPANDATA.DB_NAME) == conn_params.get( + "database" + ) or conn_params.get("dbname") + assert rollback_span["data"].get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) + assert rollback_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 rollback statements are siblings + assert rollback_span["parent_span_id"] == insert_span["parent_span_id"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_db_atomic_executemany_exception(sentry_init, client, capture_events): + sentry_init( + integrations=[DjangoIntegration(db_transaction_spans=True)], + send_default_pii=True, + traces_sample_rate=1.0, + ) + + events = capture_events() + + with start_transaction(name="test_transaction"): + from django.db import connection, transaction + + try: + 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) + 1 / 0 + except ZeroDivisionError: + pass + + (event,) = events + + # Ensure operation is rolled back + assert not User.objects.exists() + + assert event["contexts"]["trace"]["origin"] == "manual" + + rollback_spans = [ + span + for span in event["spans"] + if span["data"].get(SPANDATA.DB_OPERATION) == SPANNAME.DB_ROLLBACK + ] + assert len(rollback_spans) == 1 + rollback_span = rollback_spans[0] + assert rollback_span["origin"] == "auto.db.django" + + # Verify other database attributes + assert rollback_span["data"].get(SPANDATA.DB_SYSTEM) == "sqlite" + conn_params = connection.get_connection_params() + assert rollback_span["data"].get(SPANDATA.DB_NAME) is not None + assert rollback_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 rollback statements are siblings + for insert_span in insert_spans: + assert rollback_span["parent_span_id"] == insert_span["parent_span_id"]