From e4674e64e77a901c171f7da204b1d9d15ebbd7a1 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 1 Sep 2022 13:55:13 +0530 Subject: [PATCH 01/11] uninstruemnt existing instances before uninstrumenting fastapi class --- .../opentelemetry/instrumentation/fastapi/__init__.py | 7 +++++++ .../tests/test_fastapi_instrumentation.py | 11 +++++++++++ 2 files changed, 18 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 1c76c8bb50..8c50851d77 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -226,6 +226,8 @@ def _instrument(self, **kwargs): fastapi.FastAPI = _InstrumentedFastAPI def _uninstrument(self, **kwargs): + for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: + FastAPIInstrumentor.uninstrument_app(instance) fastapi.FastAPI = self._original_fastapi @@ -235,6 +237,7 @@ class _InstrumentedFastAPI(fastapi.FastAPI): _server_request_hook: _ServerRequestHookT = None _client_request_hook: _ClientRequestHookT = None _client_response_hook: _ClientResponseHookT = None + _instrumented_fastapi_apps = set() def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -247,6 +250,10 @@ def __init__(self, *args, **kwargs): client_response_hook=_InstrumentedFastAPI._client_response_hook, tracer_provider=_InstrumentedFastAPI._tracer_provider, ) + _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) + + def __del__(self): + _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) def _get_route_details(scope): diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 0d42e7533c..7e9cacfa93 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -89,6 +89,17 @@ def test_instrument_app_with_instrument(self): for span in spans: self.assertIn("/foobar", span.name) + def test_uninstrument_after_instrument(self): + if not isinstance(self, TestAutoInstrumentation): + self._instrumentor.instrument() + app = self._create_fastapi_app() + client = TestClient(app) + client.get("/foobar") + self._instrumentor.uninstrument() + client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + def test_uninstrument_app(self): self._client.get("/foobar") spans = self.memory_exporter.get_finished_spans() From 46542a8b0a342f5b4731f3eb3d2589329d5a03ba Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 1 Sep 2022 14:11:22 +0530 Subject: [PATCH 02/11] fix lint --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8c50851d77..43be148aee 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -251,7 +251,7 @@ def __init__(self, *args, **kwargs): tracer_provider=_InstrumentedFastAPI._tracer_provider, ) _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) - + def __del__(self): _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) From bd6b77984f241c9573a86a3155bfe035484f03be Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Thu, 1 Sep 2022 20:12:33 +0530 Subject: [PATCH 03/11] fix instrument after uninstrument --- .../instrumentation/fastapi/__init__.py | 4 +- .../tests/test_fastapi_instrumentation.py | 40 ++++++++++++++----- 2 files changed, 32 insertions(+), 12 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 43be148aee..08c6a6f785 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -224,10 +224,12 @@ def _instrument(self, **kwargs): else parse_excluded_urls(_excluded_urls) ) fastapi.FastAPI = _InstrumentedFastAPI + for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: + self.instrument_app(instance) def _uninstrument(self, **kwargs): for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: - FastAPIInstrumentor.uninstrument_app(instance) + self.uninstrument_app(instance) fastapi.FastAPI = self._original_fastapi diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 7e9cacfa93..4076a550df 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -89,17 +89,6 @@ def test_instrument_app_with_instrument(self): for span in spans: self.assertIn("/foobar", span.name) - def test_uninstrument_after_instrument(self): - if not isinstance(self, TestAutoInstrumentation): - self._instrumentor.instrument() - app = self._create_fastapi_app() - client = TestClient(app) - client.get("/foobar") - self._instrumentor.uninstrument() - client.get("/foobar") - spans = self.memory_exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - def test_uninstrument_app(self): self._client.get("/foobar") spans = self.memory_exporter.get_finished_spans() @@ -284,6 +273,35 @@ def test_request(self): for span in spans: self.assertEqual(span.resource.attributes["key1"], "value1") self.assertEqual(span.resource.attributes["key2"], "value2") + + def test_uninstrument_after_instrument(self): + app = self._create_fastapi_app() + client = TestClient(app) + client.get("/foobar") + self._instrumentor.uninstrument() + client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + + # def test_instrument_after_uninstrument(self): + # instrumentor = otel_fastapi.FastAPIInstrumentor() + # # tracer_provider = None + # resource = Resource.create({"key1": "value1", "key2": "value2"}) + # tracer_provider, exporter = self.create_tracer_provider(resource=resource) + # instrumentor.instrument(tracer_provider=tracer_provider) + # app = self._create_fastapi_app() + # client = TestClient(app) + # client.get("/foobar") + # spans = self.memory_exporter.get_finished_spans() + # self.assertEqual(len(spans), 3) + # instrumentor.uninstrument() + # client.get("/foobar") + # spans = self.memory_exporter.get_finished_spans() + # self.assertEqual(len(spans), 3) + # instrumentor.instrument(tracer_provider=tracer_provider) + # client.get("/foobar") + # spans = self.memory_exporter.get_finished_spans() + # self.assertEqual(len(spans), 6) def tearDown(self): self._instrumentor.uninstrument() From 192923ad90d565b90c662ce7d9114ff06e90a506 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Mon, 5 Sep 2022 17:32:23 +0530 Subject: [PATCH 04/11] changes but with error --- .../instrumentation/fastapi/__init__.py | 36 ++++++++++---- .../tests/test_fastapi_instrumentation.py | 48 +++++++++++-------- 2 files changed, 55 insertions(+), 29 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 08c6a6f785..41f1e20605 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -140,7 +140,7 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span -from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls +from opentelemetry.util.http import ExcludeList, get_excluded_urls, parse_excluded_urls _excluded_urls_from_env = get_excluded_urls("FASTAPI") _logger = logging.getLogger(__name__) @@ -175,7 +175,7 @@ def instrument_app( if excluded_urls is None: excluded_urls = _excluded_urls_from_env else: - excluded_urls = parse_excluded_urls(excluded_urls) + excluded_urls = excluded_urls if type(excluded_urls) == ExcludeList else parse_excluded_urls(excluded_urls) app.add_middleware( OpenTelemetryMiddleware, @@ -187,6 +187,8 @@ def instrument_app( tracer_provider=tracer_provider, ) app._is_instrumented_by_opentelemetry = True + if not app in _InstrumentedFastAPI._instrumented_fastapi_apps: + _InstrumentedFastAPI._instrumented_fastapi_apps.add(app) else: _logger.warning( "Attempting to instrument FastAPI app while already instrumented" @@ -218,18 +220,31 @@ def _instrument(self, **kwargs): "client_response_hook" ) _excluded_urls = kwargs.get("excluded_urls") - _InstrumentedFastAPI._excluded_urls = ( - _excluded_urls_from_env - if _excluded_urls is None - else parse_excluded_urls(_excluded_urls) - ) + if _excluded_urls is None: + _InstrumentedFastAPI._excluded_urls = _excluded_urls_from_env + elif type(_excluded_urls) == ExcludeList: + _InstrumentedFastAPI._excluded_urls = _excluded_urls + else: + _InstrumentedFastAPI._excluded_urls = parse_excluded_urls(_excluded_urls) fastapi.FastAPI = _InstrumentedFastAPI for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: - self.instrument_app(instance) + self.instrument_app( + instance, + # server_request_hook=_InstrumentedFastAPI._server_request_hook, + # client_request_hook=_InstrumentedFastAPI._client_request_hook, + # client_response_hook=_InstrumentedFastAPI._client_response_hook, + # tracer_provider=_InstrumentedFastAPI._tracer_provider, + # excluded_urls=_InstrumentedFastAPI._excluded_urls, + ) def _uninstrument(self, **kwargs): for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: self.uninstrument_app(instance) + _InstrumentedFastAPI._instrumented_fastapi_apps = { + instance + for instance in _InstrumentedFastAPI._instrumented_fastapi_apps + if type(instance) == _InstrumentedFastAPI + } fastapi.FastAPI = self._original_fastapi @@ -255,6 +270,11 @@ def __init__(self, *args, **kwargs): _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) def __del__(self): + print("-----------") + print("-----------") + print("Yes it got deleted") + print("-----------") + print("-----------") _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 4076a550df..ee665f6fbb 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -273,7 +273,7 @@ def test_request(self): for span in spans: self.assertEqual(span.resource.attributes["key1"], "value1") self.assertEqual(span.resource.attributes["key2"], "value2") - + def test_uninstrument_after_instrument(self): app = self._create_fastapi_app() client = TestClient(app) @@ -282,26 +282,32 @@ def test_uninstrument_after_instrument(self): client.get("/foobar") spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) - - # def test_instrument_after_uninstrument(self): - # instrumentor = otel_fastapi.FastAPIInstrumentor() - # # tracer_provider = None - # resource = Resource.create({"key1": "value1", "key2": "value2"}) - # tracer_provider, exporter = self.create_tracer_provider(resource=resource) - # instrumentor.instrument(tracer_provider=tracer_provider) - # app = self._create_fastapi_app() - # client = TestClient(app) - # client.get("/foobar") - # spans = self.memory_exporter.get_finished_spans() - # self.assertEqual(len(spans), 3) - # instrumentor.uninstrument() - # client.get("/foobar") - # spans = self.memory_exporter.get_finished_spans() - # self.assertEqual(len(spans), 3) - # instrumentor.instrument(tracer_provider=tracer_provider) - # client.get("/foobar") - # spans = self.memory_exporter.get_finished_spans() - # self.assertEqual(len(spans), 6) + + def test_instrument_after_uninstrument(self): + # self._instrumentor.instrument_app(self._app) + # print(self._app.user_middleware) + self._instrumentor.uninstrument() + # self._app = None + # instrumentor = otel_fastapi.FastAPIInstrumentor() + # resource = Resource.create({"key1": "value1", "key2": "value2"}) + # tracer_provider, exporter = self.create_tracer_provider( + # resource=resource + # ) + self._instrumentor.instrument() + # self._app = self._create_fastapi_app() + # selfclient = TestClient(app) + # breakpoint() + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + self._instrumentor.uninstrument() + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 3) + self._instrumentor.instrument() + self._client.get("/foobar") + spans = self.memory_exporter.get_finished_spans() + self.assertEqual(len(spans), 6) def tearDown(self): self._instrumentor.uninstrument() From 17df34878b5b160731d930b680b471a93d1caf45 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 6 Sep 2022 10:05:36 +0530 Subject: [PATCH 05/11] add more uninstrumentation logic --- .../instrumentation/fastapi/__init__.py | 17 ++++------ .../tests/test_fastapi_instrumentation.py | 31 +++++++++---------- 2 files changed, 20 insertions(+), 28 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 41f1e20605..968864e340 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -187,7 +187,7 @@ def instrument_app( tracer_provider=tracer_provider, ) app._is_instrumented_by_opentelemetry = True - if not app in _InstrumentedFastAPI._instrumented_fastapi_apps: + if app not in _InstrumentedFastAPI._instrumented_fastapi_apps: _InstrumentedFastAPI._instrumented_fastapi_apps.add(app) else: _logger.warning( @@ -230,11 +230,11 @@ def _instrument(self, **kwargs): for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: self.instrument_app( instance, - # server_request_hook=_InstrumentedFastAPI._server_request_hook, - # client_request_hook=_InstrumentedFastAPI._client_request_hook, - # client_response_hook=_InstrumentedFastAPI._client_response_hook, - # tracer_provider=_InstrumentedFastAPI._tracer_provider, - # excluded_urls=_InstrumentedFastAPI._excluded_urls, + server_request_hook=_InstrumentedFastAPI._server_request_hook, + client_request_hook=_InstrumentedFastAPI._client_request_hook, + client_response_hook=_InstrumentedFastAPI._client_response_hook, + tracer_provider=_InstrumentedFastAPI._tracer_provider, + excluded_urls=_InstrumentedFastAPI._excluded_urls, ) def _uninstrument(self, **kwargs): @@ -270,11 +270,6 @@ def __init__(self, *args, **kwargs): _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) def __del__(self): - print("-----------") - print("-----------") - print("Yes it got deleted") - print("-----------") - print("-----------") _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index ee665f6fbb..be9eb19025 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -284,29 +284,26 @@ def test_uninstrument_after_instrument(self): self.assertEqual(len(spans), 3) def test_instrument_after_uninstrument(self): - # self._instrumentor.instrument_app(self._app) - # print(self._app.user_middleware) self._instrumentor.uninstrument() - # self._app = None - # instrumentor = otel_fastapi.FastAPIInstrumentor() - # resource = Resource.create({"key1": "value1", "key2": "value2"}) - # tracer_provider, exporter = self.create_tracer_provider( - # resource=resource - # ) - self._instrumentor.instrument() - # self._app = self._create_fastapi_app() - # selfclient = TestClient(app) - # breakpoint() + self._app = None + instrumentor = otel_fastapi.FastAPIInstrumentor() + resource = Resource.create({"key1": "value1", "key2": "value2"}) + tracer_provider, exporter = self.create_tracer_provider( + resource=resource + ) + instrumentor.instrument(tracer_provider=tracer_provider) + self._app = self._create_fastapi_app() + self._client = TestClient(self._app) self._client.get("/foobar") - spans = self.memory_exporter.get_finished_spans() + spans = exporter.get_finished_spans() self.assertEqual(len(spans), 3) - self._instrumentor.uninstrument() + instrumentor.uninstrument() self._client.get("/foobar") - spans = self.memory_exporter.get_finished_spans() + spans = exporter.get_finished_spans() self.assertEqual(len(spans), 3) - self._instrumentor.instrument() + instrumentor.instrument(tracer_provider=tracer_provider) self._client.get("/foobar") - spans = self.memory_exporter.get_finished_spans() + spans = exporter.get_finished_spans() self.assertEqual(len(spans), 6) def tearDown(self): From d48a1bf73c3839c5c7d0ee4b7c31884ab3e6815e Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 6 Sep 2022 10:36:24 +0530 Subject: [PATCH 06/11] fix lint --- .../instrumentation/fastapi/__init__.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 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 968864e340..825262f903 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -140,7 +140,11 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span -from opentelemetry.util.http import ExcludeList, get_excluded_urls, parse_excluded_urls +from opentelemetry.util.http import ( + ExcludeList, + get_excluded_urls, + parse_excluded_urls, +) _excluded_urls_from_env = get_excluded_urls("FASTAPI") _logger = logging.getLogger(__name__) @@ -175,7 +179,11 @@ def instrument_app( if excluded_urls is None: excluded_urls = _excluded_urls_from_env else: - excluded_urls = excluded_urls if type(excluded_urls) == ExcludeList else parse_excluded_urls(excluded_urls) + excluded_urls = ( + excluded_urls + if type(excluded_urls) == ExcludeList + else parse_excluded_urls(excluded_urls) + ) app.add_middleware( OpenTelemetryMiddleware, @@ -225,7 +233,9 @@ def _instrument(self, **kwargs): elif type(_excluded_urls) == ExcludeList: _InstrumentedFastAPI._excluded_urls = _excluded_urls else: - _InstrumentedFastAPI._excluded_urls = parse_excluded_urls(_excluded_urls) + _InstrumentedFastAPI._excluded_urls = parse_excluded_urls( + _excluded_urls + ) fastapi.FastAPI = _InstrumentedFastAPI for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: self.instrument_app( From e979ef9b92701431bf65d958fcec2d1078155d98 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 6 Sep 2022 10:43:47 +0530 Subject: [PATCH 07/11] add changelog --- CHANGELOG.md | 2 ++ .../tests/test_fastapi_instrumentation.py | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a2b306157..e750246843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208)) - `opentelemetry-instrumentation-aiohttp-client` Fix producing additional spans with each newly created ClientSession - ([#1246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1246)) +- Fix uninstrumentation of existing app instances in FastAPI + ([#1258](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1258)) ## [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-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index be9eb19025..b1a2c60bd3 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -285,7 +285,6 @@ def test_uninstrument_after_instrument(self): def test_instrument_after_uninstrument(self): self._instrumentor.uninstrument() - self._app = None instrumentor = otel_fastapi.FastAPIInstrumentor() resource = Resource.create({"key1": "value1", "key2": "value2"}) tracer_provider, exporter = self.create_tracer_provider( From 2a1e08aa4ce53f82f885c448c182b7b69fa07cc6 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 6 Sep 2022 11:37:36 +0530 Subject: [PATCH 08/11] remove type and use isinstance --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 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 825262f903..915e45fb5e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -181,7 +181,7 @@ def instrument_app( else: excluded_urls = ( excluded_urls - if type(excluded_urls) == ExcludeList + if isinstance(excluded_urls, ExcludeList) else parse_excluded_urls(excluded_urls) ) @@ -230,7 +230,7 @@ def _instrument(self, **kwargs): _excluded_urls = kwargs.get("excluded_urls") if _excluded_urls is None: _InstrumentedFastAPI._excluded_urls = _excluded_urls_from_env - elif type(_excluded_urls) == ExcludeList: + elif isinstance(_excluded_urls, ExcludeList): _InstrumentedFastAPI._excluded_urls = _excluded_urls else: _InstrumentedFastAPI._excluded_urls = parse_excluded_urls( @@ -253,7 +253,7 @@ def _uninstrument(self, **kwargs): _InstrumentedFastAPI._instrumented_fastapi_apps = { instance for instance in _InstrumentedFastAPI._instrumented_fastapi_apps - if type(instance) == _InstrumentedFastAPI + if isinstance(instance, _InstrumentedFastAPI) } fastapi.FastAPI = self._original_fastapi From 7cc1cb426a619ef048392efab47b80b7cbbb856c Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 13 Sep 2022 16:58:07 +0530 Subject: [PATCH 09/11] revert changes --- .../instrumentation/fastapi/__init__.py | 37 +++++-------------- .../tests/test_fastapi_instrumentation.py | 35 ++---------------- 2 files changed, 13 insertions(+), 59 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 9ce5143b15..ad50547f7f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -182,11 +182,7 @@ def instrument_app( if excluded_urls is None: excluded_urls = _excluded_urls_from_env else: - excluded_urls = ( - excluded_urls - if isinstance(excluded_urls, ExcludeList) - else parse_excluded_urls(excluded_urls) - ) + excluded_urls = parse_excluded_urls(excluded_urls) meter = get_meter(__name__, __version__, meter_provider) app.add_middleware( @@ -233,34 +229,18 @@ def _instrument(self, **kwargs): "client_response_hook" ) _excluded_urls = kwargs.get("excluded_urls") - if _excluded_urls is None: - _InstrumentedFastAPI._excluded_urls = _excluded_urls_from_env - elif isinstance(_excluded_urls, ExcludeList): - _InstrumentedFastAPI._excluded_urls = _excluded_urls - else: - _InstrumentedFastAPI._excluded_urls = parse_excluded_urls( - _excluded_urls - ) + _InstrumentedFastAPI._excluded_urls = ( + _excluded_urls_from_env + if _excluded_urls is None + else parse_excluded_urls(_excluded_urls) + ) _InstrumentedFastAPI._meter_provider = kwargs.get("meter_provider") fastapi.FastAPI = _InstrumentedFastAPI - for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: - self.instrument_app( - instance, - server_request_hook=_InstrumentedFastAPI._server_request_hook, - client_request_hook=_InstrumentedFastAPI._client_request_hook, - client_response_hook=_InstrumentedFastAPI._client_response_hook, - tracer_provider=_InstrumentedFastAPI._tracer_provider, - excluded_urls=_InstrumentedFastAPI._excluded_urls, - ) def _uninstrument(self, **kwargs): for instance in _InstrumentedFastAPI._instrumented_fastapi_apps: self.uninstrument_app(instance) - _InstrumentedFastAPI._instrumented_fastapi_apps = { - instance - for instance in _InstrumentedFastAPI._instrumented_fastapi_apps - if isinstance(instance, _InstrumentedFastAPI) - } + _InstrumentedFastAPI._instrumented_fastapi_apps.clear() fastapi.FastAPI = self._original_fastapi @@ -292,7 +272,8 @@ def __init__(self, *args, **kwargs): _InstrumentedFastAPI._instrumented_fastapi_apps.add(self) def __del__(self): - _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) + if self in _InstrumentedFastAPI._instrumented_fastapi_apps: + _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self) def _get_route_details(scope): diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 6419190219..fff3578b13 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -274,16 +274,11 @@ def test_metric_uninstruemnt_app(self): self.assertEqual(point.value, 0) def test_metric_uninstrument(self): - # instrumenting class and creating app to send request - self._instrumentor.instrument() - app = self._create_fastapi_app() - client = TestClient(app) - client.get("/foobar") - # uninstrumenting class and creating the app again + if not isinstance(self, TestAutoInstrumentation): + self._instrumentor.instrument() + self._client.get("/foobar") self._instrumentor.uninstrument() - app = self._create_fastapi_app() - client = TestClient(app) - client.get("/foobar") + self._client.get("/foobar") metrics_list = self.memory_metrics_reader.get_metrics_data() for metric in ( @@ -425,28 +420,6 @@ def test_uninstrument_after_instrument(self): spans = self.memory_exporter.get_finished_spans() self.assertEqual(len(spans), 3) - def test_instrument_after_uninstrument(self): - self._instrumentor.uninstrument() - instrumentor = otel_fastapi.FastAPIInstrumentor() - resource = Resource.create({"key1": "value1", "key2": "value2"}) - tracer_provider, exporter = self.create_tracer_provider( - resource=resource - ) - instrumentor.instrument(tracer_provider=tracer_provider) - self._app = self._create_fastapi_app() - self._client = TestClient(self._app) - self._client.get("/foobar") - spans = exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - instrumentor.uninstrument() - self._client.get("/foobar") - spans = exporter.get_finished_spans() - self.assertEqual(len(spans), 3) - instrumentor.instrument(tracer_provider=tracer_provider) - self._client.get("/foobar") - spans = exporter.get_finished_spans() - self.assertEqual(len(spans), 6) - def tearDown(self): self._instrumentor.uninstrument() super().tearDown() From b6a8172f0f4c4d7ec75a242b1359d43943846bb3 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 13 Sep 2022 17:11:37 +0530 Subject: [PATCH 10/11] fix lint --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 1 - 1 file changed, 1 deletion(-) 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 ad50547f7f..26608a180f 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -143,7 +143,6 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span from opentelemetry.util.http import ( - ExcludeList, get_excluded_urls, parse_excluded_urls, ) From 5cea0b8373482dd95e2fe751962aab0ce7f65bc3 Mon Sep 17 00:00:00 2001 From: "anshul.asawa" Date: Tue, 13 Sep 2022 17:15:42 +0530 Subject: [PATCH 11/11] fix generate --- .../src/opentelemetry/instrumentation/fastapi/__init__.py | 5 +---- 1 file changed, 1 insertion(+), 4 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 26608a180f..a4245bab70 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py @@ -142,10 +142,7 @@ def client_response_hook(span: Span, message: dict): from opentelemetry.metrics import get_meter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import Span -from opentelemetry.util.http import ( - get_excluded_urls, - parse_excluded_urls, -) +from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls _excluded_urls_from_env = get_excluded_urls("FASTAPI") _logger = logging.getLogger(__name__)