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

asyncio context does not chain sub-tasks #247

Closed
thehesiod opened this issue Apr 12, 2017 · 6 comments
Closed

asyncio context does not chain sub-tasks #247

thehesiod opened this issue Apr 12, 2017 · 6 comments
Labels
Milestone

Comments

@thehesiod
Copy link
Contributor

thehesiod commented Apr 12, 2017

while browsing the code I noticed in https://github.com/DataDog/dd-trace-py/blob/master/ddtrace/contrib/asyncio/provider.py#L34 where it's associating a context with the current task, note that when doing things like asyncio.gather it will create new tasks so you won't be able to retrieve the context for the sub-tasks. In our product I wrote a monkeypatch to the task constructor to ensure that sub-tasks get associated with their "parent" tasks.

@palazzem
Copy link

Thank you @thehesiod for notifying this use case! we'll plan updating our code so that asyncio.gather will have the proper Context instance when the task-switching is in place.

@palazzem palazzem added the bug label Apr 12, 2017
@thehesiod
Copy link
Contributor Author

btw, working on this right now along with adding support for aiopg + aiobotocore, PR coming out soon

@palazzem
Copy link

It would be great if you can contribute on that! thank you a lot! About the monkey-patching, while it's our common approach for third-party libraries, we usually prefer not to touch Python internals unless it's safe and supported by the language. But we're happy to see your proposal and continue this discussion whenever the PR is out!

Thanks a lot!

@thehesiod
Copy link
Contributor Author

ok, have a PR in progress: #248

@thehesiod
Copy link
Contributor Author

ok, PR works, but having some issues setting up the test system

@palazzem palazzem modified the milestone: 0.9.0 Apr 17, 2017
@palazzem palazzem self-assigned this Apr 17, 2017
@palazzem palazzem removed their assignment May 19, 2017
@palazzem
Copy link

palazzem commented Jul 2, 2017

Fixed in #260 and #297. Closing since it will be released in the next 0.9.0 version.

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

No branches or pull requests

2 participants