Support cursor based queries

[bartv]

Description

This PR add support for tracing on queries using a cursor instead of directly on the connection.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Test against a code base that uses cursor extensively

Does This PR Require a Core Repo Change?

• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: bartv / name: Bart Vanbrabant (fd5f5f7, 705572c, 4a70b8b, 04bd445, 2ada10f, 75ec544, 3518ad5, a3631b5, df0232e, 381de4f, 4cb9c28, f5c115f, dce2556, 88cdf1f, 30b4f90, 07367b6)
• ✅ login: ocelotl / name: Diego Hurtado (5af4d51)

[xrmx]

Thanks for the PR but you need to add tests

[bartv]

Thanks for the PR but you need to add tests

What do you have in mind? The existing code is also just testing if the function gets wrapped. Do the same for the Cursor class?

[xrmx]

Thanks for the PR but you need to add tests

What do you have in mind? The existing code is also just testing if the function gets wrapped. Do the same for the Cursor class?

Given the code you have added would be nice to assert that a Span gets created with the attributes you set

[xrmx]

While you are at it please add a test asserting that the span gets created. You are adding 40 lines that are not tested

[bartv]

Do you have examples on how to do this? The current code does not have any of this at the moment. This also means I will need a postgres database to actually connect to.

[xrmx]

Do you have examples on how to do this? The current code does not have any of this at the moment. This also means I will need a postgres database to actually connect to.

This is redis instrumentation mocking the wrapped function:
https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2493/files#diff-83dd33b6baa2337f393ad1da857dbc683f48d0f52b460c5900c0c63139f2c279R235

Can something like this work?

[bartv]

It would test if we can instrument a mock and that it generates spans. Not sure what the value is of that?

[xrmx]

It would test if we can instrument a mock and that it generates spans. Not sure what the value is of that?

The value is to check that _do_cursor_execute does what we expect it to do 😅

[bartv]

This project also seems to use pytest however it is not clear to me how it can execute async test cases. Do you have pointer on this?

[xrmx]

This project also seems to use pytest however it is not clear to me how it can execute async test cases. Do you have pointer on this?

I would take a look at the instrumentation current tests in instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py

[bartv]

I looked at them. The problem is that there is something different there that makes pytest execute those tests correct and not the ones that I added.

Both when invoking pytest directly and using the predefined tox environments.

instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py::TestAsyncPGInstrumentation::test_cursor_span_creation
  /usr/lib64/python3.11/unittest/case.py:579: RuntimeWarning: coroutine 'TestAsyncPGInstrumentation.test_cursor_span_creation' was never awaited
    if method() is not None:
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py::TestAsyncPGInstrumentation::test_cursor_span_creation
  /usr/lib64/python3.11/unittest/case.py:678: DeprecationWarning: It is deprecated to return a value that is not None from a test case (<bound method TestAsyncPGInstrumentation.test_cursor_span_creation of <tests.test_asyncpg_wrapper.TestAsyncPGInstrumentation testMethod=test_cursor_span_creation>>)
    return self.run(*args, **kwds)

It does not detect them as async. I will push my change, maybe you can spot the issue. This test should fail but it doesn't because it never gets awaited.

[xrmx]

@bartv regarding async tests #2540

[xrmx]

@bartv next time please configure pre-commit as documented in CONTRIBUTING

[bartv]

@bartv next time please configure pre-commit as documented in CONTRIBUTING

ok. done. Do you squash merge or do I need to squash the commit history myself?

[bartv]

How do I handle this error?

lint-instrumentation-asyncpg: commands[4]> pylint /home/runner/work/opentelemetry-python-contrib/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-asyncpg/tests
************* Module tests.test_asyncpg_wrapper
instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py:103:24: C2801: Unnecessarily calls dunder method __anext__. Use anext built-in function. (unnecessary-dunder-call)

anext only exists in python3.10 and later. I switched to calling __anext__ directly to make the tests pass on 3.8

[xrmx]

How do I handle this error?

lint-instrumentation-asyncpg: commands[4]> pylint /home/runner/work/opentelemetry-python-contrib/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-asyncpg/tests
************* Module tests.test_asyncpg_wrapper
instrumentation/opentelemetry-instrumentation-asyncpg/tests/test_asyncpg_wrapper.py:103:24: C2801: Unnecessarily calls dunder method __anext__. Use anext built-in function. (unnecessary-dunder-call)

anext only exists in python3.10 and later. I switched to calling anext directly to make the tests pass on 3.8

Does something like this work? Otherwise just silence the warning with a pylint comment and open an issue with the exact line to fix properly

diff --git a/.pylintrc b/.pylintrc
index 114dadef..dc3a0525 100644
--- a/.pylintrc
+++ b/.pylintrc
@@ -45,6 +45,8 @@ suggestion-mode=yes
 # active Python interpreter and may run arbitrary code.
 unsafe-load-any-extension=no
 
+# Run python dependant checks considering the baseline version
+py-version=3.8
 
 [MESSAGES CONTROL]
 

[bartv]

@xrmx What do I need to do to get this merged?

[ocelotl]

@xrmx What do I need to do to get this merged?

I'll merge this PR ✌️