feat(auto_source_config): Derive in-app stack trace rules #87842

armenzg · 2025-03-25T13:22:14Z

This is the logic to mark frames as in-app when the files are found in the developers' GitHub repositories.

This is very important for languages where the SDK or our internal rules cannot determine which frames are in-app and which are not.

The initial language to get support for this is Java.

This will run in dry run mode to discover errors or scaling problems.

codecov · 2025-03-25T13:53:18Z

Codecov Report

Attention: Patch coverage is 96.33028% with 4 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...uto_source_code_config/in_app_stack_trace_rules.py	87.50%	4 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #87842       +/-   ##
===========================================
+ Coverage   42.12%   87.74%   +45.61%     
===========================================
  Files        9892     9924       +32     
  Lines      561937   563914     +1977     
  Branches    22190    22190               
===========================================
+ Hits       236735   494815   +258080     
+ Misses     324771    68668   -256103     
  Partials      431      431

armenzg · 2025-03-25T13:27:57Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

-        expected_stack_root: str,
-        expected_source_root: str,
+        expected_stack_root: str | None = None,
+        expected_source_root: str | None = None,


Once this merges, this won't be necessary.

armenzg · 2025-03-25T13:28:26Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

@@ -100,12 +102,13 @@ def _process_and_assert_configuration_changes(
            )
            code_mappings = RepositoryProjectPathConfig.objects.all()
            assert len(code_mappings) == expected_num_code_mappings
-            code_mapping = code_mappings.filter(


This block won't be necessary when #87776 merges.

armenzg · 2025-03-25T14:17:19Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

@@ -133,7 +146,8 @@ def _process_and_assert_no_configuration_changes(
        repo_trees: Mapping[str, Sequence[str]],
        frames: Sequence[Mapping[str, str | bool]],
        platform: str,
-    ) -> tuple[list[CodeMapping], list[str]]:


We don't need to return in_app_stacrace_rules since we can test it within this function.

tests/sentry/issues/auto_source_code_config/test_process_event.py

armenzg · 2025-03-25T14:27:19Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

+            frames=[
+                {"module": "com.example.foo.Bar", "abs_path": "Bar.kt"},
+                {"module": "com.example.foo.Baz", "abs_path": "Baz.kt"},
+                {"module": "com.example.utils.Helper", "abs_path": "Helper.kt"},


There's the utils app and the foo app under the same domain, thus, we will have one in-app stack trace rule but two code mappings.

lobsterkatie

This LGTM! Can't wait to see the effects once it's turned on!

src/sentry/grouping/api.py

lobsterkatie · 2025-03-26T00:16:21Z

src/sentry/grouping/api.py

+                enhancements_string = (
+                    f"{automatic_enhancements}\n{enhancements_string}"
+                    if enhancements_string
+                    else automatic_enhancements
+                )


Nit: This can be simpler. If enhancements_string is falsy, the worst that happens is we add an extra newline to the auto enhancements.

Suggested change

enhancements_string = (

f"{automatic_enhancements}\n{enhancements_string}"

if enhancements_string

else automatic_enhancements

)

enhancements_string = f"{automatic_enhancements}\n{enhancements_string}"

lobsterkatie · 2025-03-26T00:16:43Z

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

+
+logger = logging.getLogger(__name__)
+
+OPTION_KEY = "sentry:automatic_grouping_enhancements"


What would you think of derived_grouping_enhancements as an option name rather than automatic_grouping_enhancements?

If I didn't know anything about how this worked, my first thought seeing the description "auto enhancements" would be that they were enhancements applied automatically. OTOH, that's not a lie - we do apply them automatically. OTOH, we also apply the default and custom enhancements automatically, so under my gut-reaction interpretation "automatic" isn't a great differentiator.

Works for me 👍🏻

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

lobsterkatie · 2025-03-26T00:24:42Z

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

+    # For packages including the domain name, we want to have less granularity to create less rules, thus,
+    # reducing the number of rules applied to the stack trace.
+    # com/example/foo/bar -> com.example
+    if stacktrace_root.find("/") > 2:


Doesn't this want to be >= 2? You've already stripped the trailing slash, but you still want to catch examples like com/example/foo, right?

lobsterkatie · 2025-03-26T00:25:20Z

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

+        if len(parts) > 2:
+            stacktrace_root = "/".join(parts[:2])
+
+    return f"stack.module:{stacktrace_root.replace('/', '.')}.** +app"


Will this rule match something at the base level of the stacktrace root (module = com.example)? IOW, can .** match with nothing, or does the . mean that it'd have to be com.example. (trailing period) to match? (IDK if that's even a reasonable scenario, though.)

Also, does our glob matching treat . the same way it treats /? Put another way, does it recognize a dot-separated string as a path, or does it fall back to literal string matching?

lobsterkatie · 2025-03-26T00:26:07Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

+            assert event.data["metadata"]["in_app_frame_mix"] == "system-only"
+            assert len(event.data["hashes"]) == 1  # Only system hash
+            system_only_hash = event.data["hashes"][0]
+            first_enhancements_hash = event.data["grouping_config"]["enhancements"]


I get why you're referring to this as a "hash", but that means something different than the "hash" on the line before. Maybe first_enhancements_base64_string or something?

tests/sentry/issues/auto_source_code_config/test_process_event.py

This is the logic to mark frames as in-app when the files are found in the developers' GitHub repositories. This is very important for languages where the SDK or our internal rules cannot determine which frames are in-app vs not. The initial language to get support for this is Java. For now, this will run as dry-run to discover any errors or escaling problems.

Co-authored-by: Katie Byers <katie.byers@sentry.io>

armenzg · 2025-03-26T18:11:35Z

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -593,9 +593,12 @@ def get_path_from_module(module: str, abs_path: str) -> tuple[str, str]:
    if "." not in module:
        raise DoesNotFollowJavaPackageNamingConvention

-    # If module has a dot, take everything before the last dot


These changes simplify this code block.

It also fixes a bug where we were not restricting the length of the stack root, thus, we would end up with com/example/ and many more levels deep (not intended behaviour).

There's a test down below that ensures we get the right number of levels.

Inputs:

repo_file="src/com/example/foo/bar/Baz.kt"

self.frame(module="com.example.foo.bar.Baz", abs_path="Baz.kt", in_app=False)

Expected:

stack_root="com/example/", source_root="src/com/example/"

in_app_rule="stack.module:com.example.** +app"

MichaelSun48 · 2025-03-26T18:23:34Z

src/sentry/grouping/api.py

@@ -87,6 +88,7 @@ def get_config_dict(self, project: Project) -> GroupingConfig:
        }

    def _get_enhancements(self, project: Project) -> str:


