-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Crons: Support upsert for monitors #42881
Comments
Routing to @getsentry/crons for triage, due by Tue Jan 10 2023 17:01:14 GMT+0000. ⏲️ |
This one seems like it should be more important than on the backlog, or at least the insert part. We were looking at switching from ohdear.app to Sentry crons, but as far as I can see we can't create the monitors via the api which is a non-starter for us (automatically deployed applications for customers that require their own monitor) and must instead manually create the monitors via the UI first. |
Hey @hailwood, agreed 100%, thanks for sharing your use case. We've moved this task up in our priority list and hope to iterate on it soon. I'll reach back out when I have updates to share. |
One of the questions I think we'll need to address with this is what alerting looks like by default for monitors that are not created in the app. There's likely a future where alerting will be configured at the same time as when you configure a new monitor. So we might need to either have a configuration for "new monitors get these alert settings" or provide a way to define alerting via the API. Refs #43753 |
This is now in the home stretch! We're just waiting on some SDK updates and this will be available! |
Our initial SDK integration for this has just dropped! Go check it out and play with it! We've also added documentation describing how to use upsert as part of the checkin API here: https://docs.sentry.io/product/crons/getting-started/http/#creating-or-updating-a-monitor-through-a-check-in-optional |
Problem Statement
To cleanly integrate with frameworks like Celerybeat, Sidekiq, and Laravel's cron monitoring, we want to be able to easily and automatically register monitors. There are two different scenarios for automatically registering these:
Basically in both situations we want to be able to upsert and reference a monitor without storing an internal GUID mapping, as both situations are effectively stateless.
Solution Brainstorm
Ignoring our current APIs, you could imagine something akin to:
POST /monitors/checkins/ {"monitor": {"name": "my-unique-name", "schedule": "* * * * *"}, "status": "ok"}
Effectively allowing you to specify and define the monitor in the same payload as the check-in. Now this could work but its still a fairly heavy payload to craft, especially when we're talking about crontab and curl.
What we'd love to achieve is something like:
curl -x POST https://myorg.sentry.io/api/monitors/my-unique-name/checkins/ -d '{"status": "ok"}'
The benefit here is this is minimal JSON (less error prone), but the issue is
my-unique-name
needs to be:Alternatively we could have a shortened API using a few more HTTP standards, such as query params, and opening it up to a GET request akin to a webhook:
curl https://myorg.sentry.io/hooks/monitors/checkin/?id=my-unique-name&status=ok
From a framework level, no matter the API, we'd effectively call some variation of this upsert endpoint when we send a check-in. Meaning whenever a Celerybeat task runs, we'd also send along the monitor details (or worst case, we'd make a second API call to ensure the monitor is created).
The last item here to note is, what do we do if we dont have a schedule? Do we support creating (upserting) a monitor without one? We could. We would then either need to prompt the user for configuration in the UI, or we could try to automatically learn the schedule after a few runs.
Refs #42283
Tasks
The text was updated successfully, but these errors were encountered: