Skip to content

opentelemetry-instrumentation-psycopg claims to work for async queries, but doesn't record time #2486

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
samuelcolvin opened this issue May 1, 2024 · 3 comments · May be fixed by #3324
Open
Labels
bug Something isn't working

Comments

@samuelcolvin
Copy link
Contributor

This was orignally reported as pydantic/logfire#65 which has version details etc.

But in summary, using opentelemetry-instrumentation-psycopg="0.45b0" to instrument asyncio queries, doesn't work properly.

The cause is as follows:

async def execute(self, *args, **kwargs):
return await _cursor_tracer.traced_execution(
self, super().execute, *args, **kwargs
)

calls

def execute(self, *args, **kwargs):
return _cursor_tracer.traced_execution(
self.__wrapped__, self.__wrapped__.execute, *args, **kwargs
)

calls

def traced_execution(
self,
cursor,
query_method: typing.Callable[..., typing.Any],
*args: typing.Tuple[typing.Any, typing.Any],
**kwargs: typing.Dict[typing.Any, typing.Any],
):

which isn't designed to support an awaitable query_method, so it immediately returns a future, which gets awaited after the span has closed, hence zero time spans:

From Logfire, we see:
image

when we'd expect something similar to what's observed with sync calls:
image

I'm not clear why TracedCursorProxy is required rather than just using CursorTracer, but either way, I'd suggest implementing an async varient of traced_execution which tries to share as much logic as possible with the sync method.

If you agree, I'll try to create a PR for this over the next week or so.

@xrmx
Copy link
Contributor

xrmx commented May 13, 2024

cc @federicobond

@federicobond
Copy link
Member

Thanks for the ping @xrmx. Agree that this is a bug and your proposed solution sounds reasonable @samuelcolvin, happy to review your PR

@Kludex
Copy link
Contributor

Kludex commented Dec 10, 2024

If someone can review this: #3067, and #3068... I'm happy to work on this, properly.

Also, knowing the reason for TracedCursorProxy would be nice.

ibash added a commit to ibash/opentelemetry-python-contrib that referenced this issue Mar 4, 2025
This copies the traced_execution of AsyncCursorTracer except
query_method is awaited within the span.

Fixes open-telemetry#2486
@ibash ibash linked a pull request Mar 4, 2025 that will close this issue
7 tasks
ibash added a commit to ibash/opentelemetry-python-contrib that referenced this issue Mar 5, 2025
This copies the traced_execution of AsyncCursorTracer except
query_method is awaited within the span.

Fixes open-telemetry#2486
ibash added a commit to ibash/opentelemetry-python-contrib that referenced this issue Mar 25, 2025
This copies the traced_execution of AsyncCursorTracer except
query_method is awaited within the span.

Fixes open-telemetry#2486
ibash added a commit to ibash/opentelemetry-python-contrib that referenced this issue Mar 25, 2025
This copies the traced_execution of AsyncCursorTracer except
query_method is awaited within the span.

Fixes open-telemetry#2486
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants