diff --git a/0005-test-Allow-passing-of-PostgreSQL-port.patch b/0005-test-Allow-passing-of-PostgreSQL-port.patch new file mode 100644 index 0000000..504f104 --- /dev/null +++ b/0005-test-Allow-passing-of-PostgreSQL-port.patch @@ -0,0 +1,58 @@ +From 08fc215667b4b85dd791ac4d1f0dd188f3ec575f Mon Sep 17 00:00:00 2001 +From: Roman Inflianskas +Date: Thu, 11 Jul 2024 17:29:00 +0300 +Subject: [PATCH] test: Allow passing of PostgreSQL port + +In some environments, port `5432` might be already opened by some other +process. Allow passing an arbitrary port via the +`SENTPY_PYTHON_TEST_POSTGRES_PORT` environmental variable. +--- + tests/integrations/asyncpg/test_asyncpg.py | 2 +- + tests/integrations/django/myapp/settings.py | 2 +- + tests/integrations/django/test_basic.py | 4 +++- + 3 files changed, 5 insertions(+), 3 deletions(-) + +diff --git a/tests/integrations/asyncpg/test_asyncpg.py b/tests/integrations/asyncpg/test_asyncpg.py +index 94b02f4c..e36d15c5 100644 +--- a/tests/integrations/asyncpg/test_asyncpg.py ++++ b/tests/integrations/asyncpg/test_asyncpg.py +@@ -13,7 +13,7 @@ import os + + + PG_HOST = os.getenv("SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost") +-PG_PORT = 5432 ++PG_PORT = int(os.getenv("SENTRY_PYTHON_TEST_POSTGRES_PORT", "5432")) + PG_USER = os.getenv("SENTRY_PYTHON_TEST_POSTGRES_USER", "postgres") + PG_PASSWORD = os.getenv("SENTRY_PYTHON_TEST_POSTGRES_PASSWORD", "sentry") + PG_NAME = os.getenv("SENTRY_PYTHON_TEST_POSTGRES_NAME", "postgres") +diff --git a/tests/integrations/django/myapp/settings.py b/tests/integrations/django/myapp/settings.py +index 8956357a..0678762b 100644 +--- a/tests/integrations/django/myapp/settings.py ++++ b/tests/integrations/django/myapp/settings.py +@@ -122,7 +122,7 @@ try: + DATABASES["postgres"] = { + "ENGINE": db_engine, + "HOST": os.environ.get("SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost"), +- "PORT": 5432, ++ "PORT": int(os.environ.get("SENTRY_PYTHON_TEST_POSTGRES_PORT", "5432")), + "USER": os.environ.get("SENTRY_PYTHON_TEST_POSTGRES_USER", "postgres"), + "PASSWORD": os.environ.get("SENTRY_PYTHON_TEST_POSTGRES_PASSWORD", "sentry"), + "NAME": os.environ.get( +diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py +index f79c6e13..1505204f 100644 +--- a/tests/integrations/django/test_basic.py ++++ b/tests/integrations/django/test_basic.py +@@ -626,7 +626,9 @@ def test_db_connection_span_data(sentry_init, client, capture_events): + assert data.get(SPANDATA.SERVER_ADDRESS) == os.environ.get( + "SENTRY_PYTHON_TEST_POSTGRES_HOST", "localhost" + ) +- assert data.get(SPANDATA.SERVER_PORT) == "5432" ++ assert data.get(SPANDATA.SERVER_PORT) == os.environ.get( ++ "SENTRY_PYTHON_TEST_POSTGRES_PORT", "5432" ++ ) + + + def test_set_db_data_custom_backend(): +-- +2.45.2 + diff --git a/0006-test-tracing-Test-add_query_source-with-modules-outs.patch b/0006-test-tracing-Test-add_query_source-with-modules-outs.patch new file mode 100644 index 0000000..04781f4 --- /dev/null +++ b/0006-test-tracing-Test-add_query_source-with-modules-outs.patch @@ -0,0 +1,238 @@ +From da164dad9eea765f2156cc44ef9a86a848aff4d9 Mon Sep 17 00:00:00 2001 +From: Roman Inflianskas +Date: Thu, 18 Jul 2024 18:54:14 +0300 +Subject: [PATCH 1/2] test(tracing): Test `add_query_source` with modules + outside of project root + +When packages added in `in_app_include` are installed to a location +outside of the project root directory, span from those packages are not +extended with OTel compatible source code information. Cases include +running Python from virtualenv created outside of the project root +directory or Python packages installed into the system using package +managers. This results in an inconsistency: spans from the same project +are be different, depending on the deployment method. + +The change extends `test_query_source_with_in_app_include` to test the +simulation of Django installed outside of the project root. + +The steps to manually reproduce the issue are as follows +(case: a virtual environment created outside of the project root): +```bash +docker run --replace --rm --detach \ + --name sentry-postgres \ + --env POSTGRES_USER=sentry \ + --env POSTGRES_PASSWORD=sentry \ + --publish 5432:5432 \ + postgres +distrobox create \ + --image ubuntu:24.04 \ + --name sentry-test-in_app_include-venv +distrobox enter sentry-test-in_app_include-venv +python3 -m venv /tmp/.venv-test-in_app_include +source /tmp/.venv-test-in_app_include/bin/activate +pip install \ + -r requirements-devenv.txt \ + pytest-django \ + psycopg2-binary \ + -e .[django] +export SENTRY_PYTHON_TEST_POSTGRES_USER=sentry +export SENTRY_PYTHON_TEST_POSTGRES_PASSWORD=sentry +pytest tests/integrations/django/test_db_query_data.py \ + -k test_query_source_with_in_app_include # FAIL +``` + +The steps to manually reproduce the issue are as follows +(case: Django is installed through system packages): +```bash +docker run --replace --rm --detach \ + --name sentry-postgres \ + --env POSTGRES_USER=sentry \ + --env POSTGRES_PASSWORD=sentry \ + --publish 5432:5432 \ + postgres +distrobox create \ + --image ubuntu:24.04 \ + --name sentry-test-in_app_include-os +distrobox enter sentry-test-in_app_include-os +sudo apt install \ + python3-django python3-pytest python3-pytest-cov \ + python3-pytest-django python3-jsonschema python3-urllib3 \ + python3-certifi python3-werkzeug python3-psycopg2 +export SENTRY_PYTHON_TEST_POSTGRES_USER=sentry +export SENTRY_PYTHON_TEST_POSTGRES_PASSWORD=sentry +pytest tests/integrations/django/test_db_query_data.py \ + -k test_query_source_with_in_app_include # FAIL +``` +--- + sentry_sdk/tracing_utils.py | 21 ++++---- + sentry_sdk/utils.py | 5 +- + .../integrations/django/test_db_query_data.py | 54 +++++++++++++++++-- + 3 files changed, 66 insertions(+), 14 deletions(-) + +diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py +index ba20dc84..47e14ecd 100644 +--- a/sentry_sdk/tracing_utils.py ++++ b/sentry_sdk/tracing_utils.py +@@ -170,6 +170,14 @@ def maybe_create_breadcrumbs_from_span(scope, span): + ) + + ++def _get_frame_module_abs_path(frame): ++ # type: (FrameType) -> Optional[str] ++ try: ++ return frame.f_code.co_filename ++ except Exception: ++ return None ++ ++ + def add_query_source(span): + # type: (sentry_sdk.tracing.Span) -> None + """ +@@ -200,10 +208,7 @@ def add_query_source(span): + # Find the correct frame + frame = sys._getframe() # type: Union[FrameType, None] + while frame is not None: +- try: +- abs_path = frame.f_code.co_filename +- except Exception: +- abs_path = "" ++ abs_path = _get_frame_module_abs_path(frame) + + try: + namespace = frame.f_globals.get("__name__") # type: Optional[str] +@@ -224,7 +229,8 @@ def add_query_source(span): + should_be_included = True + + if ( +- abs_path.startswith(project_root) ++ abs_path is not None ++ and abs_path.startswith(project_root) + and should_be_included + and not is_sentry_sdk_frame + ): +@@ -250,10 +256,7 @@ def add_query_source(span): + if namespace is not None: + span.set_data(SPANDATA.CODE_NAMESPACE, namespace) + +- try: +- filepath = frame.f_code.co_filename +- except Exception: +- filepath = None ++ filepath = _get_frame_module_abs_path(frame) + if filepath is not None: + if namespace is not None: + in_app_path = filename_for_module(namespace, filepath) +diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py +index 8a805d3d..fcbff45e 100644 +--- a/sentry_sdk/utils.py ++++ b/sentry_sdk/utils.py +@@ -1058,8 +1058,11 @@ def _module_in_list(name, items): + + + def _is_external_source(abs_path): +- # type: (str) -> bool ++ # type: (Optional[str]) -> bool + # check if frame is in 'site-packages' or 'dist-packages' ++ if abs_path is None: ++ return False ++ + external_source = ( + re.search(r"[\\/](?:dist|site)-packages[\\/]", abs_path) is not None + ) +diff --git a/tests/integrations/django/test_db_query_data.py b/tests/integrations/django/test_db_query_data.py +index 087fc5ad..265c6a79 100644 +--- a/tests/integrations/django/test_db_query_data.py ++++ b/tests/integrations/django/test_db_query_data.py +@@ -1,7 +1,9 @@ ++import contextlib + import os + + import pytest + from datetime import datetime ++from pathlib import Path, PurePosixPath, PureWindowsPath + from unittest import mock + + from django import VERSION as DJANGO_VERSION +@@ -15,14 +17,20 @@ except ImportError: + from werkzeug.test import Client + + from sentry_sdk import start_transaction ++from sentry_sdk._types import TYPE_CHECKING + from sentry_sdk.consts import SPANDATA + from sentry_sdk.integrations.django import DjangoIntegration +-from sentry_sdk.tracing_utils import record_sql_queries ++from sentry_sdk.tracing_utils import _get_frame_module_abs_path, record_sql_queries ++from sentry_sdk.utils import _module_in_list + + from tests.conftest import unpack_werkzeug_response + from tests.integrations.django.utils import pytest_mark_django_db_decorator + from tests.integrations.django.myapp.wsgi import application + ++if TYPE_CHECKING: ++ from types import FrameType ++ from typing import Optional ++ + + @pytest.fixture + def client(): +@@ -283,7 +291,10 @@ def test_query_source_with_in_app_exclude(sentry_init, client, capture_events): + + @pytest.mark.forked + @pytest_mark_django_db_decorator(transaction=True) +-def test_query_source_with_in_app_include(sentry_init, client, capture_events): ++@pytest.mark.parametrize("django_outside_of_project_root", [False, True]) ++def test_query_source_with_in_app_include( ++ sentry_init, client, capture_events, django_outside_of_project_root ++): + sentry_init( + integrations=[DjangoIntegration()], + send_default_pii=True, +@@ -301,8 +312,43 @@ def test_query_source_with_in_app_include(sentry_init, client, capture_events): + + events = capture_events() + +- _, status, _ = unpack_werkzeug_response(client.get(reverse("postgres_select_orm"))) +- assert status == "200 OK" ++ # Simulate Django installation outside of the project root ++ original_get_frame_module_abs_path = _get_frame_module_abs_path ++ ++ def patched_get_frame_module_abs_path_function(frame): ++ # type: (FrameType) -> Optional[str] ++ result = original_get_frame_module_abs_path(frame) ++ if result is None: ++ return result ++ ++ namespace = frame.f_globals.get("__name__") ++ if _module_in_list(namespace, ["django"]): ++ # Since the result is used as `str` only and not accessed, ++ # it is sufficient to generate non-existent path ++ # that would be located outside of the project root. ++ # Use UNC path for simplicity on Windows. ++ non_existent_prefix = ( ++ PureWindowsPath("//outside-of-project-root") ++ if os.name == "nt" ++ else PurePosixPath("/outside-of-project-root") ++ ) ++ result = str(non_existent_prefix.joinpath(*Path(result).parts[1:])) ++ return result ++ ++ patched_get_frame_module_abs_path = ( ++ mock.patch( ++ "sentry_sdk.tracing_utils._get_frame_module_abs_path", ++ patched_get_frame_module_abs_path_function, ++ ) ++ if django_outside_of_project_root ++ else contextlib.suppress() ++ ) ++ ++ with patched_get_frame_module_abs_path: ++ _, status, _ = unpack_werkzeug_response( ++ client.get(reverse("postgres_select_orm")) ++ ) ++ assert status == "200 OK" + + (event,) = events + for span in event["spans"]: +-- +2.45.2 + diff --git a/0007-fix-tracing-Fix-add_query_source-with-modules-outsid.patch b/0007-fix-tracing-Fix-add_query_source-with-modules-outsid.patch new file mode 100644 index 0000000..3eda4db --- /dev/null +++ b/0007-fix-tracing-Fix-add_query_source-with-modules-outsid.patch @@ -0,0 +1,97 @@ +From 7ff39563c20b83809ae2378479d0324880231b33 Mon Sep 17 00:00:00 2001 +From: Roman Inflianskas +Date: Thu, 18 Jul 2024 16:57:21 +0300 +Subject: [PATCH 2/2] fix(tracing): Fix `add_query_source` with modules outside + of project root + +Fix: https://github.com/getsentry/sentry-python/issues/3312 + +Previously, when packages added in `in_app_include` were installed +to a location outside of the project root directory, span from +those packages were not extended with OTel compatible source code +information. Cases include running Python from virtualenv created +outside of the project root directory or Python packages installed into +the system using package managers. This resulted in an inconsistency: +spans from the same project would be different, depending on the +deployment method. + +In this change, the logic was slightly changed to avoid these +discrepancies and conform to the requirements, described in the PR with +better setting of in-app in stack frames: +https://github.com/getsentry/sentry-python/pull/1894#issue-1579192436. +Note that the `_module_in_list` function returns `False` if `name` is +`None` or `items` are falsy, hence extra check before function call can +be omitted to simplify code. +--- + sentry_sdk/tracing_utils.py | 22 ++++++++-------------- + sentry_sdk/utils.py | 6 +++--- + 2 files changed, 11 insertions(+), 17 deletions(-) + +diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py +index 47e14ecd..bfcf8d0a 100644 +--- a/sentry_sdk/tracing_utils.py ++++ b/sentry_sdk/tracing_utils.py +@@ -22,6 +22,7 @@ from sentry_sdk.utils import ( + is_sentry_url, + _is_external_source, + _module_in_list, ++ _is_in_project_root, + ) + from sentry_sdk._types import TYPE_CHECKING + +@@ -218,21 +219,14 @@ def add_query_source(span): + is_sentry_sdk_frame = namespace is not None and namespace.startswith( + "sentry_sdk." + ) ++ should_be_included = _module_in_list(namespace, in_app_include) ++ should_be_excluded = _is_external_source(abs_path) or _module_in_list( ++ namespace, in_app_exclude ++ ) + +- should_be_included = not _is_external_source(abs_path) +- if namespace is not None: +- if in_app_exclude and _module_in_list(namespace, in_app_exclude): +- should_be_included = False +- if in_app_include and _module_in_list(namespace, in_app_include): +- # in_app_include takes precedence over in_app_exclude, so doing it +- # at the end +- should_be_included = True +- +- if ( +- abs_path is not None +- and abs_path.startswith(project_root) +- and should_be_included +- and not is_sentry_sdk_frame ++ if not is_sentry_sdk_frame and ( ++ should_be_included ++ or (_is_in_project_root(abs_path, project_root) and not should_be_excluded) + ): + break + +diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py +index fcbff45e..22df7aef 100644 +--- a/sentry_sdk/utils.py ++++ b/sentry_sdk/utils.py +@@ -1043,7 +1043,7 @@ def event_from_exception( + + + def _module_in_list(name, items): +- # type: (str, Optional[List[str]]) -> bool ++ # type: (Optional[str], Optional[List[str]]) -> bool + if name is None: + return False + +@@ -1070,8 +1070,8 @@ def _is_external_source(abs_path): + + + def _is_in_project_root(abs_path, project_root): +- # type: (str, Optional[str]) -> bool +- if project_root is None: ++ # type: (Optional[str], Optional[str]) -> bool ++ if abs_path is None or project_root is None: + return False + + # check if path is in the project root +-- +2.45.2 + diff --git a/python-sentry-sdk.spec b/python-sentry-sdk.spec index 5482504..6398c0a 100644 --- a/python-sentry-sdk.spec +++ b/python-sentry-sdk.spec @@ -66,10 +66,22 @@ Patch1: 0002-ref-tests-Unhardcode-integration-list.patch Patch2: 0003-fix-tests-Fix-exception-on-copying-frame.f_locals-Py.patch # Upstream PR: https://github.com/getsentry/sentry-python/pull/3272 Patch3: 0004-fix-utils-Handle-partialmethod-in-qualname_from_func.patch +# Patch for non-default PostgreSQL port +# Upstream PR: https://github.com/getsentry/sentry-python/pull/3281 +Patch4: 0005-test-Allow-passing-of-PostgreSQL-port.patch +# Patches for testing and fixing logic for handling `in_app_include` in `add_query_source` +# Upstream PR: https://github.com/getsentry/sentry-python/pull/3313 +Patch5: 0006-test-tracing-Test-add_query_source-with-modules-outs.patch +Patch6: 0007-fix-tracing-Fix-add_query_source-with-modules-outsid.patch BuildArch: noarch BuildRequires: python3-devel %if %{with tests} +%if 0%{?fedora} == 39 +BuildRequires: postgresql-test-rpm-macros +%else +BuildRequires: postgresql15-test-rpm-macros +%endif BuildRequires: python3dist(botocore) BuildRequires: python3dist(certifi) BuildRequires: python3dist(djangorestframework) @@ -119,6 +131,7 @@ Summary: %{summary} # List of names of extras & toxenvs included %global components %{shrink: aiohttp + asyncpg celery django falcon @@ -161,7 +174,6 @@ Summary: %{summary} # List of names of extras included (if not present in components) %global extras %{shrink: %{components} - asyncpg bottle flask grpcio @@ -197,7 +209,6 @@ Summary: %{summary} # List of names of toxenvs excluded (if not present in components_excluded) # ariadne: no_ariadne # asgi: async_asgi_testclient is unpackaged yet -# asyncpg: require a local PostgreSQL instance running # aws_lambda: aws_lambda requires credentials # boto3: require network # bottle: new_werkzeug @@ -225,7 +236,6 @@ Summary: %{summary} %{toxenvs_excluded_by_components} %{toxenv}-ariadne %{toxenv}-asgi - %{toxenv}-asyncpg %{toxenv}-aws_lambda %{toxenv}-boto3 %{toxenv}-bottle @@ -363,7 +373,6 @@ skip_import_check="${skip_import_check-} -e sentry_sdk.integrations.tornado" # Tests # Deselect/ignore: - # not in tox.ini, probably broken ignore="${ignore-} --ignore=tests/integrations/wsgi" @@ -380,20 +389,6 @@ deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_ deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_cache_spans_middleware" deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_cache_spans_templatetag" deselect="${deselect-} --deselect=tests/integrations/aiohttp/test_aiohttp.py::test_span_origin" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_db_connection_span_data" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_django_connect_breadcrumbs" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_django_connect_trace" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_queryset_repr" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_response_trace" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_sql_dict_query_params" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_sql_psycopg2_placeholders" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_sql_psycopg2_string_composition" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_sql_queries" -deselect="${deselect-} --deselect=tests/integrations/django/test_basic.py::test_user_captured" -deselect="${deselect-} --deselect=tests/integrations/django/test_data_scrubbing.py::test_scrub_django_custom_session_cookies_filtered" -deselect="${deselect-} --deselect=tests/integrations/django/test_data_scrubbing.py::test_scrub_django_session_cookies_filtered" -deselect="${deselect-} --deselect=tests/integrations/django/test_data_scrubbing.py::test_scrub_django_session_cookies_removed" -deselect="${deselect-} --deselect=tests/integrations/django/test_db_query_data.py" deselect="${deselect-} --deselect=tests/integrations/requests/test_requests.py::test_omit_url_data_if_parsing_fails" deselect="${deselect-} --deselect=tests/integrations/requests/test_requests.py::test_crumb_capture" deselect="${deselect-} --deselect=tests/integrations/stdlib/test_httplib.py::test_span_origin" @@ -406,9 +401,6 @@ ignore="${ignore-} --ignore=tests/integrations/socket" # https://github.com/getsentry/sentry-python/blob/2.7.1/tests/integrations/aws_lambda/test_aws.py#L6 ignore="${ignore-} --ignore=tests/integrations/aws_lambda/" -# require a local PostgreSQL instance running -ignore="${ignore-} --ignore=tests/integrations/asyncpg" - # testing suite relies on the test to be executed in clean env deselect="${deselect-} --deselect=tests/test_basics.py::test_auto_enabling_integrations_catches_import_error" @@ -450,6 +442,14 @@ diff <(echo "$defined_toxenvs") <(echo "$tox_ini_toxenvs") %{_bindir}/redis-server --bind 127.0.0.1 --port 6379 & REDIS_SERVER_PID=$! +# Start postresql-server, which is required for some integrations tests. +%postgresql_tests_run +export SENTRY_PYTHON_TEST_POSTGRES_USER=sentry_test_user +export SENTRY_PYTHON_TEST_POSTGRES_PASSWORD=sentry_test_password +export SENTRY_PYTHON_TEST_POSTGRES_NAME=sentry_test_name +export SENTRY_PYTHON_TEST_POSTGRES_PORT=$PGTESTS_PORT +psql -c "CREATE ROLE $SENTRY_PYTHON_TEST_POSTGRES_USER WITH LOGIN SUPERUSER PASSWORD '$SENTRY_PYTHON_TEST_POSTGRES_PASSWORD';" +createdb $SENTRY_PYTHON_TEST_POSTGRES_NAME --owner $SENTRY_PYTHON_TEST_POSTGRES_USER DJANGO_SETTINGS_MODULE=tests.integrations.django.myapp.settings %tox -e %{toxenvs_csv} -- -- ${deselect-} ${ignore-}