From e6c4a1e7b11a2bfaa28faf4fdadadf3854d39eb4 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 9 Feb 2024 11:53:17 -0500 Subject: [PATCH 1/6] [boto3sqs] Instrument `Session` and `resource` This commit addresses the following open issues: - https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1699 - https://github.com/open-telemetry/opentelemetry-python-contrib/issues/1996 There are four ways to access the SQS API via `boto3`: - `client = boto3.client("sqs")` - `client = boto3.Session().client("sqs")` - `sqs = boto3.resource("sqs")` - `sqs = boto3.Session().resource("sqs")` The existing wrapper tied into `boto3.client` to wrap a generated `botocore.client.SQS` class. The change here covers the three missing initialization methods. --- .../instrumentation/boto3sqs/__init__.py | 3 + .../tests/test_boto3sqs_instrumentation.py | 56 +++++++++++++++---- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py index ee7f4a59a6..c7faf5f66e 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py @@ -383,6 +383,9 @@ def client_wrapper(wrapped, instance, args, kwargs): return retval wrap_function_wrapper(boto3, "client", client_wrapper) + wrap_function_wrapper(boto3, "resource", client_wrapper) + wrap_function_wrapper(boto3.Session, "client", client_wrapper) + wrap_function_wrapper(boto3.Session, "resource", client_wrapper) def _decorate_sqs(self, sqs_class: type) -> None: """ diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py index a6ca0e062b..86cd29ff50 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py @@ -37,8 +37,17 @@ from opentelemetry.trace.span import Span, format_span_id, format_trace_id -def _make_sqs_client(): - return boto3.client( +def _make_sqs_client(*, session=False): + return (boto3.Session() if session else boto3).client( + "sqs", + region_name="us-east-1", + aws_access_key_id="dummy", + aws_secret_access_key="dummy", + ) + + +def _make_sqs_resource(*, session=False): + return (boto3.Session() if session else boto3).resource( "sqs", region_name="us-east-1", aws_access_key_id="dummy", @@ -67,19 +76,44 @@ def _active_instrumentor(): Boto3SQSInstrumentor().uninstrument() def test_instrument_api_before_client_init(self) -> None: - with self._active_instrumentor(): - client = _make_sqs_client() - self._assert_instrumented(client) + for session in (False, True): + with self._active_instrumentor(): + client = _make_sqs_client(session=session) + self._assert_instrumented(client) def test_instrument_api_after_client_init(self) -> None: - client = _make_sqs_client() - with self._active_instrumentor(): - self._assert_instrumented(client) + for session in (False, True): + client = _make_sqs_client(session=session) + with self._active_instrumentor(): + self._assert_instrumented(client) def test_instrument_multiple_clients(self): - with self._active_instrumentor(): - self._assert_instrumented(_make_sqs_client()) - self._assert_instrumented(_make_sqs_client()) + for session in (False, True): + with self._active_instrumentor(): + self._assert_instrumented(_make_sqs_client(session=session)) + self._assert_instrumented(_make_sqs_client(session=session)) + + def test_instrument_api_before_resource_init(self) -> None: + for session in (False, True): + with self._active_instrumentor(): + sqs = _make_sqs_resource(session=session) + self._assert_instrumented(sqs.meta.client) + + def test_instrument_api_after_client_init(self) -> None: + for session in (False, True): + sqs = _make_sqs_resource(session=session) + with self._active_instrumentor(): + self._assert_instrumented(sqs.meta.client) + + def test_instrument_multiple_clients(self): + for session in (False, True): + with self._active_instrumentor(): + self._assert_instrumented( + _make_sqs_resource(session=session).meta.client + ) + self._assert_instrumented( + _make_sqs_resource(session=session).meta.client + ) class TestBoto3SQSGetter(TestCase): From 9d6315cc77f784a2bf5adf64f4b180e2cabe4f32 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 9 Feb 2024 12:07:03 -0500 Subject: [PATCH 2/6] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3157bca33e..6ce784e7b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-boto3sqs` Instrument Session and resource + ([#2161](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2161)) - Drop uspport for 3.7 ([#2151](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2151)) - `opentelemetry-resource-detector-azure` Added 10s timeout to VM Resource Detector From 9e4682ad54a154f997b7d0d58477470d76c231e5 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 9 Feb 2024 12:18:22 -0500 Subject: [PATCH 3/6] rename duplicate test methods --- .../tests/test_boto3sqs_instrumentation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py index 86cd29ff50..8a490d0db4 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py @@ -99,13 +99,13 @@ def test_instrument_api_before_resource_init(self) -> None: sqs = _make_sqs_resource(session=session) self._assert_instrumented(sqs.meta.client) - def test_instrument_api_after_client_init(self) -> None: + def test_instrument_api_after_resource_init(self) -> None: for session in (False, True): sqs = _make_sqs_resource(session=session) with self._active_instrumentor(): self._assert_instrumented(sqs.meta.client) - def test_instrument_multiple_clients(self): + def test_instrument_multiple_resources(self): for session in (False, True): with self._active_instrumentor(): self._assert_instrumented( From 6bfaf34e5550711cc12b3cdd9a3a9d349f758119 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 9 Feb 2024 14:25:43 -0500 Subject: [PATCH 4/6] implement uninstrument --- .../src/opentelemetry/instrumentation/boto3sqs/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py index c7faf5f66e..2ea448af11 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py @@ -437,6 +437,9 @@ def _instrument(self, **kwargs: Dict[str, Any]) -> None: def _uninstrument(self, **kwargs: Dict[str, Any]) -> None: unwrap(boto3, "client") + unwrap(boto3, "resource") + unwrap(boto3.Session, "client") + unwrap(boto3.Session, "resource") for client_cls in botocore.client.BaseClient.__subclasses__(): self._un_decorate_sqs(client_cls) From f634266dbfe7359a5a36e88fdd47cdeeae6901b2 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Fri, 23 Feb 2024 17:31:10 -0500 Subject: [PATCH 5/6] [boto3sqs] Reduce number of wrapper targets There are actually 6 ways to initialize a boto3 API object. ```py boto3.client() # Using default global session boto3.resource() # Using default global session boto3.Session().client() # Using "re-exported" session.Session boto3.Session().resource() # Using "re-exported" session.Session boto3.session.Session().client() # Using session.Session directly boto3.session.Session().resource() # Using session.Session directly ``` We only have to patch `session.Session.client` to catch all the cases. - https://github.com/boto/boto3/blob/b3c158c62aa2a1314dc0ec78caea1ea976abd1a0/boto3/session.py#L217-L229 - https://github.com/boto/boto3/blob/b3c158c62aa2a1314dc0ec78caea1ea976abd1a0/boto3/session.py#L446-L457 --- .../instrumentation/boto3sqs/__init__.py | 12 +++--------- .../tests/test_boto3sqs_instrumentation.py | 16 +++++++++++++++- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py index 2ea448af11..c0231f81e4 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/src/opentelemetry/instrumentation/boto3sqs/__init__.py @@ -31,7 +31,7 @@ import logging from typing import Any, Collection, Dict, Generator, List, Mapping, Optional -import boto3 +import boto3.session import botocore.client from wrapt import wrap_function_wrapper @@ -382,10 +382,7 @@ def client_wrapper(wrapped, instance, args, kwargs): self._decorate_sqs(type(retval)) return retval - wrap_function_wrapper(boto3, "client", client_wrapper) - wrap_function_wrapper(boto3, "resource", client_wrapper) - wrap_function_wrapper(boto3.Session, "client", client_wrapper) - wrap_function_wrapper(boto3.Session, "resource", client_wrapper) + wrap_function_wrapper(boto3.session.Session, "client", client_wrapper) def _decorate_sqs(self, sqs_class: type) -> None: """ @@ -436,10 +433,7 @@ def _instrument(self, **kwargs: Dict[str, Any]) -> None: self._decorate_sqs(client_cls) def _uninstrument(self, **kwargs: Dict[str, Any]) -> None: - unwrap(boto3, "client") - unwrap(boto3, "resource") - unwrap(boto3.Session, "client") - unwrap(boto3.Session, "resource") + unwrap(boto3.session.Session, "client") for client_cls in botocore.client.BaseClient.__subclasses__(): self._un_decorate_sqs(client_cls) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py index 8a490d0db4..5ee604b97e 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py @@ -57,7 +57,6 @@ def _make_sqs_resource(*, session=False): class TestBoto3SQSInstrumentor(TestCase): def _assert_instrumented(self, client): - self.assertIsInstance(boto3.client, FunctionWrapper) self.assertIsInstance(client.send_message, BoundFunctionWrapper) self.assertIsInstance(client.send_message_batch, BoundFunctionWrapper) self.assertIsInstance(client.receive_message, BoundFunctionWrapper) @@ -66,6 +65,17 @@ def _assert_instrumented(self, client): client.delete_message_batch, BoundFunctionWrapper ) + def _assert_uninstrumented(self, client): + self.assertNotIsInstance(client.send_message, BoundFunctionWrapper) + self.assertNotIsInstance( + client.send_message_batch, BoundFunctionWrapper + ) + self.assertNotIsInstance(client.receive_message, BoundFunctionWrapper) + self.assertNotIsInstance(client.delete_message, BoundFunctionWrapper) + self.assertNotIsInstance( + client.delete_message_batch, BoundFunctionWrapper + ) + @staticmethod @contextmanager def _active_instrumentor(): @@ -80,12 +90,14 @@ def test_instrument_api_before_client_init(self) -> None: with self._active_instrumentor(): client = _make_sqs_client(session=session) self._assert_instrumented(client) + self._assert_uninstrumented(client) def test_instrument_api_after_client_init(self) -> None: for session in (False, True): client = _make_sqs_client(session=session) with self._active_instrumentor(): self._assert_instrumented(client) + self._assert_uninstrumented(client) def test_instrument_multiple_clients(self): for session in (False, True): @@ -98,12 +110,14 @@ def test_instrument_api_before_resource_init(self) -> None: with self._active_instrumentor(): sqs = _make_sqs_resource(session=session) self._assert_instrumented(sqs.meta.client) + self._assert_uninstrumented(sqs.meta.client) def test_instrument_api_after_resource_init(self) -> None: for session in (False, True): sqs = _make_sqs_resource(session=session) with self._active_instrumentor(): self._assert_instrumented(sqs.meta.client) + self._assert_uninstrumented(sqs.meta.client) def test_instrument_multiple_resources(self): for session in (False, True): From 6787796877979e5bd4964de0ce4ec59bea96d9f4 Mon Sep 17 00:00:00 2001 From: Matt Oberle Date: Sun, 21 Apr 2024 23:40:23 -0400 Subject: [PATCH 6/6] Remove unused import --- .../tests/test_boto3sqs_instrumentation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py index 5ee604b97e..7f7f00cf0a 100644 --- a/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-boto3sqs/tests/test_boto3sqs_instrumentation.py @@ -20,7 +20,7 @@ import boto3 from botocore.awsrequest import AWSResponse -from wrapt import BoundFunctionWrapper, FunctionWrapper +from wrapt import BoundFunctionWrapper from opentelemetry.instrumentation.boto3sqs import ( Boto3SQSGetter,