Skip to content
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

Add support for cron jobs in ARQ integration #2088

Merged

Conversation

lewazo
Copy link
Contributor

@lewazo lewazo commented May 8, 2023

This adds support for logging exceptions that occurs in ARQ cron jobs to Sentry.

Previously, only the tasks defined in the functions argument of the ARQ Worker Settings class were supported. Cron jobs were being ignored.

Since Cron Jobs run automatically by the worker object, we need to patch the worker instead of the func function that was previously patched.

assert error_event["exception"]["values"][0]["mechanism"]["type"] == "arq"
loop = asyncio.get_event_loop()
task = loop.create_task(worker.async_run())
await asyncio.sleep(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed because the cron job automatically execute after the worker starts.

There's no way to actually wait for a cron job to finish other than adding some delay before asserting results.

@antonpirker
Copy link
Member

Hey @lewazo !
Thanks for the PR!
It will take some time until we have the capacity to review this, please be patient.

@github-actions
Copy link

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@antonpirker
Copy link
Member

ping

@github-actions
Copy link

github-actions bot commented Jul 5, 2023

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@antonpirker antonpirker self-assigned this Jul 7, 2023
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!
Looks good to me, and does what it should!

@antonpirker
Copy link
Member

antonpirker commented Jul 10, 2023

Btw @lewazo and @iam-abbas Sentry has now a "Crons" feature, where cron jobs can submit their config (when it should run) on creation of a cron job and then can have check ins on each run (and on each success and failure) and then one has a nice UI in Sentry to see if the crons are running on time and if there are errors. Here is part of the implementation in Celery: https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/celery.py#L500-L518

Maybe you want to add this too for ARQ?

@antonpirker antonpirker merged commit 1c8b4e0 into getsentry:master Jul 10, 2023
244 checks passed
@lewazo
Copy link
Contributor Author

lewazo commented Jul 17, 2023

Thanks @antonpirker that looks like a nice feature indeed and sounds perfect for this integration.

I'll reserve some time to take a look at implementing it for ARQ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants