Skip to content

Commit

Permalink
fix(tracing): fix logic for setting in_app flag
Browse files Browse the repository at this point in the history
Previously, in case packages added in `in_app_include` were installed
into a location outside of the project root directory, stack frames from
those packages were not marked as `in_app`. 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 inconsistency: traces from the same project would have different
`in_app` flags depending on the deployment method.

Steps to reproduce (virtualenv outside of project root):
```
$ docker run --replace --rm --name sentry-postgres -e POSTGRES_USER=sentry -e POSTGRES_PASSWORD=sentry -d -p 5432:5432 postgres
$ distrobox create -i ubuntu:24.04 -n 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
$ pytest tests/integrations/django/test_db_query_data.py::test_query_source_with_in_app_include  # FAIL

```

Steps to reproduce (system packages):
```
$ docker run --replace --rm --name sentry-postgres -e POSTGRES_USER=sentry -e POSTGRES_PASSWORD=sentry -d -p 5432:5432 postgres
$ distrobox create -i ubuntu:24.04 -n 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
$ pytest tests/integrations/django/test_db_query_data.py::test_query_source_with_in_app_include  # FAIL
```

In this change, the logic was slightly changed to avoid these
discrepancies and conform to the requirements, described in the PR with
`in_app` flag introduction:
getsentry#1894 (comment).
Note that the `_module_in_list` function returns `False` if `name` is
`None`, hence extra check before function call can be omitted to simplify
code.
  • Loading branch information
rominf committed Jul 18, 2024
1 parent e0a9b5f commit 2568b9c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 14 deletions.
21 changes: 8 additions & 13 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
is_sentry_url,
_is_external_source,
_module_in_list,
_is_in_project_root,
)
from sentry_sdk._types import TYPE_CHECKING

Expand Down Expand Up @@ -218,20 +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.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

Expand Down
2 changes: 1 addition & 1 deletion sentry_sdk/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 2568b9c

Please sign in to comment.