From 5b8c5efba52f2d23d88f043df38e5d0cc8daf08e Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Wed, 5 Mar 2025 07:31:55 -0500 Subject: [PATCH 1/3] feat(derived_code_mappings): Add metric for creating a repository and dry_run mode This will track when a repository is created and when any of the two metrics would have created a repository or code mapping (useful for Java's dry run). --- .../auto_source_code_config/code_mapping.py | 2 +- .../auto_source_code_config/constants.py | 1 + .../issues/auto_source_code_config/task.py | 60 ++++++++++++------- .../test_process_event.py | 33 +++++++++- 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/src/sentry/issues/auto_source_code_config/code_mapping.py b/src/sentry/issues/auto_source_code_config/code_mapping.py index de95a8d5700877..fb2357766d8f91 100644 --- a/src/sentry/issues/auto_source_code_config/code_mapping.py +++ b/src/sentry/issues/auto_source_code_config/code_mapping.py @@ -589,7 +589,7 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]: raise DoesNotFollowJavaPackageNamingConvention # If module has a dot, take everything before the last dot - # com.example.foo.Bar$InnerClass -> com/example/foo/ + # com.example.foo.Bar$InnerClass -> com/example/foo stack_root = module.rsplit(".", 1)[0].replace(".", "/") file_path = f"{stack_root}/{abs_path}" diff --git a/src/sentry/issues/auto_source_code_config/constants.py b/src/sentry/issues/auto_source_code_config/constants.py index e04efbeb5b3818..8f3602ff52f50f 100644 --- a/src/sentry/issues/auto_source_code_config/constants.py +++ b/src/sentry/issues/auto_source_code_config/constants.py @@ -1,3 +1,4 @@ +METRIC_PREFIX = "auto_source_code_config" SUPPORTED_INTEGRATIONS = ["github"] # XXX: We may want to change these constants into a configuration object # Any new languages should also require updating the stacktraceLink.tsx and repo_trees.py SUPPORTED_EXTENSIONS diff --git a/src/sentry/issues/auto_source_code_config/task.py b/src/sentry/issues/auto_source_code_config/task.py index 5fba0e8b5f66f7..d11a27203e0b9f 100644 --- a/src/sentry/issues/auto_source_code_config/task.py +++ b/src/sentry/issues/auto_source_code_config/task.py @@ -22,6 +22,7 @@ from sentry.utils import metrics from sentry.utils.locking import UnableToAcquireLock +from .constants import METRIC_PREFIX from .integration_utils import ( InstallationCannotGetTreesError, InstallationNotFoundError, @@ -79,8 +80,7 @@ def process_event(project_id: int, group_id: int, event_id: str) -> list[CodeMap trees = get_trees_for_org(installation, org, extra) trees_helper = CodeMappingTreesHelper(trees) code_mappings = trees_helper.generate_code_mappings(frames_to_process, platform) - if not is_dry_run_platform(platform): - set_project_codemappings(code_mappings, installation, project, platform) + create_repos_and_code_mappings(code_mappings, installation, project, platform) except (InstallationNotFoundError, InstallationCannotGetTreesError): pass @@ -157,7 +157,7 @@ def get_trees_for_org( return trees -def set_project_codemappings( +def create_repos_and_code_mappings( code_mappings: list[CodeMapping], installation: IntegrationInstallation, project: Project, @@ -167,6 +167,7 @@ def set_project_codemappings( Given a list of code mappings, create a new repository project path config for each mapping. """ + dry_run = is_dry_run_platform(platform) organization_integration = installation.org_integration if not organization_integration: raise InstallationNotFoundError @@ -180,25 +181,38 @@ def set_project_codemappings( ) if not repository: - repository = Repository.objects.create( - name=code_mapping.repo.name, - organization_id=organization_id, - integration_id=organization_integration.integration_id, + if not dry_run: + repository = Repository.objects.create( + name=code_mapping.repo.name, + organization_id=organization_id, + integration_id=organization_integration.integration_id, + ) + metrics.incr( + key=f"{METRIC_PREFIX}.repository.created", + tags={"platform": platform, "dry_run": dry_run}, + sample_rate=1.0, ) - _, created = RepositoryProjectPathConfig.objects.get_or_create( - project=project, - stack_root=code_mapping.stacktrace_root, - defaults={ - "repository": repository, - "organization_integration_id": organization_integration.id, - "integration_id": organization_integration.integration_id, - "organization_id": organization_integration.organization_id, - "source_root": code_mapping.source_path, - "default_branch": code_mapping.repo.branch, - "automatically_generated": True, - }, - ) - if created: - # Since it is a low volume event, we can sample at 100% - metrics.incr(key="code_mappings.created", tags={"platform": platform}, sample_rate=1.0) + # The project and stack_root are unique together + configs = RepositoryProjectPathConfig.objects.filter( + project=project, stack_root=code_mapping.stacktrace_root + ).all() + if not configs: + if not dry_run and repository is not None: + RepositoryProjectPathConfig.objects.create( + repository=repository, + project=project, + organization_integration_id=organization_integration.id, + organization_id=organization_integration.organization_id, + integration_id=organization_integration.integration_id, + stack_root=code_mapping.stacktrace_root, + source_root=code_mapping.source_path, + default_branch=code_mapping.repo.branch, + automatically_generated=True, + ) + + metrics.incr( + key=f"{METRIC_PREFIX}.code_mapping.created", + tags={"platform": platform, "dry_run": dry_run}, + sample_rate=1.0, + ) 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 c66c50462bd652..fe3b558ddac8a0 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 @@ -7,8 +7,10 @@ from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig from sentry.integrations.source_code_management.repo_trees import RepoAndBranch, RepoTree from sentry.issues.auto_source_code_config.code_mapping import CodeMapping +from sentry.issues.auto_source_code_config.constants import METRIC_PREFIX from sentry.issues.auto_source_code_config.integration_utils import InstallationNotFoundError from sentry.issues.auto_source_code_config.task import DeriveCodeMappingsErrorReason, process_event +from sentry.issues.auto_source_code_config.utils import is_dry_run_platform from sentry.models.repository import Repository from sentry.shared_integrations.exceptions import ApiError from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric @@ -78,8 +80,17 @@ def _process_and_assert_code_mapping( code_mapping = code_mappings[0] assert code_mapping.stack_root == expected_stack_root assert code_mapping.source_root == expected_source_root - mock_incr.assert_called_with( - key="code_mappings.created", tags={"platform": event.platform}, sample_rate=1.0 + dry_run = is_dry_run_platform(platform) + # Check that both metrics were called with any order + mock_incr.assert_any_call( + key=f"{METRIC_PREFIX}.code_mapping.created", + tags={"dry_run": dry_run, "platform": event.platform}, + sample_rate=1.0, + ) + mock_incr.assert_any_call( + key=f"{METRIC_PREFIX}.repository.created", + tags={"dry_run": dry_run, "platform": event.platform}, + sample_rate=1.0, ) def _process_and_assert_no_code_mapping( @@ -93,10 +104,24 @@ def _process_and_assert_no_code_mapping( patch(f"{CLIENT}.get_tree", return_value=self._repo_tree_files(repo_files)), patch(f"{CLIENT}.get_remaining_api_requests", return_value=500), patch(REPO_TREES_GET_REPOS, return_value=[self._repo_info()]), + patch("sentry.utils.metrics.incr") as mock_incr, ): event = self.create_event(frames, platform) code_mappings = process_event(self.project.id, event.group_id, event.event_id) assert not RepositoryProjectPathConfig.objects.exists() + dry_run = is_dry_run_platform(platform) + if code_mappings: + # Check that both metrics were called with any order + mock_incr.assert_any_call( + key=f"{METRIC_PREFIX}.repository.created", + tags={"platform": event.platform, "dry_run": dry_run}, + sample_rate=1.0, + ) + mock_incr.assert_any_call( + key=f"{METRIC_PREFIX}.code_mapping.created", + tags={"platform": event.platform, "dry_run": dry_run}, + sample_rate=1.0, + ) return code_mappings def frame(self, filename: str, in_app: bool | None = True) -> dict[str, str | bool]: @@ -179,15 +204,17 @@ def test_handle_existing_code_mapping(self) -> None: def test_dry_run_platform(self) -> None: frame_filename = "foo/bar.py" file_in_repo = "src/foo/bar.py" + platform = "other" with ( patch(f"{CODE_ROOT}.task.supported_platform", return_value=True), patch(f"{CODE_ROOT}.task.is_dry_run_platform", return_value=True), + override_options({"issues.auto_source_code_config.dry-run-platforms": [platform]}), ): # No code mapping will be stored, however, we get what would have been created code_mappings = self._process_and_assert_no_code_mapping( repo_files=[file_in_repo], frames=[self.frame(frame_filename, True)], - platform="other", + platform=platform, ) assert len(code_mappings) == 1 assert code_mappings[0].stacktrace_root == "foo/" From bf93f1d8d36718f22dc907b315d95e2b21238819 Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Wed, 5 Mar 2025 13:07:43 -0500 Subject: [PATCH 2/3] Go back --- .../issues/auto_source_code_config/task.py | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/sentry/issues/auto_source_code_config/task.py b/src/sentry/issues/auto_source_code_config/task.py index d11a27203e0b9f..cc7f778185c6c3 100644 --- a/src/sentry/issues/auto_source_code_config/task.py +++ b/src/sentry/issues/auto_source_code_config/task.py @@ -193,24 +193,24 @@ def create_repos_and_code_mappings( sample_rate=1.0, ) - # The project and stack_root are unique together - configs = RepositoryProjectPathConfig.objects.filter( - project=project, stack_root=code_mapping.stacktrace_root - ).all() - if not configs: - if not dry_run and repository is not None: - RepositoryProjectPathConfig.objects.create( - repository=repository, - project=project, - organization_integration_id=organization_integration.id, - organization_id=organization_integration.organization_id, - integration_id=organization_integration.integration_id, - stack_root=code_mapping.stacktrace_root, - source_root=code_mapping.source_path, - default_branch=code_mapping.repo.branch, - automatically_generated=True, - ) + created = False + if not dry_run: + # The project and stack_root are unique together + _, created = RepositoryProjectPathConfig.objects.get_or_create( + project=project, + stack_root=code_mapping.stacktrace_root, + defaults={ + "repository": repository, + "organization_integration_id": organization_integration.id, + "integration_id": organization_integration.integration_id, + "organization_id": organization_integration.organization_id, + "source_root": code_mapping.source_path, + "default_branch": code_mapping.repo.branch, + "automatically_generated": True, + }, + ) + if created or dry_run: metrics.incr( key=f"{METRIC_PREFIX}.code_mapping.created", tags={"platform": platform, "dry_run": dry_run}, From 795334d2c1055b2e1b71d950bd24455c51ac4c0a Mon Sep 17 00:00:00 2001 From: Armen Zambrano G <44410+armenzg@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:29:41 -0500 Subject: [PATCH 3/3] A more explicit solution --- .../issues/auto_source_code_config/task.py | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/sentry/issues/auto_source_code_config/task.py b/src/sentry/issues/auto_source_code_config/task.py index cc7f778185c6c3..c8f7a9cb397f18 100644 --- a/src/sentry/issues/auto_source_code_config/task.py +++ b/src/sentry/issues/auto_source_code_config/task.py @@ -193,26 +193,38 @@ def create_repos_and_code_mappings( sample_rate=1.0, ) - created = False + extra = { + "project_id": project.id, + "stack_root": code_mapping.stacktrace_root, + "repository_name": code_mapping.repo.name, + } + # The project and stack_root are unique together + existing_code_mappings = RepositoryProjectPathConfig.objects.filter( + project=project, stack_root=code_mapping.stacktrace_root + ) + if existing_code_mappings.exists(): + logger.warning("Investigate.", extra=extra) + continue + if not dry_run: - # The project and stack_root are unique together - _, created = RepositoryProjectPathConfig.objects.get_or_create( + if repository is None: # This is mostly to appease the type checker + logger.warning("Investigate.", extra=extra) + continue + + RepositoryProjectPathConfig.objects.create( project=project, stack_root=code_mapping.stacktrace_root, - defaults={ - "repository": repository, - "organization_integration_id": organization_integration.id, - "integration_id": organization_integration.integration_id, - "organization_id": organization_integration.organization_id, - "source_root": code_mapping.source_path, - "default_branch": code_mapping.repo.branch, - "automatically_generated": True, - }, + repository=repository, + organization_integration_id=organization_integration.id, + integration_id=organization_integration.integration_id, + organization_id=organization_integration.organization_id, + source_root=code_mapping.source_path, + default_branch=code_mapping.repo.branch, + automatically_generated=True, ) - if created or dry_run: - metrics.incr( - key=f"{METRIC_PREFIX}.code_mapping.created", - tags={"platform": platform, "dry_run": dry_run}, - sample_rate=1.0, - ) + metrics.incr( + key=f"{METRIC_PREFIX}.code_mapping.created", + tags={"platform": platform, "dry_run": dry_run}, + sample_rate=1.0, + )