sentry-python icon indicating copy to clipboard operation
sentry-python copied to clipboard

Crons: Celerybeat integration provides invalid Crontab schedule

Open gaprl opened this issue 1 year ago • 2 comments

How do you use Sentry?

Sentry Saas (sentry.io)

Version

2.3.1

Steps to Reproduce

Create a new Celery task with a crontab schedule of "0 0 * * saturday". The monitor for the task won't be upserted in Crons.

Expected Result

The monitor and check-ins should be created in Crons.

Actual Result

This happens because 0 0 * * saturday is not a valid crontab schedule, but it is a valid task schedule in Celery.

This is the complete payload that has failed validation on our end:

{
    "check_in_id": "82f57c609739414a8e21a88165fd52d8",
    "contexts": {
        "trace": {
            "trace_id": "8e424a9d5c7b462b8ccb638b3b9c98cc"
        }
    },
    "duration": 1602.9272508621216,
    "environment": "prod",
    "monitor_config": {
        "schedule": {
            "type": "crontab",
            "value": "0 0 * * saturday"
        },
        "timezone": "UTC"
    },
    "monitor_slug": "schedule-weekly-organization-reports-new",
    "status": "ok"
}

Here's the definition for the task: https://github.com/getsentry/sentry/blob/d92153e4f39810ed5b91a1e5e340a51e3cf507ec/src/sentry/conf/server.py#L1112-L1116

Perhaps we can map these unique Celery schedule definitions to a valid Crontab? Celery also supports solar schedules, so not sure what we want to do there.

gaprl avatar Jun 04 '24 18:06 gaprl

As for solar schedules: we currently only support crontab and schedule schedules in Celery, for everything else we print a warning: https://github.com/getsentry/sentry-python/blob/antonpirker/trace-origin-in-integrations/sentry_sdk/integrations/celery/beat.py#L90

antonpirker avatar Jun 06 '24 13:06 antonpirker

About the invalid Celery crontab definition. We probably need to improve our transaction from Celery crontab definition to the one we send to Sentry implemented here: https://github.com/getsentry/sentry-python/blob/antonpirker/trace-origin-in-integrations/sentry_sdk/integrations/celery/beat.py#L67-L75

antonpirker avatar Jun 06 '24 13:06 antonpirker

@gaprl Have you considered updating the Sentry backend to be able to interpret such schedules correctly?

imo it is not the Sentry SDK's responsibility to convert something like 0 0 * * Saturday to a format that the backend should recognize. Either we should communicate to users that such schedules are not supported, or ideally, we would update the Sentry backend to recognize these schedules.

szokeasaurusrex avatar Nov 04 '24 13:11 szokeasaurusrex