specs/python-sentry-sdk
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

Run integration tests with a PostgreSQL server

Browse source 
Enabling integration tests with a PostgreSQL server led to the discovery
of the issue, which is documented at
https://github.com/getsentry/sentry-python/issues/3312.

Short version:
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.

Add a patch to fix this issue as part of the update.

[skip changelog]
This commit is contained in:
Roman Inflianskas 2024-07-11 16:46:59 +03:00
parent 79be25abc5
commit 304f1d0bd2
No known key found for this signature in database
4 changed files with 414 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
0005-test-Allow-passing-of-PostgreSQL-port.patch Normal file
View file

@ -0,0 +1,58 @@
From 08fc215667b4b85dd791ac4d1f0dd188f3ec575f Mon Sep 17 00:00:00 2001
From: Roman Inflianskas <rominf@pm.me>
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

238
0006-test-tracing-Test-add_query_source-with-modules-outs.patch Normal file
View file

@ -0,0 +1,238 @@
From da164dad9eea765f2156cc44ef9a86a848aff4d9 Mon Sep 17 00:00:00 2001
From: Roman Inflianskas <rominf@pm.me>
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

97
0007-fix-tracing-Fix-add_query_source-with-modules-outsid.patch Normal file
View file

@ -0,0 +1,97 @@
From 7ff39563c20b83809ae2378479d0324880231b33 Mon Sep 17 00:00:00 2001
From: Roman Inflianskas <rominf@pm.me>
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

42
python-sentry-sdk.spec
View file

@ -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-}