From 9593eea8085f73ba37773321f441df4fdc51d941 Mon Sep 17 00:00:00 2001 From: Anshul Asawa <35421635+TheAnshul756@users.noreply.github.com> Date: Tue, 20 Sep 2022 23:47:06 +0530 Subject: [PATCH] Uninstruemnt existing instances before uninstrumenting falcon class (#1341) --- CHANGELOG.md | 2 + .../instrumentation/falcon/__init__.py | 54 +++++++++++++++++-- .../tests/test_falcon.py | 12 +++++ 3 files changed, 63 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d27a89531..3629cf1e58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1313](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1313)) - Fix uninstrumentation of existing app instances in FastAPI ([#1258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1258)) +- Fix uninstrumentation of existing app instances in falcon + ([#1341]https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1341) ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08 diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index b123203c77..e4f97bce93 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -194,14 +194,16 @@ def response_hook(span, req, resp): class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): + _instrumented_falcon_apps = set() + def __init__(self, *args, **kwargs): otel_opts = kwargs.pop("_otel_opts", {}) # inject trace middleware - middlewares = kwargs.pop("middleware", []) + self._middlewares_list = kwargs.pop("middleware", []) tracer_provider = otel_opts.pop("tracer_provider", None) - if not isinstance(middlewares, (list, tuple)): - middlewares = [middlewares] + if not isinstance(self._middlewares_list, (list, tuple)): + self._middlewares_list = [self._middlewares_list] self._otel_tracer = trace.get_tracer( __name__, __version__, tracer_provider @@ -215,12 +217,18 @@ def __init__(self, *args, **kwargs): otel_opts.pop("request_hook", None), otel_opts.pop("response_hook", None), ) - middlewares.insert(0, trace_middleware) - kwargs["middleware"] = middlewares + self._middlewares_list.insert(0, trace_middleware) + kwargs["middleware"] = self._middlewares_list self._otel_excluded_urls = get_excluded_urls("FALCON") + self._is_instrumented_by_opentelemetry = True + _InstrumentedFalconAPI._instrumented_falcon_apps.add(self) super().__init__(*args, **kwargs) + def __del__(self): + if self in _InstrumentedFalconAPI._instrumented_falcon_apps: + _InstrumentedFalconAPI._instrumented_falcon_apps.remove(self) + def _handle_exception( self, arg1, arg2, arg3, arg4 ): # pylint: disable=C0103 @@ -229,6 +237,9 @@ def _handle_exception( # Translation layer for handling the changed arg position of "ex" in Falcon > 2 vs # Falcon < 2 + if not self._is_instrumented_by_opentelemetry: + return super()._handle_exception(arg1, arg2, arg3, arg4) + if _falcon_version == 1: ex = arg1 req = arg2 @@ -253,6 +264,9 @@ def __call__(self, env, start_response): if self._otel_excluded_urls.url_disabled(env.get("PATH_INFO", "/")): return super().__call__(env, start_response) + if not self._is_instrumented_by_opentelemetry: + return super().__call__(env, start_response) + start_time = time_ns() span, token = _start_internal_or_server_span( @@ -414,6 +428,33 @@ class FalconInstrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> Collection[str]: return _instruments + def _remove_instrumented_middleware(self, app): + if ( + hasattr(app, "_is_instrumented_by_opentelemetry") + and app._is_instrumented_by_opentelemetry + ): + if _falcon_version == 3: + app._unprepared_middleware = [ + x + for x in app._unprepared_middleware + if not isinstance(x, _TraceMiddleware) + ] + app._middleware = app._prepare_middleware( + app._unprepared_middleware, + independent_middleware=app._independent_middleware, + ) + else: + app._middlewares_list = [ + x + for x in app._middlewares_list + if not isinstance(x, _TraceMiddleware) + ] + app._middleware = falcon.api_helpers.prepare_middleware( + app._middlewares_list, + independent_middleware=app._independent_middleware, + ) + app._is_instrumented_by_opentelemetry = False + def _instrument(self, **opts): self._original_falcon_api = getattr(falcon, _instrument_app) @@ -425,4 +466,7 @@ def __init__(self, *args, **kwargs): setattr(falcon, _instrument_app, FalconAPI) def _uninstrument(self, **kwargs): + for app in _InstrumentedFalconAPI._instrumented_falcon_apps: + self._remove_instrumented_middleware(app) + _InstrumentedFalconAPI._instrumented_falcon_apps.clear() setattr(falcon, _instrument_app, self._original_falcon_api) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 5098937b2a..cf02b52d82 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -242,6 +242,18 @@ def test_traced_not_recording(self): self.assertFalse(mock_span.set_attribute.called) self.assertFalse(mock_span.set_status.called) + def test_uninstrument_after_instrument(self): + self.client().simulate_get(path="/hello") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 1) + + FalconInstrumentor().uninstrument() + self.memory_exporter.clear() + + self.client().simulate_get(path="/hello") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 0) + class TestFalconInstrumentationWithTracerProvider(TestBase): def setUp(self):