From 95fea2bfa7eb885912752ca954db0711f96c8ab5 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Wed, 5 Jun 2024 20:14:44 +0200 Subject: [PATCH] Fix running async psycopg tests (#2540) --- .../test-requirements.txt | 1 - .../tests/test_aio_client_interceptor.py | 18 +- .../test_aio_client_interceptor_filter.py | 20 +- .../test_aio_client_interceptor_hooks.py | 18 +- .../tests/test_aio_server_interceptor.py | 19 +- .../test_aio_server_interceptor_filter.py | 18 +- .../tests/test_psycopg_integration.py | 219 +++++++++--------- tox.ini | 1 - 8 files changed, 120 insertions(+), 194 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt b/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt index 56d47af0df..9303dc27c0 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt +++ b/instrumentation/opentelemetry-instrumentation-grpc/test-requirements.txt @@ -10,7 +10,6 @@ protobuf==3.20.3 py==1.11.0 py-cpuinfo==9.0.0 pytest==7.1.3 -pytest-asyncio==0.23.5 pytest-benchmark==4.0.0 tomli==2.0.1 typing_extensions==4.9.0 diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor.py index 21dbc44066..7ae1649149 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor.py @@ -11,24 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -try: - from unittest import IsolatedAsyncioTestCase -except ImportError: - # unittest.IsolatedAsyncioTestCase was introduced in Python 3.8. It's use - # simplifies the following tests. Without it, the amount of test code - # increases significantly, with most of the additional code handling - # the asyncio set up. - from unittest import TestCase - - class IsolatedAsyncioTestCase(TestCase): - def run(self, result=None): - self.skipTest( - "This test requires Python 3.8 for unittest.IsolatedAsyncioTestCase" - ) - +from unittest import IsolatedAsyncioTestCase import grpc -import pytest import opentelemetry.instrumentation.grpc from opentelemetry import trace @@ -65,7 +50,6 @@ async def intercept_unary_unary( return await continuation(client_call_details, request) -@pytest.mark.asyncio class TestAioClientInterceptor(TestBase, IsolatedAsyncioTestCase): def setUp(self): super().setUp() diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_filter.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_filter.py index b8c408c6cf..2bd68fd492 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_filter.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_filter.py @@ -11,27 +11,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -try: - from unittest import IsolatedAsyncioTestCase -except ImportError: - # unittest.IsolatedAsyncioTestCase was introduced in Python 3.8. It's use - # simplifies the following tests. Without it, the amount of test code - # increases significantly, with most of the additional code handling - # the asyncio set up. - from unittest import TestCase - - class IsolatedAsyncioTestCase(TestCase): - def run(self, result=None): - self.skipTest( - "This test requires Python 3.8 for unittest.IsolatedAsyncioTestCase" - ) - - import os -from unittest import mock +from unittest import IsolatedAsyncioTestCase, mock import grpc -import pytest from opentelemetry.instrumentation.grpc import ( GrpcAioInstrumentorClient, @@ -50,7 +33,6 @@ def run(self, result=None): from .protobuf import test_server_pb2_grpc # pylint: disable=no-name-in-module -@pytest.mark.asyncio class TestAioClientInterceptorFiltered(TestBase, IsolatedAsyncioTestCase): def setUp(self): super().setUp() diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_hooks.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_hooks.py index 9086d8b0f7..40c2334ae7 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_hooks.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_client_interceptor_hooks.py @@ -11,24 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -try: - from unittest import IsolatedAsyncioTestCase -except ImportError: - # unittest.IsolatedAsyncioTestCase was introduced in Python 3.8. It's use - # simplifies the following tests. Without it, the amount of test code - # increases significantly, with most of the additional code handling - # the asyncio set up. - from unittest import TestCase - - class IsolatedAsyncioTestCase(TestCase): - def run(self, result=None): - self.skipTest( - "This test requires Python 3.8 for unittest.IsolatedAsyncioTestCase" - ) - +from unittest import IsolatedAsyncioTestCase import grpc -import pytest from opentelemetry.instrumentation.grpc import GrpcAioInstrumentorClient from opentelemetry.test.test_base import TestBase @@ -54,7 +39,6 @@ def response_hook_with_exception(_span, _response): raise Exception() # pylint: disable=broad-exception-raised -@pytest.mark.asyncio class TestAioClientInterceptorWithHooks(TestBase, IsolatedAsyncioTestCase): def setUp(self): super().setUp() diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py index 7b31b085de..050f6f8d13 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor.py @@ -12,26 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. import asyncio - -try: - from unittest import IsolatedAsyncioTestCase -except ImportError: - # unittest.IsolatedAsyncioTestCase was introduced in Python 3.8. It's use - # simplifies the following tests. Without it, the amount of test code - # increases significantly, with most of the additional code handling - # the asyncio set up. - from unittest import TestCase - - class IsolatedAsyncioTestCase(TestCase): - def run(self, result=None): - self.skipTest( - "This test requires Python 3.8 for unittest.IsolatedAsyncioTestCase" - ) - +from unittest import IsolatedAsyncioTestCase import grpc import grpc.aio -import pytest import opentelemetry.instrumentation.grpc from opentelemetry import trace @@ -97,7 +81,6 @@ async def run_with_test_server( return resp -@pytest.mark.asyncio class TestOpenTelemetryAioServerInterceptor(TestBase, IsolatedAsyncioTestCase): async def test_instrumentor(self): """Check that automatic instrumentation configures the interceptor""" diff --git a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor_filter.py b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor_filter.py index 837d9c7618..34b755ced8 100644 --- a/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor_filter.py +++ b/instrumentation/opentelemetry-instrumentation-grpc/tests/test_aio_server_interceptor_filter.py @@ -11,25 +11,10 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -try: - from unittest import IsolatedAsyncioTestCase -except ImportError: - # unittest.IsolatedAsyncioTestCase was introduced in Python 3.8. It's use - # simplifies the following tests. Without it, the amount of test code - # increases significantly, with most of the additional code handling - # the asyncio set up. - from unittest import TestCase - - class IsolatedAsyncioTestCase(TestCase): - def run(self, result=None): - self.skipTest( - "This test requires Python 3.8 for unittest.IsolatedAsyncioTestCase" - ) - +from unittest import IsolatedAsyncioTestCase import grpc import grpc.aio -import pytest from opentelemetry import trace from opentelemetry.instrumentation.grpc import ( @@ -68,7 +53,6 @@ async def run_with_test_server( return resp -@pytest.mark.asyncio class TestOpenTelemetryAioServerInterceptor(TestBase, IsolatedAsyncioTestCase): async def test_instrumentor(self): """Check that automatic instrumentation configures the interceptor""" diff --git a/instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py b/instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py index e2b3ed917a..5a5b39d80b 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg/tests/test_psycopg_integration.py @@ -12,9 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -import asyncio import types -from unittest import mock +from unittest import IsolatedAsyncioTestCase, mock import psycopg @@ -114,6 +113,10 @@ def cursor(self): return cur return MockAsyncCursor() + def execute(self, query, params=None, *, prepare=None, binary=False): + cur = self.cursor() + return cur.execute(query, params, prepare=prepare) + def get_dsn_parameters(self): # pylint: disable=no-self-use return {"dbname": "test"} @@ -124,7 +127,8 @@ async def __aexit__(self, *args): return mock.MagicMock(spec=types.MethodType) -class TestPostgresqlIntegration(TestBase): +class PostgresqlIntegrationTestMixin: + # pylint: disable=invalid-name def setUp(self): super().setUp() self.cursor_mock = mock.patch( @@ -148,6 +152,7 @@ def setUp(self): self.connection_sync_mock.start() self.connection_async_mock.start() + # pylint: disable=invalid-name def tearDown(self): super().tearDown() self.memory_exporter.clear() @@ -159,6 +164,8 @@ def tearDown(self): with self.disable_logging(): PsycopgInstrumentor().uninstrument() + +class TestPostgresqlIntegration(PostgresqlIntegrationTestMixin, TestBase): # pylint: disable=unused-argument def test_instrumentor(self): PsycopgInstrumentor().instrument() @@ -221,60 +228,6 @@ def test_instrumentor_with_connection_class(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) - async def test_wrap_async_connection_class_with_cursor(self): - PsycopgInstrumentor().instrument() - - async def test_async_connection(): - acnx = await psycopg.AsyncConnection.connect(database="test") - async with acnx as cnx: - async with cnx.cursor() as cursor: - await cursor.execute("SELECT * FROM test") - - asyncio.run(test_async_connection()) - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 1) - span = spans_list[0] - - # Check version and name in span's instrumentation info - self.assertEqualSpanInstrumentationInfo( - span, opentelemetry.instrumentation.psycopg - ) - - # check that no spans are generated after uninstrument - PsycopgInstrumentor().uninstrument() - - asyncio.run(test_async_connection()) - - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 1) - - # pylint: disable=unused-argument - async def test_instrumentor_with_async_connection_class(self): - PsycopgInstrumentor().instrument() - - async def test_async_connection(): - acnx = await psycopg.AsyncConnection.connect(database="test") - async with acnx as cnx: - await cnx.execute("SELECT * FROM test") - - asyncio.run(test_async_connection()) - - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 1) - span = spans_list[0] - - # Check version and name in span's instrumentation info - self.assertEqualSpanInstrumentationInfo( - span, opentelemetry.instrumentation.psycopg - ) - - # check that no spans are generated after uninstrument - PsycopgInstrumentor().uninstrument() - asyncio.run(test_async_connection()) - - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 1) - def test_span_name(self): PsycopgInstrumentor().instrument() @@ -301,33 +254,6 @@ def test_span_name(self): self.assertEqual(spans_list[4].name, "query") self.assertEqual(spans_list[5].name, "query") - async def test_span_name_async(self): - PsycopgInstrumentor().instrument() - - cnx = psycopg.AsyncConnection.connect(database="test") - async with cnx.cursor() as cursor: - await cursor.execute("Test query", ("param1Value", False)) - await cursor.execute( - """multi - line - query""" - ) - await cursor.execute("tab\tseparated query") - await cursor.execute("/* leading comment */ query") - await cursor.execute( - "/* leading comment */ query /* trailing comment */" - ) - await cursor.execute("query /* trailing comment */") - - spans_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans_list), 6) - self.assertEqual(spans_list[0].name, "Test") - self.assertEqual(spans_list[1].name, "multi") - self.assertEqual(spans_list[2].name, "tab") - self.assertEqual(spans_list[3].name, "query") - self.assertEqual(spans_list[4].name, "query") - self.assertEqual(spans_list[5].name, "query") - # pylint: disable=unused-argument def test_not_recording(self): mock_tracer = mock.Mock() @@ -348,26 +274,6 @@ def test_not_recording(self): PsycopgInstrumentor().uninstrument() - # pylint: disable=unused-argument - async def test_not_recording_async(self): - mock_tracer = mock.Mock() - mock_span = mock.Mock() - mock_span.is_recording.return_value = False - mock_tracer.start_span.return_value = mock_span - PsycopgInstrumentor().instrument() - with mock.patch("opentelemetry.trace.get_tracer") as tracer: - tracer.return_value = mock_tracer - cnx = psycopg.AsyncConnection.connect(database="test") - async with cnx.cursor() as cursor: - query = "SELECT * FROM test" - cursor.execute(query) - self.assertFalse(mock_span.is_recording()) - self.assertTrue(mock_span.is_recording.called) - self.assertFalse(mock_span.set_attribute.called) - self.assertFalse(mock_span.set_status.called) - - PsycopgInstrumentor().uninstrument() - # pylint: disable=unused-argument def test_custom_tracer_provider(self): resource = resources.Resource.create({}) @@ -477,3 +383,108 @@ def test_sqlcommenter_disabled(self, event_mocked): cursor.execute(query) kwargs = event_mocked.call_args[1] self.assertEqual(kwargs["enable_commenter"], False) + + +class TestPostgresqlIntegrationAsync( + PostgresqlIntegrationTestMixin, TestBase, IsolatedAsyncioTestCase +): + async def test_wrap_async_connection_class_with_cursor(self): + PsycopgInstrumentor().instrument() + + async def test_async_connection(): + acnx = await psycopg.AsyncConnection.connect("test") + async with acnx as cnx: + async with cnx.cursor() as cursor: + await cursor.execute("SELECT * FROM test") + + await test_async_connection() + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + + # Check version and name in span's instrumentation info + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.psycopg + ) + + # check that no spans are generated after uninstrument + PsycopgInstrumentor().uninstrument() + + await test_async_connection() + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + + # pylint: disable=unused-argument + async def test_instrumentor_with_async_connection_class(self): + PsycopgInstrumentor().instrument() + + async def test_async_connection(): + acnx = await psycopg.AsyncConnection.connect("test") + async with acnx as cnx: + await cnx.execute("SELECT * FROM test") + + await test_async_connection() + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + span = spans_list[0] + + # Check version and name in span's instrumentation info + self.assertEqualSpanInstrumentationInfo( + span, opentelemetry.instrumentation.psycopg + ) + + # check that no spans are generated after uninstrument + PsycopgInstrumentor().uninstrument() + await test_async_connection() + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + + async def test_span_name_async(self): + PsycopgInstrumentor().instrument() + + cnx = await psycopg.AsyncConnection.connect("test") + async with cnx.cursor() as cursor: + await cursor.execute("Test query", ("param1Value", False)) + await cursor.execute( + """multi + line + query""" + ) + await cursor.execute("tab\tseparated query") + await cursor.execute("/* leading comment */ query") + await cursor.execute( + "/* leading comment */ query /* trailing comment */" + ) + await cursor.execute("query /* trailing comment */") + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 6) + self.assertEqual(spans_list[0].name, "Test") + self.assertEqual(spans_list[1].name, "multi") + self.assertEqual(spans_list[2].name, "tab") + self.assertEqual(spans_list[3].name, "query") + self.assertEqual(spans_list[4].name, "query") + self.assertEqual(spans_list[5].name, "query") + + # pylint: disable=unused-argument + async def test_not_recording_async(self): + mock_tracer = mock.Mock() + mock_span = mock.Mock() + mock_span.is_recording.return_value = False + mock_tracer.start_span.return_value = mock_span + PsycopgInstrumentor().instrument() + with mock.patch("opentelemetry.trace.get_tracer") as tracer: + tracer.return_value = mock_tracer + cnx = await psycopg.AsyncConnection.connect("test") + async with cnx.cursor() as cursor: + query = "SELECT * FROM test" + await cursor.execute(query) + self.assertFalse(mock_span.is_recording()) + self.assertTrue(mock_span.is_recording.called) + self.assertFalse(mock_span.set_attribute.called) + self.assertFalse(mock_span.set_status.called) + + PsycopgInstrumentor().uninstrument() diff --git a/tox.ini b/tox.ini index e97476a850..63c80083d4 100644 --- a/tox.ini +++ b/tox.ini @@ -373,7 +373,6 @@ deps = test: pytest-benchmark coverage: pytest coverage: pytest-cov - grpc: pytest-asyncio ; FIXME: add coverage testing ; FIXME: add mypy testing