getsentry / sentry

Developer-first error tracking and performance monitoring
https://sentry.io
Other
39.11k stars 4.2k forks source link

Crons: Allow `PUT /organizations/{orgId}/monitors/{monitorId}/checkin/{checin_id}/` to create, not just update #44643

Open evanpurkhiser opened 1 year ago

evanpurkhiser commented 1 year ago

We should allow for monitor checkins to be provided a checkin ID on creation.

This provides better ergonomics from the client side since the client now no longer needs to parse the response json just to get the checkin ID to use for updates.

This could look like

local checkin_id=$(uuidgen)

curl -X POST \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"id": "${checkin_id}", "status": "in_progress"}'

# Run my cron job

curl -X PUT \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/${checkin_id}/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "ok"}'

We could also allow for PUT to be idempotent and also create a checkin when the checkin doesn't exist.

local checkin_id=$(uuidgen)

# Creates a checkin
curl -X PUT \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/${checkin_id}/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "in_progress"}'

# Run my cron job

# Marks a checkin as completed
curl -X PUT \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/${checkin_id}/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "ok"}'

Alternatives

Accept: plain/text

Instead of returning JSON from new checkins, we could allow Accept: plain/text to get back the checkin ID to make it easy to use from bash scripts

This could look like this then

local checkin_id=$(curl -X POST \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/" \
    --silent \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --header 'Accept: plain/text' \
    --data-raw '{"status": "in_progress"}')

# Run my cron job

curl -X PUT \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/${checkin_id}/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "ok"}'

Downsides of this approach

That's a lot more headers to pass, along with the silent option.

Using the existing latest checkin ID

We support a latest checkin_id, which we don't really expose anywhere afaik

curl -X POST \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "in_progress"}')

# Run my cron job

curl -X PUT \
    "https://example-org.sentry.io/api/0/monitors/my-monitor/checkins/latest/" \
    --header 'Authorization: DSN https://examplePublicKey@o0.ingest.sentry.io/0' \
    --header 'Content-Type: application/json' \
    --data-raw '{"status": "ok"}'

Downsides of this approach

You better be damn sure that you won't have overlapping monitor runs, otherwise you'll end up in a world of confusing debugging here.

getsantry[bot] commented 1 year ago

Routing to @getsentry/crons for triage, due by (sfo). ⏲️

getsantry[bot] commented 1 year ago

Routing to @getsentry/product-owners-crons for triage ⏲️