From bb46dc042cef645a5f873d925bab39ef3862770a Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Sat, 24 Oct 2026 13:33:31 -0700 Subject: [PATCH 1/4] fix(native): Bound flamegraph inline expansion allocations --- src/sentry/lang/native/flamegraphs.py | 10 +++ src/sentry/lang/native/processing.py | 6 +- tests/sentry/lang/native/test_processing.py | 80 ++++++++++++++++++++- 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/src/sentry/lang/native/flamegraphs.py b/src/sentry/lang/native/flamegraphs.py index 2ffe7369ab54..6e90714de0fc 100644 --- a/src/sentry/lang/native/flamegraphs.py +++ b/src/sentry/lang/native/flamegraphs.py @@ -18,6 +18,12 @@ MAX_FLAMEGRAPH_SIZE = 10 * 1024 * 1024 MAX_SYMBOLICATION_FRAMES = 2500 MAX_SAMPLE_COUNT = 9007199254740991 +# Bound node allocations independently of the serialized attachment size. +MAX_EXPANDED_NODES = 100_000 + + +class FlamegraphExpansionLimitExceeded(Exception): + pass class _StringField(serializers.Field): @@ -113,11 +119,15 @@ def _is_valid_flamegraph(payload: Any) -> bool: def expand_inline_frames( nodes: list[dict[str, Any]], chains: dict[int, list[int]] ) -> list[dict[str, Any]]: + if len(nodes) > MAX_EXPANDED_NODES: + raise FlamegraphExpansionLimitExceeded # Keep captured indices stable so roots and shared child references remain valid. output = [dict(node) for node in nodes] for index, original in enumerate(nodes): current = output[index] for frame_id in chains.get(original["frame_id"], [])[1:]: + if len(output) >= MAX_EXPANDED_NODES: + raise FlamegraphExpansionLimitExceeded child = {"frame_id": frame_id, "sample_count": original["sample_count"]} current["children"] = [len(output)] output.append(child) diff --git a/src/sentry/lang/native/processing.py b/src/sentry/lang/native/processing.py index c8374077f915..63c3aedc612d 100644 --- a/src/sentry/lang/native/processing.py +++ b/src/sentry/lang/native/processing.py @@ -14,6 +14,7 @@ from sentry.lang.native.error import SymbolicationFailed, write_error from sentry.lang.native.flamegraphs import ( MAX_SYMBOLICATION_FRAMES, + FlamegraphExpansionLimitExceeded, Flamegraphs, expand_inline_frames, ) @@ -548,7 +549,10 @@ def process_native_stacktraces(symbolicator: Symbolicator, data: Any) -> Any: resolved_stacktraces = response.get("stacktraces", []) if len(resolved_stacktraces) != len(frame_ids): continue - output = _merge_flamegraph_frames(payload, frame_ids, resolved_stacktraces) + try: + output = _merge_flamegraph_frames(payload, frame_ids, resolved_stacktraces) + except FlamegraphExpansionLimitExceeded: + continue attachments_changed |= flamegraphs.save(attachment_index, output) stacktrace_infos = [ stacktrace diff --git a/tests/sentry/lang/native/test_processing.py b/tests/sentry/lang/native/test_processing.py index 2eb4a5a46d06..9566d7f96dcb 100644 --- a/tests/sentry/lang/native/test_processing.py +++ b/tests/sentry/lang/native/test_processing.py @@ -21,7 +21,11 @@ get_attachments_for_event, store_attachments_for_event, ) -from sentry.lang.native.flamegraphs import _is_valid_flamegraph +from sentry.lang.native.flamegraphs import ( + FlamegraphExpansionLimitExceeded, + _is_valid_flamegraph, + expand_inline_frames, +) from sentry.lang.native.processing import ( ELECTRON_FIRST_MODULE_REWRITE_RULES, _get_flamegraph_stacktraces, @@ -292,6 +296,38 @@ def test_flamegraph_indexed_inline_expansion(flamegraph_payload, nodes) -> None: assert _is_valid_flamegraph(output) +def test_flamegraph_inline_expansion_at_node_limit() -> None: + nodes = [ + {"frame_id": 0, "sample_count": 10, "children": [1]}, + {"frame_id": 0, "sample_count": 10}, + ] + with mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", 4): + assert expand_inline_frames(nodes, {0: [0, 1]}) == [ + {"frame_id": 0, "sample_count": 10, "children": [2]}, + {"frame_id": 0, "sample_count": 10, "children": [3]}, + {"frame_id": 1, "sample_count": 10, "children": [1]}, + {"frame_id": 1, "sample_count": 10}, + ] + + +def test_flamegraph_inline_expansion_discards_partial_rewrite(flamegraph_payload) -> None: + flamegraph_payload["nodes"] = [ + {"frame_id": 0, "sample_count": 10, "children": [1]}, + {"frame_id": 0, "sample_count": 10}, + ] + original = deepcopy(flamegraph_payload) + with ( + mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", 3), + pytest.raises(FlamegraphExpansionLimitExceeded), + ): + _merge_flamegraph_frames( + flamegraph_payload, + [0], + [{"frames": [{"function": "outer"}, {"function": "inner"}]}], + ) + assert flamegraph_payload == original + + def test_flamegraph_repeated_child_edges_are_not_materialized_as_paths(flamegraph_payload) -> None: depth = 31 flamegraph_payload["nodes"] = [ @@ -604,6 +640,48 @@ def test_flamegraph_large_frame_metadata_is_not_duplicated(cached_flamegraph) -> assert processed["_symbolication"]["status"] == "completed" +@django_db_all +@pytest.mark.parametrize("node_limit", [0, 1]) +def test_flamegraph_node_limit_preserves_original(cached_flamegraph, node_limit) -> None: + data, payload, symbolicator = cached_flamegraph + original = deepcopy(data) + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", node_limit), + ): + assert process_native_stacktraces(symbolicator, data) is None + assert data == original + assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( + payload + ) + + +@django_db_all +def test_flamegraph_node_limit_does_not_block_event_symbolication(cached_flamegraph) -> None: + data, payload, symbolicator = cached_flamegraph + data["platform"] = "native" + data["exception"] = {"values": [deepcopy(NATIVE_EXCEPTION)]} + symbolicator.process_payload.side_effect = [ + symbolicator.process_payload.return_value, + { + "status": "completed", + "modules": [], + "stacktraces": [ + {"frames": [{"original_index": 0, "function": "main", "status": "symbolicated"}]} + ], + }, + ] + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", 1), + ): + assert process_native_stacktraces(symbolicator, data) is data + assert data["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] == "main" + assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( + payload + ) + + @django_db_all def test_flamegraph_oversized_rewrite_preserves_original(cached_flamegraph) -> None: data, payload, symbolicator = cached_flamegraph From e2e27d904e7690acd3f48d79a16fa13794ef5259 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Sat, 24 Oct 2026 13:54:55 -0700 Subject: [PATCH 2/4] fix(native): Type inline expansion boundary test nodes --- tests/sentry/lang/native/test_processing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/lang/native/test_processing.py b/tests/sentry/lang/native/test_processing.py index 9566d7f96dcb..e408a1fb9f29 100644 --- a/tests/sentry/lang/native/test_processing.py +++ b/tests/sentry/lang/native/test_processing.py @@ -297,7 +297,7 @@ def test_flamegraph_indexed_inline_expansion(flamegraph_payload, nodes) -> None: def test_flamegraph_inline_expansion_at_node_limit() -> None: - nodes = [ + nodes: list[dict[str, Any]] = [ {"frame_id": 0, "sample_count": 10, "children": [1]}, {"frame_id": 0, "sample_count": 10}, ] From 6185f38f90ce4ea55999a02e4484797c6ecda7d0 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Sat, 24 Oct 2026 14:10:53 -0700 Subject: [PATCH 3/4] fix(native): Isolate flamegraph failures from crash symbolication --- src/sentry/lang/native/flamegraphs.py | 56 +++-- src/sentry/lang/native/processing.py | 104 ++++++--- src/sentry/tasks/store.py | 2 +- src/sentry/tasks/symbolication.py | 4 +- tests/sentry/lang/native/test_processing.py | 220 ++++++++++++++++++-- 5 files changed, 318 insertions(+), 68 deletions(-) diff --git a/src/sentry/lang/native/flamegraphs.py b/src/sentry/lang/native/flamegraphs.py index 6e90714de0fc..7278020bc83b 100644 --- a/src/sentry/lang/native/flamegraphs.py +++ b/src/sentry/lang/native/flamegraphs.py @@ -2,17 +2,18 @@ from typing import Any import rapidjson +import sentry_sdk from rest_framework import serializers from sentry import features from sentry.attachments import ( CachedAttachment, - MissingAttachmentChunks, get_attachments_for_event, store_attachments_for_event, ) from sentry.ingest.consumer import CACHE_TIMEOUT from sentry.models.project import Project +from sentry.utils import metrics # See: https://develop.sentry.dev/sdk/telemetry/attachments/#flame-graph-validation-and-limits MAX_FLAMEGRAPH_SIZE = 10 * 1024 * 1024 @@ -144,34 +145,57 @@ def __init__(self, project: Project, data: Any): self._attachments = list(get_attachments_for_event(data)) def get_flamegraphs(self) -> Generator[tuple[int, dict[str, Any]]]: - if not any(attachment.type == "event.flamegraph" for attachment in self._attachments): - return + # Imported at invocation time to avoid the symbolication task's processing import cycle. + from sentry.tasks.symbolication import SymbolicationTimeout + if not features.has("organizations:flamegraph-attachments", self._project.organization): return for index, attachment in enumerate(self._attachments): if attachment.type != "event.flamegraph": continue + reason = "load_error" try: raw = attachment.load_data(self._project) - except (MissingAttachmentChunks, FileNotFoundError): - continue - if len(raw) > MAX_FLAMEGRAPH_SIZE: - continue - try: + if len(raw) > MAX_FLAMEGRAPH_SIZE: + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "raw_size"}) + continue + reason = "invalid_json" payload = rapidjson.loads(raw, allow_nan=False) - except (rapidjson.JSONDecodeError, UnicodeDecodeError): - continue - if not isinstance(payload, dict): - continue - state = payload.get("_symbolication") - if isinstance(state, dict) and state.get("status") == "completed": + if not isinstance(payload, dict): + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "invalid_schema"} + ) + continue + state = payload.get("_symbolication") + if isinstance(state, dict) and state.get("status") == "completed": + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "already_completed"} + ) + continue + nodes = payload.get("nodes") + if isinstance(nodes, list) and len(nodes) > MAX_EXPANDED_NODES: + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "node_limit"}) + continue + reason = "invalid_schema" + if not _is_valid_flamegraph(payload): + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "invalid_schema"} + ) + continue + except SymbolicationTimeout as error: + sentry_sdk.capture_exception(error) + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "timeout"}) + return + except Exception as error: + sentry_sdk.capture_exception(error) + metrics.incr("process.native.flamegraph.skipped", tags={"reason": reason}) continue - if _is_valid_flamegraph(payload): - yield index, payload + yield index, payload def save(self, attachment_index: int, payload: dict[str, Any]) -> bool: serialized = rapidjson.dumps(payload, ensure_ascii=False, allow_nan=False).encode() if len(serialized) > MAX_FLAMEGRAPH_SIZE: + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "rewrite_size"}) return False attachment = self._attachments[attachment_index] metadata = attachment.meta() diff --git a/src/sentry/lang/native/processing.py b/src/sentry/lang/native/processing.py index 63c3aedc612d..5af9321b0b24 100644 --- a/src/sentry/lang/native/processing.py +++ b/src/sentry/lang/native/processing.py @@ -6,11 +6,10 @@ from typing import Any import sentry_sdk -from requests.exceptions import RequestException from symbolic.debuginfo import normalize_debug_id from symbolic.exceptions import ParseDebugIdError -from sentry import options +from sentry import features, options from sentry.lang.native.error import SymbolicationFailed, write_error from sentry.lang.native.flamegraphs import ( MAX_SYMBOLICATION_FRAMES, @@ -33,6 +32,7 @@ signal_from_data, ) from sentry.models.eventerror import EventErrorType +from sentry.models.project import Project from sentry.options.rollout import in_random_rollout from sentry.stacktraces.functions import trim_function_name from sentry.stacktraces.processing import StacktraceInfo, find_stacktraces_in_data @@ -527,33 +527,69 @@ def _merge_flamegraph_frames( def process_native_stacktraces(symbolicator: Symbolicator, data: Any) -> Any: - flamegraphs = Flamegraphs(symbolicator.project, data) + event_changed = _process_native_event_stacktraces(symbolicator, data) + attachments_changed = _process_flamegraphs(symbolicator, data) + return data if event_changed or attachments_changed else None + + +def _process_flamegraphs(symbolicator: Symbolicator, data: Any) -> bool: + # Imported at invocation time to avoid the symbolication task's processing import cycle. + from sentry.tasks.symbolication import SymbolicationTimeout + attachments_changed = False - for attachment_index, payload in flamegraphs.get_flamegraphs(): - modules, frame_ids, stacktraces = _get_flamegraph_stacktraces(payload) - if not stacktraces: - continue - try: - response = symbolicator.process_payload( - platform=payload["platform"], - stacktraces=stacktraces, - modules=modules, - frame_order=FrameOrder.caller_first, - apply_source_context=False, - ) - except (OSError, RequestException) as error: - sentry_sdk.capture_exception(error) - continue - if not response or response.get("status") != "completed": - continue - resolved_stacktraces = response.get("stacktraces", []) - if len(resolved_stacktraces) != len(frame_ids): - continue - try: - output = _merge_flamegraph_frames(payload, frame_ids, resolved_stacktraces) - except FlamegraphExpansionLimitExceeded: - continue - attachments_changed |= flamegraphs.save(attachment_index, output) + try: + flamegraphs = Flamegraphs(symbolicator.project, data) + for attachment_index, payload in flamegraphs.get_flamegraphs(): + reason = "processing_error" + try: + modules, frame_ids, stacktraces = _get_flamegraph_stacktraces(payload) + if not stacktraces: + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "no_stacktraces"} + ) + continue + reason = "request_error" + response = symbolicator.process_payload( + platform=payload["platform"], + stacktraces=stacktraces, + modules=modules, + frame_order=FrameOrder.caller_first, + apply_source_context=False, + ) + if not response or response.get("status") != "completed": + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "request_error"} + ) + continue + resolved_stacktraces = response.get("stacktraces", []) + if len(resolved_stacktraces) != len(frame_ids): + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "invalid_response"} + ) + continue + reason = "merge_error" + output = _merge_flamegraph_frames(payload, frame_ids, resolved_stacktraces) + reason = "save_error" + attachments_changed |= flamegraphs.save(attachment_index, output) + except SymbolicationTimeout as error: + sentry_sdk.capture_exception(error) + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "timeout"}) + break + except FlamegraphExpansionLimitExceeded as error: + sentry_sdk.capture_exception(error) + metrics.incr( + "process.native.flamegraph.skipped", tags={"reason": "expansion_limit"} + ) + except Exception as error: + sentry_sdk.capture_exception(error) + metrics.incr("process.native.flamegraph.skipped", tags={"reason": reason}) + except Exception as error: + sentry_sdk.capture_exception(error) + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "processing_error"}) + return attachments_changed + + +def _process_native_event_stacktraces(symbolicator: Symbolicator, data: Any) -> Any: stacktrace_infos = [ stacktrace for stacktrace in find_stacktraces_in_data(data) @@ -576,7 +612,7 @@ def process_native_stacktraces(symbolicator: Symbolicator, data: Any) -> Any: ] if not any(stacktrace["frames"] for stacktrace in stacktraces): - return data if attachments_changed else None + return None signal = signal_from_data(data) @@ -730,7 +766,7 @@ def emit_apple_symbol_stats(apple_symbol_stats, data): def get_native_symbolication_functions( - data: Mapping[str, Any], stacktraces: list[StacktraceInfo] + data: Mapping[str, Any], stacktraces: list[StacktraceInfo], project: Project ) -> list[SymbolicatorFunction]: """ Yields the symbolication functions that will process the event, @@ -745,8 +781,12 @@ def get_native_symbolication_functions( functions.append(SymbolicatorFunction.native) return functions # early return to prevent duplicates - if has_native_stacktraces(stacktraces) or any( - attachment.get("type") == "event.flamegraph" for attachment in data.get("_attachments", []) + if has_native_stacktraces(stacktraces) or ( + features.has("organizations:flamegraph-attachments", project.organization) + and any( + attachment.get("type") == "event.flamegraph" + for attachment in data.get("_attachments", []) + ) ): # Run native symbolication first, to prevent sending minidump / ACR events to # symbolicator twice (after symbolication a minidump event will have native stacktraces). diff --git a/src/sentry/tasks/store.py b/src/sentry/tasks/store.py index 9b28cb4e0e67..cc86297bc35d 100644 --- a/src/sentry/tasks/store.py +++ b/src/sentry/tasks/store.py @@ -223,7 +223,7 @@ def _do_preprocess_event( # The event will be submitted to Symbolicator for all returned platforms, # one after the other, so we handle mixed stacktraces. stacktraces = find_stacktraces_in_data(data) - symbolicate_functions = get_symbolication_functions(data, stacktraces) + symbolicate_functions = get_symbolication_functions(data, stacktraces, project) metrics.incr( "events.to-symbolicate", tags={f.value: True for f in symbolicate_functions}, diff --git a/src/sentry/tasks/symbolication.py b/src/sentry/tasks/symbolication.py index e21238dabad3..4b2063379235 100644 --- a/src/sentry/tasks/symbolication.py +++ b/src/sentry/tasks/symbolication.py @@ -41,7 +41,7 @@ def get_symbolication_functions( - data: Mapping[str, Any], stacktraces: list[StacktraceInfo] + data: Mapping[str, Any], stacktraces: list[StacktraceInfo], project: Project ) -> list[SymbolicatorFunction]: """Returns a list of Symbolicator platforms that apply to this event.""" @@ -56,7 +56,7 @@ def get_symbolication_functions( if is_js_event(data, stacktraces): functions.append(SymbolicatorFunction.js) - functions.extend(get_native_symbolication_functions(data, stacktraces)) + functions.extend(get_native_symbolication_functions(data, stacktraces, project)) return functions diff --git a/tests/sentry/lang/native/test_processing.py b/tests/sentry/lang/native/test_processing.py index e408a1fb9f29..bd400dce4db9 100644 --- a/tests/sentry/lang/native/test_processing.py +++ b/tests/sentry/lang/native/test_processing.py @@ -39,6 +39,8 @@ from sentry.models.eventerror import EventErrorType from sentry.objectstore import UsecaseId, get_session from sentry.stacktraces.processing import find_stacktraces_in_data +from sentry.tasks import store +from sentry.tasks.symbolication import SymbolicationTimeout, symbolicate_event from sentry.testutils.helpers.features import Feature from sentry.testutils.pytest.fixtures import django_db_all from sentry.testutils.skips import requires_objectstore @@ -62,6 +64,7 @@ } +@django_db_all @pytest.mark.parametrize( ("exceptions", "expected_functions"), [ @@ -98,7 +101,9 @@ ], ) def test_get_native_symbolication_functions( - exceptions: list[dict[str, Any]], expected_functions: list[SymbolicatorFunction] + default_project, + exceptions: list[dict[str, Any]], + expected_functions: list[SymbolicatorFunction], ) -> None: # Relay sets the platform of minidump/applecrashreport events to "native", # so a native platform alone must not schedule a `native` symbolication run. @@ -109,7 +114,7 @@ def test_get_native_symbolication_functions( } stacktraces = find_stacktraces_in_data(data) - functions = list(get_native_symbolication_functions(data, stacktraces)) + functions = list(get_native_symbolication_functions(data, stacktraces, default_project)) assert functions == expected_functions @@ -236,9 +241,28 @@ def test_flamegraph_symbolication_is_idempotent(cached_flamegraph) -> None: assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == before -def test_route_flamegraph_to_native_symbolication() -> None: +@django_db_all +@pytest.mark.parametrize( + ("enabled", "expected"), [(False, []), (True, [SymbolicatorFunction.native])] +) +def test_route_flamegraph_to_native_symbolication(default_project, enabled, expected) -> None: data = {"platform": "other", "_attachments": [{"type": "event.flamegraph"}]} - assert get_native_symbolication_functions(data, []) == [SymbolicatorFunction.native] + with Feature({"organizations:flamegraph-attachments": enabled}): + assert get_native_symbolication_functions(data, [], default_project) == expected + + +@django_db_all +@pytest.mark.parametrize("enabled", [False, True]) +def test_route_native_stacktraces_with_flamegraph_flag(default_project, enabled) -> None: + data = { + "platform": "native", + "exception": {"values": [deepcopy(NATIVE_EXCEPTION)]}, + "_attachments": [{"type": "event.flamegraph"}], + } + with Feature({"organizations:flamegraph-attachments": enabled}): + assert get_native_symbolication_functions( + data, find_stacktraces_in_data(data), default_project + ) == [SymbolicatorFunction.native] @pytest.fixture @@ -457,6 +481,141 @@ def cached_flamegraph(default_project, flamegraph_payload): return data, payload, symbolicator +@pytest.fixture +def native_event_with_flamegraph(cached_flamegraph): + data, payload, symbolicator = cached_flamegraph + data["platform"] = "native" + data["exception"] = {"values": [deepcopy(NATIVE_EXCEPTION)]} + symbolicator.process_payload.side_effect = [ + { + "status": "completed", + "modules": [], + "stacktraces": [ + {"frames": [{"original_index": 0, "function": "main", "status": "symbolicated"}]} + ], + }, + symbolicator.process_payload.return_value, + ] + return data, payload, symbolicator + + +@django_db_all +@pytest.mark.parametrize( + ("target", "reason"), + [ + ("sentry.attachments.CachedAttachment.load_data", "load_error"), + ("sentry.lang.native.flamegraphs._is_valid_flamegraph", "invalid_schema"), + ("sentry.lang.native.processing._get_flamegraph_stacktraces", "processing_error"), + ("sentry.lang.native.processing._merge_flamegraph_frames", "merge_error"), + ("sentry.lang.native.flamegraphs.Flamegraphs.save", "save_error"), + ], +) +def test_flamegraph_exception_does_not_mark_event_fatal( + native_event_with_flamegraph, target, reason +): + data, _, symbolicator = native_event_with_flamegraph + error = RuntimeError("attachment failure") + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch(target, side_effect=error), + mock.patch("sentry.tasks.symbolication.Symbolicator", return_value=symbolicator), + mock.patch.object(store, "submit_process") as submit, + mock.patch("sentry_sdk.capture_exception") as capture, + mock.patch("sentry.utils.metrics.incr") as incr, + ): + symbolicate_event(data=data) + processed = submit.call_args.kwargs["data"] + assert processed["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] == "main" + assert not processed.get("_metrics", {}).get("flag.processing.fatal", False) + capture.assert_any_call(error) + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": reason}) + + +@django_db_all +def test_flamegraph_timeout_stops_remaining_attachments(multiple_flamegraphs): + data, _, _, symbolicator = multiple_flamegraphs + data["platform"] = "native" + data["exception"] = {"values": [deepcopy(NATIVE_EXCEPTION)]} + error = SymbolicationTimeout() + symbolicator.process_payload.side_effect = [ + { + "status": "completed", + "modules": [], + "stacktraces": [ + {"frames": [{"original_index": 0, "function": "main", "status": "symbolicated"}]} + ], + }, + error, + ] + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.tasks.symbolication.Symbolicator", return_value=symbolicator), + mock.patch.object(store, "submit_process") as submit, + mock.patch("sentry_sdk.capture_exception") as capture, + mock.patch("sentry.utils.metrics.incr") as incr, + ): + symbolicate_event(data=data) + processed = submit.call_args.kwargs["data"] + assert processed["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] == "main" + assert not processed.get("_metrics", {}).get("flag.processing.fatal", False) + assert symbolicator.process_payload.call_count == 2 + capture.assert_any_call(error) + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": "timeout"}) + + +@django_db_all +@pytest.mark.parametrize("response", [None, {"status": "failed"}]) +def test_flamegraph_processed_after_failed_event_response(native_event_with_flamegraph, response): + data, _, symbolicator = native_event_with_flamegraph + symbolicator.process_payload.side_effect = [response, symbolicator.process_payload.return_value] + with Feature({"organizations:flamegraph-attachments": True}): + assert process_native_stacktraces(symbolicator, data) is data + processed = orjson.loads(next(get_attachments_for_event(data)).load_data(symbolicator.project)) + assert processed["_symbolication"]["status"] == "completed" + assert processed["frames"][0]["function"] == "outer" + assert data["errors"] + + +@django_db_all +def test_flamegraph_load_error_does_not_skip_later_attachments(multiple_flamegraphs): + data, first, _, symbolicator = multiple_flamegraphs + original_load = CachedAttachment.load_data + + def load(attachment, project): + if attachment.id == 0: + raise RuntimeError("attachment unavailable") + return original_load(attachment, project) + + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch.object(CachedAttachment, "load_data", load), + ): + assert process_native_stacktraces(symbolicator, data) is data + first_attachment, _, second_attachment = get_attachments_for_event(data) + assert first_attachment.load_data(symbolicator.project) == orjson.dumps(first) + processed = orjson.loads(second_attachment.load_data(symbolicator.project)) + assert processed["frames"][0]["function"] == "outer" + assert processed["_symbolication"]["status"] == "completed" + + +@django_db_all +def test_flamegraph_node_limit_precedes_validation(cached_flamegraph): + data, payload, symbolicator = cached_flamegraph + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", 0), + mock.patch("sentry.lang.native.flamegraphs._is_valid_flamegraph") as validate, + mock.patch("sentry.utils.metrics.incr") as incr, + ): + assert process_native_stacktraces(symbolicator, data) is None + validate.assert_not_called() + symbolicator.process_payload.assert_not_called() + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": "node_limit"}) + assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( + payload + ) + + @pytest.mark.parametrize( ("field", "value"), [("data", "invalid"), ("data", None), ("addr_mode", 1), ("platform", [])], @@ -501,7 +660,7 @@ def test_flamegraph_inline_frames_do_not_inherit_captured_metadata(flamegraph_pa @django_db_all -@pytest.mark.parametrize("error", [TimeoutError(), Timeout(), ConnectionError()]) +@pytest.mark.parametrize("error", [TimeoutError(), Timeout(), ConnectionError(), RuntimeError()]) def test_flamegraph_transport_error_does_not_block_event_symbolication( cached_flamegraph, error ) -> None: @@ -509,7 +668,6 @@ def test_flamegraph_transport_error_does_not_block_event_symbolication( data["platform"] = "native" data["exception"] = {"values": [deepcopy(NATIVE_EXCEPTION)]} symbolicator.process_payload.side_effect = [ - error, { "status": "completed", "modules": [], @@ -517,9 +675,14 @@ def test_flamegraph_transport_error_does_not_block_event_symbolication( {"frames": [{"original_index": 0, "function": "main", "status": "symbolicated"}]} ], }, + error, ] - with Feature({"organizations:flamegraph-attachments": True}): + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.utils.metrics.incr") as incr, + ): assert process_native_stacktraces(symbolicator, data) is data + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": "request_error"}) assert symbolicator.process_payload.call_count == 2 assert data["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] == "main" assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( @@ -641,15 +804,17 @@ def test_flamegraph_large_frame_metadata_is_not_duplicated(cached_flamegraph) -> @django_db_all -@pytest.mark.parametrize("node_limit", [0, 1]) -def test_flamegraph_node_limit_preserves_original(cached_flamegraph, node_limit) -> None: +@pytest.mark.parametrize(("node_limit", "reason"), [(0, "node_limit"), (1, "expansion_limit")]) +def test_flamegraph_node_limit_preserves_original(cached_flamegraph, node_limit, reason) -> None: data, payload, symbolicator = cached_flamegraph original = deepcopy(data) with ( Feature({"organizations:flamegraph-attachments": True}), mock.patch("sentry.lang.native.flamegraphs.MAX_EXPANDED_NODES", node_limit), + mock.patch("sentry.utils.metrics.incr") as incr, ): assert process_native_stacktraces(symbolicator, data) is None + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": reason}) assert data == original assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( payload @@ -662,7 +827,6 @@ def test_flamegraph_node_limit_does_not_block_event_symbolication(cached_flamegr data["platform"] = "native" data["exception"] = {"values": [deepcopy(NATIVE_EXCEPTION)]} symbolicator.process_payload.side_effect = [ - symbolicator.process_payload.return_value, { "status": "completed", "modules": [], @@ -670,6 +834,7 @@ def test_flamegraph_node_limit_does_not_block_event_symbolication(cached_flamegr {"frames": [{"original_index": 0, "function": "main", "status": "symbolicated"}]} ], }, + symbolicator.process_payload.return_value, ] with ( Feature({"organizations:flamegraph-attachments": True}), @@ -689,8 +854,10 @@ def test_flamegraph_oversized_rewrite_preserves_original(cached_flamegraph) -> N with ( Feature({"organizations:flamegraph-attachments": True}), mock.patch("sentry.lang.native.flamegraphs.MAX_FLAMEGRAPH_SIZE", len(raw)), + mock.patch("sentry.utils.metrics.incr") as incr, ): assert process_native_stacktraces(symbolicator, data) is None + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": "rewrite_size"}) symbolicator.process_payload.assert_called_once() assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == raw @@ -763,14 +930,22 @@ def test_flamegraph_frame_limit(cached_flamegraph) -> None: @django_db_all @pytest.mark.parametrize( - "response", - [{"status": "failed"}, {"status": "pending"}, {"status": "completed", "stacktraces": []}], + ("response", "reason"), + [ + ({"status": "failed"}, "request_error"), + ({"status": "pending"}, "request_error"), + ({"status": "completed", "stacktraces": []}, "invalid_response"), + ], ) -def test_flamegraph_failure_preserves_attachment(cached_flamegraph, response) -> None: +def test_flamegraph_failure_preserves_attachment(cached_flamegraph, response, reason) -> None: data, payload, symbolicator = cached_flamegraph symbolicator.process_payload.return_value = response - with Feature({"organizations:flamegraph-attachments": True}): + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.utils.metrics.incr") as incr, + ): assert process_native_stacktraces(symbolicator, data) is None + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": reason}) assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == orjson.dumps( payload ) @@ -778,9 +953,16 @@ def test_flamegraph_failure_preserves_attachment(cached_flamegraph, response) -> @django_db_all @pytest.mark.parametrize( - "raw", [b"invalid json", b"\xff", b"null", b'{"version":"2"}', b" " * (10 * 1024 * 1024 + 1)] + ("raw", "reason"), + [ + (b"invalid json", "invalid_json"), + (b"\xff", "invalid_json"), + (b"null", "invalid_schema"), + (b'{"version":"2"}', "invalid_schema"), + (b" " * (10 * 1024 * 1024 + 1), "raw_size"), + ], ) -def test_flamegraph_invalid_attachment(cached_flamegraph, raw) -> None: +def test_flamegraph_invalid_attachment(cached_flamegraph, raw, reason) -> None: data, _, symbolicator = cached_flamegraph store_attachments_for_event( symbolicator.project, @@ -793,8 +975,12 @@ def test_flamegraph_invalid_attachment(cached_flamegraph, raw) -> None: ], timeout=60, ) - with Feature({"organizations:flamegraph-attachments": True}): + with ( + Feature({"organizations:flamegraph-attachments": True}), + mock.patch("sentry.utils.metrics.incr") as incr, + ): assert process_native_stacktraces(symbolicator, data) is None + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": reason}) symbolicator.process_payload.assert_not_called() assert next(get_attachments_for_event(data)).load_data(symbolicator.project) == raw From a35859ed7c167dcfa04f61c5f2813dc5cc62a7e1 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Sat, 24 Oct 2026 14:17:40 -0700 Subject: [PATCH 4/4] perf(native): Skip flag checks without flamegraph attachments --- src/sentry/lang/native/flamegraphs.py | 16 ++++++------- src/sentry/lang/native/processing.py | 4 ++-- tests/sentry/lang/native/test_processing.py | 25 +++++++++++++++++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/sentry/lang/native/flamegraphs.py b/src/sentry/lang/native/flamegraphs.py index 7278020bc83b..e78efcb471b4 100644 --- a/src/sentry/lang/native/flamegraphs.py +++ b/src/sentry/lang/native/flamegraphs.py @@ -8,6 +8,7 @@ from sentry import features from sentry.attachments import ( CachedAttachment, + MissingAttachmentChunks, get_attachments_for_event, store_attachments_for_event, ) @@ -145,9 +146,8 @@ def __init__(self, project: Project, data: Any): self._attachments = list(get_attachments_for_event(data)) def get_flamegraphs(self) -> Generator[tuple[int, dict[str, Any]]]: - # Imported at invocation time to avoid the symbolication task's processing import cycle. - from sentry.tasks.symbolication import SymbolicationTimeout - + if not any(attachment.type == "event.flamegraph" for attachment in self._attachments): + return if not features.has("organizations:flamegraph-attachments", self._project.organization): return for index, attachment in enumerate(self._attachments): @@ -155,7 +155,11 @@ def get_flamegraphs(self) -> Generator[tuple[int, dict[str, Any]]]: continue reason = "load_error" try: - raw = attachment.load_data(self._project) + try: + raw = attachment.load_data(self._project) + except (MissingAttachmentChunks, FileNotFoundError): + metrics.incr("process.native.flamegraph.skipped", tags={"reason": "load_error"}) + continue if len(raw) > MAX_FLAMEGRAPH_SIZE: metrics.incr("process.native.flamegraph.skipped", tags={"reason": "raw_size"}) continue @@ -182,10 +186,6 @@ def get_flamegraphs(self) -> Generator[tuple[int, dict[str, Any]]]: "process.native.flamegraph.skipped", tags={"reason": "invalid_schema"} ) continue - except SymbolicationTimeout as error: - sentry_sdk.capture_exception(error) - metrics.incr("process.native.flamegraph.skipped", tags={"reason": "timeout"}) - return except Exception as error: sentry_sdk.capture_exception(error) metrics.incr("process.native.flamegraph.skipped", tags={"reason": reason}) diff --git a/src/sentry/lang/native/processing.py b/src/sentry/lang/native/processing.py index 5af9321b0b24..680aea8c346a 100644 --- a/src/sentry/lang/native/processing.py +++ b/src/sentry/lang/native/processing.py @@ -782,11 +782,11 @@ def get_native_symbolication_functions( return functions # early return to prevent duplicates if has_native_stacktraces(stacktraces) or ( - features.has("organizations:flamegraph-attachments", project.organization) - and any( + any( attachment.get("type") == "event.flamegraph" for attachment in data.get("_attachments", []) ) + and features.has("organizations:flamegraph-attachments", project.organization) ): # Run native symbolication first, to prevent sending minidump / ACR events to # symbolicator twice (after symbolication a minidump event will have native stacktraces). diff --git a/tests/sentry/lang/native/test_processing.py b/tests/sentry/lang/native/test_processing.py index bd400dce4db9..90885f42c12b 100644 --- a/tests/sentry/lang/native/test_processing.py +++ b/tests/sentry/lang/native/test_processing.py @@ -23,6 +23,7 @@ ) from sentry.lang.native.flamegraphs import ( FlamegraphExpansionLimitExceeded, + Flamegraphs, _is_valid_flamegraph, expand_inline_frames, ) @@ -251,6 +252,26 @@ def test_route_flamegraph_to_native_symbolication(default_project, enabled, expe assert get_native_symbolication_functions(data, [], default_project) == expected +@django_db_all +@pytest.mark.parametrize("attachments", [[], [{"type": "event.attachment"}]]) +def test_route_without_flamegraph_skips_feature_check(default_project, attachments) -> None: + data = {"platform": "python", "_attachments": attachments} + with mock.patch("sentry.features.has", return_value=True) as has: + assert get_native_symbolication_functions(data, [], default_project) == [] + has.assert_not_called() + + +@django_db_all +@pytest.mark.parametrize("attachments", [[], [{"type": "event.attachment"}]]) +def test_get_flamegraphs_without_flamegraph_skips_feature_check( + default_project, attachments +) -> None: + data = {"platform": "native", "_attachments": attachments} + with mock.patch("sentry.features.has", return_value=True) as has: + assert list(Flamegraphs(default_project, data).get_flamegraphs()) == [] + has.assert_not_called() + + @django_db_all @pytest.mark.parametrize("enabled", [False, True]) def test_route_native_stacktraces_with_flamegraph_flag(default_project, enabled) -> None: @@ -708,9 +729,13 @@ def test_flamegraph_unavailable_data_does_not_block_event_symbolication( with ( Feature({"organizations:flamegraph-attachments": True}), mock.patch.object(CachedAttachment, "load_data", side_effect=error), + mock.patch("sentry_sdk.capture_exception") as capture, + mock.patch("sentry.utils.metrics.incr") as incr, ): assert process_native_stacktraces(symbolicator, data) is data assert data["exception"]["values"][0]["stacktrace"]["frames"][0]["function"] == "main" + capture.assert_not_called() + incr.assert_any_call("process.native.flamegraph.skipped", tags={"reason": "load_error"}) @django_db_all