-
-
Notifications
You must be signed in to change notification settings - Fork 59
Sentry monitors #585
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: main
Are you sure you want to change the base?
Sentry monitors #585
Conversation
adds check in at highstate start and resulting curl at end so we can track highstate times
salt/base/auto-highstate.sls
Outdated
15m-interval-highstate: | ||
cron.present: | ||
- identifier: 15m-interval-highstate | ||
- name: "timeout 5m salt-call state.highstate >> /var/log/salt/cron-highstate.log 2>&1" | ||
- name: {{ '/usr/local/bin/sentry-checkin.sh' if sentry_enabled else 'timeout 5m salt-call state.highstate >> /var/log/salt/cron-highstate.log 2>&1' }} |
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.
Suggestion here. Have sentry-checkin.sh
take a full command as an argument ala:
#!/bin/bash
while [[ $# -gt 0 ]]; do
case "$1" in
"--")
# Discard all options before `--`
shift
break
;;
*)
# Optionally if the script would need to accept options
# They could be processed using additional cases instead of `*`
shift
;;
esac
done
# Evaluate all arguments after `--` as a bash command
eval "$@"
And be invoked as:
/usr/local/bin/sentry-checkin.sh -- timeout 5m salt-call state.highstate
Then this script line can become:
name: "{% if sentry_enabled %}/usr/local/bin/sentry-checkin.sh -- {% endif %}timeout 5m salt-call state.highstate >> /var/log/salt/cron-highstate.log 2>&1"
This will allow us to keep from duplicating the command inside the script.
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.
i think i did this as part of 1456a65
pillar/base/sentry.sls
Outdated
@@ -0,0 +1,6 @@ | |||
sentry: |
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.
Feels weird that these are all in base, and that all are in clear-text.
It appears to me that everything here is sufficient for someone to report check-ins on our behalf, including updating the interval.
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.
this makes it more interesting i missed that you could check in on a non existent monitor!
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.
wdyt about 1456a65
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.
and i can rotate out the test projects and such so that the keys from git are stale
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.
@@ -0,0 +1,5 @@ | |||
secrets: |
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.
I don’t know if these really should even be here—it doesn't hurt much but its random processes that will not succeed since these projects dont exist.
It is maybe helpful as a reminder that the secrets in prod need to be set up, though?
todo:
|
Description
https://python-software-foundation.sentry.io/issues/alerts/rules/?project=4509407540346880
The only downside is that org tokens have a limited scope (I think only
org:ci
currently) and we needalerts:write/read
andproject:read
... so this currently is using my token and in prod the same would need to happen.