Skip to content

Commit

Permalink
Revert "feat(save_event): Always use cache for stacktrace processing (#…
Browse files Browse the repository at this point in the history
…56413)"

This reverts commit 8bc7aec.

Co-authored-by: lynnagara <1779792+lynnagara@users.noreply.github.com>
  • Loading branch information
getsentry-bot and lynnagara committed Sep 19, 2023
1 parent dc8c226 commit bb386fa
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 10 deletions.
4 changes: 4 additions & 0 deletions src/sentry/event_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2412,10 +2412,14 @@ def _calculate_event_grouping(
Main entrypoint for modifying/enhancing and grouping an event, writes
hashes back into event payload.
"""
load_stacktrace_from_cache = bool(event.org_can_load_stacktrace_from_cache)
metric_tags: MutableTags = {
"grouping_config": grouping_config["id"],
"platform": event.platform or "unknown",
"loading_from_cache": load_stacktrace_from_cache,
}
# This will help us differentiate when a transaction uses caching vs not
sentry_sdk.set_tag("stacktrace.loaded_from_cache", load_stacktrace_from_cache)

with metrics.timer("event_manager.normalize_stacktraces_for_grouping", tags=metric_tags):
with sentry_sdk.start_span(op="event_manager.normalize_stacktraces_for_grouping"):
Expand Down
12 changes: 10 additions & 2 deletions src/sentry/eventstore/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from django.conf import settings
from django.utils.encoding import force_str

from sentry import eventtypes
from sentry import eventtypes, features
from sentry.db.models import NodeData
from sentry.grouping.result import CalculatedHashes
from sentry.interfaces.base import Interface, get_interfaces
Expand Down Expand Up @@ -419,7 +419,11 @@ def normalize_stacktraces_for_grouping(self, grouping_config) -> None:
"""
from sentry.stacktraces.processing import normalize_stacktraces_for_grouping

normalize_stacktraces_for_grouping(self.data, grouping_config)
normalize_stacktraces_for_grouping(
self.data,
grouping_config,
load_stacktrace_from_cache=self.org_can_load_stacktrace_from_cache,
)

# We have modified event data, so any cached interfaces have to be reset:
self.__dict__.pop("interfaces", None)
Expand Down Expand Up @@ -487,6 +491,10 @@ def get_span_groupings(
def organization(self) -> Organization:
return self.project.organization

@property
def org_can_load_stacktrace_from_cache(self) -> bool:
return features.has("organizations:stacktrace-processing-caching", self.organization)

@property
def version(self) -> str:
return cast(str, self.data.get("version", "5"))
Expand Down
27 changes: 21 additions & 6 deletions src/sentry/grouping/enhancer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def apply_modifications_to_frame(
platform: str,
exception_data: dict[str, Any],
extra_fingerprint: str = "",
load_stacktrace_from_cache: bool = False,
) -> None:
"""This applies the frame modifications to the frames itself. This does not affect grouping."""
in_memory_cache: dict[str, str] = {}
Expand All @@ -157,7 +158,12 @@ def apply_modifications_to_frame(
cache_key = f"stacktrace_hash.{stacktrace_fingerprint}"
use_cache = bool(stacktrace_fingerprint)
if use_cache:
frames_changed = _update_frames_from_cached_values(frames, cache_key, platform)
frames_changed = _update_frames_from_cached_values(
frames,
cache_key,
platform,
load_from_cache=load_stacktrace_from_cache,
)
if frames_changed:
logger.info("The frames have been loaded from the cache. Skipping some work.")
return
Expand Down Expand Up @@ -497,21 +503,26 @@ def visit_quoted_ident(self, node, children):


def _update_frames_from_cached_values(
frames: Sequence[dict[str, Any]], cache_key: str, platform: str
frames: Sequence[dict[str, Any]], cache_key: str, platform: str, load_from_cache: bool = False
) -> bool:
"""
This will update the frames of the stacktrace if it's been cached.
Returns True if the merged has correctly happened.
Set load_from_cache to True to actually change the frames.
Returns if the merged has correctly happened.
"""
frames_changed = False
changed_frames_values = cache.get(cache_key, {})

# This helps tracking changes in the hit/miss ratio of the cache
metrics.incr(
f"{DATADOG_KEY}.cache.get",
tags={"success": bool(changed_frames_values), "platform": platform},
tags={
"success": bool(changed_frames_values),
"platform": platform,
"loading_from_cache": load_from_cache,
},
)
if changed_frames_values:
if changed_frames_values and load_from_cache:
try:
for frame, changed_frame_values in zip(frames, changed_frames_values):
if changed_frame_values.get("in_app") is not None:
Expand All @@ -534,7 +545,11 @@ def _update_frames_from_cached_values(

metrics.incr(
f"{DATADOG_KEY}.merged_cached_values",
tags={"success": frames_changed, "platform": platform},
tags={
"success": frames_changed,
"platform": platform,
"loading_from_cache": load_from_cache,
},
)
return frames_changed

Expand Down
8 changes: 6 additions & 2 deletions src/sentry/stacktraces/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ def _normalize_in_app(stacktrace: Sequence[dict[str, str]]) -> str:


def normalize_stacktraces_for_grouping(
data: MutableMapping[str, Any], grouping_config=None
data: MutableMapping[str, Any], grouping_config=None, load_stacktrace_from_cache: bool = False
) -> None:
"""
Applies grouping enhancement rules and ensure in_app is set on all frames.
Expand Down Expand Up @@ -341,7 +341,11 @@ def normalize_stacktraces_for_grouping(
for frames, stacktrace_container in zip(stacktrace_frames, stacktrace_containers):
# This call has a caching mechanism when the same stacktrace and rules are used
grouping_config.enhancements.apply_modifications_to_frame(
frames, platform, stacktrace_container, extra_fingerprint=grouping_config.id
frames,
platform,
stacktrace_container,
extra_fingerprint=grouping_config.id,
load_stacktrace_from_cache=load_stacktrace_from_cache,
)

# normalize `in_app` values, noting and storing the event's mix of in-app and system frames, so
Expand Down

0 comments on commit bb386fa

Please sign in to comment.