uninstruemnt existing instances before uninstrumenting fastapi class (#…

…1258)

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 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
 

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

@@ -192,6 +192,8 @@ def instrument_app(
                 meter=meter,
             )
             app._is_instrumented_by_opentelemetry = True
+            if app not in _InstrumentedFastAPI._instrumented_fastapi_apps:
+                _InstrumentedFastAPI._instrumented_fastapi_apps.add(app)
         else:
             _logger.warning(
                 "Attempting to instrument FastAPI app while already instrumented"
@@ -232,6 +234,9 @@ def _instrument(self, **kwargs):
         fastapi.FastAPI = _InstrumentedFastAPI
 
     def _uninstrument(self, **kwargs):
+        for instance in _InstrumentedFastAPI._instrumented_fastapi_apps:
+            self.uninstrument_app(instance)
+        _InstrumentedFastAPI._instrumented_fastapi_apps.clear()
         fastapi.FastAPI = self._original_fastapi
 
 
@@ -242,6 +247,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)
@@ -259,6 +265,11 @@ def __init__(self, *args, **kwargs):
             meter=meter,
         )
         self._is_instrumented_by_opentelemetry = True
+        _InstrumentedFastAPI._instrumented_fastapi_apps.add(self)
+
+    def __del__(self):
+        if self in _InstrumentedFastAPI._instrumented_fastapi_apps:
+            _InstrumentedFastAPI._instrumented_fastapi_apps.remove(self)
 
 
 def _get_route_details(scope):

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 (
@@ -416,6 +411,15 @@ def test_mulitple_way_instrumentation(self):
                 count += 1
         self.assertEqual(count, 1)
 
+    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 tearDown(self):
         self._instrumentor.uninstrument()
         super().tearDown()