Better setting of in-app in stack frames (getsentry#1894)

antonpirker · web-flow · commit bb20fc6e6ad5 · 2023-02-15T11:51:26.000+01:00
How the in_app flag is set in stack trace frames (in set_in_app_in_frames()): - If there is already in_app set, it is left untouched. - If there is a module in the frame and it is in the in_app_includes -> in_app=True - If there is a module in the frame and it is in the in_app_excludes -> in_app=False - If there is an abs_path in the frame and the path is in /side-packages/ or /dist-packages/ -> in_app=False - If there is an abs_path in the frame and it starts with the current working directory of the process -> in_app=True - If nothing of the above is true, there will be no in_app set. Fixes getsentry#1754 Fixes getsentry#320
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -90,6 +90,14 @@ def _get_options(*args, **kwargs):
     if rv["instrumenter"] is None:
         rv["instrumenter"] = INSTRUMENTER.SENTRY
 
+    if rv["project_root"] is None:
+        try:
+            project_root = os.getcwd()
+        except Exception:
+            project_root = None
+
+        rv["project_root"] = project_root
+
     return rv
 
 
@@ -103,6 +111,7 @@ class _Client(object):
     def __init__(self, *args, **kwargs):
         # type: (*Any, **Any) -> None
         self.options = get_options(*args, **kwargs)  # type: Dict[str, Any]
+
         self._init_impl()
 
     def __getstate__(self):
@@ -222,7 +231,10 @@ def _prepare_event(
             event["platform"] = "python"
 
         event = handle_in_app(
-            event, self.options["in_app_exclude"], self.options["in_app_include"]
+            event,
+            self.options["in_app_exclude"],
+            self.options["in_app_include"],
+            self.options["project_root"],
         )
 
         # Postprocess the event here so that annotated types do
diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py
@@ -123,6 +123,7 @@ def __init__(
         proxy_headers=None,  # type: Optional[Dict[str, str]]
         instrumenter=INSTRUMENTER.SENTRY,  # type: Optional[str]
         before_send_transaction=None,  # type: Optional[TransactionProcessor]
+        project_root=None,  # type: Optional[str]
     ):
         # type: (...) -> None
         pass
diff --git a/sentry_sdk/profiler.py b/sentry_sdk/profiler.py
@@ -27,9 +27,9 @@
 from sentry_sdk._types import MYPY
 from sentry_sdk.utils import (
     filename_for_module,
-    handle_in_app_impl,
     logger,
     nanosecond_time,
+    set_in_app_in_frames,
 )
 
 if MYPY:
@@ -627,14 +627,14 @@ def process(self):
         }
 
     def to_json(self, event_opt, options):
-        # type: (Any, Dict[str, Any]) -> Dict[str, Any]
+        # type: (Any, Dict[str, Any], Dict[str, Any]) -> Dict[str, Any]
         profile = self.process()
 
-        handle_in_app_impl(
+        set_in_app_in_frames(
             profile["frames"],
             options["in_app_exclude"],
             options["in_app_include"],
-            default_in_app=False,  # Do not default a frame to `in_app: True`
+            options["project_root"],
         )
 
         return {
diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py
@@ -762,44 +762,54 @@ def iter_event_frames(event):
             yield frame
 
 
-def handle_in_app(event, in_app_exclude=None, in_app_include=None):
-    # type: (Dict[str, Any], Optional[List[str]], Optional[List[str]]) -> Dict[str, Any]
+def handle_in_app(event, in_app_exclude=None, in_app_include=None, project_root=None):
+    # type: (Dict[str, Any], Optional[List[str]], Optional[List[str]], Optional[str]) -> Dict[str, Any]
     for stacktrace in iter_event_stacktraces(event):
-        handle_in_app_impl(
+        set_in_app_in_frames(
             stacktrace.get("frames"),
             in_app_exclude=in_app_exclude,
             in_app_include=in_app_include,
+            project_root=project_root,
         )
 
     return event
 
 
-def handle_in_app_impl(frames, in_app_exclude, in_app_include, default_in_app=True):
-    # type: (Any, Optional[List[str]], Optional[List[str]], bool) -> Optional[Any]
+def set_in_app_in_frames(frames, in_app_exclude, in_app_include, project_root=None):
+    # type: (Any, Optional[List[str]], Optional[List[str]], Optional[str]) -> Optional[Any]
     if not frames:
         return None
 
-    any_in_app = False
     for frame in frames:
-        in_app = frame.get("in_app")
-        if in_app is not None:
-            if in_app:
-                any_in_app = True
+        # if frame has already been marked as in_app, skip it
+        current_in_app = frame.get("in_app")
+        if current_in_app is not None:
             continue
 
         module = frame.get("module")
-        if not module:
-            continue
-        elif _module_in_set(module, in_app_include):
+
+        # check if module in frame is in the list of modules to include
+        if _module_in_list(module, in_app_include):
             frame["in_app"] = True
-            any_in_app = True
-        elif _module_in_set(module, in_app_exclude):
+            continue
+
+        # check if module in frame is in the list of modules to exclude
+        if _module_in_list(module, in_app_exclude):
             frame["in_app"] = False
+            continue
 
-    if default_in_app and not any_in_app:
-        for frame in frames:
-            if frame.get("in_app") is None:
-                frame["in_app"] = True
+        # if frame has no abs_path, skip further checks
+        abs_path = frame.get("abs_path")
+        if abs_path is None:
+            continue
+
+        if _is_external_source(abs_path):
+            frame["in_app"] = False
+            continue
+
+        if _is_in_project_root(abs_path, project_root):
+            frame["in_app"] = True
+            continue
 
     return frames
 
@@ -847,13 +857,39 @@ def event_from_exception(
     )
 
 
-def _module_in_set(name, set):
+def _module_in_list(name, items):
     # type: (str, Optional[List[str]]) -> bool
-    if not set:
+    if name is None:
+        return False
+
+    if not items:
         return False
-    for item in set or ():
+
+    for item in items:
         if item == name or name.startswith(item + "."):
             return True
+
+    return False
+
+
+def _is_external_source(abs_path):
+    # type: (str) -> bool
+    # check if frame is in 'site-packages' or 'dist-packages'
+    external_source = (
+        re.search(r"[\\/](?:dist|site)-packages[\\/]", abs_path) is not None
+    )
+    return external_source
+
+
+def _is_in_project_root(abs_path, project_root):
+    # type: (str, Optional[str]) -> bool
+    if project_root is None:
+        return False
+
+    # check if path is in the project root
+    if abs_path.startswith(project_root):
+        return True
+
     return False
 
 
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -601,7 +601,6 @@ def test_template_exception(
 
     assert template_frame["post_context"] == ["11\n", "12\n", "13\n", "14\n", "15\n"]
     assert template_frame["lineno"] == 10
-    assert template_frame["in_app"]
     assert template_frame["filename"].endswith("error.html")
 
     filenames = [
diff --git a/tests/test_client.py b/tests/test_client.py
@@ -401,7 +401,6 @@ def test_attach_stacktrace_in_app(sentry_init, capture_events):
     pytest_frames = [f for f in frames if f["module"].startswith("_pytest")]
     assert pytest_frames
     assert all(f["in_app"] is False for f in pytest_frames)
-    assert any(f["in_app"] for f in frames)
 
 
 def test_attach_stacktrace_disabled(sentry_init, capture_events):
diff --git a/tests/utils/test_general.py b/tests/utils/test_general.py
