diff --git a/src/sentry/issues/auto_source_code_config/constants.py b/src/sentry/issues/auto_source_code_config/constants.py index 5625d0d5afb243..9f428bc1a51188 100644 --- a/src/sentry/issues/auto_source_code_config/constants.py +++ b/src/sentry/issues/auto_source_code_config/constants.py @@ -9,7 +9,7 @@ DERIVED_ENHANCEMENTS_OPTION_KEY = "sentry:derived_grouping_enhancements" SUPPORTED_INTEGRATIONS = [IntegrationProviderSlug.GITHUB.value] STACK_ROOT_MAX_LEVEL = 4 -# Stacktrace roots that match one of these will have three levels of granularity +# Stacktrace roots that match one of these will have an extra level of granularity # com.au, co.uk, org.uk, gov.uk, net.uk, edu.uk, ct.uk # This list does not have to be exhaustive as the fallback is two levels of granularity SECOND_LEVEL_TLDS = ("com", "co", "org", "gov", "net", "edu") diff --git a/src/sentry/issues/auto_source_code_config/frame_info.py b/src/sentry/issues/auto_source_code_config/frame_info.py index 3b63867d2f3875..b921279dac4d3a 100644 --- a/src/sentry/issues/auto_source_code_config/frame_info.py +++ b/src/sentry/issues/auto_source_code_config/frame_info.py @@ -2,9 +2,10 @@ import re from abc import ABC, abstractmethod -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from typing import Any +from sentry import options from sentry.integrations.source_code_management.repo_trees import get_extension from .constants import SECOND_LEVEL_TLDS, STACK_ROOT_MAX_LEVEL @@ -42,7 +43,7 @@ def __init__(self, frame: Mapping[str, Any]) -> None: self.process_frame(frame) def __repr__(self) -> str: - return f"FrameInfo: {self.raw_path}" + return f"FrameInfo: {self.raw_path} stack_root: {self.stack_root}" def __eq__(self, other: object) -> bool: if not isinstance(other, FrameInfo): @@ -148,6 +149,14 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]: # Gets rid of the class name parts = module.rsplit(".", 1)[0].split(".") dirpath = "/".join(parts) + granularity = get_granularity(parts) + + stack_root = "/".join(parts[:granularity]) + "/" + file_path = f"{dirpath}/{abs_path}" + return stack_root, file_path + + +def get_granularity(parts: Sequence[str]) -> int: # a.Bar, Bar.kt -> stack_root: a/, file_path: a/Bar.kt granularity = 1 @@ -163,6 +172,10 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]: # file_path: uk/co/example/foo/bar/Baz.kt granularity = STACK_ROOT_MAX_LEVEL - stack_root = "/".join(parts[:granularity]) + "/" - file_path = f"{dirpath}/{abs_path}" - return stack_root, file_path + elif options.get("auto_source_code_config.multi_module_java"): + # com.example.multi.foo.bar.Baz$InnerClass, Baz.kt -> + # stack_root: com/example/multi/foo/ + # file_path: com/example/multi/foo/bar/Baz.kt + granularity = STACK_ROOT_MAX_LEVEL + + return granularity diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index 1c7ba24774c00c..a8ddd368033de0 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -903,6 +903,13 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) +register( + "auto_source_code_config.multi_module_java", + type=Bool, + default=False, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) + register( "issues.severity.first-event-severity-calculation-projects-allowlist", type=Sequence, diff --git a/src/sentry/utils/event_frames.py b/src/sentry/utils/event_frames.py index 25e695c476ad78..ab2562a5570648 100644 --- a/src/sentry/utils/event_frames.py +++ b/src/sentry/utils/event_frames.py @@ -45,6 +45,7 @@ class SdkFrameMunger: def java_frame_munger(frame: EventFrame) -> str | None: stacktrace_path = None if not frame.module or not frame.abs_path: + logger.warning("Module or absPath is missing", extra={"frame": frame}) return None from sentry.issues.auto_source_code_config.errors import ( diff --git a/tests/sentry/issues/auto_source_code_config/test_frame_info.py b/tests/sentry/issues/auto_source_code_config/test_frame_info.py index 6aa8da07b700a7..a3c34bee2f7a97 100644 --- a/tests/sentry/issues/auto_source_code_config/test_frame_info.py +++ b/tests/sentry/issues/auto_source_code_config/test_frame_info.py @@ -49,7 +49,9 @@ class TestFrameInfo: def test_frame_filename_repr(self) -> None: path = "getsentry/billing/tax/manager.py" - assert create_frame_info({"filename": path}).__repr__() == f"FrameInfo: {path}" + frame_info = create_frame_info({"filename": path}) + expected = f"FrameInfo: {path} stack_root: {frame_info.stack_root}" + assert frame_info.__repr__() == expected @pytest.mark.parametrize("filepath", UNSUPPORTED_FRAME_FILENAMES) def test_raises_unsupported(self, filepath: str) -> None: @@ -88,6 +90,8 @@ def test_java_raises_exception( with pytest.raises(expected_exception): create_frame_info(frame, "java") + # Only necessary while auto_source_code_config.multi_module_java is used + @pytest.mark.django_db @pytest.mark.parametrize( "frame, expected_stack_root, expected_normalized_path", [ diff --git a/tests/sentry/issues/auto_source_code_config/test_process_event.py b/tests/sentry/issues/auto_source_code_config/test_process_event.py index e6144a05d4f0f0..33228e8445a439 100644 --- a/tests/sentry/issues/auto_source_code_config/test_process_event.py +++ b/tests/sentry/issues/auto_source_code_config/test_process_event.py @@ -176,11 +176,11 @@ def _process_and_assert_configuration_changes( expected_new_code_mappings ) for expected_cm in expected_new_code_mappings: - code_mapping = current_code_mappings.filter( - stack_root=expected_cm["stack_root"], - source_root=expected_cm["source_root"], - ).first() + code_mapping = current_code_mappings.get( + project_id=self.project.id, stack_root=expected_cm["stack_root"] + ) assert code_mapping is not None + assert code_mapping.source_root == expected_cm["source_root"] assert code_mapping.repository.name == expected_cm["repo_name"] else: assert current_code_mappings.count() == starting_code_mappings_count @@ -1003,3 +1003,64 @@ def test_categorized_frames_are_not_processed(self) -> None: expected_new_code_mappings=[self.code_mapping("android/app/", "src/android/app/")], expected_new_in_app_stack_trace_rules=["stack.module:android.app.** +app"], ) + + def test_multi_module_with_old_granularity(self) -> None: + java_module_prefix = "com.example.multi" + module_prefix = java_module_prefix.replace(".", "/") + "/" + repo_trees = { + REPO1: [ + f"modules/modX/{module_prefix}foo/Bar.kt", + f"modules/modY/{module_prefix}bar/Baz.kt", + ] + } + frames = [ + self.frame_from_module(f"{java_module_prefix}.foo.Bar", "Bar.kt"), + self.frame_from_module(f"{java_module_prefix}.bar.Baz", "Baz.kt"), + ] + self._process_and_assert_configuration_changes( + repo_trees=repo_trees, + frames=frames, + platform=self.platform, + expected_new_code_mappings=[ + # It's missing the extra granularity + # It's going to pick modY since it is the first frame processed, thus, + # the other frame will not have a working code mapping + self.code_mapping("com/example/multi/", "modules/modY/com/example/multi/") + ], + expected_new_in_app_stack_trace_rules=["stack.module:com.example.** +app"], + ) + + def test_multi_module(self) -> None: + # Some Java projects have all modules under the same com/foo/bar directory + # however, some projects have different modules under different directories + # Case 1: + # com.example.multi.foo -> modules/com/example/multi/foo/Bar.kt + # com.example.multi.bar -> modules/com/example/multi/bar/Baz.kt + # Case 2: + # com.example.multi.foo -> modules/modX/com/example/multi/foo/Bar.kt (Notice modX infix) + # com.example.multi.bar -> modules/modY/com/example/multi/bar/Baz.kt (Notice modY infix) + java_module_prefix = "com.example.multi" + module_prefix = java_module_prefix.replace(".", "/") + "/" + repo_trees = { + REPO1: [ + f"modules/modX/{module_prefix}foo/Bar.kt", + f"modules/modY/{module_prefix}bar/Baz.kt", + ] + } + frames = [ + self.frame_from_module(f"{java_module_prefix}.foo.Bar", "Bar.kt"), + self.frame_from_module(f"{java_module_prefix}.bar.Baz", "Baz.kt"), + ] + with self.options({"auto_source_code_config.multi_module_java": True}): + self._process_and_assert_configuration_changes( + repo_trees=repo_trees, + frames=frames, + platform=self.platform, + expected_new_code_mappings=[ + self.code_mapping(f"{module_prefix}foo/", f"modules/modX/{module_prefix}foo/"), + self.code_mapping(f"{module_prefix}bar/", f"modules/modY/{module_prefix}bar/"), + ], + expected_new_in_app_stack_trace_rules=[ + f"stack.module:{java_module_prefix}.** +app" + ], + ) diff --git a/tests/sentry/issues/ownership/test_grammar.py b/tests/sentry/issues/ownership/test_grammar.py index fbae958c005fdd..1fd6aee7a2bc96 100644 --- a/tests/sentry/issues/ownership/test_grammar.py +++ b/tests/sentry/issues/ownership/test_grammar.py @@ -230,6 +230,8 @@ def test_matcher_test_threads() -> None: assert not Matcher("url", "*.py").test(data, munged_data) +# Only necessary while auto_source_code_config.multi_module_java is used +@pytest.mark.django_db def test_matcher_test_platform_java_threads() -> None: data = { "platform": "java", diff --git a/tests/sentry/utils/test_event_frames.py b/tests/sentry/utils/test_event_frames.py index 3745ba74322d0a..f9837c28568f7d 100644 --- a/tests/sentry/utils/test_event_frames.py +++ b/tests/sentry/utils/test_event_frames.py @@ -1,5 +1,7 @@ import unittest +import pytest + from sentry.testutils.cases import TestCase from sentry.utils.event_frames import ( EventFrame, @@ -76,6 +78,8 @@ def test_platform_other(self) -> None: def test_platform_sdk_name_not_supported(self) -> None: assert not munged_filename_and_frames("javascript", [], "munged", "sdk.other") + # Only necessary while auto_source_code_config.multi_module_java is used + @pytest.mark.django_db def test_supported_platform_sdk_name_not_required(self) -> None: frames = [ { @@ -88,6 +92,8 @@ def test_supported_platform_sdk_name_not_required(self) -> None: class JavaFilenameMungingTestCase(unittest.TestCase): + # Only necessary while auto_source_code_config.multi_module_java is used + @pytest.mark.django_db def test_platform_java(self) -> None: frames = [ { @@ -140,6 +146,8 @@ def test_platform_java_do_not_follow_java_package_naming_convention_does_not_rai munged = munged_filename_and_frames("java", [frame]) assert munged is None + # Only necessary while auto_source_code_config.multi_module_java is used + @pytest.mark.django_db def test_platform_android_kotlin(self) -> None: exception_frames = [ {