Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(save_event): Always use cache for stacktrace processing #56413

Merged
merged 2 commits into from
Sep 19, 2023

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Sep 18, 2023

We enabled it for EA customers last week without any issues. It reduces the stacktrace normalization by 40%. It's most noticeable with Native events.

We enabled it for EA customers last week without any issues.
It reduces the stacktrace normalization by 40%.
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 18, 2023
@armenzg armenzg self-assigned this Sep 18, 2023
@@ -2411,14 +2411,10 @@ 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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We remove the method in eventstore/models.

@@ -491,10 +487,6 @@ 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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature still shows up in src/sentry/features/__init__.py since it's still in use in getsentry. We can remove it once we remove it's use from getsentry.

)
if changed_frames_values and load_from_cache:
if changed_frames_values:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what enables it for everyone.

@armenzg armenzg marked this pull request as ready for review September 18, 2023 19:14
@armenzg armenzg requested a review from a team as a code owner September 18, 2023 19:14
@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

Merging #56413 (334fa4c) into master (f1361a8) will increase coverage by 0.00%.
Report is 22 commits behind head on master.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           master   #56413    +/-   ##
========================================
  Coverage   78.64%   78.64%            
========================================
  Files        5075     5076     +1     
  Lines      218067   218205   +138     
  Branches    36905    36934    +29     
========================================
+ Hits       171490   171615   +125     
- Misses      41044    41046     +2     
- Partials     5533     5544    +11     
Files Changed Coverage
src/sentry/stacktraces/processing.py ø
src/sentry/eventstore/models.py 100.00%
src/sentry/grouping/enhancer/__init__.py 100.00%

@armenzg armenzg enabled auto-merge (squash) September 19, 2023 15:08
@armenzg armenzg merged commit 8bc7aec into master Sep 19, 2023
50 checks passed
@armenzg armenzg deleted the enable-stacktrace-caching-for-all-orgs branch September 19, 2023 15:16
@lynnagara lynnagara added the Trigger: Revert add to a merged PR to revert it (skips CI) label Sep 19, 2023
@getsentry-bot
Copy link
Contributor

PR reverted: bb386fa

getsentry-bot added a commit that referenced this pull request Sep 19, 2023
@lynnagara
Copy link
Member

Revert due to large backlogs seen in ingest-attachments that align with this change.

armenzg added a commit that referenced this pull request Sep 20, 2023
We enabled it for EA customers last week without any issues. It reduces
the stacktrace normalization by 40%. It's most noticeable with Native
events.

A repeat of #56413 which got reverted by mistake.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Trigger: Revert add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants