python-sensor icon indicating copy to clipboard operation
python-sensor copied to clipboard

use ContextVarsScopeManager instead of AsyncioScopeManager for python >= 3.7

Open BohdanKaminskyi opened this issue 2 years ago • 4 comments

Per opentracing documentation, ContextVarsScopeManager should be used for async code in python >= 3.7 .

For me, main issue was that, parent_span context was not propagated for coroutines running inside asyncio.wait() or asyncio.gather()

BohdanKaminskyi avatar Jun 23 '22 11:06 BohdanKaminskyi

Hi @pglombardo , it looks like there are 4 tests which failing due to difference in inner realization of ContextVarsScopeManager to AsyncioScopeManager: it lacks scope_manager._set_task_scope method, instead it uses _set_scope method but with different signature. Not sure how to properly fix these tests for py37+ . Could you please suggest what can I do to fix the issue? Thank you!

BohdanKaminskyi avatar Jun 23 '22 13:06 BohdanKaminskyi

Hi @BohdanKaminskyi - I no longer maintain this repository and have lost track of recent opentracing/telemetry scope management changes. Someone from the team should pick up on this soon I would think.

pglombardo avatar Jun 23 '22 14:06 pglombardo

@Ferenc- Hi, are you maintaining this repo? Maybe you can assist me on issue I mentioned https://github.com/instana/python-sensor/pull/374#issuecomment-1164385117

BohdanKaminskyi avatar Jun 23 '22 14:06 BohdanKaminskyi

Hi @BohdanKaminskyi,

Thank you for taking the initiative!
Would it be possible to start by creating a new test case for your original issue,
so your use case doesn't get broken in a future iteration?

Support for pre 3.7 runtimes is about to be dropped from the main branch,
so all the version checking will likely not be needed at the very end.

Well about the different signature of _set_task_scope vs _set_scope,
namely the second not taking a task argument,
the way I see it, that appears to be the commented purpose of ContextVarsScopeManager,
that you don't have to deal with setting the scope of the task manually,
but the scope should be propagated automatically.
While with the old AsyncioScopeManager it was explicitely needed to do manually. So is it possible, that we just have to stop trying to do that manually?

Ferenc- avatar Jun 28 '22 09:06 Ferenc-

Not relevant anymore. Closing.

Ferenc- avatar Mar 13 '24 12:03 Ferenc-