feat(code-mappings): Add new function for applying code mappings to s…

…tack frames (try 2) (#63128)

There are a couple problems with the current code mapping flow that are
addressed in this commit:

1. Code mapping logic is duplicated (and slightly different) in
stacktrace linking and suspect commits.

To ensure a consistent experience, stacktrace linking and suspect
commits should apply code mappings in a similar way. By introducing a
new function convert_stacktrace_frame_path_to_source_path(), we can use
it in both locations to guarantee that the implementations do not
diverge. (Note that this function is tested but not yet used - will
update stacktrace linking and suspect commits in a separate PR)

2. Code mappings only apply to filename, not abs_path

Certain platforms have trouble creating valid code mappings because
filename only contains the file name and the folder structure is in
abs_path (see
#43516 (comment)).
By adding it as a fallback check in
convert_stacktrace_frame_path_to_source_path() we can support these
platforms.

Related to the above concern with abs_path, I also modified
get_sorted_code_mapping_configs() to check for absolute paths in the
stack_root while sorting code mappings. Without this check, the sorting
does not work as expected.

src/sentry/integrations/utils/code_mapping.py

@@ -8,6 +8,7 @@
 from sentry.models.project import Project
 from sentry.models.repository import Repository
 from sentry.services.hybrid_cloud.integration.model import RpcOrganizationIntegration
+from sentry.utils.event_frames import EventFrame, try_munge_frame_path
 
 logger = logging.getLogger(__name__)
 logger.setLevel(logging.INFO)
@@ -100,6 +101,38 @@ def filter_source_code_files(files: List[str]) -> List[str]:
     return _supported_files
 
 
+def convert_stacktrace_frame_path_to_source_path(
+    frame: EventFrame,
+    code_mapping: RepositoryProjectPathConfig,
+    platform: str | None,
+    sdk_name: str | None,
+) -> str | None:
+    """
+    Applies the given code mapping to the given stacktrace frame and returns the source path.
+
+    If the code mapping does not apply to the frame, returns None.
+    """
+
+    # In most cases, code mappings get applied to frame.filename, but some platforms such as Java
+    # contain folder info in other parts of the frame (e.g. frame.module="com.example.app.MainActivity"
+    # gets transformed to "com/example/app/MainActivity.java"), so in those cases we use the
+    # transformed path instead.
+    stacktrace_path = (
+        try_munge_frame_path(frame=frame, platform=platform, sdk_name=sdk_name) or frame.filename
+    )
+
+    if stacktrace_path and stacktrace_path.startswith(code_mapping.stack_root):
+        return stacktrace_path.replace(code_mapping.stack_root, code_mapping.source_root, 1)
+
+    # Some platforms only provide the file's name without folder paths, so we
+    # need to use the absolute path instead. If the code mapping has a non-empty
+    # stack_root value and it matches the absolute path, we do the mapping on it.
+    if frame.abs_path and frame.abs_path.startswith(code_mapping.stack_root):
+        return frame.abs_path.replace(code_mapping.stack_root, code_mapping.source_root, 1)
+
+    return None
+
+
 # XXX: Look at sentry.interfaces.stacktrace and maybe use that
 class FrameFilename:
     def __init__(self, frame_file_path: str) -> None:
@@ -433,6 +466,7 @@ def get_sorted_code_mapping_configs(project: Project) -> List[RepositoryProjectP
     """
     Returns the code mapping config list for a project sorted based on precedence.
     User generated code mappings are evaluated before Sentry generated code mappings.
+    Code mappings with absolute path stack roots are evaluated before relative path stack roots.
     Code mappings with more defined stack trace roots are evaluated before less defined stack trace
     roots.
 
@@ -455,10 +489,15 @@ def get_sorted_code_mapping_configs(project: Project) -> List[RepositoryProjectP
             for index, sorted_config in enumerate(sorted_configs):
                 # This check will ensure that all user defined code mappings will come before Sentry generated ones
                 if (
-                    sorted_config.automatically_generated and not config.automatically_generated
-                ) or (  # Insert more defined stack roots before less defined ones
-                    (sorted_config.automatically_generated == config.automatically_generated)
-                    and config.stack_root.startswith(sorted_config.stack_root)
+                    (sorted_config.automatically_generated and not config.automatically_generated)
+                    or (  # Insert absolute paths before relative paths
+                        not sorted_config.stack_root.startswith("/")
+                        and config.stack_root.startswith("/")
+                    )
+                    or (  # Insert more defined stack roots before less defined ones
+                        (sorted_config.automatically_generated == config.automatically_generated)
+                        and config.stack_root.startswith(sorted_config.stack_root)
+                    )
                 ):
                     sorted_configs.insert(index, config)
                     inserted = True

src/sentry/utils/event_frames.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import inspect
 from copy import deepcopy
 from dataclasses import dataclass, field
 from typing import (
@@ -18,10 +19,25 @@
 from sentry.utils.safe import PathSearchable, get_path
 
 
+@dataclass(frozen=True)
+class EventFrame:
+    lineno: Optional[int] = None
+    in_app: Optional[bool] = None
+    abs_path: Optional[str] = None
+    filename: Optional[str] = None
+    function: Optional[str] = None
+    package: Optional[str] = None
+    module: Optional[str] = None
+
+    @classmethod
+    def from_dict(cls, data: Mapping[str, Any]) -> EventFrame:
+        return cls(**{k: v for k, v in data.items() if k in inspect.signature(cls).parameters})
+
+
 # mypy hack to work around callable assuing the first arg of callable is 'self'
 # https://github.com/python/mypy/issues/5485
 class FrameMunger(Protocol):
-    def __call__(self, key: str, frame: MutableMapping[str, Any]) -> bool:
+    def __call__(self, frame: EventFrame) -> str | None:
         pass
 
 
@@ -32,51 +48,48 @@ class SdkFrameMunger:
     supported_sdks: Set[str] = field(default_factory=set)
 
 
-def java_frame_munger(key: str, frame: MutableMapping[str, Any]) -> bool:
-    if frame.get("filename") is None or frame.get("module") is None:
-        return False
-    if "/" not in str(frame.get("filename")) and frame.get("module"):
+def java_frame_munger(frame: EventFrame) -> str | None:
+    if not frame.filename or not frame.module:
+        return None
+    if "/" not in str(frame.filename) and frame.module:
         # Replace the last module segment with the filename, as the
         # terminal element in a module path is the class
-        module = frame["module"].split(".")
-        module[-1] = frame["filename"]
-        frame[key] = "/".join(module)
-        return True
-    return False
+        module = frame.module.split(".")
+        module[-1] = frame.filename
+        return "/".join(module)
+    return None
 
 
-def cocoa_frame_munger(key: str, frame: MutableMapping[str, Any]) -> bool:
-    if not frame.get("package") or not frame.get("abs_path"):
-        return False
+def cocoa_frame_munger(frame: EventFrame) -> str | None:
+    if not frame.package or not frame.abs_path:
+        return None
 
-    rel_path = package_relative_path(frame.get("abs_path"), frame.get("package"))
+    rel_path = package_relative_path(frame.abs_path, frame.package)
     if rel_path:
-        frame[key] = rel_path
-        return True
-    return False
+        return rel_path
+    return None
 
 
-def flutter_frame_munger(key: str, frame: MutableMapping[str, Any]) -> bool:
-    if not frame.get("abs_path"):
-        return False
+def flutter_frame_munger(frame: EventFrame) -> str | None:
+    if not frame.abs_path:
+        return None
 
-    abs_path = str(frame.get("abs_path"))
+    abs_path = str(frame.abs_path)
 
     if abs_path.startswith("dart:"):
-        return False
+        return None
     elif abs_path.startswith("package:"):
-        if not frame.get("package"):
-            return False
+        if not frame.package:
+            return None
 
-        pkg = frame.get("package")
+        pkg = frame.package
         if abs_path.find(f"package:{pkg}") == -1:
-            return False
+            return None
         else:
             src_path = abs_path.replace(f"package:{pkg}", "", 1).strip("/")
             if src_path:
-                frame[key] = src_path
-                return True
-    return False
+                return src_path
+    return None
 
 
 def package_relative_path(abs_path: str | None, package: str | None) -> str | None:
@@ -106,6 +119,23 @@ def get_sdk_name(event_data: PathSearchable) -> Optional[str]:
     return get_path(event_data, "sdk", "name", filter=True) or None
 
 
+def try_munge_frame_path(
+    frame: EventFrame,
+    platform: str | None = None,
+    sdk_name: str | None = None,
+) -> str | None:
+    """
+    Applies platform-specific frame munging for filename pathing.
+
+    If munging was successful, return the munged filename, otherwise return None.
+    """
+    munger = platform and PLATFORM_FRAME_MUNGER.get(platform)
+    if not munger or (munger.requires_sdk and sdk_name not in munger.supported_sdks):
+        return None
+
+    return munger.frame_munger(frame)
+
+
 def munged_filename_and_frames(
     platform: str,
     data_frames: Sequence[Mapping[str, Any]],
@@ -127,7 +157,10 @@ def munged_filename_and_frames(
     )
     frames_updated = False
     for frame in copy_frames:
-        frames_updated |= munger.frame_munger(key, frame)
+        munged_filename = munger.frame_munger(EventFrame.from_dict(frame))
+        if munged_filename:
+            frame[key] = munged_filename
+            frames_updated = True
     return (key, copy_frames) if frames_updated else None
 
 

tests/sentry/integrations/utils/test_code_mapping.py

@@ -9,6 +9,7 @@
     Repo,
     RepoTree,
     UnsupportedFrameFilename,
+    convert_stacktrace_frame_path_to_source_path,
     filter_source_code_files,
     get_extension,
     get_sorted_code_mapping_configs,
@@ -19,6 +20,7 @@
 from sentry.silo.base import SiloMode
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import assume_test_silo_mode
+from sentry.utils.event_frames import EventFrame
 
 sentry_files = [
     "bin/__init__.py",
@@ -328,6 +330,77 @@ def test_normalized_stack_and_source_roots_starts_with_app_dot_dot_slash(self):
         assert source_path == ""
 
 
+class TestConvertStacktraceFramePathToSourcePath(TestCase):
+    def setUp(self):
+        super()
+        with assume_test_silo_mode(SiloMode.CONTROL):
+            self.integration = self.create_provider_integration(provider="example", name="Example")
+            self.integration.add_organization(self.organization, self.user)
+            self.oi = OrganizationIntegration.objects.get(integration_id=self.integration.id)
+
+        self.repo = self.create_repo(
+            project=self.project,
+            name="getsentry/sentry",
+        )
+
+        self.code_mapping_empty = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="",
+            source_root="src/",
+        )
+        self.code_mapping_abs_path = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="/Users/Foo/src/sentry/",
+            source_root="src/sentry/",
+        )
+        self.code_mapping_file = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="sentry/",
+            source_root="src/sentry/",
+        )
+
+    def test_convert_stacktrace_frame_path_to_source_path_empty(self):
+        assert (
+            convert_stacktrace_frame_path_to_source_path(
+                frame=EventFrame(filename="sentry/file.py"),
+                code_mapping=self.code_mapping_empty,
+                platform="python",
+                sdk_name="sentry.python",
+            )
+            == "src/sentry/file.py"
+        )
+
+    def test_convert_stacktrace_frame_path_to_source_path_abs_path(self):
+        assert (
+            convert_stacktrace_frame_path_to_source_path(
+                frame=EventFrame(
+                    filename="file.py", abs_path="/Users/Foo/src/sentry/folder/file.py"
+                ),
+                code_mapping=self.code_mapping_abs_path,
+                platform="python",
+                sdk_name="sentry.python",
+            )
+            == "src/sentry/folder/file.py"
+        )
+
+    def test_convert_stacktrace_frame_path_to_source_path_java(self):
+        assert (
+            convert_stacktrace_frame_path_to_source_path(
+                frame=EventFrame(filename="File.java", module="sentry.module.File"),
+                code_mapping=self.code_mapping_file,
+                platform="java",
+                sdk_name="sentry.java",
+            )
+            == "src/sentry/module/File.java"
+        )
+
+
 class TestGetSortedCodeMappingConfigs(TestCase):
     def setUp(self):
         super()
@@ -390,9 +463,19 @@ def test_get_sorted_code_mapping_configs(self):
             source_root="",
             automatically_generated=True,
         )
+        # Created by user, well defined stack root that references abs_path
+        code_mapping6 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="/Users/User/code/src/getsentry/src/sentry/",
+            source_root="",
+            automatically_generated=False,
+        )
 
         # Expected configs: stack_root, automatically_generated
         expected_config_order = [
+            code_mapping6,  # "/Users/User/code/src/getsentry/src/sentry/", False
             code_mapping3,  # "usr/src/getsentry/", False
             code_mapping4,  # "usr/src/", False
             code_mapping1,  # "", False