-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(feedback): add a tag for whether spam detection was applied #97452
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #97452 +/- ##
=======================================
Coverage 80.58% 80.58%
=======================================
Files 8557 8557
Lines 376461 376462 +1
Branches 24468 24468
=======================================
+ Hits 303369 303382 +13
+ Misses 72722 72710 -12
Partials 370 370 |
event_fixed["tags"]["has_spam_filter"] = "true" if is_message_spam is not None else "false" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, the is_mesage_spam
value is currently also added to evidenceData on line ~70 in this same file:
sentry/src/sentry/feedback/usecases/ingest/create_feedback.py
Lines 70 to 74 in ccd6841
if is_message_spam is True: | |
evidence_data["is_spam"] = is_message_spam # Used by alerts post process. | |
evidence_display.append( | |
IssueEvidence(name="is_spam", value=str(is_message_spam), important=False) | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh, i think we could do better if we unconditionally recorded whether spam detection was even executed. Projects have a setting where that can be disabled, so for each feedback we could say "spam detected: disabled" OR "spam detection: enabled, is_spam: Nope"
and put the pair into context, instead of tags
@@ -380,6 +380,8 @@ def create_feedback_issue( | |||
if event_fixed.get("release"): | |||
event_fixed["tags"]["release"] = event_fixed["release"] | |||
|
|||
event_fixed["tags"]["has_spam_filter"] = "true" if is_message_spam is not None else "false" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please prefix sentry tags with sentry.
Named it
has_spam_filter
so it'll appear underhas_linked_error
- another tag we attach.Relates to REPLAY-602: [User Feedback] Unhelpful feedback and profanity should be detected as spam