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

feat(arq): add arq integration #1872

Merged
merged 17 commits into from
Feb 22, 2023

Conversation

Zhenay
Copy link
Contributor

@Zhenay Zhenay commented Jan 28, 2023

Add support for arq https://arq-docs.helpmanual.io/

See #1728

@Zhenay Zhenay force-pushed the Zhenay/1728-arq-integration branch from 0d03c77 to 250aa4f Compare January 28, 2023 18:24
@sl0thentr0py sl0thentr0py added Status: Backlog New Integration Integrating with a new framework or library labels Jan 30, 2023
@sl0thentr0py sl0thentr0py linked an issue Jan 30, 2023 that may be closed by this pull request
@antonpirker
Copy link
Member

Hey @Zhenay !

Thanks for the great work. Looks good!
The coverage report is somehow misleading, because I thing it has way more coverage but because it is async code the coverage is calculated wrong.

I will create a test project for this today.

Could you try to find out why the coverage is wrong? This is the code I used for local coverage measurements:

coverage erase
./scripts/runtox.sh "py3.10-arq" --cov=tests --cov=sentry_sdk --cov-report= --cov-branch
coverage combine .coverage*
coverage html

@Zhenay
Copy link
Contributor Author

Zhenay commented Feb 22, 2023

Hey @antonpirker!

I'll take a look at the coverage problems this evening.

@antonpirker
Copy link
Member

One thing @Zhenay :

There is the possibility of distributed tracing (to see performance data from the python process that put a job into the queue combined with the performance data of the job being executed by the worker)

In rq the trace information is put into the jobs meta data when enqueuing the job:
https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/rq.py#L109-L111

and then fetched from this meta again when the job is executed:
https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/rq.py#L67-L72

Is there something like this in arq? Would be a great feature to have.

@Zhenay
Copy link
Contributor Author

Zhenay commented Feb 22, 2023

@antonpirker, unfortunately arq jobs hasn't meta data.

I can make a pr to arq. But suppose it will be more chances to be accepted, if this is merged.

@antonpirker
Copy link
Member

@antonpirker, unfortunately arq jobs hasn't meta data.

I can make a pr to arq. But suppose it will be more chances to be accepted, if this is merged.

Ok, it is just a nice to have for now, so we can move forward without it.

@Zhenay
Copy link
Contributor Author

Zhenay commented Feb 22, 2023

The coverage was wrong because I had forgot to add pytest-asyncio for arq into tox.ini.

@antonpirker
Copy link
Member

I have now made a small fix to your PR to clone the sentry hub. Because before it happened that the spans that where created when doing a http request (I used the demo from the arq website) then multiple downloads (from multiple jobs) showed up under the same job in Sentry.
But now everything looks good.

@Zhenay
Copy link
Contributor Author

Zhenay commented Feb 22, 2023

Great, should I do anything else?

@antonpirker antonpirker merged commit f3b3f65 into getsentry:master Feb 22, 2023
@antonpirker
Copy link
Member

No, I think its good.
I have merged it and it will release it soon.

@antonpirker
Copy link
Member

Oh @Zhenay you can add a PR to https://github.com/getsentry/sentry-docs for a documentation on how to use the integration. Something short like this: https://docs.sentry.io/platforms/python/configuration/integrations/sqlalchemy/

Just that have something to copy and paste, to be up and running in no time. Would be great!

@antonpirker
Copy link
Member

Oh, and have you already sent me your shipping address (for the Huey integration)? If not please send me your shipping address and t-shirt size to anton.pirker@sentry.io

Thanks!

@Zhenay
Copy link
Contributor Author

Zhenay commented Feb 22, 2023

I'll create PR to docs soon.
Yes, I've already sent the address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Integration Integrating with a new framework or library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arq support
3 participants