Skip to content

Commit 4636afc

Browse files
fix(tracing): Fix add_query_source with modules outside of project root (#3313)
Fix: #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: #1894 (comment). --------- Co-authored-by: Daniel Szoke <[email protected]> Co-authored-by: Daniel Szoke <[email protected]>
1 parent aed18d4 commit 4636afc

File tree

2 files changed

+125
-13
lines changed

2 files changed

+125
-13
lines changed

sentry_sdk/tracing_utils.py

+29-13
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,26 @@ def _get_frame_module_abs_path(frame):
180180
return None
181181

182182

183+
def _should_be_included(
184+
is_sentry_sdk_frame, # type: bool
185+
namespace, # type: Optional[str]
186+
in_app_include, # type: Optional[list[str]]
187+
in_app_exclude, # type: Optional[list[str]]
188+
abs_path, # type: Optional[str]
189+
project_root, # type: Optional[str]
190+
):
191+
# type: (...) -> bool
192+
# in_app_include takes precedence over in_app_exclude
193+
should_be_included = _module_in_list(namespace, in_app_include)
194+
should_be_excluded = _is_external_source(abs_path) or _module_in_list(
195+
namespace, in_app_exclude
196+
)
197+
return not is_sentry_sdk_frame and (
198+
should_be_included
199+
or (_is_in_project_root(abs_path, project_root) and not should_be_excluded)
200+
)
201+
202+
183203
def add_query_source(span):
184204
# type: (sentry_sdk.tracing.Span) -> None
185205
"""
@@ -221,19 +241,15 @@ def add_query_source(span):
221241
"sentry_sdk."
222242
)
223243

224-
# in_app_include takes precedence over in_app_exclude
225-
should_be_included = (
226-
not (
227-
_is_external_source(abs_path)
228-
or _module_in_list(namespace, in_app_exclude)
229-
)
230-
) or _module_in_list(namespace, in_app_include)
231-
232-
if (
233-
_is_in_project_root(abs_path, project_root)
234-
and should_be_included
235-
and not is_sentry_sdk_frame
236-
):
244+
should_be_included = _should_be_included(
245+
is_sentry_sdk_frame=is_sentry_sdk_frame,
246+
namespace=namespace,
247+
in_app_include=in_app_include,
248+
in_app_exclude=in_app_exclude,
249+
abs_path=abs_path,
250+
project_root=project_root,
251+
)
252+
if should_be_included:
237253
break
238254

239255
frame = frame.f_back

tests/test_tracing_utils.py

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
from dataclasses import asdict, dataclass
2+
from typing import Optional, List
3+
4+
from sentry_sdk.tracing_utils import _should_be_included
5+
import pytest
6+
7+
8+
def id_function(val):
9+
# type: (object) -> str
10+
if isinstance(val, ShouldBeIncludedTestCase):
11+
return val.id
12+
13+
14+
@dataclass(frozen=True)
15+
class ShouldBeIncludedTestCase:
16+
id: str
17+
is_sentry_sdk_frame: bool
18+
namespace: Optional[str] = None
19+
in_app_include: Optional[List[str]] = None
20+
in_app_exclude: Optional[List[str]] = None
21+
abs_path: Optional[str] = None
22+
project_root: Optional[str] = None
23+
24+
25+
@pytest.mark.parametrize(
26+
"test_case, expected",
27+
[
28+
(
29+
ShouldBeIncludedTestCase(
30+
id="Frame from Sentry SDK",
31+
is_sentry_sdk_frame=True,
32+
),
33+
False,
34+
),
35+
(
36+
ShouldBeIncludedTestCase(
37+
id="Frame from Django installed in virtualenv inside project root",
38+
is_sentry_sdk_frame=False,
39+
abs_path="/home/username/some_project/.venv/lib/python3.12/site-packages/django/db/models/sql/compiler",
40+
project_root="/home/username/some_project",
41+
namespace="django.db.models.sql.compiler",
42+
in_app_include=["django"],
43+
),
44+
True,
45+
),
46+
(
47+
ShouldBeIncludedTestCase(
48+
id="Frame from project",
49+
is_sentry_sdk_frame=False,
50+
abs_path="/home/username/some_project/some_project/__init__.py",
51+
project_root="/home/username/some_project",
52+
namespace="some_project",
53+
),
54+
True,
55+
),
56+
(
57+
ShouldBeIncludedTestCase(
58+
id="Frame from project module in `in_app_exclude`",
59+
is_sentry_sdk_frame=False,
60+
abs_path="/home/username/some_project/some_project/exclude_me/some_module.py",
61+
project_root="/home/username/some_project",
62+
namespace="some_project.exclude_me.some_module",
63+
in_app_exclude=["some_project.exclude_me"],
64+
),
65+
False,
66+
),
67+
(
68+
ShouldBeIncludedTestCase(
69+
id="Frame from system-wide installed Django",
70+
is_sentry_sdk_frame=False,
71+
abs_path="/usr/lib/python3.12/site-packages/django/db/models/sql/compiler",
72+
project_root="/home/username/some_project",
73+
namespace="django.db.models.sql.compiler",
74+
),
75+
False,
76+
),
77+
(
78+
ShouldBeIncludedTestCase(
79+
id="Frame from system-wide installed Django with `django` in `in_app_include`",
80+
is_sentry_sdk_frame=False,
81+
abs_path="/usr/lib/python3.12/site-packages/django/db/models/sql/compiler",
82+
project_root="/home/username/some_project",
83+
namespace="django.db.models.sql.compiler",
84+
in_app_include=["django"],
85+
),
86+
True,
87+
),
88+
],
89+
ids=id_function,
90+
)
91+
def test_should_be_included(test_case, expected):
92+
# type: (ShouldBeIncludedTestCase, bool) -> None
93+
"""Checking logic, see: https://github.com/getsentry/sentry-python/issues/3312"""
94+
kwargs = asdict(test_case)
95+
kwargs.pop("id")
96+
assert _should_be_included(**kwargs) == expected

0 commit comments

Comments
 (0)