Browse Source

Revert "ref(code mapping): Remove feature flags (#45296)"

This reverts commit b94ac9b9d889c05b6e14418b5ac83f1cef59be48.

Co-authored-by: lobsterkatie <14812505+lobsterkatie@users.noreply.github.com>
getsentry-bot 2 years ago
parent
commit
74d6d29262

+ 7 - 0
src/sentry/api/endpoints/organization_derive_code_mappings.py

@@ -4,6 +4,7 @@ from rest_framework import status
 from rest_framework.request import Request
 from rest_framework.response import Response
 
+from sentry import features
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.organization import (
     OrganizationEndpoint,
@@ -35,6 +36,9 @@ class OrganizationDeriveCodeMappingsEndpoint(OrganizationEndpoint):
         :param string stacktraceFilename:
         :auth: required
         """
+        if not features.has("organizations:derive-code-mappings", organization):
+            return Response(status=status.HTTP_403_FORBIDDEN)
+
         stacktrace_filename = request.GET.get("stacktraceFilename")
         installation, _ = get_installation(organization)
         if not installation:
@@ -65,6 +69,9 @@ class OrganizationDeriveCodeMappingsEndpoint(OrganizationEndpoint):
         :param string sourceRoot:
         :auth: required
         """
+        if not features.has("organizations:derive-code-mappings", organization):
+            return Response(status=status.HTTP_403_FORBIDDEN)
+
         installation, organization_integration = get_installation(organization)
         if not installation:
             return self.respond(

+ 4 - 0
src/sentry/conf/server.py

@@ -1022,6 +1022,10 @@ SENTRY_FEATURES = {
     "organizations:auto-enable-codecov": False,
     # Enables getting commit sha from git blame for codecov.
     "organizations:codecov-commit-sha-from-git-blame": False,
+    # Enables automatically deriving of code mappings
+    "organizations:derive-code-mappings": False,
+    # Enables automatically deriving of code mappings as a dry run for early adopters
+    "organizations:derive-code-mappings-dry-run": False,
     # Enable advanced search features, like negation and wildcard matching.
     "organizations:advanced-search": True,
     # Use metrics as the dataset for crash free metric alerts

+ 2 - 0
src/sentry/features/__init__.py

@@ -225,6 +225,8 @@ default_manager.add("organizations:sso-basic", OrganizationFeature)
 default_manager.add("organizations:sso-saml2", OrganizationFeature)
 default_manager.add("organizations:source-maps-cta", OrganizationFeature, True)
 default_manager.add("organizations:team-insights", OrganizationFeature)
+default_manager.add("organizations:derive-code-mappings", OrganizationFeature)
+default_manager.add("organizations:derive-code-mappings-dry-run", OrganizationFeature)
 default_manager.add("organizations:codecov-stacktrace-integration", OrganizationFeature, True)
 default_manager.add("organizations:codecov-stacktrace-integration-v2", OrganizationFeature, True)
 default_manager.add("organizations:auto-enable-codecov", OrganizationFeature)

+ 36 - 1
src/sentry/tasks/derive_code_mappings.py

@@ -5,6 +5,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Tuple
 
 from sentry_sdk import set_tag, set_user
 
+from sentry import features
 from sentry.db.models.fields.node import NodeData
 from sentry.integrations.utils.code_mapping import CodeMapping, CodeMappingTreesHelper
 from sentry.locks import locks
@@ -74,6 +75,7 @@ def process_error(error: ApiError, extra: Dict[str, str]) -> None:
 def derive_code_mappings(
     project_id: int,
     data: NodeData,
+    dry_run=False,
 ) -> None:
     """
     Derive code mappings for a project given data from a recent event.
@@ -89,8 +91,11 @@ def derive_code_mappings(
     extra = {
         "organization.slug": org.slug,
     }
+    feat_key = "organizations:derive-code-mappings"
+    # Check the feature flag again to ensure the feature is still enabled.
+    org_has_flag = features.has(feat_key, org) or features.has(f"{feat_key}-dry-run", org)
 
-    if data["platform"] not in SUPPORTED_LANGUAGES:
+    if not org_has_flag or not data["platform"] in SUPPORTED_LANGUAGES:
         logger.info("Event should not be processed.", extra=extra)
         return
 
@@ -125,6 +130,10 @@ def derive_code_mappings(
 
     trees_helper = CodeMappingTreesHelper(trees)
     code_mappings = trees_helper.generate_code_mappings(stacktrace_paths)
+    if dry_run:
+        report_project_codemappings(code_mappings, stacktrace_paths, project)
+        return
+
     set_project_codemappings(code_mappings, organization_integration, project)
 
 
@@ -219,3 +228,29 @@ def set_project_codemappings(
                     "existing_code_mapping": cm,
                 },
             )
+
+
+def report_project_codemappings(
+    code_mappings: List[CodeMapping],
+    stacktrace_paths: List[str],
+    project: Project,
+) -> None:
+    """
+    Log the code mappings that would be created for a project.
+    """
+    extra = {
+        "org": project.organization.slug,
+        "project": project.slug,
+        "code_mappings": code_mappings,
+        "stacktrace_paths": stacktrace_paths,
+    }
+    if code_mappings:
+        msg = "Code mappings would have been created."
+    else:
+        msg = "NO code mappings would have been created."
+    existing_code_mappings = RepositoryProjectPathConfig.objects.filter(project=project)
+    if existing_code_mappings.exists():
+        msg = "Code mappings already exist."
+        extra["existing_code_mappings"] = existing_code_mappings
+
+    logger.info(msg, extra=extra)

+ 15 - 5
src/sentry/tasks/post_process.py

@@ -755,12 +755,22 @@ def process_code_mappings(job: PostProcessJob) -> None:
         org = event.project.organization
         org_slug = org.slug
         next_time = timezone.now() + timedelta(hours=1)
+        has_normal_run_flag = features.has("organizations:derive-code-mappings", org)
+        has_dry_run_flag = features.has("organizations:derive-code-mappings-dry-run", org)
 
-        logger.info(
-            f"derive_code_mappings: Queuing code mapping derivation for {project.slug=} {event.group_id=}."
-            + f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
-        )
-        derive_code_mappings.delay(project.id, event.data, dry_run=False)
+        if has_normal_run_flag:
+            logger.info(
+                f"derive_code_mappings: Queuing code mapping derivation for {project.slug=} {event.group_id=}."
+                + f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
+            )
+            derive_code_mappings.delay(project.id, event.data, dry_run=False)
+        # Derive code mappings with dry_run=True to validate the generated mappings.
+        elif has_dry_run_flag:
+            logger.info(
+                f"derive_code_mappings: Queuing dry run code mapping derivation for {project.slug=} {event.group_id=}."
+                + f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
+            )
+            derive_code_mappings.delay(project.id, event.data, dry_run=True)
 
     except Exception:
         logger.exception("derive_code_mappings: Failed to process code mappings")

+ 2 - 0
tests/sentry/api/endpoints/test_organization_derive_code_mappings.py

@@ -7,10 +7,12 @@ from sentry.models.integrations.integration import Integration
 from sentry.models.integrations.repository_project_path_config import RepositoryProjectPathConfig
 from sentry.models.repository import Repository
 from sentry.testutils import APITestCase
+from sentry.testutils.helpers.features import apply_feature_flag_on_cls
 from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test
 
 
 @region_silo_test(stable=True)
+@apply_feature_flag_on_cls("organizations:derive-code-mappings")
 class OrganizationDeriveCodeMappingsTest(APITestCase):
     def setUp(self):
         super().setUp()

+ 66 - 0
tests/sentry/tasks/test_derive_code_mappings.py

@@ -13,6 +13,7 @@ from sentry.models.repository import Repository
 from sentry.shared_integrations.exceptions.base import ApiError
 from sentry.tasks.derive_code_mappings import derive_code_mappings, identify_stacktrace_paths
 from sentry.testutils import TestCase
+from sentry.testutils.helpers import apply_feature_flag_on_cls, with_feature
 from sentry.utils.locking import UnableToAcquireLock
 
 
@@ -34,6 +35,7 @@ class BaseDeriveCodeMappings(TestCase):
         return self.store_event(data=test_data, project_id=self.project.id).data
 
 
+@apply_feature_flag_on_cls("organizations:derive-code-mappings")
 class TestTaskBehavior(BaseDeriveCodeMappings):
     """Test task behavior that is not language specific."""
 
@@ -122,6 +124,7 @@ class TestJavascriptDeriveCodeMappings(BaseDeriveCodeMappings):
         assert stacktrace_paths == []
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_starts_with_period_slash(self):
         repo_name = "foo/bar"
         with patch(
@@ -141,6 +144,7 @@ class TestJavascriptDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.repository.name == repo_name
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_starts_with_period_slash_no_containing_directory(self):
         repo_name = "foo/bar"
         with patch(
@@ -160,6 +164,7 @@ class TestJavascriptDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.repository.name == repo_name
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_one_to_one_match(self):
         repo_name = "foo/bar"
         with patch(
@@ -198,6 +203,7 @@ class TestRubyDeriveCodeMappings(BaseDeriveCodeMappings):
         assert set(stacktrace_paths) == {"some/path/test.rb", "lib/tasks/crontask.rake"}
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_rb(self):
         repo_name = "foo/bar"
         with patch(
@@ -213,6 +219,7 @@ class TestRubyDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.repository.name == repo_name
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_rake(self):
         repo_name = "foo/bar"
         with patch(
@@ -258,6 +265,7 @@ class TestNodeDeriveCodeMappings(BaseDeriveCodeMappings):
         }
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_starts_with_app(self):
         repo_name = "foo/bar"
         with patch(
@@ -273,6 +281,7 @@ class TestNodeDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.repository.name == repo_name
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_starts_with_multiple_dot_dot_slash(self):
         repo_name = "foo/bar"
         with patch(
@@ -288,6 +297,7 @@ class TestNodeDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.repository.name == repo_name
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_starts_with_app_dot_dot_slash(self):
         repo_name = "foo/bar"
         with patch(
@@ -338,6 +348,23 @@ class TestPythonDeriveCodeMappings(BaseDeriveCodeMappings):
             "sentry/tasks.py",
         ]
 
+    @with_feature({"organizations:derive-code-mappings": False})
+    def test_feature_off(self):
+        event = self.store_event(data=self.test_data, project_id=self.project.id)
+
+        assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
+
+        with patch(
+            "sentry.tasks.derive_code_mappings.identify_stacktrace_paths",
+            return_value={
+                self.project: ["sentry/models/release.py", "sentry/tasks.py"],
+            },
+        ) as mock_identify_stacktraces, self.tasks():
+            derive_code_mappings(self.project.id, event.data)
+
+        assert mock_identify_stacktraces.call_count == 0
+        assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
+
     @patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
     @patch(
         "sentry.integrations.utils.code_mapping.CodeMappingTreesHelper.generate_code_mappings",
@@ -349,6 +376,7 @@ class TestPythonDeriveCodeMappings(BaseDeriveCodeMappings):
             )
         ],
     )
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_single_project(
         self, mock_generate_code_mappings, mock_get_trees_for_org
     ):
@@ -386,6 +414,7 @@ class TestPythonDeriveCodeMappings(BaseDeriveCodeMappings):
         ],
     )
     @patch("sentry.tasks.derive_code_mappings.logger")
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_duplicates(
         self, mock_logger, mock_generate_code_mappings, mock_get_trees_for_org
     ):
@@ -422,7 +451,43 @@ class TestPythonDeriveCodeMappings(BaseDeriveCodeMappings):
         assert code_mapping.first().automatically_generated is False
         assert mock_logger.info.call_count == 1
 
+    @patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
+    @patch(
+        "sentry.integrations.utils.code_mapping.CodeMappingTreesHelper.generate_code_mappings",
+        return_value=[
+            CodeMapping(
+                repo=Repo(name="repo", branch="master"),
+                stacktrace_root="sentry/models",
+                source_path="src/sentry/models",
+            )
+        ],
+    )
+    @patch("sentry.tasks.derive_code_mappings.logger")
+    @with_feature("organizations:derive-code-mappings")
+    def test_derive_code_mappings_dry_run(
+        self, mock_logger, mock_generate_code_mappings, mock_get_trees_for_org
+    ):
+
+        event = self.store_event(data=self.test_data, project_id=self.project.id)
+
+        assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
+
+        with patch(
+            "sentry.tasks.derive_code_mappings.identify_stacktrace_paths",
+            return_value=["sentry/models/release.py", "sentry/tasks.py"],
+        ) as mock_identify_stacktraces, self.tasks():
+            derive_code_mappings(self.project.id, event.data, dry_run=True)
+
+        assert mock_logger.info.call_count == 1
+        assert mock_identify_stacktraces.call_count == 1
+        assert mock_get_trees_for_org.call_count == 1
+        assert mock_generate_code_mappings.call_count == 1
+
+        # We should not create the code mapping for dry runs
+        assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
+
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_stack_and_source_root_do_not_match(self):
         repo_name = "foo/bar"
         with patch(
@@ -438,6 +503,7 @@ class TestPythonDeriveCodeMappings(BaseDeriveCodeMappings):
             assert code_mapping.source_root == "src/sentry/"
 
     @responses.activate
+    @with_feature("organizations:derive-code-mappings")
     def test_derive_code_mappings_no_normalization(self):
         repo_name = "foo/bar"
         with patch(

+ 3 - 1
tests/sentry/tasks/test_post_process.py

@@ -48,7 +48,7 @@ from sentry.tasks.post_process import (
 )
 from sentry.testutils import SnubaTestCase, TestCase
 from sentry.testutils.cases import BaseTestCase
-from sentry.testutils.helpers import with_feature
+from sentry.testutils.helpers import apply_feature_flag_on_cls, with_feature
 from sentry.testutils.helpers.datetime import before_now, iso_format
 from sentry.testutils.helpers.eventprocessing import write_event_to_cache
 from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
@@ -164,6 +164,8 @@ class CorePostProcessGroupTestMixin(BasePostProgressGroupMixin):
         assert event_processing_store.get(cache_key) is None
 
 
+@apply_feature_flag_on_cls("organizations:derive-code-mappings")
+@apply_feature_flag_on_cls("organizations:derive-code-mappings-dry-run")
 class DeriveCodeMappingsProcessGroupTestMixin(BasePostProgressGroupMixin):
     def _call_post_process_group(self, data: Dict[str, str]) -> None:
         event = self.create_event(data=data, project_id=self.project.id)