fix instrumentation of connection when pool.acquire was called multip… #381
Conversation
sartx
requested review from
a team,
toumorokoshi and
aabmass
and removed request for
a team
| return get_traced_connection_proxy( | ||
| connection, db_api_integration, *args, **kwargs | ||
| ) | ||
| if not hasattr(connection, "__wrapped__"): |
There was a problem hiding this comment.
we probably could check if the wrapper type is the same as the instrumented type but not sure if it's worth it. This is probably enough,.
There was a problem hiding this comment.
I'd probably have argued for something a bit clearer that it was wrapped by opentelemetry, such as __wrapped_by_opentelemetry__. But agreed it's probably overkill: highly unlikely to overlap.
There was a problem hiding this comment.
I agree with you. if not isinstance(connection, AsyncProxyObject): - I think that this option more accurately defines the case of verification.
There was a problem hiding this comment.
It does but not sure if it is worth the extra check :) Probably we should do what @toumorokoshi suggested in a separate PR so all instrumentations benefit from it,.
tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py
Outdated
Show resolved
Hide resolved
|
Thanks! |
…le times
Description
Fix appearing multiple nested spans when instrumented
aiopg.poolwas used. The reason of the bug was thatpool.acquireinstrumented connection every time. But pool reuses old connections which had already been instrumented. This fix addes checking connection. Instrumenting connection only if it was not instrumented.Fixes #336
Type of change
How Has This Been Tested?
See added integration test in this PR.
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.