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

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

Open
samuelcolvin opened this issue May 1, 2024 · 2 comments
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants