From 5fc1b4e457d130ea452898302654d9f8a7276f57 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 30 Apr 2021 11:25:53 -0700 Subject: [PATCH 01/22] Create changelog copy.yml --- .github/workflows/changelog copy.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .github/workflows/changelog copy.yml diff --git a/.github/workflows/changelog copy.yml b/.github/workflows/changelog copy.yml new file mode 100644 index 0000000000..ee5f4c2d9b --- /dev/null +++ b/.github/workflows/changelog copy.yml @@ -0,0 +1,11 @@ + +# This action adds a comment on every new contributor's first PR to refer to the CONTRIBUTING.md section +# about new insrumentations and expectations from contributors regarding them. + +name: contributing-message + +steps: +- uses: actions/first-interaction@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + pr-message: 'Message that will be displayed on users' first pr. Look, a `code block` for markdown.' From 1882a7045ca69e961eb0b3954dc3ef83cec93c39 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 30 Apr 2021 11:33:53 -0700 Subject: [PATCH 02/22] test --- .github/workflows/changelog copy.yml | 11 ----------- .github/workflows/contributing-message.yml | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 11 deletions(-) delete mode 100644 .github/workflows/changelog copy.yml create mode 100644 .github/workflows/contributing-message.yml diff --git a/.github/workflows/changelog copy.yml b/.github/workflows/changelog copy.yml deleted file mode 100644 index ee5f4c2d9b..0000000000 --- a/.github/workflows/changelog copy.yml +++ /dev/null @@ -1,11 +0,0 @@ - -# This action adds a comment on every new contributor's first PR to refer to the CONTRIBUTING.md section -# about new insrumentations and expectations from contributors regarding them. - -name: contributing-message - -steps: -- uses: actions/first-interaction@v1 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - pr-message: 'Message that will be displayed on users' first pr. Look, a `code block` for markdown.' diff --git a/.github/workflows/contributing-message.yml b/.github/workflows/contributing-message.yml new file mode 100644 index 0000000000..4e796e3ec7 --- /dev/null +++ b/.github/workflows/contributing-message.yml @@ -0,0 +1,14 @@ + +# This action adds a comment on every new contributor's first PR to refer to the CONTRIBUTING.md section +# about new insrumentations and expectations from contributors regarding them. + +name: contributing-message + +jobs: + stale: + runs-on: ubuntu-latest + steps: + - uses: actions/first-interaction@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + pr-message: 'Message that will be displayed on users' first pr. Look, a `code block` for markdown.' From c095206a6a193e60168ab3d0fe8fe9aec977f562 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 30 Apr 2021 11:34:25 -0700 Subject: [PATCH 03/22] name --- .github/workflows/contributing-message.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/contributing-message.yml b/.github/workflows/contributing-message.yml index 4e796e3ec7..d1a0ec6a8e 100644 --- a/.github/workflows/contributing-message.yml +++ b/.github/workflows/contributing-message.yml @@ -5,7 +5,7 @@ name: contributing-message jobs: - stale: + contributing-message: runs-on: ubuntu-latest steps: - uses: actions/first-interaction@v1 From f3c5162855ace300fa7d36788aeef1bfb276b55b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 1 Jun 2021 17:38:59 -0700 Subject: [PATCH 04/22] Delete contributing-message.yml --- .github/workflows/contributing-message.yml | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 .github/workflows/contributing-message.yml diff --git a/.github/workflows/contributing-message.yml b/.github/workflows/contributing-message.yml deleted file mode 100644 index d1a0ec6a8e..0000000000 --- a/.github/workflows/contributing-message.yml +++ /dev/null @@ -1,14 +0,0 @@ - -# This action adds a comment on every new contributor's first PR to refer to the CONTRIBUTING.md section -# about new insrumentations and expectations from contributors regarding them. - -name: contributing-message - -jobs: - contributing-message: - runs-on: ubuntu-latest - steps: - - uses: actions/first-interaction@v1 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - pr-message: 'Message that will be displayed on users' first pr. Look, a `code block` for markdown.' From cbb0bb411b0190e4d78531a49c6244b82ea8c904 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 23 Jun 2021 13:03:26 -0700 Subject: [PATCH 05/22] fastapi --- .../aiohttp_client/__init__.py | 4 +- .../instrumentation/fastapi/__init__.py | 22 ++++++++- .../tests/test_fastapi_instrumentation.py | 17 +++++++ .../instrumentation/flask/__init__.py | 29 +++++------ .../instrumentation/psycopg2/__init__.py | 48 +++++++++++++------ .../instrumentation/instrumentor.py | 14 ++++-- 6 files changed, 97 insertions(+), 37 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index d4000584bb..c32efab5a5 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -259,7 +259,7 @@ def instrumented_init(wrapped, instance, args, kwargs): span_name=span_name, tracer_provider=tracer_provider, ) - trace_config.opentelemetry_aiohttp_instrumented = True + trace_config._is_instrumented_by_opentelemetry = True trace_configs.append(trace_config) kwargs["trace_configs"] = trace_configs @@ -282,7 +282,7 @@ def _uninstrument_session(client_session: aiohttp.ClientSession): client_session._trace_configs = [ trace_config for trace_config in trace_configs - if not hasattr(trace_config, "opentelemetry_aiohttp_instrumented") + if not hasattr(trace_config, "_is_instrumented_by_opentelemetry") ] diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 933b027018..6d78eeb047 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -12,9 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging from typing import Collection import fastapi +from starlette import middleware from starlette.routing import Match from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware @@ -24,6 +26,7 @@ from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls _excluded_urls_from_env = get_excluded_urls("FASTAPI") +_logger = logging.getLogger(__name__) class FastAPIInstrumentor(BaseInstrumentor): @@ -39,7 +42,7 @@ def instrument_app( app: fastapi.FastAPI, tracer_provider=None, excluded_urls=None, ): """Instrument an uninstrumented FastAPI application.""" - if not getattr(app, "is_instrumented_by_opentelemetry", False): + if not getattr(app, "_is_instrumented_by_opentelemetry", False): if excluded_urls is None: excluded_urls = _excluded_urls_from_env else: @@ -51,7 +54,22 @@ def instrument_app( span_details_callback=_get_route_details, tracer_provider=tracer_provider, ) - app.is_instrumented_by_opentelemetry = True + app._is_instrumented_by_opentelemetry = True + + @staticmethod + def uninstrument_app(app: fastapi.FastAPI): + if not hasattr(app, "_is_instrumented_by_opentelemetry"): + app._is_instrumented_by_opentelemetry = False + + if app._is_instrumented_by_opentelemetry: + app.user_middleware = [x for x in app.user_middleware if x.cls is not OpenTelemetryMiddleware] + app.middleware_stack = app.build_middleware_stack() + app._is_instrumented_by_opentelemetry = False + else: + _logger.warning( + "Attempting to uninstrument FastAPI " + "app while already uninstrumented" + ) def instrumentation_dependencies(self) -> Collection[str]: return _instruments diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index e3e6f8f4c5..f9744a143d 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -57,6 +57,23 @@ def tearDown(self): super().tearDown() self.env_patch.stop() self.exclude_patch.stop() + with self.disable_logging(): + self._instrumentor.uninstrument_app(self._app) + + def test_uninstrument_app(self): + # uninstrument_app only called for manual instrumentation + if not isinstance(self, TestAutoInstrumentation): + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware + self._app.add_middleware(HTTPSRedirectMiddleware) + self._instrumentor.uninstrument_app(self._app) + self._client = TestClient(self._app) + resp = self._client.get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) def test_basic_fastapi_call(self): self._client.get("/foobar") diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index f37859ac30..5a581b00cd 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -229,18 +229,21 @@ def _instrument(self, **kwargs): _InstrumentedFlask._request_hook = request_hook if callable(response_hook): _InstrumentedFlask._response_hook = response_hook - flask.Flask = _InstrumentedFlask tracer_provider = kwargs.get("tracer_provider") _InstrumentedFlask._tracer_provider = tracer_provider flask.Flask = _InstrumentedFlask + def _uninstrument(self, **kwargs): + flask.Flask = self._original_flask + + @staticmethod def instrument_app( - self, app, request_hook=None, response_hook=None, tracer_provider=None - ): # pylint: disable=no-self-use - if not hasattr(app, "_is_instrumented"): - app._is_instrumented = False + app, request_hook=None, response_hook=None, tracer_provider=None + ): + if not hasattr(app, "_is_instrumented_by_opentelemetry"): + app._is_instrumented_by_opentelemetry = False - if not app._is_instrumented: + if not app._is_instrumented_by_opentelemetry: app._original_wsgi_app = app.wsgi_app app.wsgi_app = _rewrapped_app(app.wsgi_app, response_hook) @@ -256,14 +259,12 @@ def instrument_app( "Attempting to instrument Flask app while already instrumented" ) - def _uninstrument(self, **kwargs): - flask.Flask = self._original_flask - - def uninstrument_app(self, app): # pylint: disable=no-self-use - if not hasattr(app, "_is_instrumented"): - app._is_instrumented = False + @staticmethod + def uninstrument_app(app): + if not hasattr(app, "_is_instrumented_by_opentelemetry"): + app._is_instrumented_by_opentelemetry = False - if app._is_instrumented: + if app._is_instrumented_by_opentelemetry: app.wsgi_app = app._original_wsgi_app # FIXME add support for other Flask blueprints that are not None @@ -271,7 +272,7 @@ def uninstrument_app(self, app): # pylint: disable=no-self-use app.teardown_request_funcs[None].remove(_teardown_request) del app._original_wsgi_app - app._is_instrumented = False + app._is_instrumented_by_opentelemetry = False else: _logger.warning( "Attempting to uninstrument Flask " diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py index 675d9ec258..75165b3ed9 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py @@ -53,6 +53,7 @@ from opentelemetry.instrumentation.psycopg2.package import _instruments from opentelemetry.instrumentation.psycopg2.version import __version__ +_logger = getLogger(__name__) _OTEL_CURSOR_FACTORY_KEY = "_otel_orig_cursor_factory" @@ -91,24 +92,43 @@ def _uninstrument(self, **kwargs): dbapi.unwrap_connect(psycopg2, "connect") # TODO(owais): check if core dbapi can do this for all dbapi implementations e.g, pymysql and mysql + @staticmethod def instrument_connection( - self, connection, tracer_provider=None - ): # pylint: disable=no-self-use - setattr( - connection, _OTEL_CURSOR_FACTORY_KEY, connection.cursor_factory - ) - connection.cursor_factory = _new_cursor_factory( - tracer_provider=tracer_provider - ) + connection, tracer_provider=None + ): + if not hasattr(connection, "_is_instrumented_by_opentelemetry"): + connection._is_instrumented_by_opentelemetry = False + + if not connection._is_instrumented_by_opentelemetry: + setattr( + connection, _OTEL_CURSOR_FACTORY_KEY, connection.cursor_factory + ) + connection.cursor_factory = _new_cursor_factory( + tracer_provider=tracer_provider + ) + else: + _logger.warning( + "Attempting to instrument Psycopg connection while already instrumented" + ) return connection # TODO(owais): check if core dbapi can do this for all dbapi implementations e.g, pymysql and mysql - def uninstrument_connection( - self, connection - ): # pylint: disable=no-self-use - connection.cursor_factory = getattr( - connection, _OTEL_CURSOR_FACTORY_KEY, None - ) + @staticmethod + def uninstrument_connection(connection): + if not hasattr(connection, "_is_instrumented_by_opentelemetry"): + connection._is_instrumented_by_opentelemetry = False + + if connection._is_instrumented_by_opentelemetry: + connection.cursor_factory = getattr( + connection, _OTEL_CURSOR_FACTORY_KEY, None + ) + connection._is_instrumented_by_opentelemetry = False + else: + _logger.warning( + "Attempting to uninstrument Psycopg " + "connection while already uninstrumented" + ) + return connection diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py index 43a4ea59bb..317a4a1d3e 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py @@ -43,7 +43,7 @@ class BaseInstrumentor(ABC): """ _instance = None - _is_instrumented = False + _is_instrumented_by_opentelemetry = False def __new__(cls, *args, **kwargs): @@ -52,6 +52,10 @@ def __new__(cls, *args, **kwargs): return cls._instance + @property + def is_instrumented_by_opentelemetry(self): + return self._is_instrumented_by_opentelemetry + @abstractmethod def instrumentation_dependencies(self) -> Collection[str]: """Return a list of python packages with versions that the will be instrumented. @@ -90,7 +94,7 @@ def instrument(self, **kwargs): ``opentelemetry-instrument`` command does. """ - if self._is_instrumented: + if self._is_instrumented_by_opentelemetry: _LOG.warning("Attempting to instrument while already instrumented") return None @@ -105,7 +109,7 @@ def instrument(self, **kwargs): result = self._instrument( # pylint: disable=assignment-from-no-return **kwargs ) - self._is_instrumented = True + self._is_instrumented_by_opentelemetry = True return result def uninstrument(self, **kwargs): @@ -115,9 +119,9 @@ def uninstrument(self, **kwargs): usage of ``kwargs``. """ - if self._is_instrumented: + if self._is_instrumented_by_opentelemetry: result = self._uninstrument(**kwargs) - self._is_instrumented = False + self._is_instrumented_by_opentelemetry = False return result _LOG.warning("Attempting to uninstrument while already uninstrumented") From 59de42220a97cef0f6021891005a20dd8b0d97a9 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 23 Jun 2021 14:35:49 -0700 Subject: [PATCH 06/22] flask, distro --- .../opentelemetry/instrumentation/aiopg/wrappers.py | 11 +++++++++-- .../opentelemetry/instrumentation/flask/__init__.py | 2 +- opentelemetry-instrumentation/tests/test_distro.py | 4 ++-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py index 706648d643..c613b72d1e 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py @@ -38,6 +38,7 @@ from opentelemetry.instrumentation.aiopg.aiopg_integration import ( AiopgIntegration, + AsyncProxyObject, get_traced_connection_proxy, ) from opentelemetry.instrumentation.aiopg.version import __version__ @@ -150,6 +151,10 @@ def instrument_connection( Returns: An instrumented connection. """ + if isinstance(connection, AsyncProxyObject): + logger.warning("Connection already instrumented") + return connection + db_integration = AiopgIntegration( name, database_system, @@ -158,7 +163,9 @@ def instrument_connection( tracer_provider=tracer_provider, ) db_integration.get_connection_attributes(connection) - return get_traced_connection_proxy(connection, db_integration) + proxy = get_traced_connection_proxy(connection, db_integration) + proxy._is_instrumented_by_opentelemetry = True + return proxy def uninstrument_connection(connection): @@ -170,7 +177,7 @@ def uninstrument_connection(connection): Returns: An uninstrumented connection. """ - if isinstance(connection, wrapt.ObjectProxy): + if isinstance(connection, AsyncProxyObject): return connection.__wrapped__ logger.warning("Connection is not instrumented") diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 5a581b00cd..a588c4b4f1 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -253,7 +253,7 @@ def instrument_app( app._before_request = _before_request app.before_request(_before_request) app.teardown_request(_teardown_request) - app._is_instrumented = True + app._is_instrumented_by_opentelemetry = True else: _logger.warning( "Attempting to instrument Flask app while already instrumented" diff --git a/opentelemetry-instrumentation/tests/test_distro.py b/opentelemetry-instrumentation/tests/test_distro.py index 2346d5709f..399b3f8a65 100644 --- a/opentelemetry-instrumentation/tests/test_distro.py +++ b/opentelemetry-instrumentation/tests/test_distro.py @@ -53,6 +53,6 @@ def test_load_instrumentor(self): instrumentor = MockInstrumetor() entry_point = MockEntryPoint(MockInstrumetor) - self.assertFalse(instrumentor._is_instrumented) + self.assertFalse(instrumentor._is_instrumented_by_opentelemetry) distro.load_instrumentor(entry_point) - self.assertTrue(instrumentor._is_instrumented) + self.assertTrue(instrumentor._is_instrumented_by_opentelemetry) From b023a5ac42d94462977e0432eac964caf02ebee0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Wed, 23 Jun 2021 15:19:12 -0700 Subject: [PATCH 07/22] Update wrappers.py --- .../src/opentelemetry/instrumentation/aiopg/wrappers.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py index c613b72d1e..10c807d9ac 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py @@ -163,9 +163,7 @@ def instrument_connection( tracer_provider=tracer_provider, ) db_integration.get_connection_attributes(connection) - proxy = get_traced_connection_proxy(connection, db_integration) - proxy._is_instrumented_by_opentelemetry = True - return proxy + return get_traced_connection_proxy(connection, db_integration) def uninstrument_connection(connection): From 55fd0db14bea9b18156a5ac35f8f5507a3b5c71b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Jun 2021 11:16:32 -0700 Subject: [PATCH 08/22] httpx --- .../instrumentation/httpx/__init__.py | 40 +++++++++++++++---- .../tests/test_httpx_integration.py | 30 +++++--------- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index fa3d29faf2..83c6891457 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging import typing import httpx @@ -31,6 +32,8 @@ from opentelemetry.trace.span import Span from opentelemetry.trace.status import Status +_logger = logging.getLogger(__name__) + URL = typing.Tuple[bytes, bytes, typing.Optional[int], bytes] Headers = typing.List[typing.Tuple[bytes, bytes]] RequestHook = typing.Callable[[Span, "RequestInfo"], None] @@ -332,6 +335,7 @@ def _instrument_client( ) else: raise TypeError("Invalid client provided") + client._original_transport = transport client._transport = telemetry_transport @@ -346,7 +350,8 @@ def _uninstrument_client( ) -> None: """Disables instrumentation for the given Client or AsyncClient""" # pylint: disable=protected-access - unwrap(client, "send") + client._transport = client._original_transport + del client._original_transport class HTTPXClientInstrumentor(BaseInstrumentor): @@ -395,12 +400,21 @@ def instrument_client( response_hook: A hook that receives the span, request, and response that is called right before the span ends """ - _instrument_client( - client, - tracer_provider=tracer_provider, - request_hook=request_hook, - response_hook=response_hook, - ) + if not hasattr(client, "_is_instrumented_by_opentelemetry"): + client._is_instrumented_by_opentelemetry = False + + if not client._is_instrumented_by_opentelemetry: + _instrument_client( + client, + tracer_provider=tracer_provider, + request_hook=request_hook, + response_hook=response_hook, + ) + client._is_instrumented_by_opentelemetry = True + else: + _logger.warning( + "Attempting to instrument Httpx client while already instrumented" + ) @staticmethod def uninstrument_client( @@ -411,4 +425,14 @@ def uninstrument_client( Args: client: The httpx Client or AsyncClient instance """ - _uninstrument_client(client) + if not hasattr(client, "_is_instrumented_by_opentelemetry"): + client._is_instrumented_by_opentelemetry = False + + if client._is_instrumented_by_opentelemetry: + _uninstrument_client(client) + client._is_instrumented_by_opentelemetry = False + else: + _logger.warning( + "Attempting to uninstrument Httpx " + "client while already uninstrumented" + ) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index e6d8427341..6a671953d8 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -375,9 +375,9 @@ def create_client( pass def setUp(self): + super().setUp() self.client = self.create_client() HTTPXClientInstrumentor().instrument() - super().setUp() def tearDown(self): super().tearDown() @@ -504,33 +504,21 @@ def test_uninstrument(self): HTTPXClientInstrumentor().instrument() def test_uninstrument_client(self): - HTTPXClientInstrumentor().uninstrument_client(self.client) - - result = self.perform_request(self.URL) - - self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=0) + HTTPXClientInstrumentor().uninstrument() + HTTPXClientInstrumentor().instrument_client(self.client) - def test_uninstrument_new_client(self): - client1 = self.create_client() - HTTPXClientInstrumentor().uninstrument_client(client1) + result = self.perform_request(url=self.URL) - result = self.perform_request(self.URL, client=client1) self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=0) + self.assert_span(num_spans=1) - # Test that other clients as well as instance client is still - # instrumented - client2 = self.create_client() - result = self.perform_request(self.URL, client=client2) - self.assertEqual(result.text, "Hello!") - self.assert_span() + HTTPXClientInstrumentor().uninstrument_client(self.client) - self.memory_exporter.clear() + result = self.perform_request(url=self.URL) - result = self.perform_request(self.URL) self.assertEqual(result.text, "Hello!") - self.assert_span() + self.assert_span(num_spans=1) + HTTPXClientInstrumentor().instrument() class TestSyncIntegration(BaseTestCases.BaseManualTest): From 15f77d718b7b55951d6bb25cb389f0d92c5731f5 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Jun 2021 15:50:43 -0700 Subject: [PATCH 09/22] aiopg --- .../src/opentelemetry/instrumentation/aiopg/wrappers.py | 5 ++--- .../src/opentelemetry/instrumentation/dbapi/__init__.py | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py index 10c807d9ac..b84927a84b 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py @@ -38,7 +38,6 @@ from opentelemetry.instrumentation.aiopg.aiopg_integration import ( AiopgIntegration, - AsyncProxyObject, get_traced_connection_proxy, ) from opentelemetry.instrumentation.aiopg.version import __version__ @@ -151,7 +150,7 @@ def instrument_connection( Returns: An instrumented connection. """ - if isinstance(connection, AsyncProxyObject): + if isinstance(connection, wrapt.ObjectProxy): logger.warning("Connection already instrumented") return connection @@ -175,7 +174,7 @@ def uninstrument_connection(connection): Returns: An uninstrumented connection. """ - if isinstance(connection, AsyncProxyObject): + if isinstance(connection, wrapt.ObjectProxy): return connection.__wrapped__ logger.warning("Connection is not instrumented") diff --git a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py index 2f03d196b5..b7ce6d13b6 100644 --- a/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-dbapi/src/opentelemetry/instrumentation/dbapi/__init__.py @@ -180,6 +180,10 @@ def instrument_connection( Returns: An instrumented connection. """ + if isinstance(connection, wrapt.ObjectProxy): + logger.warning("Connection already instrumented") + return connection + db_integration = DatabaseApiIntegration( name, database_system, From 353834314fa637788dbc2e75527968344f1cf409 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Jun 2021 16:18:54 -0700 Subject: [PATCH 10/22] psycopg2 --- .../instrumentation/psycopg2/__init__.py | 20 +++------- .../tests/test_psycopg2_integration.py | 38 ++++++++++++++++++- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py index 75165b3ed9..c81744f595 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py @@ -39,6 +39,7 @@ --- """ +import logging import typing from typing import Collection @@ -53,7 +54,7 @@ from opentelemetry.instrumentation.psycopg2.package import _instruments from opentelemetry.instrumentation.psycopg2.version import __version__ -_logger = getLogger(__name__) +_logger = logging.getLogger(__name__) _OTEL_CURSOR_FACTORY_KEY = "_otel_orig_cursor_factory" @@ -106,6 +107,7 @@ def instrument_connection( connection.cursor_factory = _new_cursor_factory( tracer_provider=tracer_provider ) + connection._is_instrumented_by_opentelemetry = True else: _logger.warning( "Attempting to instrument Psycopg connection while already instrumented" @@ -115,19 +117,9 @@ def instrument_connection( # TODO(owais): check if core dbapi can do this for all dbapi implementations e.g, pymysql and mysql @staticmethod def uninstrument_connection(connection): - if not hasattr(connection, "_is_instrumented_by_opentelemetry"): - connection._is_instrumented_by_opentelemetry = False - - if connection._is_instrumented_by_opentelemetry: - connection.cursor_factory = getattr( - connection, _OTEL_CURSOR_FACTORY_KEY, None - ) - connection._is_instrumented_by_opentelemetry = False - else: - _logger.warning( - "Attempting to uninstrument Psycopg " - "connection while already uninstrumented" - ) + connection.cursor_factory = getattr( + connection, _OTEL_CURSOR_FACTORY_KEY, None + ) return connection diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py b/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py index fa8172ba15..8219e2a6b5 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg2/tests/test_psycopg2_integration.py @@ -172,7 +172,25 @@ def test_instrument_connection(self): self.assertEqual(len(spans_list), 1) # pylint: disable=unused-argument - def test_uninstrument_connection(self): + def test_instrument_connection_with_instrument(self): + cnx = psycopg2.connect(database="test") + query = "SELECT * FROM test" + cursor = cnx.cursor() + cursor.execute(query) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 0) + + Psycopg2Instrumentor().instrument() + cnx = Psycopg2Instrumentor().instrument_connection(cnx) + cursor = cnx.cursor() + cursor.execute(query) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + + # pylint: disable=unused-argument + def test_uninstrument_connection_with_instrument(self): Psycopg2Instrumentor().instrument() cnx = psycopg2.connect(database="test") query = "SELECT * FROM test" @@ -188,3 +206,21 @@ def test_uninstrument_connection(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + + # pylint: disable=unused-argument + def test_uninstrument_connection_with_instrument_connection(self): + cnx = psycopg2.connect(database="test") + Psycopg2Instrumentor().instrument_connection(cnx) + query = "SELECT * FROM test" + cursor = cnx.cursor() + cursor.execute(query) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + + cnx = Psycopg2Instrumentor().uninstrument_connection(cnx) + cursor = cnx.cursor() + cursor.execute(query) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) From 10656382001f10bdbcc599e687952854651c2b0a Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Jun 2021 16:54:53 -0700 Subject: [PATCH 11/22] aiopg --- .../tests/test_aiopg_integration.py | 17 +++++++++++++++ .../instrumentation/fastapi/__init__.py | 21 ++++++++----------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py index fd7ca16cc3..9b4cd60cc1 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/tests/test_aiopg_integration.py @@ -204,6 +204,23 @@ def test_instrument_connection(self): spans_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans_list), 1) + def test_instrument_connection_after_instrument(self): + cnx = async_call(aiopg.connect(database="test")) + query = "SELECT * FROM test" + cursor = async_call(cnx.cursor()) + async_call(cursor.execute(query)) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 0) + + AiopgInstrumentor().instrument() + cnx = AiopgInstrumentor().instrument_connection(cnx) + cursor = async_call(cnx.cursor()) + async_call(cursor.execute(query)) + + spans_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans_list), 1) + def test_custom_tracer_provider_instrument_connection(self): resource = resources.Resource.create( {"service.name": "db-test-service"} diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 6d78eeb047..0416720713 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -42,6 +42,9 @@ def instrument_app( app: fastapi.FastAPI, tracer_provider=None, excluded_urls=None, ): """Instrument an uninstrumented FastAPI application.""" + if not hasattr(app, "_is_instrumented_by_opentelemetry"): + app._is_instrumented_by_opentelemetry = False + if not getattr(app, "_is_instrumented_by_opentelemetry", False): if excluded_urls is None: excluded_urls = _excluded_urls_from_env @@ -55,22 +58,16 @@ def instrument_app( tracer_provider=tracer_provider, ) app._is_instrumented_by_opentelemetry = True - - @staticmethod - def uninstrument_app(app: fastapi.FastAPI): - if not hasattr(app, "_is_instrumented_by_opentelemetry"): - app._is_instrumented_by_opentelemetry = False - - if app._is_instrumented_by_opentelemetry: - app.user_middleware = [x for x in app.user_middleware if x.cls is not OpenTelemetryMiddleware] - app.middleware_stack = app.build_middleware_stack() - app._is_instrumented_by_opentelemetry = False else: _logger.warning( - "Attempting to uninstrument FastAPI " - "app while already uninstrumented" + "Attempting to instrument FastAPI app while already instrumented" ) + @staticmethod + def uninstrument_app(app: fastapi.FastAPI): + app.user_middleware = [x for x in app.user_middleware if x.cls is not OpenTelemetryMiddleware] + app.middleware_stack = app.build_middleware_stack() + def instrumentation_dependencies(self) -> Collection[str]: return _instruments From 8bc3d31beab1c89da443160ead69820499bdc511 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 24 Jun 2021 16:54:59 -0700 Subject: [PATCH 12/22] fastapi --- .../tests/test_fastapi_instrumentation.py | 43 +++++++++++++------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index f9744a143d..d983780f61 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -17,6 +17,7 @@ import fastapi from fastapi.testclient import TestClient +from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry.sdk.resources import Resource @@ -58,22 +59,40 @@ def tearDown(self): self.env_patch.stop() self.exclude_patch.stop() with self.disable_logging(): + self._instrumentor.uninstrument() self._instrumentor.uninstrument_app(self._app) + def test_instrument_app_with_instrument(self): + if not isinstance(self, TestAutoInstrumentation): + self._instrumentor.instrument() + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + for span in spans: + self.assertIn("/foobar", span.name) + def test_uninstrument_app(self): - # uninstrument_app only called for manual instrumentation + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware + self._app.add_middleware(HTTPSRedirectMiddleware) + self._instrumentor.uninstrument_app(self._app) + print(self._app.user_middleware[0].cls) + self.assertFalse(isinstance(self._app.user_middleware[0].cls, OpenTelemetryMiddleware)) + self._client = TestClient(self._app) + resp = self._client.get("/foobar") + self.assertEqual(200, resp.status_code) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 3) + + def test_uninstrument_app_after_instrument(self): if not isinstance(self, TestAutoInstrumentation): - self._client.get("/foobar") - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware - self._app.add_middleware(HTTPSRedirectMiddleware) - self._instrumentor.uninstrument_app(self._app) - self._client = TestClient(self._app) - resp = self._client.get("/foobar") - self.assertEqual(200, resp.status_code) - span_list = self.memory_exporter.get_finished_spans() - self.assertEqual(len(span_list), 3) + self._instrumentor.instrument() + self._instrumentor.uninstrument_app(self._app) + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) def test_basic_fastapi_call(self): self._client.get("/foobar") From 6f9ab2087f7cef6edb2cb411a9bd4d8a28c60b41 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 10:01:53 -0700 Subject: [PATCH 13/22] flask --- .../instrumentation/flask/__init__.py | 9 ++------ .../tests/test_programmatic.py | 21 ++++++++++++++++++- .../instrumentation/instrumentor.py | 2 +- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index a588c4b4f1..6fdccaf31c 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -193,7 +193,7 @@ class _InstrumentedFlask(flask.Flask): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self._original_wsgi_ = self.wsgi_app + self._original_wsgi_app = self.wsgi_app self.wsgi_app = _rewrapped_app( self.wsgi_app, _InstrumentedFlask._response_hook @@ -261,18 +261,13 @@ def instrument_app( @staticmethod def uninstrument_app(app): - if not hasattr(app, "_is_instrumented_by_opentelemetry"): - app._is_instrumented_by_opentelemetry = False - - if app._is_instrumented_by_opentelemetry: + if hasattr(app, "_original_wsgi_app"): app.wsgi_app = app._original_wsgi_app # FIXME add support for other Flask blueprints that are not None app.before_request_funcs[None].remove(app._before_request) app.teardown_request_funcs[None].remove(_teardown_request) del app._original_wsgi_app - - app._is_instrumented_by_opentelemetry = False else: _logger.warning( "Attempting to uninstrument Flask " diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 8eb916b3d0..4e3e313945 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -79,7 +79,16 @@ def tearDown(self): with self.disable_logging(): FlaskInstrumentor().uninstrument_app(self.app) - def test_uninstrument(self): + def test_instrument_app_and_instrument(self): + FlaskInstrumentor().instrument() + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + FlaskInstrumentor().uninstrument() + + def test_uninstrument_app(self): resp = self.client.get("/hello/123") self.assertEqual(200, resp.status_code) self.assertEqual([b"Hello: 123"], list(resp.response)) @@ -94,6 +103,16 @@ def test_uninstrument(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) + def test_uninstrument_app_after_instrument(self): + FlaskInstrumentor().instrument() + FlaskInstrumentor().uninstrument_app(self.app) + resp = self.client.get("/hello/123") + self.assertEqual(200, resp.status_code) + self.assertEqual([b"Hello: 123"], list(resp.response)) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 0) + FlaskInstrumentor().uninstrument() + # pylint: disable=no-member def test_only_strings_in_environ(self): """ diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py index 317a4a1d3e..74ebe86746 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/instrumentor.py @@ -103,7 +103,7 @@ def instrument(self, **kwargs): if not skip_dep_check: conflict = self._check_dependency_conflicts() if conflict: - _LOG.warning(conflict) + _LOG.error(conflict) return None result = self._instrument( # pylint: disable=assignment-from-no-return From 2d0ead57396ff440d88aeb63fa503f3e2f29fb21 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 10:38:24 -0700 Subject: [PATCH 14/22] flask --- .../opentelemetry/instrumentation/fastapi/__init__.py | 1 + .../src/opentelemetry/instrumentation/flask/__init__.py | 1 + .../tests/test_httpx_integration.py | 9 +++++++++ 3 files changed, 11 insertions(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 0416720713..621c6c5f21 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -67,6 +67,7 @@ def instrument_app( def uninstrument_app(app: fastapi.FastAPI): app.user_middleware = [x for x in app.user_middleware if x.cls is not OpenTelemetryMiddleware] app.middleware_stack = app.build_middleware_stack() + app._is_instrumented_by_opentelemetry = False def instrumentation_dependencies(self) -> Collection[str]: return _instruments diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 6fdccaf31c..d07ca0f1c2 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -268,6 +268,7 @@ def uninstrument_app(app): app.before_request_funcs[None].remove(app._before_request) app.teardown_request_funcs[None].remove(_teardown_request) del app._original_wsgi_app + app._is_instrumented_by_opentelemetry = False else: _logger.warning( "Attempting to uninstrument Flask " diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 6a671953d8..070018f1e1 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -495,6 +495,15 @@ def test_instrument_client(self): # instrument again to avoid annoying warning message HTTPXClientInstrumentor().instrument() + def test_instrument_client_after_instrument(self): + print(HTTPXClientInstrumentor()._is_instrumented_by_opentelemetry) + client = self.create_client() + print(client._transport) + HTTPXClientInstrumentor().instrument_client(client) + result = self.perform_request(self.URL, client=client) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=1) + def test_uninstrument(self): HTTPXClientInstrumentor().uninstrument() result = self.perform_request(self.URL) From 8e72e7ced8992f3af48a65c1e37d85c9d76b6d90 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 16:21:38 -0700 Subject: [PATCH 15/22] flask --- .../src/opentelemetry/instrumentation/flask/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index d07ca0f1c2..c27207d196 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -194,6 +194,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._original_wsgi_app = self.wsgi_app + self._is_instrumented_by_opentelemetry = True self.wsgi_app = _rewrapped_app( self.wsgi_app, _InstrumentedFlask._response_hook From b66885c716690f5d33fcbf45c747af0a90c8ab6e Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 16:24:55 -0700 Subject: [PATCH 16/22] httpx --- .../instrumentation/httpx/__init__.py | 40 ++++--------------- .../tests/test_httpx_integration.py | 39 +++++++++--------- 2 files changed, 29 insertions(+), 50 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 83c6891457..fa3d29faf2 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import logging import typing import httpx @@ -32,8 +31,6 @@ from opentelemetry.trace.span import Span from opentelemetry.trace.status import Status -_logger = logging.getLogger(__name__) - URL = typing.Tuple[bytes, bytes, typing.Optional[int], bytes] Headers = typing.List[typing.Tuple[bytes, bytes]] RequestHook = typing.Callable[[Span, "RequestInfo"], None] @@ -335,7 +332,6 @@ def _instrument_client( ) else: raise TypeError("Invalid client provided") - client._original_transport = transport client._transport = telemetry_transport @@ -350,8 +346,7 @@ def _uninstrument_client( ) -> None: """Disables instrumentation for the given Client or AsyncClient""" # pylint: disable=protected-access - client._transport = client._original_transport - del client._original_transport + unwrap(client, "send") class HTTPXClientInstrumentor(BaseInstrumentor): @@ -400,21 +395,12 @@ def instrument_client( response_hook: A hook that receives the span, request, and response that is called right before the span ends """ - if not hasattr(client, "_is_instrumented_by_opentelemetry"): - client._is_instrumented_by_opentelemetry = False - - if not client._is_instrumented_by_opentelemetry: - _instrument_client( - client, - tracer_provider=tracer_provider, - request_hook=request_hook, - response_hook=response_hook, - ) - client._is_instrumented_by_opentelemetry = True - else: - _logger.warning( - "Attempting to instrument Httpx client while already instrumented" - ) + _instrument_client( + client, + tracer_provider=tracer_provider, + request_hook=request_hook, + response_hook=response_hook, + ) @staticmethod def uninstrument_client( @@ -425,14 +411,4 @@ def uninstrument_client( Args: client: The httpx Client or AsyncClient instance """ - if not hasattr(client, "_is_instrumented_by_opentelemetry"): - client._is_instrumented_by_opentelemetry = False - - if client._is_instrumented_by_opentelemetry: - _uninstrument_client(client) - client._is_instrumented_by_opentelemetry = False - else: - _logger.warning( - "Attempting to uninstrument Httpx " - "client while already uninstrumented" - ) + _uninstrument_client(client) diff --git a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py index 070018f1e1..e6d8427341 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/tests/test_httpx_integration.py @@ -375,9 +375,9 @@ def create_client( pass def setUp(self): - super().setUp() self.client = self.create_client() HTTPXClientInstrumentor().instrument() + super().setUp() def tearDown(self): super().tearDown() @@ -495,15 +495,6 @@ def test_instrument_client(self): # instrument again to avoid annoying warning message HTTPXClientInstrumentor().instrument() - def test_instrument_client_after_instrument(self): - print(HTTPXClientInstrumentor()._is_instrumented_by_opentelemetry) - client = self.create_client() - print(client._transport) - HTTPXClientInstrumentor().instrument_client(client) - result = self.perform_request(self.URL, client=client) - self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=1) - def test_uninstrument(self): HTTPXClientInstrumentor().uninstrument() result = self.perform_request(self.URL) @@ -513,21 +504,33 @@ def test_uninstrument(self): HTTPXClientInstrumentor().instrument() def test_uninstrument_client(self): - HTTPXClientInstrumentor().uninstrument() - HTTPXClientInstrumentor().instrument_client(self.client) + HTTPXClientInstrumentor().uninstrument_client(self.client) - result = self.perform_request(url=self.URL) + result = self.perform_request(self.URL) self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=1) + self.assert_span(num_spans=0) - HTTPXClientInstrumentor().uninstrument_client(self.client) + def test_uninstrument_new_client(self): + client1 = self.create_client() + HTTPXClientInstrumentor().uninstrument_client(client1) - result = self.perform_request(url=self.URL) + result = self.perform_request(self.URL, client=client1) + self.assertEqual(result.text, "Hello!") + self.assert_span(num_spans=0) + # Test that other clients as well as instance client is still + # instrumented + client2 = self.create_client() + result = self.perform_request(self.URL, client=client2) self.assertEqual(result.text, "Hello!") - self.assert_span(num_spans=1) - HTTPXClientInstrumentor().instrument() + self.assert_span() + + self.memory_exporter.clear() + + result = self.perform_request(self.URL) + self.assertEqual(result.text, "Hello!") + self.assert_span() class TestSyncIntegration(BaseTestCases.BaseManualTest): From 6fd6fc2682a7205b77179311be68578be052d12a Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 16:55:45 -0700 Subject: [PATCH 17/22] lint --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 6 +++++- .../tests/test_fastapi_instrumentation.py | 6 +++++- .../src/opentelemetry/instrumentation/psycopg2/__init__.py | 4 +--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py index 621c6c5f21..7847eccb20 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -65,7 +65,11 @@ def instrument_app( @staticmethod def uninstrument_app(app: fastapi.FastAPI): - app.user_middleware = [x for x in app.user_middleware if x.cls is not OpenTelemetryMiddleware] + app.user_middleware = [ + x + for x in app.user_middleware + if x.cls is not OpenTelemetryMiddleware + ] app.middleware_stack = app.build_middleware_stack() app._is_instrumented_by_opentelemetry = False diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index d983780f61..62bf9f4c64 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -79,7 +79,11 @@ def test_uninstrument_app(self): self._app.add_middleware(HTTPSRedirectMiddleware) self._instrumentor.uninstrument_app(self._app) print(self._app.user_middleware[0].cls) - self.assertFalse(isinstance(self._app.user_middleware[0].cls, OpenTelemetryMiddleware)) + self.assertFalse( + isinstance( + self._app.user_middleware[0].cls, OpenTelemetryMiddleware + ) + ) self._client = TestClient(self._app) resp = self._client.get("/foobar") self.assertEqual(200, resp.status_code) diff --git a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py index c81744f595..4285fec1d0 100644 --- a/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-psycopg2/src/opentelemetry/instrumentation/psycopg2/__init__.py @@ -94,9 +94,7 @@ def _uninstrument(self, **kwargs): # TODO(owais): check if core dbapi can do this for all dbapi implementations e.g, pymysql and mysql @staticmethod - def instrument_connection( - connection, tracer_provider=None - ): + def instrument_connection(connection, tracer_provider=None): if not hasattr(connection, "_is_instrumented_by_opentelemetry"): connection._is_instrumented_by_opentelemetry = False From 46ab6ccc38d974060807a247273585b1682cd1c0 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 17:00:03 -0700 Subject: [PATCH 18/22] chanelog --- CHANGELOG.md | 2 ++ .../tests/test_fastapi_instrumentation.py | 1 + 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51c1880f66..5db70d5bc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#538](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/538)) - Changed the psycopg2-binary to psycopg2 as dependency in production ([#543](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/543)) +- Implement consistent way of checking if instrumentation is already instrumented + ([#549](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/549)) ### Added - `opentelemetry-instrumentation-httpx` Add `httpx` instrumentation diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 62bf9f4c64..79f27e03be 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -76,6 +76,7 @@ def test_uninstrument_app(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware + self._app.add_middleware(HTTPSRedirectMiddleware) self._instrumentor.uninstrument_app(self._app) print(self._app.user_middleware[0].cls) From 78c81823af2778e619b02b0b5224f12b4946b616 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 17:06:58 -0700 Subject: [PATCH 19/22] lint --- .../tests/test_fastapi_instrumentation.py | 2 +- .../instrumentation/pymysql/__init__.py | 32 ++++++++++--------- .../instrumentation/pyramid/__init__.py | 8 +++-- .../instrumentation/sqlite3/__init__.py | 24 +++++++------- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 79f27e03be..f34a990d4f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -17,9 +17,9 @@ import fastapi from fastapi.testclient import TestClient -from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware import opentelemetry.instrumentation.fastapi as otel_fastapi +from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware from opentelemetry.sdk.resources import Resource from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.test_base import TestBase diff --git a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py index e70d4354f3..bc2168936c 100644 --- a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py @@ -50,15 +50,16 @@ from opentelemetry.instrumentation.pymysql.version import __version__ -class PyMySQLInstrumentor(BaseInstrumentor): - _CONNECTION_ATTRIBUTES = { - "database": "db", - "port": "port", - "host": "host", - "user": "user", - } +_CONNECTION_ATTRIBUTES = { + "database": "db", + "port": "port", + "host": "host", + "user": "user", +} +_DATABASE_SYSTEM = "mysql" + - _DATABASE_SYSTEM = "mysql" +class PyMySQLInstrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments @@ -73,8 +74,8 @@ def _instrument(self, **kwargs): __name__, pymysql, "connect", - self._DATABASE_SYSTEM, - self._CONNECTION_ATTRIBUTES, + _DATABASE_SYSTEM, + _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, ) @@ -83,8 +84,8 @@ def _uninstrument(self, **kwargs): """"Disable PyMySQL instrumentation""" dbapi.unwrap_connect(pymysql, "connect") - # pylint:disable=no-self-use - def instrument_connection(self, connection, tracer_provider=None): + @staticmethod + def instrument_connection(connection, tracer_provider=None): """Enable instrumentation in a PyMySQL connection. Args: @@ -99,13 +100,14 @@ def instrument_connection(self, connection, tracer_provider=None): return dbapi.instrument_connection( __name__, connection, - self._DATABASE_SYSTEM, - self._CONNECTION_ATTRIBUTES, + _DATABASE_SYSTEM, + _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, ) - def uninstrument_connection(self, connection): + @staticmethod + def uninstrument_connection(connection): """Disable instrumentation in a PyMySQL connection. Args: diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py index dc8ed6e7a9..44ee512bdd 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py @@ -78,6 +78,7 @@ --- """ +from os import stat import typing from typing import Collection @@ -140,8 +141,8 @@ def _uninstrument(self, **kwargs): """"Disable Pyramid instrumentation""" unwrap(Configurator, "__init__") - # pylint:disable=no-self-use - def instrument_config(self, config): + @staticmethod + def instrument_config(config): """Enable instrumentation in a Pyramid configurator. Args: @@ -149,5 +150,6 @@ def instrument_config(self, config): """ config.include("opentelemetry.instrumentation.pyramid.callbacks") - def uninstrument_config(self, config): + @staticmethod + def uninstrument_config(config): config.add_settings({SETTING_TRACE_ENABLED: False}) diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py index f8cd6e88cc..182589d222 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py @@ -48,12 +48,13 @@ from opentelemetry.instrumentation.sqlite3.version import __version__ from opentelemetry.trace import get_tracer +# No useful attributes of sqlite3 connection object +_CONNECTION_ATTRIBUTES = {} + +_DATABASE_SYSTEM = "sqlite" -class SQLite3Instrumentor(BaseInstrumentor): - # No useful attributes of sqlite3 connection object - _CONNECTION_ATTRIBUTES = {} - _DATABASE_SYSTEM = "sqlite" +class SQLite3Instrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments @@ -68,8 +69,8 @@ def _instrument(self, **kwargs): __name__, sqlite3, "connect", - self._DATABASE_SYSTEM, - self._CONNECTION_ATTRIBUTES, + _DATABASE_SYSTEM, + _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, ) @@ -78,8 +79,8 @@ def _uninstrument(self, **kwargs): """"Disable SQLite3 instrumentation""" dbapi.unwrap_connect(sqlite3, "connect") - # pylint:disable=no-self-use - def instrument_connection(self, connection, tracer_provider=None): + @staticmethod + def instrument_connection(connection, tracer_provider=None): """Enable instrumentation in a SQLite connection. Args: @@ -94,13 +95,14 @@ def instrument_connection(self, connection, tracer_provider=None): return dbapi.instrument_connection( __name__, connection, - self._DATABASE_SYSTEM, - self._CONNECTION_ATTRIBUTES, + _DATABASE_SYSTEM, + _CONNECTION_ATTRIBUTES, version=__version__, tracer_provider=tracer_provider, ) - def uninstrument_connection(self, connection): + @staticmethod + def uninstrument_connection(connection): """Disable instrumentation in a SQLite connection. Args: From ed69c230a66ccb612eb8937fd972d411137a244b Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 25 Jun 2021 17:14:43 -0700 Subject: [PATCH 20/22] lint --- .../src/opentelemetry/instrumentation/pymysql/__init__.py | 2 -- .../src/opentelemetry/instrumentation/pyramid/__init__.py | 2 +- .../src/opentelemetry/instrumentation/sqlite3/__init__.py | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py index bc2168936c..0b524ec7f6 100644 --- a/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pymysql/src/opentelemetry/instrumentation/pymysql/__init__.py @@ -49,7 +49,6 @@ from opentelemetry.instrumentation.pymysql.package import _instruments from opentelemetry.instrumentation.pymysql.version import __version__ - _CONNECTION_ATTRIBUTES = { "database": "db", "port": "port", @@ -60,7 +59,6 @@ class PyMySQLInstrumentor(BaseInstrumentor): - def instrumentation_dependencies(self) -> Collection[str]: return _instruments diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py index 44ee512bdd..fcf8caae67 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py @@ -78,8 +78,8 @@ --- """ -from os import stat import typing +from os import stat from typing import Collection from pyramid.config import Configurator diff --git a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py index 182589d222..a0842214e5 100644 --- a/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-sqlite3/src/opentelemetry/instrumentation/sqlite3/__init__.py @@ -55,7 +55,6 @@ class SQLite3Instrumentor(BaseInstrumentor): - def instrumentation_dependencies(self) -> Collection[str]: return _instruments From 865ca59e1ace7b7dd5b810953348b352fae7e309 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 28 Jun 2021 09:01:22 -0700 Subject: [PATCH 21/22] lint --- .../tests/test_fastapi_instrumentation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index f34a990d4f..6d0f16f19b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -75,6 +75,7 @@ def test_uninstrument_app(self): self._client.get("/foobar") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) + # pylint: disable=import-outside-toplevel from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware self._app.add_middleware(HTTPSRedirectMiddleware) From 6959f225f208cad69605831f2ce9f3302d19932c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 8 Jul 2021 09:37:35 -0700 Subject: [PATCH 22/22] comments --- .../instrumentation/aiopg/aiopg_integration.py | 9 ++------- .../src/opentelemetry/instrumentation/aiopg/wrappers.py | 5 +++-- .../opentelemetry/instrumentation/pyramid/__init__.py | 1 - 3 files changed, 5 insertions(+), 10 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 86a600499a..d140c18838 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 @@ -1,10 +1,7 @@ import typing import wrapt -from aiopg.utils import ( # pylint: disable=no-name-in-module - _ContextManager, - _PoolAcquireContextManager, -) +from aiopg.utils import _ContextManager, _PoolAcquireContextManager from opentelemetry.instrumentation.dbapi import ( CursorTracer, @@ -64,9 +61,7 @@ def __init__(self, connection, *args, **kwargs): def cursor(self, *args, **kwargs): coro = self._cursor(*args, **kwargs) - return _ContextManager( # pylint: disable=no-value-for-parameter - coro - ) + return _ContextManager(coro) async def _cursor(self, *args, **kwargs): # pylint: disable=protected-access diff --git a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py index c651d9b780..f786772c49 100644 --- a/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py +++ b/instrumentation/opentelemetry-instrumentation-aiopg/src/opentelemetry/instrumentation/aiopg/wrappers.py @@ -41,6 +41,7 @@ from opentelemetry.instrumentation.aiopg.aiopg_integration import ( AiopgIntegration, + AsyncProxyObject, get_traced_connection_proxy, ) from opentelemetry.instrumentation.aiopg.version import __version__ @@ -153,7 +154,7 @@ def instrument_connection( Returns: An instrumented connection. """ - if isinstance(connection, wrapt.ObjectProxy): + if isinstance(connection, AsyncProxyObject): logger.warning("Connection already instrumented") return connection @@ -177,7 +178,7 @@ def uninstrument_connection(connection): Returns: An uninstrumented connection. """ - if isinstance(connection, wrapt.ObjectProxy): + if isinstance(connection, AsyncProxyObject): return connection.__wrapped__ logger.warning("Connection is not instrumented") diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py index fcf8caae67..401b6bb3cf 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/src/opentelemetry/instrumentation/pyramid/__init__.py @@ -79,7 +79,6 @@ """ import typing -from os import stat from typing import Collection from pyramid.config import Configurator