Just curious - where/how is this enhancement string used?

Primarily, it is used as part of save_event to determine which frames should be considered for app or system hash calculations.

MichaelSun48 · 2025-03-26T18:27:08Z

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

+    if stacktrace_root == "":
+        raise ValueError("Stacktrace root is empty")
+
+    parts = stacktrace_root.rstrip("/").split("/", 2)


Do we need to account for backslash paths too?

Not for Java.
When we generate the code mappings, we replace periods (e.g. com.foo) with forward slashes (e.g. com/foo).

MichaelSun48 · 2025-03-26T18:31:26Z

tests/sentry/issues/auto_source_code_config/test_process_event.py

+            platform=self.platform,
+            expected_new_code_mappings=[self.code_mapping("com/example/", "src/com/example/")],
+            expected_in_app_stack_trace_rules=["stack.module:com.example.** +app"],
+            expected_num_code_mappings=0,


This API is a bit confusing to me — the expected_num_code_mappings seems to conflict with the expected_new_code_mappings.

It is. I intend to fix it. Since we're in dry_run mode, it won't create them.

tests/sentry/issues/auto_source_code_config/test_process_event.py

MichaelSun48 · 2025-03-26T18:35:17Z

src/sentry/issues/auto_source_code_config/in_app_stack_trace_rules.py

+    if not platform_config.is_dry_run_platform():
+        project.update_option(DERIVED_ENHANCEMENTS_OPTION_KEY, "\n".join(rules))


Should we be checking for duplicates here?

Good catch!

Locally, I have a test and a fix for this. I will push it later.

This is the logic to mark frames as in-app when the files are found in the developers' GitHub repositories. This is very important for languages where the SDK or our internal rules cannot determine which frames are in-app and which are not. The initial language to get support for this is Java. This will run in dry run mode to discover errors or scaling problems. --------- Co-authored-by: Katie Byers <katie.byers@sentry.io>

armenzg self-assigned this Mar 25, 2025

github-actions bot added the Scope: Backend label Mar 25, 2025

