Skip to content

Commit 74d6d29

Browse files
Revert "ref(code mapping): Remove feature flags (#45296)"
This reverts commit b94ac9b. Co-authored-by: lobsterkatie <[email protected]>
1 parent e5fb8b1 commit 74d6d29

File tree

8 files changed

+135
-7
lines changed

8 files changed

+135
-7
lines changed

src/sentry/api/endpoints/organization_derive_code_mappings.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from rest_framework.request import Request
55
from rest_framework.response import Response
66

7+
from sentry import features
78
from sentry.api.base import region_silo_endpoint
89
from sentry.api.bases.organization import (
910
OrganizationEndpoint,
@@ -35,6 +36,9 @@ def get(self, request: Request, organization: Organization) -> Response:
3536
:param string stacktraceFilename:
3637
:auth: required
3738
"""
39+
if not features.has("organizations:derive-code-mappings", organization):
40+
return Response(status=status.HTTP_403_FORBIDDEN)
41+
3842
stacktrace_filename = request.GET.get("stacktraceFilename")
3943
installation, _ = get_installation(organization)
4044
if not installation:
@@ -65,6 +69,9 @@ def post(self, request: Request, organization: Organization) -> Response:
6569
:param string sourceRoot:
6670
:auth: required
6771
"""
72+
if not features.has("organizations:derive-code-mappings", organization):
73+
return Response(status=status.HTTP_403_FORBIDDEN)
74+
6875
installation, organization_integration = get_installation(organization)
6976
if not installation:
7077
return self.respond(

src/sentry/conf/server.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,10 @@ def SOCIAL_AUTH_DEFAULT_USERNAME():
10221022
"organizations:auto-enable-codecov": False,
10231023
# Enables getting commit sha from git blame for codecov.
10241024
"organizations:codecov-commit-sha-from-git-blame": False,
1025+
# Enables automatically deriving of code mappings
1026+
"organizations:derive-code-mappings": False,
1027+
# Enables automatically deriving of code mappings as a dry run for early adopters
1028+
"organizations:derive-code-mappings-dry-run": False,
10251029
# Enable advanced search features, like negation and wildcard matching.
10261030
"organizations:advanced-search": True,
10271031
# Use metrics as the dataset for crash free metric alerts

src/sentry/features/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,8 @@
225225
default_manager.add("organizations:sso-saml2", OrganizationFeature)
226226
default_manager.add("organizations:source-maps-cta", OrganizationFeature, True)
227227
default_manager.add("organizations:team-insights", OrganizationFeature)
228+
default_manager.add("organizations:derive-code-mappings", OrganizationFeature)
229+
default_manager.add("organizations:derive-code-mappings-dry-run", OrganizationFeature)
228230
default_manager.add("organizations:codecov-stacktrace-integration", OrganizationFeature, True)
229231
default_manager.add("organizations:codecov-stacktrace-integration-v2", OrganizationFeature, True)
230232
default_manager.add("organizations:auto-enable-codecov", OrganizationFeature)

src/sentry/tasks/derive_code_mappings.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from sentry_sdk import set_tag, set_user
77

8+
from sentry import features
89
from sentry.db.models.fields.node import NodeData
910
from sentry.integrations.utils.code_mapping import CodeMapping, CodeMappingTreesHelper
1011
from sentry.locks import locks
@@ -74,6 +75,7 @@ def process_error(error: ApiError, extra: Dict[str, str]) -> None:
7475
def derive_code_mappings(
7576
project_id: int,
7677
data: NodeData,
78+
dry_run=False,
7779
) -> None:
7880
"""
7981
Derive code mappings for a project given data from a recent event.
@@ -89,8 +91,11 @@ def derive_code_mappings(
8991
extra = {
9092
"organization.slug": org.slug,
9193
}
94+
feat_key = "organizations:derive-code-mappings"
95+
# Check the feature flag again to ensure the feature is still enabled.
96+
org_has_flag = features.has(feat_key, org) or features.has(f"{feat_key}-dry-run", org)
9297

93-
if data["platform"] not in SUPPORTED_LANGUAGES:
98+
if not org_has_flag or not data["platform"] in SUPPORTED_LANGUAGES:
9499
logger.info("Event should not be processed.", extra=extra)
95100
return
96101

@@ -125,6 +130,10 @@ def derive_code_mappings(
125130

126131
trees_helper = CodeMappingTreesHelper(trees)
127132
code_mappings = trees_helper.generate_code_mappings(stacktrace_paths)
133+
if dry_run:
134+
report_project_codemappings(code_mappings, stacktrace_paths, project)
135+
return
136+
128137
set_project_codemappings(code_mappings, organization_integration, project)
129138

130139

@@ -219,3 +228,29 @@ def set_project_codemappings(
219228
"existing_code_mapping": cm,
220229
},
221230
)
231+
232+
233+
def report_project_codemappings(
234+
code_mappings: List[CodeMapping],
235+
stacktrace_paths: List[str],
236+
project: Project,
237+
) -> None:
238+
"""
239+
Log the code mappings that would be created for a project.
240+
"""
241+
extra = {
242+
"org": project.organization.slug,
243+
"project": project.slug,
244+
"code_mappings": code_mappings,
245+
"stacktrace_paths": stacktrace_paths,
246+
}
247+
if code_mappings:
248+
msg = "Code mappings would have been created."
249+
else:
250+
msg = "NO code mappings would have been created."
251+
existing_code_mappings = RepositoryProjectPathConfig.objects.filter(project=project)
252+
if existing_code_mappings.exists():
253+
msg = "Code mappings already exist."
254+
extra["existing_code_mappings"] = existing_code_mappings
255+
256+
logger.info(msg, extra=extra)

src/sentry/tasks/post_process.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -755,12 +755,22 @@ def process_code_mappings(job: PostProcessJob) -> None:
755755
org = event.project.organization
756756
org_slug = org.slug
757757
next_time = timezone.now() + timedelta(hours=1)
758+
has_normal_run_flag = features.has("organizations:derive-code-mappings", org)
759+
has_dry_run_flag = features.has("organizations:derive-code-mappings-dry-run", org)
758760

759-
logger.info(
760-
f"derive_code_mappings: Queuing code mapping derivation for {project.slug=} {event.group_id=}."
761-
+ f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
762-
)
763-
derive_code_mappings.delay(project.id, event.data, dry_run=False)
761+
if has_normal_run_flag:
762+
logger.info(
763+
f"derive_code_mappings: Queuing code mapping derivation for {project.slug=} {event.group_id=}."
764+
+ f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
765+
)
766+
derive_code_mappings.delay(project.id, event.data, dry_run=False)
767+
# Derive code mappings with dry_run=True to validate the generated mappings.
768+
elif has_dry_run_flag:
769+
logger.info(
770+
f"derive_code_mappings: Queuing dry run code mapping derivation for {project.slug=} {event.group_id=}."
771+
+ f" Future events in {org_slug=} will not have not have code mapping derivation until {next_time}"
772+
)
773+
derive_code_mappings.delay(project.id, event.data, dry_run=True)
764774

765775
except Exception:
766776
logger.exception("derive_code_mappings: Failed to process code mappings")

tests/sentry/api/endpoints/test_organization_derive_code_mappings.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
from sentry.models.integrations.repository_project_path_config import RepositoryProjectPathConfig
88
from sentry.models.repository import Repository
99
from sentry.testutils import APITestCase
10+
from sentry.testutils.helpers.features import apply_feature_flag_on_cls
1011
from sentry.testutils.silo import exempt_from_silo_limits, region_silo_test
1112

1213

1314
@region_silo_test(stable=True)
15+
@apply_feature_flag_on_cls("organizations:derive-code-mappings")
1416
class OrganizationDeriveCodeMappingsTest(APITestCase):
1517
def setUp(self):
1618
super().setUp()

tests/sentry/tasks/test_derive_code_mappings.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from sentry.shared_integrations.exceptions.base import ApiError
1414
from sentry.tasks.derive_code_mappings import derive_code_mappings, identify_stacktrace_paths
1515
from sentry.testutils import TestCase
16+
from sentry.testutils.helpers import apply_feature_flag_on_cls, with_feature
1617
from sentry.utils.locking import UnableToAcquireLock
1718

1819

@@ -34,6 +35,7 @@ def generate_data(self, frames: List[Dict[str, Union[str, bool]]], platform: str
3435
return self.store_event(data=test_data, project_id=self.project.id).data
3536

3637

38+
@apply_feature_flag_on_cls("organizations:derive-code-mappings")
3739
class TestTaskBehavior(BaseDeriveCodeMappings):
3840
"""Test task behavior that is not language specific."""
3941

@@ -122,6 +124,7 @@ def test_find_stacktrace_paths_bad_data(self):
122124
assert stacktrace_paths == []
123125

124126
@responses.activate
127+
@with_feature("organizations:derive-code-mappings")
125128
def test_derive_code_mappings_starts_with_period_slash(self):
126129
repo_name = "foo/bar"
127130
with patch(
@@ -141,6 +144,7 @@ def test_derive_code_mappings_starts_with_period_slash(self):
141144
assert code_mapping.repository.name == repo_name
142145

143146
@responses.activate
147+
@with_feature("organizations:derive-code-mappings")
144148
def test_derive_code_mappings_starts_with_period_slash_no_containing_directory(self):
145149
repo_name = "foo/bar"
146150
with patch(
@@ -160,6 +164,7 @@ def test_derive_code_mappings_starts_with_period_slash_no_containing_directory(s
160164
assert code_mapping.repository.name == repo_name
161165

162166
@responses.activate
167+
@with_feature("organizations:derive-code-mappings")
163168
def test_derive_code_mappings_one_to_one_match(self):
164169
repo_name = "foo/bar"
165170
with patch(
@@ -198,6 +203,7 @@ def test_find_stacktrace_paths_single_project(self):
198203
assert set(stacktrace_paths) == {"some/path/test.rb", "lib/tasks/crontask.rake"}
199204

200205
@responses.activate
206+
@with_feature("organizations:derive-code-mappings")
201207
def test_derive_code_mappings_rb(self):
202208
repo_name = "foo/bar"
203209
with patch(
@@ -213,6 +219,7 @@ def test_derive_code_mappings_rb(self):
213219
assert code_mapping.repository.name == repo_name
214220

215221
@responses.activate
222+
@with_feature("organizations:derive-code-mappings")
216223
def test_derive_code_mappings_rake(self):
217224
repo_name = "foo/bar"
218225
with patch(
@@ -258,6 +265,7 @@ def test_find_stacktrace_paths_single_project(self):
258265
}
259266

260267
@responses.activate
268+
@with_feature("organizations:derive-code-mappings")
261269
def test_derive_code_mappings_starts_with_app(self):
262270
repo_name = "foo/bar"
263271
with patch(
@@ -273,6 +281,7 @@ def test_derive_code_mappings_starts_with_app(self):
273281
assert code_mapping.repository.name == repo_name
274282

275283
@responses.activate
284+
@with_feature("organizations:derive-code-mappings")
276285
def test_derive_code_mappings_starts_with_multiple_dot_dot_slash(self):
277286
repo_name = "foo/bar"
278287
with patch(
@@ -288,6 +297,7 @@ def test_derive_code_mappings_starts_with_multiple_dot_dot_slash(self):
288297
assert code_mapping.repository.name == repo_name
289298

290299
@responses.activate
300+
@with_feature("organizations:derive-code-mappings")
291301
def test_derive_code_mappings_starts_with_app_dot_dot_slash(self):
292302
repo_name = "foo/bar"
293303
with patch(
@@ -338,6 +348,23 @@ def test_handle_duplicate_filenames_in_stacktrace(self):
338348
"sentry/tasks.py",
339349
]
340350

351+
@with_feature({"organizations:derive-code-mappings": False})
352+
def test_feature_off(self):
353+
event = self.store_event(data=self.test_data, project_id=self.project.id)
354+
355+
assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
356+
357+
with patch(
358+
"sentry.tasks.derive_code_mappings.identify_stacktrace_paths",
359+
return_value={
360+
self.project: ["sentry/models/release.py", "sentry/tasks.py"],
361+
},
362+
) as mock_identify_stacktraces, self.tasks():
363+
derive_code_mappings(self.project.id, event.data)
364+
365+
assert mock_identify_stacktraces.call_count == 0
366+
assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
367+
341368
@patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
342369
@patch(
343370
"sentry.integrations.utils.code_mapping.CodeMappingTreesHelper.generate_code_mappings",
@@ -349,6 +376,7 @@ def test_handle_duplicate_filenames_in_stacktrace(self):
349376
)
350377
],
351378
)
379+
@with_feature("organizations:derive-code-mappings")
352380
def test_derive_code_mappings_single_project(
353381
self, mock_generate_code_mappings, mock_get_trees_for_org
354382
):
@@ -386,6 +414,7 @@ def test_skips_not_supported_platforms(self):
386414
],
387415
)
388416
@patch("sentry.tasks.derive_code_mappings.logger")
417+
@with_feature("organizations:derive-code-mappings")
389418
def test_derive_code_mappings_duplicates(
390419
self, mock_logger, mock_generate_code_mappings, mock_get_trees_for_org
391420
):
@@ -422,7 +451,43 @@ def test_derive_code_mappings_duplicates(
422451
assert code_mapping.first().automatically_generated is False
423452
assert mock_logger.info.call_count == 1
424453

454+
@patch("sentry.integrations.github.GitHubIntegration.get_trees_for_org")
455+
@patch(
456+
"sentry.integrations.utils.code_mapping.CodeMappingTreesHelper.generate_code_mappings",
457+
return_value=[
458+
CodeMapping(
459+
repo=Repo(name="repo", branch="master"),
460+
stacktrace_root="sentry/models",
461+
source_path="src/sentry/models",
462+
)
463+
],
464+
)
465+
@patch("sentry.tasks.derive_code_mappings.logger")
466+
@with_feature("organizations:derive-code-mappings")
467+
def test_derive_code_mappings_dry_run(
468+
self, mock_logger, mock_generate_code_mappings, mock_get_trees_for_org
469+
):
470+
471+
event = self.store_event(data=self.test_data, project_id=self.project.id)
472+
473+
assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
474+
475+
with patch(
476+
"sentry.tasks.derive_code_mappings.identify_stacktrace_paths",
477+
return_value=["sentry/models/release.py", "sentry/tasks.py"],
478+
) as mock_identify_stacktraces, self.tasks():
479+
derive_code_mappings(self.project.id, event.data, dry_run=True)
480+
481+
assert mock_logger.info.call_count == 1
482+
assert mock_identify_stacktraces.call_count == 1
483+
assert mock_get_trees_for_org.call_count == 1
484+
assert mock_generate_code_mappings.call_count == 1
485+
486+
# We should not create the code mapping for dry runs
487+
assert not RepositoryProjectPathConfig.objects.filter(project_id=self.project.id).exists()
488+
425489
@responses.activate
490+
@with_feature("organizations:derive-code-mappings")
426491
def test_derive_code_mappings_stack_and_source_root_do_not_match(self):
427492
repo_name = "foo/bar"
428493
with patch(
@@ -438,6 +503,7 @@ def test_derive_code_mappings_stack_and_source_root_do_not_match(self):
438503
assert code_mapping.source_root == "src/sentry/"
439504

440505
@responses.activate
506+
@with_feature("organizations:derive-code-mappings")
441507
def test_derive_code_mappings_no_normalization(self):
442508
repo_name = "foo/bar"
443509
with patch(

tests/sentry/tasks/test_post_process.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
)
4949
from sentry.testutils import SnubaTestCase, TestCase
5050
from sentry.testutils.cases import BaseTestCase
51-
from sentry.testutils.helpers import with_feature
51+
from sentry.testutils.helpers import apply_feature_flag_on_cls, with_feature
5252
from sentry.testutils.helpers.datetime import before_now, iso_format
5353
from sentry.testutils.helpers.eventprocessing import write_event_to_cache
5454
from sentry.testutils.performance_issues.store_transaction import PerfIssueTransactionTestMixin
@@ -164,6 +164,8 @@ def test_processing_cache_cleared_with_commits(self):
164164
assert event_processing_store.get(cache_key) is None
165165

166166

167+
@apply_feature_flag_on_cls("organizations:derive-code-mappings")
168+
@apply_feature_flag_on_cls("organizations:derive-code-mappings-dry-run")
167169
class DeriveCodeMappingsProcessGroupTestMixin(BasePostProgressGroupMixin):
168170
def _call_post_process_group(self, data: Dict[str, str]) -> None:
169171
event = self.create_event(data=data, project_id=self.project.id)

0 commit comments

Comments
 (0)