-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add Disabled Alerts section #7917
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/docs/product/alerts/index.mdx
Outdated
|
||
To ensure that your alerts are working properly without causing unnecessary noise, Sentry will proactively disable an alert from sending if we detect that the alert: | ||
|
||
- Is a duplicate of an existing alert rule. |
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.
For the first 2 bullet points, this is something that we block going forward, and disabled already created rules that matched the criteria in a migration. I don't think we should add docs for something we ran once - if instead we didn't block these from being created and continually ran migrations to disable them (which would be a silly idea) I could see adding this, but as it stands now I don't think these should be included.
The 3rd one I hesitate to agree should be listed because for one, we changed it to 5000, and for another, when we run it again we may change the time period and number of triggers. @rachrwang what do you think?
It's also untrue that anyone in the organization can re-enable an alert, they must have the correct permissions (or if that is true, it's a mistake and we should fix it).
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.
Good point... Are there any other conditions for disabling an alert? How's the following revision for the entire section?
"Sentry has disabled duplicate alerts and alerts with no actions. If a noisy alert is sent to a communication channel and receives no engagement for 30 days, Sentry will proactively disable it to reduce unnecessary noise.
Alert owners can re-enable an alert by editing its conditions and re-saving it. Alerts need to pass Sentry’s checks before they can be saved. See best practices for guidance on writing useful alerts."
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.
Can we be even more vague 😅 we might not always do the last 30 days.
Maybe we could link to the team permissions docs for the last part about who can re-enable, it's just anyone who has alert edit access.
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.
Ok, introduced additional vagueness and added the link. LMK if you or @rachrwang have any other suggestions!
fix links
Pre-merge checklist
If you work at Sentry, you're able to merge your own PR without review, but please don't unless there's a good reason.
Description of changes
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Extra resources