armenzg commented Mar 25, 2025

View reviewed changes

armenzg requested a review from MichaelSun48 March 25, 2025 14:28

armenzg force-pushed the in_app_stacktrace/auto_source_config/armenzg branch from 2564abb to d519a3b Compare March 25, 2025 18:28

vercel bot deployed to Preview March 25, 2025 18:34 View deployment

armenzg requested a review from lobsterkatie March 25, 2025 20:07

vercel bot deployed to Preview March 25, 2025 20:11 View deployment

armenzg marked this pull request as ready for review March 25, 2025 20:33

armenzg requested review from a team as code owners March 25, 2025 20:33

lobsterkatie approved these changes Mar 26, 2025

View reviewed changes

vercel bot deployed to Preview March 26, 2025 11:52 View deployment

armenzg changed the base branch from master to ref/multiple_code_mappings/auto_source_config/armenzg March 26, 2025 14:50

vercel bot deployed to Preview March 26, 2025 14:52 View deployment

Base automatically changed from ref/multiple_code_mappings/auto_source_config/armenzg to master March 26, 2025 17:50

armenzg and others added 8 commits March 26, 2025 14:09

Apply suggestions from code review

4627f6e

Co-authored-by: Katie Byers <katie.byers@sentry.io>

Reduce diff

c44bfe6

Reduce diff

746441b

Reduce diff

ed698e5

Minor changes

23a941a

Refactor and fix

e91b9b5

Minor change

f52e04a

armenzg force-pushed the in_app_stacktrace/auto_source_config/armenzg branch from 2c93f6e to f52e04a Compare March 26, 2025 18:10

armenzg changed the title ~~feat(auto_source_config): Create in-app stack trace rules~~ feat(auto_source_config): Derive in-app stack trace rules Mar 26, 2025

vercel bot deployed to Preview March 26, 2025 18:15 View deployment

armenzg commented Mar 26, 2025

View reviewed changes

MichaelSun48 reviewed Mar 26, 2025

View reviewed changes

Fix

0d5a25a

vercel bot deployed to Preview March 26, 2025 18:59 View deployment

armenzg added 2 commits March 27, 2025 08:16

Prevent clobbering of rules

a021092

Rename variable

6592b9b

vercel bot deployed to Preview March 27, 2025 12:25 View deployment

armenzg enabled auto-merge (squash) March 27, 2025 12:40

armenzg merged commit d3afa10 into master Mar 27, 2025
48 checks passed

armenzg deleted the in_app_stacktrace/auto_source_config/armenzg branch March 27, 2025 12:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(auto_source_config): Derive in-app stack trace rules #87842

feat(auto_source_config): Derive in-app stack trace rules #87842

armenzg commented Mar 25, 2025

codecov bot commented Mar 25, 2025 •

edited

Loading

armenzg Mar 25, 2025

armenzg Mar 25, 2025

armenzg Mar 25, 2025

armenzg Mar 25, 2025

lobsterkatie left a comment

lobsterkatie Mar 26, 2025

lobsterkatie Mar 26, 2025

armenzg Mar 26, 2025

lobsterkatie Mar 26, 2025

lobsterkatie Mar 26, 2025

lobsterkatie Mar 26, 2025

armenzg Mar 26, 2025

MichaelSun48 Mar 26, 2025

armenzg Mar 26, 2025

MichaelSun48 Mar 26, 2025

armenzg Mar 27, 2025

MichaelSun48 Mar 26, 2025

armenzg Mar 26, 2025

MichaelSun48 Mar 26, 2025

armenzg Mar 26, 2025

armenzg Mar 27, 2025


		logger = logging.getLogger(__name__)

		OPTION_KEY = "sentry:automatic_grouping_enhancements"

		@@ -87,6 +88,7 @@ def get_config_dict(self, project: Project) -> GroupingConfig:
		}

		def _get_enhancements(self, project: Project) -> str:

		if not platform_config.is_dry_run_platform():
		project.update_option(DERIVED_ENHANCEMENTS_OPTION_KEY, "\n".join(rules))

feat(auto_source_config): Derive in-app stack trace rules #87842

feat(auto_source_config): Derive in-app stack trace rules #87842

Conversation

armenzg commented Mar 25, 2025

codecov bot commented Mar 25, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lobsterkatie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 25, 2025 •

edited

Loading