From 0206312757820e642fa3dcd191a8735a2429c1cb Mon Sep 17 00:00:00 2001 From: asagitullin Date: Thu, 25 Mar 2021 16:54:03 +0500 Subject: [PATCH 1/6] fix instrumentation of connection when pool.acquire was called multiple times --- .../aiopg/aiopg_integration.py | 8 +-- .../tests/aiopg/test_aiopg_functional.py | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py index 8fd67aae4d..993c458403 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py @@ -86,9 +86,11 @@ def acquire(self): async def _acquire(self): # pylint: disable=protected-access connection = await self.__wrapped__._acquire() - return get_traced_connection_proxy( - connection, db_api_integration, *args, **kwargs - ) + if not hasattr(connection, '__wrapped__'): + connection = get_traced_connection_proxy( + connection, db_api_integration, *args, **kwargs + ) + return connection return TracedPoolProxy(pool, *args, **kwargs) diff --git a/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py new file mode 100644 index 0000000000..83d4f1a49c --- /dev/null +++ b/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py @@ -0,0 +1,52 @@ +import asyncio +import os + +import aiopg + +from opentelemetry.instrumentation.aiopg import AiopgInstrumentor +from opentelemetry.test.test_base import TestBase + +POSTGRES_HOST = os.getenv("POSTGRESQL_HOST", "localhost") +POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) +POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME", "opentelemetry-tests") +POSTGRES_PASSWORD = os.getenv("POSTGRESQL_PASSWORD", "testpassword") +POSTGRES_USER = os.getenv("POSTGRESQL_USER", "testuser") + + +def async_call(coro): + loop = asyncio.get_event_loop() + return loop.run_until_complete(coro) + + +class TestFunctionalAsyncPG(TestBase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls._connection = None + cls._cursor = None + cls._tracer = cls.tracer_provider.get_tracer(__name__) + AiopgInstrumentor().instrument(tracer_provider=cls.tracer_provider) + cls._dsn = ( + f"dbname='{POSTGRES_DB_NAME}' user='{POSTGRES_USER}' password='{POSTGRES_PASSWORD}'" + f" host='{POSTGRES_HOST}' port='{POSTGRES_PORT}'" + ) + + @classmethod + def tearDownClass(cls): + AiopgInstrumentor().uninstrument() + + def test_instrumented_pool_with_multiple_acquires(self, *_, **__): + async def double_asquire(): + pool = await aiopg.create_pool(dsn=self._dsn) + async with pool.acquire() as conn: + async with conn.cursor() as cursor: + query = "SELECT 1" + await cursor.execute(query) + async with pool.acquire() as conn: + async with conn.cursor() as cursor: + query = "SELECT 1" + await cursor.execute(query) + + async_call(double_asquire()) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) From 1fa67b8f7cdb25b553bc6ee88e896bd1dbd11ddc Mon Sep 17 00:00:00 2001 From: asagitullin Date: Thu, 25 Mar 2021 17:02:27 +0500 Subject: [PATCH 2/6] add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4102c1f8c3..a19e6ceed5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#350](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/350)) - `opentelemetry-exporter-datadog` Fix warning when DatadogFormat encounters a request with no DD_ORIGIN headers ([#368](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/368)). +- `opentelemetry-instrumentation-aiopg` Fix multiple nested spans when + `aiopg.pool` is used + ([#336](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/381)). ### Removed - Removing support for Python 3.5 From 566899a3b4923637a04cfddf88f8dad80b56b9ca Mon Sep 17 00:00:00 2001 From: asagitullin Date: Thu, 25 Mar 2021 17:20:15 +0500 Subject: [PATCH 3/6] fix linting --- .../opentelemetry/instrumentation/aiopg/aiopg_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py index 993c458403..d136402ef7 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py @@ -86,7 +86,7 @@ def acquire(self): async def _acquire(self): # pylint: disable=protected-access connection = await self.__wrapped__._acquire() - if not hasattr(connection, '__wrapped__'): + if not hasattr(connection, "__wrapped__"): connection = get_traced_connection_proxy( connection, db_api_integration, *args, **kwargs ) From b324b400d6392d86e221332138165e5955daa88e Mon Sep 17 00:00:00 2001 From: asagitullin Date: Thu, 25 Mar 2021 17:22:59 +0500 Subject: [PATCH 4/6] replace tests to correct file --- .../tests/aiopg/test_aiopg_functional.py | 52 ------------------- .../tests/postgres/test_aiopg_functional.py | 20 +++++++ 2 files changed, 20 insertions(+), 52 deletions(-) delete mode 100644 tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py diff --git a/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py deleted file mode 100644 index 83d4f1a49c..0000000000 --- a/tests/opentelemetry-docker-tests/tests/aiopg/test_aiopg_functional.py +++ /dev/null @@ -1,52 +0,0 @@ -import asyncio -import os - -import aiopg - -from opentelemetry.instrumentation.aiopg import AiopgInstrumentor -from opentelemetry.test.test_base import TestBase - -POSTGRES_HOST = os.getenv("POSTGRESQL_HOST", "localhost") -POSTGRES_PORT = int(os.getenv("POSTGRESQL_PORT", "5432")) -POSTGRES_DB_NAME = os.getenv("POSTGRESQL_DB_NAME", "opentelemetry-tests") -POSTGRES_PASSWORD = os.getenv("POSTGRESQL_PASSWORD", "testpassword") -POSTGRES_USER = os.getenv("POSTGRESQL_USER", "testuser") - - -def async_call(coro): - loop = asyncio.get_event_loop() - return loop.run_until_complete(coro) - - -class TestFunctionalAsyncPG(TestBase): - @classmethod - def setUpClass(cls): - super().setUpClass() - cls._connection = None - cls._cursor = None - cls._tracer = cls.tracer_provider.get_tracer(__name__) - AiopgInstrumentor().instrument(tracer_provider=cls.tracer_provider) - cls._dsn = ( - f"dbname='{POSTGRES_DB_NAME}' user='{POSTGRES_USER}' password='{POSTGRES_PASSWORD}'" - f" host='{POSTGRES_HOST}' port='{POSTGRES_PORT}'" - ) - - @classmethod - def tearDownClass(cls): - AiopgInstrumentor().uninstrument() - - def test_instrumented_pool_with_multiple_acquires(self, *_, **__): - async def double_asquire(): - pool = await aiopg.create_pool(dsn=self._dsn) - async with pool.acquire() as conn: - async with conn.cursor() as cursor: - query = "SELECT 1" - await cursor.execute(query) - async with pool.acquire() as conn: - async with conn.cursor() as cursor: - query = "SELECT 1" - await cursor.execute(query) - - async_call(double_asquire()) - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 2) diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py index 53f49c3138..acbd81275b 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py @@ -42,6 +42,10 @@ def setUpClass(cls): cls._cursor = None cls._tracer = cls.tracer_provider.get_tracer(__name__) AiopgInstrumentor().instrument(tracer_provider=cls.tracer_provider) + cls._dsn = ( + f"dbname='{POSTGRES_DB_NAME}' user='{POSTGRES_USER}' password='{POSTGRES_PASSWORD}'" + f" host='{POSTGRES_HOST}' port='{POSTGRES_PORT}'" + ) cls._connection = async_call( aiopg.connect( dbname=POSTGRES_DB_NAME, @@ -185,3 +189,19 @@ def test_callproc(self): ): async_call(self._cursor.callproc("test", ())) self.validate_spans("test") + + def test_instrumented_pool_with_multiple_acquires(self, *_, **__): + async def double_asquire(): + pool = await aiopg.create_pool(dsn=self._dsn) + async with pool.acquire() as conn: + async with conn.cursor() as cursor: + query = "SELECT 1" + await cursor.execute(query) + async with pool.acquire() as conn: + async with conn.cursor() as cursor: + query = "SELECT 1" + await cursor.execute(query) + + async_call(double_asquire()) + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 2) From 5a2038a985b80c9cbaa02340a022af16e0da7517 Mon Sep 17 00:00:00 2001 From: asagitullin Date: Thu, 25 Mar 2021 17:28:08 +0500 Subject: [PATCH 5/6] fix typo in test init --- .../tests/postgres/test_aiopg_functional.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py index acbd81275b..78abaa0b8e 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py @@ -42,10 +42,6 @@ def setUpClass(cls): cls._cursor = None cls._tracer = cls.tracer_provider.get_tracer(__name__) AiopgInstrumentor().instrument(tracer_provider=cls.tracer_provider) - cls._dsn = ( - f"dbname='{POSTGRES_DB_NAME}' user='{POSTGRES_USER}' password='{POSTGRES_PASSWORD}'" - f" host='{POSTGRES_HOST}' port='{POSTGRES_PORT}'" - ) cls._connection = async_call( aiopg.connect( dbname=POSTGRES_DB_NAME, @@ -121,6 +117,10 @@ def setUpClass(cls): cls._cursor = None cls._tracer = cls.tracer_provider.get_tracer(__name__) AiopgInstrumentor().instrument(tracer_provider=cls.tracer_provider) + cls._dsn = ( + f"dbname='{POSTGRES_DB_NAME}' user='{POSTGRES_USER}' password='{POSTGRES_PASSWORD}'" + f" host='{POSTGRES_HOST}' port='{POSTGRES_PORT}'" + ) cls._pool = async_call( aiopg.create_pool( dbname=POSTGRES_DB_NAME, From 9879b2813d8cc01310e5ebc70f9997443fadfbb0 Mon Sep 17 00:00:00 2001 From: asagitullin Date: Fri, 26 Mar 2021 09:29:47 +0500 Subject: [PATCH 6/6] make more precisely check, fix typo --- .../opentelemetry/instrumentation/aiopg/aiopg_integration.py | 2 +- .../tests/postgres/test_aiopg_functional.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py index d136402ef7..f69c1f5dde 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/aiopg_integration.py @@ -86,7 +86,7 @@ def acquire(self): async def _acquire(self): # pylint: disable=protected-access connection = await self.__wrapped__._acquire() - if not hasattr(connection, "__wrapped__"): + if not isinstance(connection, AsyncProxyObject): connection = get_traced_connection_proxy( connection, db_api_integration, *args, **kwargs ) diff --git a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py index 78abaa0b8e..6519f9ac7e 100644 --- a/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py +++ b/tests/opentelemetry-docker-tests/tests/postgres/test_aiopg_functional.py @@ -191,7 +191,7 @@ def test_callproc(self): self.validate_spans("test") def test_instrumented_pool_with_multiple_acquires(self, *_, **__): - async def double_asquire(): + async def double_acquire(): pool = await aiopg.create_pool(dsn=self._dsn) async with pool.acquire() as conn: async with conn.cursor() as cursor: @@ -202,6 +202,6 @@ async def double_asquire(): query = "SELECT 1" await cursor.execute(query) - async_call(double_asquire()) + async_call(double_acquire()) spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 2)