Skip to content

Commit ecd36f0

Browse files
armenzgandrewshie-sentry
authored andcommitted
feat(auto_source_config): Add feature flag to control Java's dry-run (#88087)
This will enable EA orgs while letting GA orgs run in dry-run mode.
1 parent 4d4feae commit ecd36f0

File tree

5 files changed

+113
-108
lines changed

5 files changed

+113
-108
lines changed

src/sentry/features/temporary.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ def register_temporary_features(manager: FeatureManager):
150150
manager.add("organizations:issue-search-group-attributes-side-query", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
151151
# Enable custom views features in the issue stream
152152
manager.add("organizations:issue-stream-custom-views", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
153+
# Control if Java dry run is enabled
154+
manager.add("organizations:auto-source-code-config-java-enabled", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
153155
# Enable left nav issue views
154156
manager.add("organizations:left-nav-issue-views", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
155157
# Enable the updated empty state for issues

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,15 @@ def save_in_app_stack_trace_rules(
2626
current_rules = set(current_enhancements.split("\n")) if current_enhancements else set()
2727

2828
united_rules = rules_from_code_mappings.union(current_rules)
29-
if not platform_config.is_dry_run_platform() and united_rules != current_rules:
29+
dry_run = platform_config.is_dry_run_platform(project.organization)
30+
if not dry_run and united_rules != current_rules:
3031
project.update_option(DERIVED_ENHANCEMENTS_OPTION_KEY, "\n".join(sorted(united_rules)))
3132

3233
new_rules_added = united_rules - current_rules
3334
metrics.incr(
3435
key=f"{METRIC_PREFIX}.in_app_stack_trace_rules.created",
3536
amount=len(new_rules_added),
36-
tags={
37-
"platform": platform_config.platform,
38-
"dry_run": platform_config.is_dry_run_platform(),
39-
},
37+
tags={"platform": platform_config.platform, "dry_run": dry_run},
4038
sample_rate=1.0,
4139
)
4240
return list(new_rules_added)

src/sentry/issues/auto_source_code_config/task.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ def create_configurations(
183183
if not org_integration:
184184
raise InstallationNotFoundError
185185

186-
dry_run = platform_config.is_dry_run_platform()
186+
dry_run = platform_config.is_dry_run_platform(project.organization)
187187
platform = platform_config.platform
188188
tags: Mapping[str, str | bool] = {"platform": platform, "dry_run": dry_run}
189189
with metrics.timer(f"{METRIC_PREFIX}.create_configurations.duration", tags=tags):

src/sentry/issues/auto_source_code_config/utils.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
from typing import Any
22

3+
from sentry import features
4+
from sentry.models.organization import Organization
5+
36
from .constants import PLATFORMS_CONFIG
47

58

@@ -35,8 +38,12 @@ def __init__(self, platform: str) -> None:
3538
def is_supported(self) -> bool:
3639
return self.config is not None
3740

38-
def is_dry_run_platform(self) -> bool:
39-
return self.config.get("dry_run", False)
41+
def is_dry_run_platform(self, org: Organization) -> bool:
42+
return (
43+
not features.has("organizations:auto-source-code-config-java-enabled", org, actor=None)
44+
if self.platform == "java"
45+
else self.config.get("dry_run", False)
46+
)
4047

4148
def extracts_filename_from_module(self) -> bool:
4249
return self.config.get("extract_filename_from_module", False)

tests/sentry/issues/auto_source_code_config/test_process_event.py

Lines changed: 98 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from sentry.shared_integrations.exceptions import ApiError
1717
from sentry.testutils.asserts import assert_failure_metric, assert_halt_metric
1818
from sentry.testutils.cases import TestCase
19+
from sentry.testutils.helpers import with_feature
1920
from sentry.testutils.silo import assume_test_silo_mode_of
2021
from sentry.testutils.skips import requires_snuba
2122
from sentry.utils.locking import UnableToAcquireLock
@@ -89,7 +90,7 @@ def _process_and_assert_configuration_changes(
8990
expected_new_in_app_stack_trace_rules: list[str] | None = None,
9091
) -> GroupEvent:
9192
platform_config = PlatformConfig(platform)
92-
dry_run = platform_config.is_dry_run_platform()
93+
dry_run = platform_config.is_dry_run_platform(self.organization)
9394
tags = {"dry_run": dry_run, "platform": platform}
9495
with (
9596
patch(f"{CLIENT}.get_tree", side_effect=create_mock_get_tree(repo_trees)),
@@ -663,6 +664,7 @@ def test_multiple_configuration_changes(self) -> None:
663664
],
664665
)
665666

667+
@with_feature({"organizations:auto-source-code-config-java-enabled": True})
666668
def test_multiple_tlds(self) -> None:
667669
# XXX: Multiple TLDs cause over in-app categorization
668670
# Think of uk.co company using packages from another uk.co company
@@ -672,64 +674,62 @@ def test_multiple_tlds(self) -> None:
672674
self.frame(module="uk.co.not-example.baz.qux", abs_path="qux.kt", in_app=False),
673675
]
674676

675-
# Let's pretend we're not running as dry-run
676-
with patch(f"{CODE_ROOT}.utils.PlatformConfig.is_dry_run_platform", return_value=False):
677-
event = self._process_and_assert_configuration_changes(
678-
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
679-
frames=frames,
680-
platform=self.platform,
681-
expected_new_code_mappings=[
682-
# XXX: Notice that we loose "example"
683-
self.code_mapping(stack_root="uk/co/", source_root="src/uk/co/"),
684-
],
685-
expected_new_in_app_stack_trace_rules=["stack.module:uk.co.** +app"],
686-
)
687-
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
688-
689-
event = self._process_and_assert_configuration_changes(
690-
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
691-
frames=frames,
692-
platform=self.platform,
693-
)
694-
# It's in-app-only because even the not-example package is in-app
695-
assert event.data["metadata"]["in_app_frame_mix"] == "in-app-only"
677+
event = self._process_and_assert_configuration_changes(
678+
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
679+
frames=frames,
680+
platform=self.platform,
681+
expected_new_code_mappings=[
682+
# XXX: Notice that we loose "example"
683+
self.code_mapping(stack_root="uk/co/", source_root="src/uk/co/"),
684+
],
685+
expected_new_in_app_stack_trace_rules=["stack.module:uk.co.** +app"],
686+
)
687+
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
696688

697-
# The developer can undo our rule
698-
self.project.update_option(
699-
"sentry:grouping_enhancements",
700-
"stack.module:uk.co.not-example.** -app",
701-
)
702-
event = self._process_and_assert_configuration_changes(
703-
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
704-
frames=frames,
705-
platform=self.platform,
706-
)
707-
assert event.data["metadata"]["in_app_frame_mix"] == "mixed"
708-
event_frames = event.data["stacktrace"]["frames"]
709-
assert event_frames[0]["module"] == "uk.co.example.foo.Bar"
710-
assert event_frames[0]["in_app"] is True
711-
assert event_frames[1]["module"] == "uk.co.not-example.baz.qux"
712-
assert event_frames[1]["in_app"] is False
689+
event = self._process_and_assert_configuration_changes(
690+
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
691+
frames=frames,
692+
platform=self.platform,
693+
)
694+
# It's in-app-only because even the not-example package is in-app
695+
assert event.data["metadata"]["in_app_frame_mix"] == "in-app-only"
713696

697+
# The developer can undo our rule
698+
self.project.update_option(
699+
"sentry:grouping_enhancements",
700+
"stack.module:uk.co.not-example.** -app",
701+
)
702+
event = self._process_and_assert_configuration_changes(
703+
repo_trees={REPO1: ["src/uk/co/example/foo/Bar.kt"]},
704+
frames=frames,
705+
platform=self.platform,
706+
)
707+
assert event.data["metadata"]["in_app_frame_mix"] == "mixed"
708+
event_frames = event.data["stacktrace"]["frames"]
709+
assert event_frames[0]["module"] == "uk.co.example.foo.Bar"
710+
assert event_frames[0]["in_app"] is True
711+
assert event_frames[1]["module"] == "uk.co.not-example.baz.qux"
712+
assert event_frames[1]["in_app"] is False
713+
714+
@with_feature({"organizations:auto-source-code-config-java-enabled": True})
714715
def test_do_not_clobber_rules(self) -> None:
715-
# Let's pretend we're not running as dry-run
716-
with patch(f"{CODE_ROOT}.utils.PlatformConfig.is_dry_run_platform", return_value=False):
717-
self._process_and_assert_configuration_changes(
718-
repo_trees={REPO1: ["src/a/Bar.java", "src/x/y/Baz.java"]},
719-
frames=[self.frame(module="a.Bar", abs_path="Bar.java", in_app=False)],
720-
platform=self.platform,
721-
expected_new_code_mappings=[self.code_mapping("a/", "src/a/")],
722-
expected_new_in_app_stack_trace_rules=["stack.module:a.** +app"],
723-
)
724-
self._process_and_assert_configuration_changes(
725-
repo_trees={REPO1: ["src/a/Bar.java", "src/x/y/Baz.java"]},
726-
frames=[self.frame(module="x.y.Baz", abs_path="Baz.java", in_app=False)],
727-
platform=self.platform,
728-
expected_new_code_mappings=[self.code_mapping("x/y/", "src/x/y/")],
729-
# Both rules should exist
730-
expected_new_in_app_stack_trace_rules=["stack.module:x.y.** +app"],
731-
)
716+
self._process_and_assert_configuration_changes(
717+
repo_trees={REPO1: ["src/a/Bar.java", "src/x/y/Baz.java"]},
718+
frames=[self.frame(module="a.Bar", abs_path="Bar.java", in_app=False)],
719+
platform=self.platform,
720+
expected_new_code_mappings=[self.code_mapping("a/", "src/a/")],
721+
expected_new_in_app_stack_trace_rules=["stack.module:a.** +app"],
722+
)
723+
self._process_and_assert_configuration_changes(
724+
repo_trees={REPO1: ["src/a/Bar.java", "src/x/y/Baz.java"]},
725+
frames=[self.frame(module="x.y.Baz", abs_path="Baz.java", in_app=False)],
726+
platform=self.platform,
727+
expected_new_code_mappings=[self.code_mapping("x/y/", "src/x/y/")],
728+
# Both rules should exist
729+
expected_new_in_app_stack_trace_rules=["stack.module:x.y.** +app"],
730+
)
732731

732+
@with_feature({"organizations:auto-source-code-config-java-enabled": True})
733733
def test_run_without_dry_run(self) -> None:
734734
repo_trees = {REPO1: ["src/com/example/foo/Bar.kt"]}
735735
frames = [
@@ -739,48 +739,46 @@ def test_run_without_dry_run(self) -> None:
739739
rule = "stack.module:com.example.**"
740740
expected_in_app_rule = f"{rule} +app"
741741

742-
# Let's pretend we're not running as dry-run
743-
with patch(f"{CODE_ROOT}.utils.PlatformConfig.is_dry_run_platform", return_value=False):
744-
event = self._process_and_assert_configuration_changes(
745-
repo_trees=repo_trees,
746-
frames=frames,
747-
platform=self.platform,
748-
expected_new_code_mappings=[
749-
self.code_mapping(stack_root="com/example/", source_root="src/com/example/"),
750-
],
751-
expected_new_in_app_stack_trace_rules=[expected_in_app_rule],
752-
)
753-
# The effects of the configuration changes will be noticed on the second event processing
754-
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
755-
assert len(event.data["hashes"]) == 1 # Only system hash
756-
system_only_hash = event.data["hashes"][0]
757-
first_enhancements_base64_string = event.data["grouping_config"]["enhancements"]
758-
group_id = event.group_id
759-
760-
# Running a second time will not create any new configurations, however,
761-
# the rules from the previous run will be applied to the event's stack trace
762-
event = self._process_and_assert_configuration_changes(
763-
repo_trees=repo_trees, frames=frames, platform=self.platform
764-
)
765-
assert event.group_id == group_id # The new rules did not cause new groups
766-
assert event.data["metadata"]["in_app_frame_mix"] == "mixed"
767-
second_enhancements_hash = event.data["grouping_config"]["enhancements"]
768-
# The enhancements now contain the automatic rule (+app)
769-
assert second_enhancements_hash != first_enhancements_base64_string
770-
assert len(event.data["hashes"]) == 2
771-
event.data["hashes"].remove(system_only_hash)
772-
in_app_hash = event.data["hashes"][0]
773-
assert in_app_hash != system_only_hash
774-
775-
# The developer will add a rule to invalidate our automatinc rule (-app)
776-
self.project.update_option("sentry:grouping_enhancements", f"{rule} -app")
777-
event = self._process_and_assert_configuration_changes(
778-
repo_trees=repo_trees, frames=frames, platform=self.platform
779-
)
780-
# Back to system-only
781-
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
782-
assert event.group_id == group_id # It still belongs to the same group
783-
assert event.data["hashes"] == [system_only_hash]
784-
# The enhancements now contain the automatic rule (+app) and the developer's rule (-app)
785-
assert event.data["grouping_config"]["enhancements"] != first_enhancements_base64_string
786-
assert event.data["grouping_config"]["enhancements"] != second_enhancements_hash
742+
event = self._process_and_assert_configuration_changes(
743+
repo_trees=repo_trees,
744+
frames=frames,
745+
platform=self.platform,
746+
expected_new_code_mappings=[
747+
self.code_mapping(stack_root="com/example/", source_root="src/com/example/"),
748+
],
749+
expected_new_in_app_stack_trace_rules=[expected_in_app_rule],
750+
)
751+
# The effects of the configuration changes will be noticed on the second event processing
752+
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
753+
assert len(event.data["hashes"]) == 1 # Only system hash
754+
system_only_hash = event.data["hashes"][0]
755+
first_enhancements_base64_string = event.data["grouping_config"]["enhancements"]
756+
group_id = event.group_id
757+
758+
# Running a second time will not create any new configurations, however,
759+
# the rules from the previous run will be applied to the event's stack trace
760+
event = self._process_and_assert_configuration_changes(
761+
repo_trees=repo_trees, frames=frames, platform=self.platform
762+
)
763+
assert event.group_id == group_id # The new rules did not cause new groups
764+
assert event.data["metadata"]["in_app_frame_mix"] == "mixed"
765+
second_enhancements_hash = event.data["grouping_config"]["enhancements"]
766+
# The enhancements now contain the automatic rule (+app)
767+
assert second_enhancements_hash != first_enhancements_base64_string
768+
assert len(event.data["hashes"]) == 2
769+
event.data["hashes"].remove(system_only_hash)
770+
in_app_hash = event.data["hashes"][0]
771+
assert in_app_hash != system_only_hash
772+
773+
# The developer will add a rule to invalidate our automatinc rule (-app)
774+
self.project.update_option("sentry:grouping_enhancements", f"{rule} -app")
775+
event = self._process_and_assert_configuration_changes(
776+
repo_trees=repo_trees, frames=frames, platform=self.platform
777+
)
778+
# Back to system-only
779+
assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
780+
assert event.group_id == group_id # It still belongs to the same group
781+
assert event.data["hashes"] == [system_only_hash]
782+
# The enhancements now contain the automatic rule (+app) and the developer's rule (-app)
783+
assert event.data["grouping_config"]["enhancements"] != first_enhancements_base64_string
784+
assert event.data["grouping_config"]["enhancements"] != second_enhancements_hash

0 commit comments

Comments
 (0)