From db40dfc26d7099b60eb22826cd08cab353d218c5 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 18 Jul 2024 18:54:14 +0300 Subject: [PATCH 1/2] chore(tracing): Refactor `tracing_utils.py` Preparation for: https://github.com/getsentry/sentry-python/pull/3313 Proposed in: https://github.com/getsentry/sentry-python/pull/3313#discussion_r1704258749 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 | 34 ++++++++++++++++++---------------- sentry_sdk/utils.py | 11 +++++++---- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 0dabfbc486..6ec765c1cc 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -21,6 +21,7 @@ to_string, is_sentry_url, _is_external_source, + _is_in_project_root, _module_in_list, ) from sentry_sdk._types import TYPE_CHECKING @@ -170,6 +171,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 +209,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] @@ -215,16 +221,15 @@ def add_query_source(span): ) 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 _module_in_list(namespace, in_app_exclude): + should_be_included = False + if _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) + _is_in_project_root(abs_path, project_root) and should_be_included and not is_sentry_sdk_frame ): @@ -250,10 +255,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 2fb7561ac8..5954337b67 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1076,7 +1076,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 @@ -1091,8 +1091,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 ) @@ -1100,8 +1103,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 From 61be4abab6d9240772a8651a256afd35ac2b77a9 Mon Sep 17 00:00:00 2001 From: Daniel Szoke Date: Mon, 26 Aug 2024 16:53:27 +0200 Subject: [PATCH 2/2] ref: Further simplify `should_be_included` logic --- sentry_sdk/tracing_utils.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 6ec765c1cc..d86a04ea47 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -220,13 +220,13 @@ def add_query_source(span): "sentry_sdk." ) - should_be_included = not _is_external_source(abs_path) - if _module_in_list(namespace, in_app_exclude): - should_be_included = False - if _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 + # in_app_include takes precedence over in_app_exclude + should_be_included = ( + not ( + _is_external_source(abs_path) + or _module_in_list(namespace, in_app_exclude) + ) + ) or _module_in_list(namespace, in_app_include) if ( _is_in_project_root(abs_path, project_root)