ref(derive_code_mappings): Process frames instead of file paths #85174

armenzg · 2025-02-13T18:10:13Z

For some languages (e.g. Java) we're going to process all frames, thus, we need to pass around more than the filenames.

armenzg · 2025-02-13T18:12:06Z

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -48,20 +49,22 @@ class UnsupportedFrameFilename(Exception):

 def derive_code_mappings(


This is the function called by the endpoint. At a later point, we will have to pass more than just the filename so we can support

armenzg · 2025-02-13T18:14:25Z

src/sentry/issues/auto_source_code_config/code_mapping.py

-    def __init__(self, frame_file_path: str) -> None:
+    def __init__(self, frame: Mapping[str, Any]) -> None:
+        # XXX: In the next PR, we will use more than just the filename
+        frame_file_path = frame["filename"]


In the PR for Java, we will use abs_path and module.

armenzg · 2025-02-13T18:14:58Z

src/sentry/issues/auto_source_code_config/code_mapping.py

@@ -187,22 +190,24 @@ def list_file_matches(self, frame_filename: FrameFilename) -> list[dict[str, str
                    )
        return file_matches

-    def _stacktrace_buckets(self, stacktraces: list[str]) -> dict[str, list[FrameFilename]]:
+    def _stacktrace_buckets(
+        self, frames: Sequence[Mapping[str, Any]]


All along we had been passing the frames but I mislabeled the variable.

armenzg · 2025-02-13T18:16:56Z

src/sentry/issues/auto_source_code_config/constants.py

@@ -11,3 +11,5 @@
 ]
 # These languages will run as dry-run mode by default
 DRY_RUN_PLATFORMS: list[str] = []
+# Some languages will also process system frames
+PROCESS_ALL_FRAMES: list[str] = []


We may one day do this for all platforms, however, for now, we will keep it to a short list.

armenzg · 2025-02-13T18:17:33Z

src/sentry/issues/auto_source_code_config/stacktraces.py

-            if frame is None:
-                continue
+            if platform in PROCESS_ALL_FRAMES:
+                frames_to_process.append(frame)


This is the functional change to this function.

armenzg · 2025-02-13T18:18:15Z

src/sentry/issues/auto_source_code_config/stacktraces.py

    exceptions = get_path(data, "exception", "values", filter=True)
    if exceptions:
        return [e["stacktrace"] for e in exceptions if get_path(e, "stacktrace", "frames")]

    stacktrace = data.get("stacktrace")
    if stacktrace and stacktrace.get("frames"):
+        logger.warning("Investigate if we use this code path in production.")


The only reason I believe we need this block is because of the way I wrote the tests. If this error doesn't happen in production I will remove it.

totally a preference thing, but I'd rather send an info event to Sentry here instead of a gcp log.

@MichaelSun48 me as well! logger.warning() captures a message in Sentry with a level of warning 👍🏻

armenzg · 2025-02-13T18:18:54Z

src/sentry/issues/endpoints/organization_derive_code_mappings.py

@@ -47,14 +47,17 @@ def get(self, request: Request, organization: Organization) -> Response:
        :param string stacktraceFilename:
        :auth: required
        """
+        # XXX: Evaluate what else does the UI send us


I have added a task so I don't forget to check if "Set up code mappings" works for Java.

tests/sentry/issues/auto_source_code_config/test_code_mapping.py

armenzg · 2025-02-13T18:20:42Z

tests/sentry/issues/auto_source_code_config/test_stacktraces.py

-def _stacktrace_with_frames(frames: list[dict[str, Any]]) -> Stacktrace:
-    return {"stacktrace": {"frames": frames}}
+def _exception_with_stacktrace(frames: list[dict[str, Any]]) -> dict[str, Any]:
+    return {"exception": {"values": [{"stacktrace": {"frames": frames}}]}}


This data structure change is what I refer to in this comment:

armenzg · 2025-02-13T18:21:12Z

tests/sentry/issues/auto_source_code_config/test_stacktraces.py

No functional changes in this test file besides data structure changes.

codecov · 2025-02-13T18:35:53Z

Codecov Report

Attention: Patch coverage is 98.52941% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ntry/issues/auto_source_code_config/stacktraces.py	92.85%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #85174       +/-   ##
===========================================
+ Coverage   46.25%   87.98%   +41.73%     
===========================================
  Files        9614     9624       +10     
  Lines      542907   543473      +566     
  Branches    21042    21029       -13     
===========================================
+ Hits       251112   478169   +227057     
+ Misses     291489    65000   -226489     
+ Partials      306      304        -2

MichaelSun48 · 2025-02-13T19:23:46Z

src/sentry/issues/auto_source_code_config/stacktraces.py

+def get_frames_to_process(
+    data: NodeData | dict[str, Any], platform: str | None = None
+) -> list[dict[str, Any]]:
+    """It flattens all processableframes from the event's data."""


Suggested change

"""It flattens all processableframes from the event's data."""

"""It flattens all processable frames from the event's data."""

MichaelSun48 · 2025-02-13T19:27:46Z

src/sentry/issues/auto_source_code_config/stacktraces.py

    exceptions = get_path(data, "exception", "values", filter=True)
    if exceptions:
        return [e["stacktrace"] for e in exceptions if get_path(e, "stacktrace", "frames")]

    stacktrace = data.get("stacktrace")
    if stacktrace and stacktrace.get("frames"):
+        logger.warning("Investigate if we use this code path in production.")


totally a preference thing, but I'd rather send an info event to Sentry here instead of a gcp log.

sentry-io · 2025-02-14T14:17:34Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ AttributeError: 'NoneType' object has no attribute 'get' sentry.tasks.post_process.post_process_group View Issue
‼️ UnsupportedFrameFilename: This path is not supported. /api/0/organizations/{organization_id_or_slug}/... View Issue
‼️ UnsupportedFrameFilename: It needs an extension. /api/0/organizations/{organization_id_or_slug}/... View Issue

_{Did you find this useful? React with a 👍 or 👎}

armenzg added 2 commits February 13, 2025 12:27

ref(derive_code_mappings): Process frames instead of file paths

63d627b

For some languages (e.g. Java) we're going to process all frames, thus, we need to pass around more than the filenames.

Fix tests

fd7aecf

armenzg self-assigned this Feb 13, 2025

github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Feb 13, 2025

armenzg added this to the Automate Code Mappings & In-App Stack Trace Rules for Java projects milestone Feb 13, 2025

vercel bot deployed to Preview February 13, 2025 18:12 View deployment

armenzg commented Feb 13, 2025

View reviewed changes

armenzg requested a review from MichaelSun48 February 13, 2025 18:21

armenzg marked this pull request as ready for review February 13, 2025 18:21

armenzg requested a review from a team as a code owner February 13, 2025 18:21

Merge branch 'master' into ref/process_frames/auto_source/armenzg

fbb8652

armenzg mentioned this pull request Feb 13, 2025

[dry run] Automatic code mappings for Java & collect metrics #85187

Closed

vercel bot deployed to Preview February 13, 2025 19:08 View deployment

armenzg added 2 commits February 13, 2025 14:17

Typing

f92162e

Fix typing

b757ef6

MichaelSun48 approved these changes Feb 13, 2025

View reviewed changes

vercel bot deployed to Preview February 13, 2025 19:37 View deployment

armenzg merged commit 3290371 into master Feb 14, 2025
49 checks passed

armenzg deleted the ref/process_frames/auto_source/armenzg branch February 14, 2025 12:29

github-actions bot locked and limited conversation to collaborators Mar 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref(derive_code_mappings): Process frames instead of file paths #85174

ref(derive_code_mappings): Process frames instead of file paths #85174

armenzg commented Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025 •

edited

Loading

armenzg Feb 13, 2025

armenzg Feb 13, 2025

MichaelSun48 Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025

armenzg Feb 13, 2025

codecov bot commented Feb 13, 2025 •

edited

Loading

MichaelSun48 Feb 13, 2025

MichaelSun48 Feb 13, 2025

sentry-io bot commented Feb 14, 2025 •

edited

Loading

		@@ -48,20 +49,22 @@ class UnsupportedFrameFilename(Exception):

		def derive_code_mappings(

	"""It flattens all processableframes from the event's data."""
	"""It flattens all processable frames from the event's data."""

ref(derive_code_mappings): Process frames instead of file paths #85174

ref(derive_code_mappings): Process frames instead of file paths #85174

Conversation

armenzg commented Feb 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armenzg Feb 13, 2025 • edited Loading

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 Feb 13, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sentry-io bot commented Feb 14, 2025 • edited Loading

Suspect Issues

armenzg Feb 13, 2025 •

edited

Loading

codecov bot commented Feb 13, 2025 •

edited

Loading

sentry-io bot commented Feb 14, 2025 •

edited

Loading