From 2f05dab6c997efeae330dcc3c87cb274345d5361 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 6 Feb 2024 16:48:45 -0500 Subject: [PATCH 1/7] Improve query source path handling Use `filename_for_module` is possible. This will create the same source paths as our stack trace handling. --- sentry_sdk/tracing_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index bc0ddc51d5..57d1e0015e 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -7,6 +7,7 @@ from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.utils import ( capture_internal_exceptions, + filename_for_module, Dsn, match_regex_list, to_string, @@ -255,7 +256,9 @@ def add_query_source(hub, span): except Exception: filepath = None if filepath is not None: - if project_root is not None and filepath.startswith(project_root): + if namespace is not None: + in_app_path = filename_for_module(namespace, filepath) + elif project_root is not None and filepath.startswith(project_root): in_app_path = filepath.replace(project_root, "").lstrip(os.sep) else: in_app_path = filepath From 69e4f6ce60a5ac9d2fa0d6ac83de54a3583087cf Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 13 Feb 2024 15:34:25 -0500 Subject: [PATCH 2/7] Disable Objective C fork safety errors These are mostly benign and show up on recent macOS versions. Turning them off so tests can succeed. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 90806b4220..34870b1ada 100644 --- a/tox.ini +++ b/tox.ini @@ -577,6 +577,7 @@ deps = setenv = PYTHONDONTWRITEBYTECODE=1 + OBJC_DISABLE_INITIALIZE_FORK_SAFETY=YES common: TESTPATH=tests gevent: TESTPATH=tests aiohttp: TESTPATH=tests/integrations/aiohttp From 99f458417660a3b7c3830f95553761d4c1411088 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 7 Feb 2024 16:01:16 -0500 Subject: [PATCH 3/7] Omit Python 2.x --- sentry_sdk/tracing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 57d1e0015e..98cdec5e38 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -256,7 +256,7 @@ def add_query_source(hub, span): except Exception: filepath = None if filepath is not None: - if namespace is not None: + if namespace is not None and not PY2: in_app_path = filename_for_module(namespace, filepath) elif project_root is not None and filepath.startswith(project_root): in_app_path = filepath.replace(project_root, "").lstrip(os.sep) From d0125d6f127ad4e853bb125585e11d0158a2fbd2 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Tue, 13 Feb 2024 17:19:35 -0500 Subject: [PATCH 4/7] Add tests - add a simple helper module to each integration test suite - load the module directly into `sys.path` so it can be referenced - check that the module name and path are correct --- sentry_sdk/tracing_utils.py | 2 +- tests/integrations/asyncpg/__init__.py | 4 ++ .../asyncpg/asyncpg_helpers/__init__.py | 0 .../asyncpg/asyncpg_helpers/helpers.py | 2 + tests/integrations/asyncpg/test_asyncpg.py | 48 ++++++++++++++ tests/integrations/django/__init__.py | 4 ++ .../django/django_helpers/__init__.py | 0 .../django/django_helpers/views.py | 9 +++ tests/integrations/django/myapp/urls.py | 6 ++ .../integrations/django/test_db_query_data.py | 58 +++++++++++++++++ tests/integrations/sqlalchemy/__init__.py | 4 ++ .../sqlalchemy/sqlalchemy_helpers/__init__.py | 0 .../sqlalchemy/sqlalchemy_helpers/helpers.py | 7 ++ .../sqlalchemy/test_sqlalchemy.py | 65 +++++++++++++++++++ 14 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 tests/integrations/asyncpg/asyncpg_helpers/__init__.py create mode 100644 tests/integrations/asyncpg/asyncpg_helpers/helpers.py create mode 100644 tests/integrations/django/django_helpers/__init__.py create mode 100644 tests/integrations/django/django_helpers/views.py create mode 100644 tests/integrations/sqlalchemy/sqlalchemy_helpers/__init__.py create mode 100644 tests/integrations/sqlalchemy/sqlalchemy_helpers/helpers.py diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 98cdec5e38..57d1e0015e 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -256,7 +256,7 @@ def add_query_source(hub, span): except Exception: filepath = None if filepath is not None: - if namespace is not None and not PY2: + if namespace is not None: in_app_path = filename_for_module(namespace, filepath) elif project_root is not None and filepath.startswith(project_root): in_app_path = filepath.replace(project_root, "").lstrip(os.sep) diff --git a/tests/integrations/asyncpg/__init__.py b/tests/integrations/asyncpg/__init__.py index 50f607f3a6..4aa8ab65e4 100644 --- a/tests/integrations/asyncpg/__init__.py +++ b/tests/integrations/asyncpg/__init__.py @@ -1,4 +1,8 @@ +import os +import sys import pytest pytest.importorskip("asyncpg") pytest.importorskip("pytest_asyncio") + +sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/asyncpg/asyncpg_helpers/__init__.py b/tests/integrations/asyncpg/asyncpg_helpers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integrations/asyncpg/asyncpg_helpers/helpers.py b/tests/integrations/asyncpg/asyncpg_helpers/helpers.py new file mode 100644 index 0000000000..8de809ba1b --- /dev/null +++ b/tests/integrations/asyncpg/asyncpg_helpers/helpers.py @@ -0,0 +1,2 @@ +async def execute_query_in_connection(query, connection): + await connection.execute(query) diff --git a/tests/integrations/asyncpg/test_asyncpg.py b/tests/integrations/asyncpg/test_asyncpg.py index 705ac83dbc..f98bda1f21 100644 --- a/tests/integrations/asyncpg/test_asyncpg.py +++ b/tests/integrations/asyncpg/test_asyncpg.py @@ -19,6 +19,7 @@ PG_PORT = 5432 +from sentry_sdk._compat import PY2 import datetime import asyncpg @@ -592,6 +593,53 @@ async def test_query_source(sentry_init, capture_events): assert data.get(SPANDATA.CODE_FUNCTION) == "test_query_source" +@pytest.mark.asyncio +async def test_query_source_with_edited_sys_path(sentry_init, capture_events): + sentry_init( + integrations=[AsyncPGIntegration()], + enable_tracing=True, + enable_db_query_source=True, + db_query_source_threshold_ms=0, + ) + + events = capture_events() + + from asyncpg_helpers.helpers import execute_query_in_connection + + with start_transaction(name="test_transaction", sampled=True): + conn: Connection = await connect(PG_CONNECTION_URI) + + await execute_query_in_connection( + "INSERT INTO users(name, password, dob) VALUES ('Alice', 'secret', '1990-12-25')", + conn, + ) + + await conn.close() + + (event,) = events + + span = event["spans"][-1] + assert span["description"].startswith("INSERT INTO") + + data = span.get("data", {}) + + assert SPANDATA.CODE_LINENO in data + assert SPANDATA.CODE_NAMESPACE in data + assert SPANDATA.CODE_FILEPATH in data + assert SPANDATA.CODE_FUNCTION in data + + assert type(data.get(SPANDATA.CODE_LINENO)) == int + assert data.get(SPANDATA.CODE_LINENO) > 0 + if not PY2: + assert data.get(SPANDATA.CODE_NAMESPACE) == "asyncpg_helpers.helpers" + assert data.get(SPANDATA.CODE_FILEPATH) == "asyncpg_helpers/helpers.py" + + is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep + assert is_relative_path + + assert data.get(SPANDATA.CODE_FUNCTION) == "execute_query_in_connection" + + @pytest.mark.asyncio async def test_no_query_source_if_duration_too_short(sentry_init, capture_events): sentry_init( diff --git a/tests/integrations/django/__init__.py b/tests/integrations/django/__init__.py index 70cc4776d5..e0ab3f2a5a 100644 --- a/tests/integrations/django/__init__.py +++ b/tests/integrations/django/__init__.py @@ -1,3 +1,7 @@ +import os +import sys import pytest pytest.importorskip("django") + +sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/django/django_helpers/__init__.py b/tests/integrations/django/django_helpers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integrations/django/django_helpers/views.py b/tests/integrations/django/django_helpers/views.py new file mode 100644 index 0000000000..a5759a5199 --- /dev/null +++ b/tests/integrations/django/django_helpers/views.py @@ -0,0 +1,9 @@ +from django.contrib.auth.models import User +from django.http import HttpResponse +from django.views.decorators.csrf import csrf_exempt + + +@csrf_exempt +def postgres_select_orm(request, *args, **kwargs): + user = User.objects.using("postgres").all().first() + return HttpResponse("ok {}".format(user)) diff --git a/tests/integrations/django/myapp/urls.py b/tests/integrations/django/myapp/urls.py index 706be13c3a..92621b07a2 100644 --- a/tests/integrations/django/myapp/urls.py +++ b/tests/integrations/django/myapp/urls.py @@ -26,6 +26,7 @@ def path(path, *args, **kwargs): from . import views +from django_helpers import views as helper_views urlpatterns = [ path("view-exc", views.view_exc, name="view_exc"), @@ -59,6 +60,11 @@ def path(path, *args, **kwargs): path("template-test3", views.template_test3, name="template_test3"), path("postgres-select", views.postgres_select, name="postgres_select"), path("postgres-select-slow", views.postgres_select_orm, name="postgres_select_orm"), + path( + "postgres-select-slow-from-supplement", + helper_views.postgres_select_orm, + name="postgres_select_slow_from_supplement", + ), path( "permission-denied-exc", views.permission_denied_exc, diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index cf2ef57358..78bd908e80 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -4,6 +4,7 @@ import pytest from datetime import datetime +from sentry_sdk._compat import PY2 from django import VERSION as DJANGO_VERSION from django.db import connections @@ -168,6 +169,63 @@ def test_query_source(sentry_init, client, capture_events): raise AssertionError("No db span found") +@pytest.mark.forked +@pytest_mark_django_db_decorator(transaction=True) +def test_query_source_with_edited_sys_path(sentry_init, client, capture_events): + client = Client(application) + + sentry_init( + integrations=[DjangoIntegration()], + send_default_pii=True, + traces_sample_rate=1.0, + enable_db_query_source=True, + db_query_source_threshold_ms=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() + + _, status, _ = unpack_werkzeug_response( + client.get(reverse("postgres_select_slow_from_supplement")) + ) + assert status == "200 OK" + + (event,) = events + for span in event["spans"]: + if span.get("op") == "db" and "auth_user" in span.get("description"): + data = span.get("data", {}) + + assert SPANDATA.CODE_LINENO in data + assert SPANDATA.CODE_NAMESPACE in data + assert SPANDATA.CODE_FILEPATH in data + assert SPANDATA.CODE_FUNCTION in data + + assert type(data.get(SPANDATA.CODE_LINENO)) == int + assert data.get(SPANDATA.CODE_LINENO) > 0 + + if not PY2: + assert ( + data.get(SPANDATA.CODE_NAMESPACE) == "supplementary_modules.views" + ) + assert ( + data.get(SPANDATA.CODE_FILEPATH) == "supplementary_modules/views.py" + ) + + is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep + assert is_relative_path + + assert data.get(SPANDATA.CODE_FUNCTION) == "postgres_select_orm" + + break + else: + raise AssertionError("No db span found") + + @pytest.mark.forked @pytest_mark_django_db_decorator(transaction=True) def test_query_source_with_in_app_exclude(sentry_init, client, capture_events): diff --git a/tests/integrations/sqlalchemy/__init__.py b/tests/integrations/sqlalchemy/__init__.py index b430bf6d43..59bd0f7e20 100644 --- a/tests/integrations/sqlalchemy/__init__.py +++ b/tests/integrations/sqlalchemy/__init__.py @@ -1,3 +1,7 @@ +import os +import sys import pytest pytest.importorskip("sqlalchemy") + +sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/sqlalchemy/sqlalchemy_helpers/__init__.py b/tests/integrations/sqlalchemy/sqlalchemy_helpers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integrations/sqlalchemy/sqlalchemy_helpers/helpers.py b/tests/integrations/sqlalchemy/sqlalchemy_helpers/helpers.py new file mode 100644 index 0000000000..ca65a88d25 --- /dev/null +++ b/tests/integrations/sqlalchemy/sqlalchemy_helpers/helpers.py @@ -0,0 +1,7 @@ +def add_model_to_session(model, session): + session.add(model) + session.commit() + + +def query_first_model_from_session(model_klass, session): + return session.query(model_klass).first() diff --git a/tests/integrations/sqlalchemy/test_sqlalchemy.py b/tests/integrations/sqlalchemy/test_sqlalchemy.py index 3f196cd0b9..b306726fb0 100644 --- a/tests/integrations/sqlalchemy/test_sqlalchemy.py +++ b/tests/integrations/sqlalchemy/test_sqlalchemy.py @@ -3,6 +3,7 @@ import sys from datetime import datetime +from sentry_sdk._compat import PY2 from sqlalchemy import Column, ForeignKey, Integer, String, create_engine from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.declarative import declarative_base @@ -449,6 +450,70 @@ class Person(Base): raise AssertionError("No db span found") +def test_query_source_with_edited_sys_path(sentry_init, capture_events): + sentry_init( + integrations=[SqlalchemyIntegration()], + enable_tracing=True, + enable_db_query_source=True, + db_query_source_threshold_ms=0, + ) + events = capture_events() + + from sqlalchemy_helpers.helpers import ( + add_model_to_session, + query_first_model_from_session, + ) + + with start_transaction(name="test_transaction", sampled=True): + Base = declarative_base() # noqa: N806 + + class Person(Base): + __tablename__ = "person" + id = Column(Integer, primary_key=True) + name = Column(String(250), nullable=False) + + engine = create_engine("sqlite:///:memory:") + Base.metadata.create_all(engine) + + Session = sessionmaker(bind=engine) # noqa: N806 + session = Session() + + bob = Person(name="Bob") + + add_model_to_session(bob, session) + + assert query_first_model_from_session(Person, session) == bob + + (event,) = events + + for span in event["spans"]: + if span.get("op") == "db" and span.get("description").startswith( + "SELECT person" + ): + data = span.get("data", {}) + + assert SPANDATA.CODE_LINENO in data + assert SPANDATA.CODE_NAMESPACE in data + assert SPANDATA.CODE_FILEPATH in data + assert SPANDATA.CODE_FUNCTION in data + + assert type(data.get(SPANDATA.CODE_LINENO)) == int + assert data.get(SPANDATA.CODE_LINENO) > 0 + if not PY2: + assert data.get(SPANDATA.CODE_NAMESPACE) == "sqlalchemy_helpers.helpers" + assert ( + data.get(SPANDATA.CODE_FILEPATH) == "sqlalchemy_helpers/helpers.py" + ) + + is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep + assert is_relative_path + + assert data.get(SPANDATA.CODE_FUNCTION) == "query_first_model_from_session" + break + else: + raise AssertionError("No db span found") + + def test_no_query_source_if_duration_too_short(sentry_init, capture_events): sentry_init( integrations=[SqlalchemyIntegration()], From 9e42b3089a365e8fd7967d9577afb0e9eecbd283 Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:18:17 -0500 Subject: [PATCH 5/7] Whoops, missed the rename --- tests/integrations/django/test_db_query_data.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 78bd908e80..1e946e53a6 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -209,12 +209,8 @@ def test_query_source_with_edited_sys_path(sentry_init, client, capture_events): assert data.get(SPANDATA.CODE_LINENO) > 0 if not PY2: - assert ( - data.get(SPANDATA.CODE_NAMESPACE) == "supplementary_modules.views" - ) - assert ( - data.get(SPANDATA.CODE_FILEPATH) == "supplementary_modules/views.py" - ) + assert data.get(SPANDATA.CODE_NAMESPACE) == "django_helpers.views" + assert data.get(SPANDATA.CODE_FILEPATH) == "django_helpers/views.py" is_relative_path = data.get(SPANDATA.CODE_FILEPATH)[0] != os.sep assert is_relative_path From 7c786fcc714373d06e231e609bbd191e5163254c Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:42:12 -0500 Subject: [PATCH 6/7] Omit Python 2 completely --- sentry_sdk/tracing_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 57d1e0015e..98cdec5e38 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -256,7 +256,7 @@ def add_query_source(hub, span): except Exception: filepath = None if filepath is not None: - if namespace is not None: + if namespace is not None and not PY2: in_app_path = filename_for_module(namespace, filepath) elif project_root is not None and filepath.startswith(project_root): in_app_path = filepath.replace(project_root, "").lstrip(os.sep) From 101f0188e8643d4c54bacb93cedee16eaac3959b Mon Sep 17 00:00:00 2001 From: George Gritsouk <989898+gggritso@users.noreply.github.com> Date: Fri, 16 Feb 2024 16:58:59 -0500 Subject: [PATCH 7/7] Improve documentation --- tests/integrations/asyncpg/__init__.py | 2 ++ tests/integrations/asyncpg/test_asyncpg.py | 5 ++++- tests/integrations/django/__init__.py | 2 ++ tests/integrations/django/test_db_query_data.py | 5 ++++- tests/integrations/sqlalchemy/__init__.py | 2 ++ tests/integrations/sqlalchemy/test_sqlalchemy.py | 5 ++++- 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/integrations/asyncpg/__init__.py b/tests/integrations/asyncpg/__init__.py index 4aa8ab65e4..d988407a2d 100644 --- a/tests/integrations/asyncpg/__init__.py +++ b/tests/integrations/asyncpg/__init__.py @@ -5,4 +5,6 @@ pytest.importorskip("asyncpg") pytest.importorskip("pytest_asyncio") +# Load `asyncpg_helpers` into the module search path to test query source path names relative to module. See +# `test_query_source_with_module_in_search_path` sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/asyncpg/test_asyncpg.py b/tests/integrations/asyncpg/test_asyncpg.py index f98bda1f21..a839031c3b 100644 --- a/tests/integrations/asyncpg/test_asyncpg.py +++ b/tests/integrations/asyncpg/test_asyncpg.py @@ -594,7 +594,10 @@ async def test_query_source(sentry_init, capture_events): @pytest.mark.asyncio -async def test_query_source_with_edited_sys_path(sentry_init, capture_events): +async def test_query_source_with_module_in_search_path(sentry_init, capture_events): + """ + Test that query source is relative to the path of the module it ran in + """ sentry_init( integrations=[AsyncPGIntegration()], enable_tracing=True, diff --git a/tests/integrations/django/__init__.py b/tests/integrations/django/__init__.py index e0ab3f2a5a..41d72f92a5 100644 --- a/tests/integrations/django/__init__.py +++ b/tests/integrations/django/__init__.py @@ -4,4 +4,6 @@ pytest.importorskip("django") +# Load `django_helpers` into the module search path to test query source path names relative to module. See +# `test_query_source_with_module_in_search_path` sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py index 1e946e53a6..92b1415f78 100644 --- a/tests/integrations/django/test_db_query_data.py +++ b/tests/integrations/django/test_db_query_data.py @@ -171,7 +171,10 @@ def test_query_source(sentry_init, client, capture_events): @pytest.mark.forked @pytest_mark_django_db_decorator(transaction=True) -def test_query_source_with_edited_sys_path(sentry_init, client, capture_events): +def test_query_source_with_module_in_search_path(sentry_init, client, capture_events): + """ + Test that query source is relative to the path of the module it ran in + """ client = Client(application) sentry_init( diff --git a/tests/integrations/sqlalchemy/__init__.py b/tests/integrations/sqlalchemy/__init__.py index 59bd0f7e20..33c43a6872 100644 --- a/tests/integrations/sqlalchemy/__init__.py +++ b/tests/integrations/sqlalchemy/__init__.py @@ -4,4 +4,6 @@ pytest.importorskip("sqlalchemy") +# Load `sqlalchemy_helpers` into the module search path to test query source path names relative to module. See +# `test_query_source_with_module_in_search_path` sys.path.insert(0, os.path.join(os.path.dirname(__file__))) diff --git a/tests/integrations/sqlalchemy/test_sqlalchemy.py b/tests/integrations/sqlalchemy/test_sqlalchemy.py index b306726fb0..08c8e29ec4 100644 --- a/tests/integrations/sqlalchemy/test_sqlalchemy.py +++ b/tests/integrations/sqlalchemy/test_sqlalchemy.py @@ -450,7 +450,10 @@ class Person(Base): raise AssertionError("No db span found") -def test_query_source_with_edited_sys_path(sentry_init, capture_events): +def test_query_source_with_module_in_search_path(sentry_init, capture_events): + """ + Test that query source is relative to the path of the module it ran in + """ sentry_init( integrations=[SqlalchemyIntegration()], enable_tracing=True,