Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uninstruemnt existing instances before uninstrumenting fastapi class #1258

Merged
merged 26 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e4674e6
uninstruemnt existing instances before uninstrumenting fastapi class
TheAnshul756 Sep 1, 2022
46542a8
fix lint
TheAnshul756 Sep 1, 2022
bd6b779
fix instrument after uninstrument
TheAnshul756 Sep 1, 2022
192923a
changes but with error
TheAnshul756 Sep 5, 2022
17df348
add more uninstrumentation logic
TheAnshul756 Sep 6, 2022
efd105d
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 6, 2022
d48a1bf
fix lint
TheAnshul756 Sep 6, 2022
28434be
Merge branch 'fix-uninstrument-fastapi' of https://github.com/TheAnsh…
TheAnshul756 Sep 6, 2022
e979ef9
add changelog
TheAnshul756 Sep 6, 2022
2a1e08a
remove type and use isinstance
TheAnshul756 Sep 6, 2022
000686c
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 7, 2022
e0ac2c8
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 8, 2022
db146ca
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 9, 2022
ab5a86e
Merge branch 'main' of https://github.com/TheAnshul756/opentelemetry-…
TheAnshul756 Sep 9, 2022
af5befa
Merge branch 'main' of https://github.com/TheAnshul756/opentelemetry-…
TheAnshul756 Sep 9, 2022
7682c2c
Merge branch 'fix-uninstrument-fastapi' of https://github.com/TheAnsh…
TheAnshul756 Sep 9, 2022
93b460d
Merge branch 'main' of https://github.com/TheAnshul756/opentelemetry-…
TheAnshul756 Sep 13, 2022
7cc1cb4
revert changes
TheAnshul756 Sep 13, 2022
f645c2b
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 13, 2022
b6a8172
fix lint
TheAnshul756 Sep 13, 2022
7a30234
Merge branch 'fix-uninstrument-fastapi' of https://github.com/TheAnsh…
TheAnshul756 Sep 13, 2022
5cea0b8
fix generate
TheAnshul756 Sep 13, 2022
7f64a44
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 15, 2022
e2e63d9
Merge branch 'main' into fix-uninstrument-fastapi
srikanthccv Sep 15, 2022
90c76da
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 16, 2022
4082be2
Merge branch 'main' into fix-uninstrument-fastapi
TheAnshul756 Sep 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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


Expand All @@ -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)
Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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()
Expand Down