diff --git a/src/sentry/integrations/utils/code_mapping.py b/src/sentry/integrations/utils/code_mapping.py index 09b712ef274380..c5e37ee6d72e29 100644 --- a/src/sentry/integrations/utils/code_mapping.py +++ b/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 diff --git a/src/sentry/utils/event_frames.py b/src/sentry/utils/event_frames.py index 0a02bd16e87299..c6a3e8b6b36d99 100644 --- a/src/sentry/utils/event_frames.py +++ b/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 diff --git a/tests/sentry/integrations/utils/test_code_mapping.py b/tests/sentry/integrations/utils/test_code_mapping.py index af9e58e679c37c..c4ea25175fd475 100644 --- a/tests/sentry/integrations/utils/test_code_mapping.py +++ b/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 diff --git a/tests/sentry/utils/test_event_frames.py b/tests/sentry/utils/test_event_frames.py index 6326c15d0b5f19..cec3fb12343972 100644 --- a/tests/sentry/utils/test_event_frames.py +++ b/tests/sentry/utils/test_event_frames.py @@ -2,6 +2,7 @@ from sentry.testutils.cases import TestCase from sentry.utils.event_frames import ( + EventFrame, cocoa_frame_munger, find_stack_frames, flutter_frame_munger, @@ -279,34 +280,27 @@ def test_simple(self): "symbol_addr": "0x102ce2b70", } - did_munge = cocoa_frame_munger("munged_filename", exception_frame) - assert did_munge - assert ( - exception_frame["munged_filename"] - == "SampleProject/Classes/App Delegate/AppDelegate.swift" - ) + munged_filename = cocoa_frame_munger(EventFrame.from_dict(exception_frame)) + assert munged_filename == "SampleProject/Classes/App Delegate/AppDelegate.swift" def test_missing_required_no_munging(self): assert cocoa_frame_munger( - "munged_filename", - { - "package": "SampleProject", - "abs_path": "SampleProject/AppDelegate.swift", - }, + EventFrame( + package="SampleProject", + abs_path="SampleProject/AppDelegate.swift", + ) ) - assert not cocoa_frame_munger("munged_filename", {}) + assert not cocoa_frame_munger(EventFrame()) assert not cocoa_frame_munger( - "munged_filename", - { - "package": "SampleProject", - }, + EventFrame( + package="SampleProject", + ) ) assert not cocoa_frame_munger( - "munged_filename", - { - "abs_path": "SampleProject/AppDelegate.swift", - }, + EventFrame( + abs_path="SampleProject/AppDelegate.swift", + ) ) def test_package_relative_repeats(self): @@ -316,10 +310,9 @@ def test_package_relative_repeats(self): "abs_path": "/Users/gszeto/code/SampleProject/more/dirs/SwiftySampleProject/SampleProject/Classes/App Delegate/AppDelegate.swift", } - did_munge = cocoa_frame_munger("munged_filename", exception_frame) - assert did_munge + munged_filename = cocoa_frame_munger(EventFrame.from_dict(exception_frame)) assert ( - exception_frame["munged_filename"] + munged_filename == "SampleProject/more/dirs/SwiftySampleProject/SampleProject/Classes/App Delegate/AppDelegate.swift" ) @@ -416,37 +409,31 @@ def test_flutter_munger_supported(self): def test_dart_prefix_not_munged(self): assert not flutter_frame_munger( - "munged_filename", - { - "abs_path": "dart:ui/a/b/test.dart", - }, + EventFrame(abs_path="dart:ui/a/b/test.dart"), ) def test_abs_path_not_present_not_munged(self): assert not flutter_frame_munger( - "munged_filename", - { - "function": "tryCatchModule", - "package": "sentry_flutter_example", - "filename": "test.dart", - }, + EventFrame( + function="tryCatchModule", + package="sentry_flutter_example", + filename="test.dart", + ) ) def test_different_package_not_munged(self): assert not flutter_frame_munger( - "munged_filename", - { - "package": "sentry_flutter_example", - "abs_path": "package:different_package/a/b/test.dart", - }, + EventFrame( + package="sentry_flutter_example", + abs_path="package:different_package/a/b/test.dart", + ) ) def test_no_package_not_munged(self): assert not flutter_frame_munger( - "munged_filename", - { - "abs_path": "package:different_package/a/b/test.dart", - }, + EventFrame( + abs_path="package:different_package/a/b/test.dart", + ) )