Browse Source

feat(code-mappings): Add precedence for multiple code mappings (#42192)

Add function to sort code mapping by precedence when multiple are
found.

Fixes WOR-2395

Co-authored-by: Armen Zambrano G <armenzg@sentry.io>
Jodi Jang 2 years ago
parent
commit
4ea1fdfcef

+ 49 - 12
src/sentry/api/endpoints/project_stacktrace_link.py

@@ -1,5 +1,6 @@
 import logging
-from typing import Dict, Mapping, Optional
+import re
+from typing import Dict, List, Mapping, Optional
 
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -129,6 +130,46 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint):  # type: ignore
 
     """
 
+    def sort_code_mapping_configs(
+        self,
+        configs: List[RepositoryProjectPathConfig],
+    ) -> List[RepositoryProjectPathConfig]:
+        """
+        Sorts the code mapping config list based on precedence.
+        User generated code mappings are evaluated before Sentry generated code mappings.
+        Code mappings with more defined stack trace roots are evaluated before less defined stack trace
+        roots.
+
+        `configs`: The list of code mapping configs
+
+        """
+        sorted_configs = []  # type: List[RepositoryProjectPathConfig]
+        regex = r"\w+"
+        inserted = False
+        for config in configs:
+            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 (
+                    sorted_config.automatically_generated == config.automatically_generated
+                    # Insert more defined stack roots before less defined ones
+                    and re.match(
+                        sorted_config.stack_root + regex,
+                        config.stack_root,
+                    )
+                ):
+                    sorted_configs.insert(index, config)
+                    inserted = True
+                    break
+            if not inserted:
+                # Insert the code mapping at the back if it's Sentry generated or at the front if it is user defined
+                if config.automatically_generated:
+                    sorted_configs.insert(len(sorted_configs), config)
+                else:
+                    sorted_configs.insert(0, config)
+        return sorted_configs
+
     def get(self, request: Request, project: Project) -> Response:
         ctx = generate_context(request.GET)
         filepath = ctx.get("file")
@@ -153,8 +194,9 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint):  # type: ignore
         configs = RepositoryProjectPathConfig.objects.filter(
             project=project, organization_integration__isnull=False
         )
+        configs = self.sort_code_mapping_configs(configs)
         derived = False
-        matched_code_mappings = []
+        current_config = None
         with configure_scope() as scope:
             set_top_tags(scope, project, ctx, len(configs) > 0)
             for config in configs:
@@ -163,7 +205,6 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint):  # type: ignore
                     # Later on, if there are matching code mappings this will be overwritten
                     result["error"] = "stack_root_mismatch"
                     continue
-                # XXX: The logic above allows all code mappings to be processed
                 if (
                     filepath.startswith(config.stack_root)
                     and config.automatically_generated is True
@@ -187,7 +228,6 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint):  # type: ignore
                     scope.set_tag("stacktrace_link.munged", True)
 
                 current_config = {"config": serialize(config, request.user), "outcome": outcome}
-                matched_code_mappings.append(current_config)
                 # use the provider key to be able to split up stacktrace
                 # link metrics by integration type
                 provider = current_config["config"]["provider"]["key"]
@@ -202,16 +242,13 @@ class ProjectStacktraceLinkEndpoint(ProjectEndpoint):  # type: ignore
             found: bool = result["sourceUrl"] is not None
             scope.set_tag("stacktrace_link.found", found)
             scope.set_tag("stacktrace_link.auto_derived", derived)
-            if matched_code_mappings:
-                # Any code mapping that matches and its results will be returned
-                result["matched_code_mappings"] = matched_code_mappings
-                last = matched_code_mappings[-1]
-                result["config"] = last["config"]  # Backwards compatible
+            if current_config:
+                result["config"] = current_config["config"]
                 if not found:
-                    result["error"] = last["outcome"]["error"]  # Backwards compatible
+                    result["error"] = current_config["outcome"]["error"]
                     # When no code mapping have been matched we have not attempted a URL
-                    if last["outcome"].get("attemptedUrl"):  # Backwards compatible
-                        result["attemptedUrl"] = last["outcome"]["attemptedUrl"]
+                    if current_config["outcome"].get("attemptedUrl"):
+                        result["attemptedUrl"] = current_config["outcome"]["attemptedUrl"]
                     if result["error"] == "stack_root_mismatch":
                         scope.set_tag("stacktrace_link.error", "stack_root_mismatch")
                     else:

+ 118 - 48
tests/sentry/api/endpoints/test_project_stacktrace_link.py

@@ -1,6 +1,7 @@
 from typing import Any, Mapping
 from unittest import mock
 
+from sentry.api.endpoints.project_stacktrace_link import ProjectStacktraceLinkEndpoint
 from sentry.integrations.example.integration import ExampleIntegration
 from sentry.models import Integration, OrganizationIntegration
 from sentry.testutils import APITestCase
@@ -36,8 +37,7 @@ def serialized_integration(integration: Integration) -> Mapping[str, Any]:
     }
 
 
-@region_silo_test
-class ProjectStacktraceLinkTest(APITestCase):
+class BaseProjectStacktraceLink(APITestCase):
     endpoint = "sentry-api-0-project-stacktrace-link"
 
     def setUp(self):
@@ -53,6 +53,29 @@ class ProjectStacktraceLinkTest(APITestCase):
         self.repo.provider = "example"
         self.repo.save()
 
+        self.login_as(self.user)
+
+    def expected_configurations(self, code_mapping) -> Mapping[str, Any]:
+        return {
+            "defaultBranch": "master",
+            "id": str(code_mapping.id),
+            "integrationId": str(self.integration.id),
+            "projectId": str(self.project.id),
+            "projectSlug": self.project.slug,
+            "provider": serialized_provider(),
+            "repoId": str(self.repo.id),
+            "repoName": self.repo.name,
+            "sourceRoot": code_mapping.source_root,
+            "stackRoot": code_mapping.stack_root,
+        }
+
+
+@region_silo_test
+class ProjectStacktraceLinkTest(BaseProjectStacktraceLink):
+    endpoint = "sentry-api-0-project-stacktrace-link"
+
+    def setUp(self):
+        BaseProjectStacktraceLink.setUp(self)
         self.code_mapping1 = self.create_code_mapping(
             organization_integration=self.oi,
             project=self.project,
@@ -70,21 +93,6 @@ class ProjectStacktraceLinkTest(APITestCase):
         )
 
         self.filepath = "usr/src/getsentry/src/sentry/src/sentry/utils/safe.py"
-        self.login_as(self.user)
-
-    def expected_configurations(self, code_mapping) -> Mapping[str, Any]:
-        return {
-            "defaultBranch": "master",
-            "id": str(code_mapping.id),
-            "integrationId": str(self.integration.id),
-            "projectId": str(self.project.id),
-            "projectSlug": self.project.slug,
-            "provider": serialized_provider(),
-            "repoId": str(self.repo.id),
-            "repoName": self.repo.name,
-            "sourceRoot": code_mapping.source_root,
-            "stackRoot": code_mapping.stack_root,
-        }
 
     def test_no_filepath(self):
         """The file query search is missing"""
@@ -218,22 +226,9 @@ class ProjectStacktraceLinkTest(APITestCase):
 
 
 @region_silo_test
-class ProjectStacktraceLinkTestMobile(APITestCase):
-    endpoint = "sentry-api-0-project-stacktrace-link"
-
+class ProjectStacktraceLinkTestMobile(BaseProjectStacktraceLink):
     def setUp(self):
-        self.integration = Integration.objects.create(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.repo.integration_id = self.integration.id
-        self.repo.provider = "example"
-        self.repo.save()
-
+        BaseProjectStacktraceLink.setUp(self)
         self.code_mapping1 = self.create_code_mapping(
             organization_integration=self.oi,
             project=self.project,
@@ -242,22 +237,6 @@ class ProjectStacktraceLinkTestMobile(APITestCase):
             source_root="",
         )
 
-        self.login_as(self.user)
-
-    def expected_configurations(self, code_mapping) -> Mapping[str, Any]:
-        return {
-            "defaultBranch": "master",
-            "id": str(code_mapping.id),
-            "integrationId": str(self.integration.id),
-            "projectId": str(self.project.id),
-            "projectSlug": self.project.slug,
-            "provider": serialized_provider(),
-            "repoId": str(self.repo.id),
-            "repoName": self.repo.name,
-            "sourceRoot": code_mapping.source_root,
-            "stackRoot": code_mapping.stack_root,
-        }
-
     @mock.patch.object(
         ExampleIntegration,
         "get_stacktrace_link",
@@ -317,3 +296,94 @@ class ProjectStacktraceLinkTestMobile(APITestCase):
         )
         assert response.data["config"] == self.expected_configurations(self.code_mapping1)
         assert response.data["sourceUrl"] == f"{example_base_url}/{file_path}"
+
+
+class ProjectStacktraceLinkTestMultipleMatches(BaseProjectStacktraceLink):
+    def setUp(self):
+        BaseProjectStacktraceLink.setUp(self)
+        # Created by the user, not well defined stack root
+        self.code_mapping1 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="",
+            source_root="",
+            automatically_generated=False,
+        )
+        # Created by automation, not as well defined stack root
+        self.code_mapping2 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="usr/src/getsentry/src/",
+            source_root="",
+            automatically_generated=True,
+        )
+        # Created by the user, well defined stack root
+        self.code_mapping3 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="usr/src/getsentry/",
+            source_root="",
+            automatically_generated=False,
+        )
+        # Created by the user, not as well defined stack root
+        self.code_mapping4 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="usr/src/",
+            source_root="",
+            automatically_generated=False,
+        )
+        # Created by automation, well defined stack root
+        self.code_mapping5 = self.create_code_mapping(
+            organization_integration=self.oi,
+            project=self.project,
+            repo=self.repo,
+            stack_root="usr/src/getsentry/src/sentry/",
+            source_root="",
+            automatically_generated=True,
+        )
+        self.code_mappings = [
+            self.code_mapping1,
+            self.code_mapping2,
+            self.code_mapping3,
+            self.code_mapping4,
+            self.code_mapping5,
+        ]
+
+        self.filepath = "usr/src/getsentry/src/sentry/src/sentry/utils/safe.py"
+
+    def test_test_multiple_code_mapping_matches_order(self):
+        project_stacktrace_link_endpoint = ProjectStacktraceLinkEndpoint()
+
+        configs = self.code_mappings
+        # Expected configs: stack_root, automatically_generated
+        expected_config_order = [
+            self.code_mapping3,  # "usr/src/getsentry/", False
+            self.code_mapping4,  # "usr/src/", False
+            self.code_mapping1,  # "", False
+            self.code_mapping5,  # "usr/src/getsentry/src/sentry/", True
+            self.code_mapping2,  # "usr/src/getsentry/src/", True
+        ]
+        sorted_configs = project_stacktrace_link_endpoint.sort_code_mapping_configs(configs)
+        assert sorted_configs == expected_config_order
+
+    def test_multiple_code_mapping_matches(self):
+        with mock.patch.object(
+            ExampleIntegration,
+            "get_stacktrace_link",
+            return_value="https://github.com/usr/src/getsentry/src/sentry/src/sentry/utils/safe.py",
+        ):
+            response = self.get_success_response(
+                self.organization.slug, self.project.slug, qs_params={"file": self.filepath}
+            )
+            # Assert that the code mapping that is user generated and has the most defined stack
+            # trace of the user generated code mappings is chosen
+            assert response.data["config"] == self.expected_configurations(self.code_mapping3)
+            assert (
+                response.data["sourceUrl"]
+                == "https://github.com/usr/src/getsentry/src/sentry/src/sentry/utils/safe.py"
+            )