Add decorator for Sentry tracing#1089
Conversation
|
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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
I would be happy to bring this to the finish line but probably best to get feedback from a maintainer first. Thank you |
|
Would it be better to have a separate decorators for transactions and spans? They have different concerns. The transaction decorator:
The span decorator:
Maybe you could also have a decorator that combines both of these, but then it needs a clear name. (I'm struggling with these questions right now as we are designing the decorators for our use case, so maybe it's a useful brain dump, but if not, feel free to ignore it) |
|
What is the status of this PR? |
|
I would be happy to bring this to the finish line but probably best to get feedback from a maintainer first. I moved it from Draft to Ready for Review. Maybe it'll get picked up by someone with this new status. cc @sl0thentr0py |
|
Hey @ynouri ! Great work! I have moved the decorator into the We would need tests for the decorator, could you please write some? |
|
Hey @ynouri Also the span.op should not be set to an arbitrary value because we index spans based on this so we have a fixed set we can choose: https://develop.sentry.dev/sdk/performance/span-operations/ (I will change your code to set it to |
sl0thentr0py
left a comment
There was a problem hiding this comment.
some convention nits, I really think we don't wanna start splitting files for python 2/3
…o feature/tracing-decorator
…ded for common tests
…stuff is installed
|
@antonpirker why are there so many test changes in this PR now? can you summarize them? |
|
Because I now install Now I changed the test setup to have a "common" test environment that is kind of like an integration, so I can install |
sentry_sdk/tracing_utils_py3.py
Outdated
| async def func_with_tracing(*args, **kwargs): | ||
| # type: (*Any, **Any) -> Any | ||
|
|
||
| span_or_trx = get_current_span_or_transaction(sentry_sdk.Hub.current) |
There was a problem hiding this comment.
also as discussed I think get_current_span here is sufficient
|
Hey @ynouri ! Thanks for the great contribution and the patience! Finally we have merged your contribution! If you want to have a small gift for your work, please send me your shipping address and your t-shirt size to anton.pirker@sentry.io and I will send you something! Thanks! |
|
Thanks @antonpirker for the heavy lifting! |
Draft proposal for #760