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

[actions][sentry] apply fingerprint rules to group issues better #455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gutsytechster
Copy link

@gutsytechster gutsytechster commented Sep 18, 2024

Sentry groups issues by the provided message argument in the implemented scenario. However, when the message includes a pattern recognizable by sentry defined here - https://github.com/getsentry/sentry/blob/master/src/sentry/grouping/parameterization.py#L56, values against these patterns are replaced with placeholders and aren't used to group issues.

E.g., if the sentry title contains hostnames with the same exception, the two would be grouped as below

Prashant Test Local | Development | Spider bookspider.com notification
Prashant Test Local | Development | Spider quotespider.com notification

grouped into

Prashant Test Local | Development | Spider <hostname> notification

This would be unintended behaviour, and we'd like to have the two spiders raise two different sentry alerts.

Providing spider's title to the fingerprint would ensure that the exact message is used to create fingerprints, and hence the two issues will have separate fingerprint values and eventually two different sentry issues.

…ues better

This allows for grouping issues by title, when it may contain pattern recognizable
via sentry and pattern values are ignored.
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.54%. Comparing base (d7d553a) to head (8450a84).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #455   +/-   ##
=======================================
  Coverage   79.54%   79.54%           
=======================================
  Files          76       76           
  Lines        3242     3242           
  Branches      539      539           
=======================================
  Hits         2579     2579           
  Misses        593      593           
  Partials       70       70           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@muzaffaryousaf
Copy link
Contributor

@gutsytechster were you able to test the changes ? Thanks.

@gutsytechster
Copy link
Author

gutsytechster commented Sep 19, 2024

Hi @muzaffaryousaf

Yes. Here's a sentry alert that grouped events of the different domain alike spiders together. This was previous to fixes.

Here's how it grouped these events
image

After fixes, the two separate events were triggered for these two spiders - bookspider.com and quotespider.com

Here's how it grouped these events.
image
image

Notice Fingerprint values in these images, where issue title is used to group on top of the {{default}} values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants