Keeping compatibility with celery 4.x.x BaseTask subclass like PeriodicTask

[kalmik]

fix #844

I'm using sentry_sdk 1.3.0 on python 2.7 with celery 4.4.6 and I'm having an issue with apply_async from PeriodicTask task instances. Following the @ahmedetefy leads I decided to open this PullRequest I'm not sure if it is the best way to fix that.

[2021-07-16 18:15:01,512: ERROR/Beat] Message Error: Couldn't apply scheduled task cron_jobs.periodic_jobs.schedule_report: unbound method apply_async() must be called with Task instance as first argument (got TaskType instance instead)
Traceback (most recent call last):
  File "/usr/local/pythonenv/myproject/local/lib/python2.7/site-packages/redbeat/schedulers.py", line 437, in maybe_due
    result = self.apply_async(entry, **kwargs)
  File "/usr/local/pythonenv/myproject/local/lib/python2.7/site-packages/celery/beat.py", line 400, in apply_async
    entry, exc=exc)), sys.exc_info()[2])
  File "/usr/local/pythonenv/myproject/local/lib/python2.7/site-packages/celery/beat.py", line 392, in apply_async
    **entry.options)
  File "/usr/local/pythonenv/myproject/local/lib/python2.7/site-packages/sentry_sdk/integrations/celery.py", line 117, in apply_async
    return f(*args, **kwargs)
SchedulingError: Couldn't apply scheduled task cron_jobs.periodic_jobs.schedule_report: unbound method apply_async() must be called with Task instance as first argument (got TaskType instance instead)```

[kalmik]

Hi @ahmedetefy What are your thoughts about this change? I would like to know if I am on the right path.

Thanks

[ahmedetefy]

Hey @kalmik
Thanks for opening the PR.

My approach was similar.. but I decided not to pursue it because it doesn't capture spans properly (for performance).
Did you manage to see a span of your celery task?

This is probably because Celery was setting the functions on this legacy Class dynamically on runtime so I was not able to monkey patch it, if I remember correctly

Anyways, considering this again maybe its not a bad idea to add this to not break backwards compatibility if we document that performance might not work properly with this hack...

Can you please document that somewhere and add a test?

[kalmik]

Sure thing, I can provide it shortly.

[kalmik]

I didn't forget about it, I've created the test and I think I could get the way to get it patched correctly (I HOPE) I think by this week I came up with the final patch.

[github-actions]

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 🥀

[imposeren]

Or at lease add information in migration guid, that old celery is not supported (I have celery version 3.1.27 in project and it also broken by newest sentry-sdk)

[antonpirker]

Hey @kalmik !

Sorry for the very late reply. I again tried to reproduce the problems described in #844 and could not reproduce it.

I also read the celery source and Sentry is currently patching celery.app.task.Task (here: https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/celery.py#L109)

This class is there in Celery 3-5:
Celery 3: https://github.com/celery/celery/blob/v3.1.26/celery/app/task.py#LL209C20-L209C20
Celery 4: https://github.com/celery/celery/blob/v4.4.7/celery/app/task.py#L144
Celery 5: https://github.com/celery/celery/blob/v5.3.1/celery/app/task.py#L164

Your patch would monkey patch BaseTask which is the same thing (see at the very last line of the links posted above)

I am sorry, but I will close this issue.

If there is still something really breaking then please create a new issue with detailed information about versions of Sentry SDK and Celery and an example on how to reproduce it.