feat(symbolication): Make frame order explicit

src/sentry/lang/java/processing.py

@@ -6,7 +6,7 @@
 from sentry.lang.java.utils import JAVA_PLATFORMS, get_jvm_images, get_proguard_images
 from sentry.lang.java.view_hierarchies import ViewHierarchies
 from sentry.lang.native.error import SymbolicationFailed, write_error
-from sentry.lang.native.symbolicator import Symbolicator
+from sentry.lang.native.symbolicator import FrameOrder, Symbolicator
 from sentry.models.eventerror import EventError
 from sentry.models.project import Project
 from sentry.models.release import Release
@@ -220,6 +220,9 @@ def process_jvm_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
         modules=modules,
         release_package=release_package,
         classes=window_class_names + exception_class_names,
+        # We are sending frames in the same order in which
+        # they were stored in the event, so this has to be "caller_first".
+        frame_order=FrameOrder.caller_first,
     )
 
     if not _handle_response_status(data, response):

src/sentry/lang/javascript/processing.py

@@ -5,7 +5,7 @@
 from sentry.debug_files.artifact_bundles import maybe_renew_artifact_bundles_from_processing
 from sentry.lang.javascript.utils import JAVASCRIPT_PLATFORMS
 from sentry.lang.native.error import SymbolicationFailed, write_error
-from sentry.lang.native.symbolicator import Symbolicator
+from sentry.lang.native.symbolicator import FrameOrder, Symbolicator
 from sentry.models.eventerror import EventError
 from sentry.stacktraces.processing import find_stacktraces_in_data
 from sentry.utils import metrics
@@ -252,6 +252,9 @@ def process_js_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
         modules=modules,
         release=data.get("release"),
         dist=data.get("dist"),
+        # We are sending frames in the same order in which
+        # they were stored in the event, so this has to be "caller_first".
+        frame_order=FrameOrder.caller_first,
     )
 
     if not _handle_response_status(data, response):

src/sentry/lang/native/processing.py

@@ -11,7 +11,7 @@
 
 from sentry import options
 from sentry.lang.native.error import SymbolicationFailed, write_error
-from sentry.lang.native.symbolicator import Symbolicator
+from sentry.lang.native.symbolicator import FrameOrder, Symbolicator
 from sentry.lang.native.utils import (
     get_event_attachment,
     get_os_from_event,
@@ -471,7 +471,13 @@ def process_native_stacktraces(symbolicator: Symbolicator, data: Any) -> Any:
 
     metrics.incr("process.native.symbolicate.request")
     response = symbolicator.process_payload(
-        platform=data.get("platform"), stacktraces=stacktraces, modules=modules, signal=signal
+        platform=data.get("platform"),
+        stacktraces=stacktraces,
+        modules=modules,
+        # Frames were reversed in `get_frames_for_symbolication`,
+        # so this has to be "callee_first".
+        frame_order=FrameOrder.callee_first,
+        signal=signal,
     )
 
     if not _handle_response_status(data, response):

src/sentry/lang/native/symbolicator.py

@@ -47,6 +47,18 @@ class SymbolicatorPlatform(Enum):
     native = "native"
 
 
+class FrameOrder(Enum):
+    """The order in which stack frames are sent to
+    and returned from Symbolicator."""
+
+    # Caller frames come before callee frames. This is the
+    # order in which stack frames are stored in events.
+    caller_first = "caller_first"
+    # Callee frames come before caller frames. This is the
+    # order in which stack frames are usually displayed.
+    callee_first = "callee_first"
+
+
 @dataclass(frozen=True)
 class SymbolicatorTaskKind:
     """Bundles information about a symbolication task:
@@ -261,8 +273,20 @@ def process_applecrashreport(self, platform: str, report: CachedAttachment):
         return process_response(res)
 
     def process_payload(
-        self, platform, stacktraces, modules, signal=None, apply_source_context=True
+        self, platform, stacktraces, modules, frame_order, signal=None, apply_source_context=True
     ):
+        """
+        Process a native event by symbolicating its frames.
+
+        :param platform: The event's platform. This should be either unset or one of "objc", "cocoa", "swift", "native", "c", "csharp".
+        :param stacktraces: The event's stacktraces. Frames must contain an `instruction_address`.
+                            Frames are expected to be ordered according to the frame_order (see below).
+        :param modules: ProGuard modules and source bundles. They must contain a `uuid` and have a
+                        `type` of either "proguard" or "source".
+        :param frame_order: The order of frames within stacktraces. See the documentation of `FrameOrder`.
+        :param signal: A numeric crash signal value. This is optional.
+        :param apply_source_context: Whether to add source context to frames.
+        """
         (sources, process_response) = sources_for_symbolication(self.project)
         scraping_config = get_scraping_config(self.project)
         json = {
@@ -271,6 +295,7 @@ def process_payload(
             "options": {
                 "dif_candidates": True,
                 "apply_source_context": apply_source_context,
+                "frame_order": frame_order.value,
             },
             "stacktraces": stacktraces,
             "modules": modules,
@@ -283,7 +308,22 @@ def process_payload(
         res = self._process("symbolicate_stacktraces", "symbolicate", json=json)
         return process_response(res)
 
-    def process_js(self, platform, stacktraces, modules, release, dist, apply_source_context=True):
+    def process_js(
+        self, platform, stacktraces, modules, release, dist, frame_order, apply_source_context=True
+    ):
+        """
+        Process a JS event by remapping its frames with sourcemaps.
+
+        :param platform: The event's platform. This should be unset, "javascript", or "node".
+        :param stacktraces: The event's stacktraces. Frames must contain a `function` and a `module`.
+                            Frames are expected to be ordered according to the frame_order (see below).
+        :param modules: Minified source files/sourcemaps Thy must contain a `type` field with value "sourcemap",
+                        a `code_file`, and a `debug_id`.
+        :param release: The event's release.
+        :param dist: The event's dist.
+        :param frame_order: The order of frames within stacktraces. See the documentation of `FrameOrder`.
+        :param apply_source_context: Whether to add source context to frames.
+        """
         source = get_internal_artifact_lookup_source(self.project)
         scraping_config = get_scraping_config(self.project)
 
@@ -292,7 +332,10 @@ def process_js(self, platform, stacktraces, modules, release, dist, apply_source
             "source": source,
             "stacktraces": stacktraces,
             "modules": modules,
-            "options": {"apply_source_context": apply_source_context},
+            "options": {
+                "apply_source_context": apply_source_context,
+                "frame_order": frame_order.value,
+            },
             "scraping": scraping_config,
         }
 
@@ -311,6 +354,7 @@ def process_jvm(
         modules,
         release_package,
         classes,
+        frame_order,
         apply_source_context=True,
     ):
         """
@@ -320,10 +364,12 @@ def process_jvm(
         :param platform: The event's platform. This should be either unset or "java".
         :param exceptions: The event's exceptions. These must contain a `type` and a `module`.
         :param stacktraces: The event's stacktraces. Frames must contain a `function` and a `module`.
+                            Frames are expected to be ordered according to the frame_order (see below).
         :param modules: ProGuard modules and source bundles. They must contain a `uuid` and have a
                         `type` of either "proguard" or "source".
         :param release_package: The name of the release's package. This is optional.
                                 Used for determining whether frames are in-app.
+        :param frame_order: The order of frames within stacktraces. See the documentation of `FrameOrder`.
         :param apply_source_context: Whether to add source context to frames.
         """
         source = get_internal_source(self.project)
@@ -335,7 +381,10 @@ def process_jvm(
             "stacktraces": stacktraces,
             "modules": modules,
             "classes": classes,
-            "options": {"apply_source_context": apply_source_context},
+            "options": {
+                "apply_source_context": apply_source_context,
+                "frame_order": frame_order.value,
+            },
         }
 
         if release_package is not None:

src/sentry/profiles/task.py

@@ -29,7 +29,12 @@
 from sentry.constants import DataCategory
 from sentry.lang.javascript.processing import _handles_frame as is_valid_javascript_frame
 from sentry.lang.native.processing import _merge_image
-from sentry.lang.native.symbolicator import Symbolicator, SymbolicatorPlatform, SymbolicatorTaskKind
+from sentry.lang.native.symbolicator import (
+    FrameOrder,
+    Symbolicator,
+    SymbolicatorPlatform,
+    SymbolicatorTaskKind,
+)
 from sentry.lang.native.utils import native_images_from_data
 from sentry.models.eventerror import EventError
 from sentry.models.files.utils import get_profiles_storage
@@ -424,6 +429,9 @@ def _symbolicate_profile(profile: Profile, project: Project) -> bool:
                     profile=profile,
                     modules=raw_modules,
                     stacktraces=raw_stacktraces,
+                    # Frames in a profile aren't inherently ordered,
+                    # but returned inlinees should be ordered callee first.
+                    frame_order=FrameOrder.callee_first,
                     platform=platform,
                 )
 
@@ -599,6 +607,7 @@ def symbolicate(
     profile: Profile,
     modules: list[Any],
     stacktraces: list[Any],
+    frame_order: FrameOrder,
     platform: str,
 ) -> Any:
     if platform in SHOULD_SYMBOLICATE_JS:
@@ -608,6 +617,7 @@ def symbolicate(
             modules=modules,
             release=profile.get("release"),
             dist=profile.get("dist"),
+            frame_order=frame_order,
             apply_source_context=False,
         )
     elif platform == "android":
@@ -617,13 +627,15 @@ def symbolicate(
             stacktraces=stacktraces,
             modules=modules,
             release_package=profile.get("transaction_metadata", {}).get("app.identifier"),
+            frame_order=frame_order,
             apply_source_context=False,
             classes=[],
         )
     return symbolicator.process_payload(
         platform=platform,
         stacktraces=stacktraces,
         modules=modules,
+        frame_order=frame_order,
         apply_source_context=False,
     )
 
@@ -638,6 +650,7 @@ def run_symbolicate(
     profile: Profile,
     modules: list[Any],
     stacktraces: list[Any],
+    frame_order: FrameOrder,
     platform: str,
 ) -> tuple[list[Any], list[Any], bool]:
     symbolication_start_time = time()
@@ -665,6 +678,7 @@ def on_symbolicator_request() -> None:
                 profile=profile,
                 stacktraces=stacktraces,
                 modules=modules,
+                frame_order=frame_order,
                 platform=platform,
             )
 
@@ -943,6 +957,9 @@ def on_symbolicator_request() -> None:
                         )
                     },
                 ],
+                # Methods in a profile aren't inherently ordered, but the order of returned
+                # inlinees should be caller first.
+                frame_order=FrameOrder.caller_first,
                 platform=profile["platform"],
             )
             if response:

tests/relay_integration/lang/javascript/test_plugin.py

@@ -354,7 +354,10 @@ def test_sourcemap_source_expansion(self) -> None:
             }
         ]
 
-        assert event.data["scraping_attempts"] == [
+        scraping_attempts = sorted(
+            event.data["scraping_attempts"], key=lambda attempt: attempt["url"]
+        )
+        assert scraping_attempts == [
             {"status": "not_attempted", "url": "http://example.com/file.min.js"},
             {"status": "not_attempted", "url": "http://example.com/file.sourcemap.js"},
             {"status": "not_attempted", "url": "http://example.com/file1.js"},
@@ -461,7 +464,10 @@ def test_sourcemap_webpack(self) -> None:
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames
 
-        assert event.data["scraping_attempts"] == [
+        scraping_attempts = sorted(
+            event.data["scraping_attempts"], key=lambda attempt: attempt["url"]
+        )
+        assert scraping_attempts == [
             {"url": "http://example.com/webpack1.min.js", "status": "not_attempted"},
             {"url": "http://example.com/webpack1.min.js.map", "status": "not_attempted"},
             {"url": "http://example.com/webpack2.min.js", "status": "not_attempted"},
@@ -564,7 +570,10 @@ def test_sourcemap_embedded_source_expansion(self) -> None:
             }
         ]
 
-        assert event.data["scraping_attempts"] == [
+        scraping_attempts = sorted(
+            event.data["scraping_attempts"], key=lambda attempt: attempt["url"]
+        )
+        assert scraping_attempts == [
             {"status": "not_attempted", "url": "http://example.com/embedded.js"},
             {"status": "not_attempted", "url": "http://example.com/embedded.js.map"},
         ]
@@ -634,7 +643,10 @@ def test_sourcemap_nofiles_source_expansion(self) -> None:
 
         assert "errors" not in event.data
 
-        assert event.data["scraping_attempts"] == [
+        scraping_attempts = sorted(
+            event.data["scraping_attempts"], key=lambda attempt: attempt["url"]
+        )
+        assert scraping_attempts == [
             {"url": "app:///nofiles.js", "status": "not_attempted"},
             {"url": "app:///nofiles.js.map", "status": "not_attempted"},
         ]
@@ -710,11 +722,14 @@ def test_indexed_sourcemap_source_expansion(self) -> None:
 
         assert "errors" not in event.data
 
-        assert event.data["scraping_attempts"] == [
-            {"status": "not_attempted", "url": "http://example.com/indexed.min.js"},
-            {"status": "not_attempted", "url": "http://example.com/indexed.sourcemap.js"},
+        scraping_attempts = sorted(
+            event.data["scraping_attempts"], key=lambda attempt: attempt["url"]
+        )
+        assert scraping_attempts == [
             {"status": "not_attempted", "url": "http://example.com/file1.js"},
             {"status": "not_attempted", "url": "http://example.com/file2.js"},
+            {"status": "not_attempted", "url": "http://example.com/indexed.min.js"},
+            {"status": "not_attempted", "url": "http://example.com/indexed.sourcemap.js"},
         ]
 
         exception = event.interfaces["exception"]