dd-trace-py
dd-trace-py copied to clipboard
DataDog
fix(context): handle cross execution tracing
Description
Previously a strong span reference was passed from task to task if a span was active. This was problematic as there would be a "race condition". Consider the following example:
async def main():
with tracer.trace("main"):
t = asyncio.create_task(task())
await t
async def task():
with tracer.trace("task"):
asyncio.sleep(0.01)
asyncio.run(main())
Running this example will result in two separate traces as the task t
is created but doesn't run until execution is yielded in main with the
await t line. By the time t runs, the main span has already
finished and so the task span creates a new trace.
This isn't desirable as we'd expect the above to result in spans in the
same trace. It's also unintuitive because moving the await t into the
with statement would subtly result in a single trace.
Fix
The fix is to maintain an executor id on all spans that are created
which correlates it with the executor it was created. The active span
updating logic is adjusted to look for and handle the executor id and
convert spans from different executors to Contexts.
Checklist
- [ ] Added to the correct milestone.
- [x] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] Corp site documentation is updated (link to the PR).
![mergify[bot] avatar](https://avatars.githubusercontent.com/in/10562?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'm sorry, I just re-read the description of the PR and I disagree with what it's trying to fix :(
Running this example will result in two separate traces as the task t is created but doesn't run until execution is yielded in main with the await t line. By the time t runs, the main span has already finished and so the task span creates a new trace.
Which is exactly what the code does and probably for a good reason. You can create thousands of tasks and await them later in different traces.
async def main():
with tracer.trace("create-tasks"):
t = asyncio.create_task(task())
t2 = asyncio.create_task(task())
with tracer.trace("run task"):
await t
with tracer.trace("run task1"):
await t
async def task():
with tracer.trace("task"):
asyncio.sleep(0.01)
asyncio.run(main())
I'm scared that this is trying to do much magic and is going to bite us later. Do we have more concrete evidence I don't have here that this PR is absolutely necessary and fixing something? 🤔
