From 65b2f8b6903be9ad5602136bce4a7028c1c335ea Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 19 Sep 2022 13:47:33 +0530 Subject: [PATCH 1/3] add uninstrument app --- .../instrumentation/falcon/__init__.py | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) 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..8b6442683a 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -194,6 +194,8 @@ def response_hook(span, req, resp): class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): + _instrumented_falcon_apis = set() + def __init__(self, *args, **kwargs): otel_opts = kwargs.pop("_otel_opts", {}) @@ -219,7 +221,13 @@ def __init__(self, *args, **kwargs): kwargs["middleware"] = middlewares self._otel_excluded_urls = get_excluded_urls("FALCON") + self._is_instrumented_by_opentelemetry = True + _InstrumentedFalconAPI._instrumented_falcon_apis.add(self) super().__init__(*args, **kwargs) + + def __del__(self): + if self in _InstrumentedFalconAPI._instrumented_falcon_apis: + _InstrumentedFalconAPI._instrumented_falcon_apis.remove(self) def _handle_exception( self, arg1, arg2, arg3, arg4 @@ -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( @@ -410,10 +424,24 @@ class FalconInstrumentor(BaseInstrumentor): See `BaseInstrumentor` """ - def instrumentation_dependencies(self) -> Collection[str]: return _instruments + @staticmethod + def uninstrument_api(api): + if hasattr(api, "_is_instrumented_by_opentelemetry") and api._is_instrumented_by_opentelemetry: + api._unprepared_middleware = [ + x + for x in api._unprepared_middleware + if not isinstance(x, _TraceMiddleware) + ] + api._middleware = api._prepare_middleware( + api._unprepared_middleware, + independent_middleware=api._independent_middleware, + ) + api._is_instrumented_by_opentelemetry = False + + def _instrument(self, **opts): self._original_falcon_api = getattr(falcon, _instrument_app) @@ -425,4 +453,6 @@ def __init__(self, *args, **kwargs): setattr(falcon, _instrument_app, FalconAPI) def _uninstrument(self, **kwargs): + for api in _InstrumentedFalconAPI._instrumented_falcon_apis: + self.uninstrument_api(api) setattr(falcon, _instrument_app, self._original_falcon_api) From fa413b5bcd830d76f1b2ecd6153437649d69e102 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 19 Sep 2022 15:03:07 +0530 Subject: [PATCH 2/3] fix uninstrumentation in falcon --- .../instrumentation/falcon/__init__.py | 27 ++++++++++--------- .../tests/test_falcon.py | 12 +++++++++ 2 files changed, 26 insertions(+), 13 deletions(-) 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 8b6442683a..ca24f863be 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -194,7 +194,7 @@ def response_hook(span, req, resp): class _InstrumentedFalconAPI(getattr(falcon, _instrument_app)): - _instrumented_falcon_apis = set() + _instrumented_falcon_apps = set() def __init__(self, *args, **kwargs): otel_opts = kwargs.pop("_otel_opts", {}) @@ -222,12 +222,12 @@ def __init__(self, *args, **kwargs): self._otel_excluded_urls = get_excluded_urls("FALCON") self._is_instrumented_by_opentelemetry = True - _InstrumentedFalconAPI._instrumented_falcon_apis.add(self) + _InstrumentedFalconAPI._instrumented_falcon_apps.add(self) super().__init__(*args, **kwargs) def __del__(self): - if self in _InstrumentedFalconAPI._instrumented_falcon_apis: - _InstrumentedFalconAPI._instrumented_falcon_apis.remove(self) + if self in _InstrumentedFalconAPI._instrumented_falcon_apps: + _InstrumentedFalconAPI._instrumented_falcon_apps.remove(self) def _handle_exception( self, arg1, arg2, arg3, arg4 @@ -428,18 +428,18 @@ def instrumentation_dependencies(self) -> Collection[str]: return _instruments @staticmethod - def uninstrument_api(api): - if hasattr(api, "_is_instrumented_by_opentelemetry") and api._is_instrumented_by_opentelemetry: - api._unprepared_middleware = [ + def uninstrument_app(app): + if hasattr(app, "_is_instrumented_by_opentelemetry") and app._is_instrumented_by_opentelemetry: + app._unprepared_middleware = [ x - for x in api._unprepared_middleware + for x in app._unprepared_middleware if not isinstance(x, _TraceMiddleware) ] - api._middleware = api._prepare_middleware( - api._unprepared_middleware, - independent_middleware=api._independent_middleware, + app._middleware = app._prepare_middleware( + app._unprepared_middleware, + independent_middleware=app._independent_middleware, ) - api._is_instrumented_by_opentelemetry = False + app._is_instrumented_by_opentelemetry = False def _instrument(self, **opts): @@ -453,6 +453,7 @@ def __init__(self, *args, **kwargs): setattr(falcon, _instrument_app, FalconAPI) def _uninstrument(self, **kwargs): - for api in _InstrumentedFalconAPI._instrumented_falcon_apis: + for api in _InstrumentedFalconAPI._instrumented_falcon_apps: self.uninstrument_api(api) + _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): From 98f89cb89ed9219bcd688827d84ffbb4160ab9f8 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 19 Sep 2022 16:20:57 +0530 Subject: [PATCH 3/3] fix uninstrumentation in falcon 1 and 2 --- CHANGELOG.md | 2 + .../instrumentation/falcon/__init__.py | 55 ++++++++++++------- 2 files changed, 36 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 539b0f293e..d6bf357ba7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - ([#1246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1246)) - Add _is_openetlemetry_instrumented check in _InstrumentedFastAPI class ([#1313](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1313)) +- 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 ca24f863be..e4f97bce93 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -200,10 +200,10 @@ 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 @@ -217,14 +217,14 @@ 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) @@ -424,23 +424,36 @@ class FalconInstrumentor(BaseInstrumentor): See `BaseInstrumentor` """ + def instrumentation_dependencies(self) -> Collection[str]: return _instruments - @staticmethod - def uninstrument_app(app): - if hasattr(app, "_is_instrumented_by_opentelemetry") and app._is_instrumented_by_opentelemetry: - 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, - ) + 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) @@ -453,7 +466,7 @@ def __init__(self, *args, **kwargs): setattr(falcon, _instrument_app, FalconAPI) def _uninstrument(self, **kwargs): - for api in _InstrumentedFalconAPI._instrumented_falcon_apps: - self.uninstrument_api(api) + 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)