-
Notifications
You must be signed in to change notification settings - Fork 624
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
fix(async-io): check for __name__ attribute when tracing coroutine #2521
Conversation
7c8f5f7
to
5f0a3ed
Compare
@povilasv please fix conflicts |
5f0a3ed
to
471533f
Compare
@xrmx done 👍 |
Anything I can do to help move this forward? Would appreciate if this would make it to next release 🙇 |
.../opentelemetry-instrumentation-asyncio/src/opentelemetry/instrumentation/asyncio/__init__.py
Outdated
Show resolved
Hide resolved
471533f
to
17a8678
Compare
Thanks for the fix! |
@@ -261,6 +261,8 @@ def trace_item(self, coro_or_future): | |||
return coro_or_future | |||
|
|||
async def trace_coroutine(self, coro): | |||
if not hasattr(coro, "__name__"): | |||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't return coro here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, wip -> #2541
somehow demo app (python anext.py) works in both cases 🤔
Description
This PR takes over stale PR #2353.
Basically we need to check if coroutine has name attribute before trying to trace, as not all coroutines have this attribute. More details are in the issue.
I've applied suggestions from @xrmx comment in here #2353 (review)
Fixes #2340
Type of change
How Has This Been Tested?
Added unit test. Ran the test without the fix and it fails with the following error:
With the fix the unit test passes.
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.