-
Notifications
You must be signed in to change notification settings - Fork 212
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
Stabilize API HTTP alarms and remove thumbnails alarms runbooks #3523
Conversation
@dhruvkb For some reason, the @openverse-bot added the label |
Full-stack documentation: https://docs.openverse.org/_preview/3523 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. Changed files 🔄: |
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.
Yay for removing alarms! I have a few comments/notes for the stabilized alarms.
documentation/meta/monitoring/runbooks/api_http_5xx_above_threshold.md
Outdated
Show resolved
Hide resolved
documentation/meta/monitoring/runbooks/api_http_5xx_above_threshold.md
Outdated
Show resolved
Hide resolved
2d2be27
to
32fea18
Compare
Co-authored-by: Madison Swain-Bowden <[email protected]>
32fea18
to
8e3d262
Compare
Oh nooo, it looks like our GitHub username parser is interpreting these as usernames 😅 I think this runbook directory might have a few such cases, since viewing Log Insights is pretty common. Would you mind adding a way to exclude that particular folder from the username parsing script? |
@AetherUnbound Can't that be a separate issue? I don't think it needs to block this information getting into Here's an issue for it: #3542 (update:) ... and here's a PR 🙂 #3543 |
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.
LGTM!
Based on the high urgency of this PR, the following reviewers are being gently reminded to review this PR: @AetherUnbound Excluding weekend1 days, this PR was ready for review 2 day(s) ago. PRs labelled with high urgency are expected to be reviewed within 2 weekday(s)2. @krysal, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
Fixes
Closes #2500 by @sarayourfriend
Closes #2502 by @sarayourfriend
Closes #2503 by @sarayourfriend
Description
This PR marks the 2xx & 5xx count over time alarms of the API as stabilized in runbooks. Since they've been running for a while, the 5xx alarm has confirmed several incidents, and while we have not had a sudden increase in requests but we still believe the alarm for 2xx responses could be useful in the future. Also, given we aren't using anymore a separated service for thumbnails it get rids of all the alarms related to that service as well.
Testing Instructions
Just check the updates docs are correct, reviewing from the link on the comment below or from the code 📖
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin