From 8a6716cd0ff2ad87634c5165590856b552c2493b Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 10:56:21 -0700 Subject: [PATCH 1/9] ep test passes --- .../tests/test_fastapi_instrumentation.py | 88 ++++++++++++++++++- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 634c74af6b..fa2fa859e6 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -14,10 +14,18 @@ # pylint: disable=too-many-lines +from pkg_resources import ( + DistributionNotFound, + EntryPoint, + get_distribution, + iter_entry_points +) import unittest from timeit import default_timer -from unittest.mock import patch - +from unittest.mock import ( + Mock, + patch +) import fastapi from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware from fastapi.responses import JSONResponse @@ -25,6 +33,7 @@ import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry import trace +from opentelemetry.instrumentation.auto_instrumentation._load import _load_instrumentors from opentelemetry.instrumentation._semconv import ( OTEL_SEMCONV_STABILITY_OPT_IN, _OpenTelemetrySemanticConventionStability, @@ -1024,6 +1033,29 @@ def client_response_hook(send_span, scope, message): ) +def get_distribution_with_fastapi(*args, **kwargs): + print("args: %s" % args) + print("kwargs: %s" % kwargs) + dist = args[0] + if dist == "fastapi ~= 0.58": + return None #Value does not matter. Only whether an exception is thrown + elif "fastapi-slim" in dist: + raise DistributionNotFound() + else: + return None #TODO: may want to change to not found + +def get_distribution_without_fastapi(*args, **kwargs): + print("args: %s" % args) + print("kwargs: %s" % kwargs) + dist = args[0] + if dist == "fastapi ~= 0.58": + return DistributionNotFound() + elif "fastapi-slim" in dist: + raise DistributionNotFound() + else: + return None #TODO: may want to change to not found + + class TestAutoInstrumentation(TestBaseAutoFastAPI): """Test the auto-instrumented variant @@ -1031,6 +1063,58 @@ class TestAutoInstrumentation(TestBaseAutoFastAPI): to both. """ + entry_point = EntryPoint.parse('fastapi = opentelemetry.instrumentation.fastapi:FastAPIInstrumentor') + + def test_entry_point_exists(self): + print("JEREVOSS eps") + for ep in iter_entry_points("opentelemetry_instrumentor"): + print("JEREVOSS ep: %s" % ep) + # if ep == TestAutoInstrumentation.entry_point: Doesn't work + # if ep.equals(TestAutoInstrumentation.entry_point): + if ( + ep.dist.key == 'opentelemetry-instrumentation-fastapi' and + ep.module_name == 'opentelemetry.instrumentation.fastapi' and + ep.attrs == ('FastAPIInstrumentor',) and + ep.name == 'fastapi' + ): + print("JEREVOSS found ep") + return + eps = iter_entry_points("opentelemetry_instrumentor") + ep = next(ep) + self.assertEquals(sep.dist.key, 'opentelemetry-instrumentation-fastapi') + self.assertEquals(ep.module_name, 'opentelemetry.instrumentation.fastapi') + self.assertEquals(ep.attrs, ('FastAPIInstrumentor',)) + self.assertEquals(ep.name, 'fastapi') + self.assertIsNone(next(ep, None)) + self.fail("Entry point not found") + + # @patch("pkg_resources.get_distribution", side_effect=get_distribution_with_fastapi) + # def test_instruments_with_fastapi_installed(self, mock_get_distribution): + # # Need to use matchers + # # mock_get_distribution.side_effect = TestAutoInstrumentationget_distribution_with_fastapi + # # mock_get_distribution("foobar").side_effect = DistributionNotFound() + # # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() + # # mock_get_distribution("fastapi ~= 0.58").return_value = None #Value does not matter. Only whether an exception is thrown + # mock_distro = Mock() + # _load_instrumentors(mock_distro) + # print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) + # # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) + # # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") + # mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) + + # @patch("pkg_resources.get_distribution", side_effect=get_distribution_without_fastapi) + # def test_instruments_without_fastapi_installed(self, mock_get_distribution): + # # mock_get_distribution.side_effect = get_distribution_without_fastapi + # # mock_get_distribution("foobar").side_effect = DistributionNotFound() + # # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() + # # mock_get_distribution("fastapi ~= 0.58").side_effect = DistributionNotFound() + # mock_distro = Mock() + # _load_instrumentors(mock_distro) + # print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) + # # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) + # # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") + # mock_distro.load_instrumentor.assert_not_called() + def _create_app(self): # instrumentation is handled by the instrument call resource = Resource.create({"key1": "value1", "key2": "value2"}) From 9a7dc72ccf35449258f8b1e5dd1ee7222f7b31ac Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 11:40:40 -0700 Subject: [PATCH 2/9] cleaned ep test --- .../tests/test_fastapi_instrumentation.py | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index fa2fa859e6..18df02207e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1066,27 +1066,13 @@ class TestAutoInstrumentation(TestBaseAutoFastAPI): entry_point = EntryPoint.parse('fastapi = opentelemetry.instrumentation.fastapi:FastAPIInstrumentor') def test_entry_point_exists(self): - print("JEREVOSS eps") - for ep in iter_entry_points("opentelemetry_instrumentor"): - print("JEREVOSS ep: %s" % ep) - # if ep == TestAutoInstrumentation.entry_point: Doesn't work - # if ep.equals(TestAutoInstrumentation.entry_point): - if ( - ep.dist.key == 'opentelemetry-instrumentation-fastapi' and - ep.module_name == 'opentelemetry.instrumentation.fastapi' and - ep.attrs == ('FastAPIInstrumentor',) and - ep.name == 'fastapi' - ): - print("JEREVOSS found ep") - return eps = iter_entry_points("opentelemetry_instrumentor") - ep = next(ep) - self.assertEquals(sep.dist.key, 'opentelemetry-instrumentation-fastapi') + ep = next(eps) + self.assertEquals(ep.dist.key, 'opentelemetry-instrumentation-fastapi') self.assertEquals(ep.module_name, 'opentelemetry.instrumentation.fastapi') self.assertEquals(ep.attrs, ('FastAPIInstrumentor',)) self.assertEquals(ep.name, 'fastapi') - self.assertIsNone(next(ep, None)) - self.fail("Entry point not found") + self.assertIsNone(next(eps, None)) # @patch("pkg_resources.get_distribution", side_effect=get_distribution_with_fastapi) # def test_instruments_with_fastapi_installed(self, mock_get_distribution): From ab1e1c2a7098523fdb65aa9423d00bc74bd48866 Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 12:26:11 -0700 Subject: [PATCH 3/9] Corrected mock paths. --- .../tests/test_fastapi_instrumentation.py | 76 ++++++++++++------- 1 file changed, 49 insertions(+), 27 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 18df02207e..f271493f4b 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -32,6 +32,10 @@ from fastapi.testclient import TestClient import opentelemetry.instrumentation.fastapi as otel_fastapi +from opentelemetry.instrumentation.dependencies import ( + get_dist_dependency_conflicts, + get_dependency_conflicts +) from opentelemetry import trace from opentelemetry.instrumentation.auto_instrumentation._load import _load_instrumentors from opentelemetry.instrumentation._semconv import ( @@ -1068,38 +1072,56 @@ class TestAutoInstrumentation(TestBaseAutoFastAPI): def test_entry_point_exists(self): eps = iter_entry_points("opentelemetry_instrumentor") ep = next(eps) - self.assertEquals(ep.dist.key, 'opentelemetry-instrumentation-fastapi') - self.assertEquals(ep.module_name, 'opentelemetry.instrumentation.fastapi') - self.assertEquals(ep.attrs, ('FastAPIInstrumentor',)) - self.assertEquals(ep.name, 'fastapi') + self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') + self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') + self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) + self.assertEqual(ep.name, 'fastapi') self.assertIsNone(next(eps, None)) # @patch("pkg_resources.get_distribution", side_effect=get_distribution_with_fastapi) - # def test_instruments_with_fastapi_installed(self, mock_get_distribution): - # # Need to use matchers - # # mock_get_distribution.side_effect = TestAutoInstrumentationget_distribution_with_fastapi - # # mock_get_distribution("foobar").side_effect = DistributionNotFound() - # # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() - # # mock_get_distribution("fastapi ~= 0.58").return_value = None #Value does not matter. Only whether an exception is thrown - # mock_distro = Mock() - # _load_instrumentors(mock_distro) - # print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) - # # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) - # # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") - # mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) + @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") + @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") + @patch("opentelemetry.instrumentation.dependencies.get_distribution") + def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): + # Need to use matchers + # mock_get_distribution.side_effect = TestAutoInstrumentationget_distribution_with_fastapi + # mock_get_distribution("foobar").side_effect = DistributionNotFound() + # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() + # mock_get_distribution("fastapi ~= 0.58").return_value = None #Value does not matter. Only whether an exception is thrown + mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts + mock_get_dependency_conflicts.side_effect = get_dependency_conflicts + mock_get_distribution.side_affect = get_distribution_with_fastapi + mock_distro = Mock() + _load_instrumentors(mock_distro) + print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) + print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) + print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) + mock_get_distribution.assert_called_once_with("fastapi~=0.58") + # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) + # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") + mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) # @patch("pkg_resources.get_distribution", side_effect=get_distribution_without_fastapi) - # def test_instruments_without_fastapi_installed(self, mock_get_distribution): - # # mock_get_distribution.side_effect = get_distribution_without_fastapi - # # mock_get_distribution("foobar").side_effect = DistributionNotFound() - # # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() - # # mock_get_distribution("fastapi ~= 0.58").side_effect = DistributionNotFound() - # mock_distro = Mock() - # _load_instrumentors(mock_distro) - # print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) - # # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) - # # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") - # mock_distro.load_instrumentor.assert_not_called() + @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") + @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") + @patch("opentelemetry.instrumentation.dependencies.get_distribution") + def test_instruments_without_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): + # mock_get_distribution.side_effect = get_distribution_without_fastapi + # mock_get_distribution("foobar").side_effect = DistributionNotFound() + # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() + # mock_get_distribution("fastapi ~= 0.58").side_effect = DistributionNotFound() + mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts + mock_get_dependency_conflicts.side_effect = get_dependency_conflicts + mock_get_distribution.side_affect = get_distribution_without_fastapi + mock_distro = Mock() + _load_instrumentors(mock_distro) + print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) + print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) + print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) + mock_get_distribution.assert_called_once_with("fastapi~=0.58") + # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) + # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") + mock_distro.load_instrumentor.assert_not_called() def _create_app(self): # instrumentation is handled by the instrument call From d478e178386686b96f843f17e363bfde6a9b7f8c Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 14:02:57 -0700 Subject: [PATCH 4/9] with installed works --- .../tests/test_fastapi_instrumentation.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index f271493f4b..065a974bc0 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1041,7 +1041,8 @@ def get_distribution_with_fastapi(*args, **kwargs): print("args: %s" % args) print("kwargs: %s" % kwargs) dist = args[0] - if dist == "fastapi ~= 0.58": + print("JEREVOSS get_distribution_with_fastapi: dist: %s" % s) + if dist == "fastapi~=0.58": return None #Value does not matter. Only whether an exception is thrown elif "fastapi-slim" in dist: raise DistributionNotFound() @@ -1052,7 +1053,8 @@ def get_distribution_without_fastapi(*args, **kwargs): print("args: %s" % args) print("kwargs: %s" % kwargs) dist = args[0] - if dist == "fastapi ~= 0.58": + print("JEREVOSS get_distribution_without_fastapi: dist: %s" % s) + if dist == "fastapi~=0.58": return DistributionNotFound() elif "fastapi-slim" in dist: raise DistributionNotFound() @@ -1098,8 +1100,15 @@ def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_ge print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) mock_get_distribution.assert_called_once_with("fastapi~=0.58") # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) - # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") - mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) + self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1) + call_args = mock_distro.load_instrumentor.call_args.args + print("JEREVOSS call_args: %s" % call_args) + ep = call_args[0] + self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') + self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') + self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) + self.assertEqual(ep.name, 'fastapi') + # mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) Doesn't work # @patch("pkg_resources.get_distribution", side_effect=get_distribution_without_fastapi) @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") @@ -1113,12 +1122,17 @@ def test_instruments_without_fastapi_installed(self, mock_get_distribution, mock mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts mock_get_dependency_conflicts.side_effect = get_dependency_conflicts mock_get_distribution.side_affect = get_distribution_without_fastapi + with self.assertRaises(DistributionNotFound): + mock_get_distribution("fastapi~=0.58") mock_distro = Mock() _load_instrumentors(mock_distro) print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) mock_get_distribution.assert_called_once_with("fastapi~=0.58") + self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 0) + call_args = mock_distro.load_instrumentor.call_args.args + print("JEREVOSS call_args: %s" % call_args) # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") mock_distro.load_instrumentor.assert_not_called() From d11d668bd68f803014d7ac707f690f6482864bcc Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 14:36:50 -0700 Subject: [PATCH 5/9] tests pass --- .../tests/test_fastapi_instrumentation.py | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 065a974bc0..973ef87432 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1038,10 +1038,11 @@ def client_response_hook(send_span, scope, message): def get_distribution_with_fastapi(*args, **kwargs): + print("JEREVOSS get_distribution_with_fastapi") print("args: %s" % args) print("kwargs: %s" % kwargs) dist = args[0] - print("JEREVOSS get_distribution_with_fastapi: dist: %s" % s) + print("JEREVOSS get_distribution_with_fastapi: dist: %s" % dist) if dist == "fastapi~=0.58": return None #Value does not matter. Only whether an exception is thrown elif "fastapi-slim" in dist: @@ -1050,15 +1051,18 @@ def get_distribution_with_fastapi(*args, **kwargs): return None #TODO: may want to change to not found def get_distribution_without_fastapi(*args, **kwargs): + print("JEREVOSS get_distribution_without_fastapi") print("args: %s" % args) print("kwargs: %s" % kwargs) dist = args[0] - print("JEREVOSS get_distribution_without_fastapi: dist: %s" % s) + print("JEREVOSS get_distribution_without_fastapi: dist: %s" % dist) if dist == "fastapi~=0.58": - return DistributionNotFound() + print("JEREVOSS get_distribution_without_fastapi IF: dist: %s" % dist) + raise DistributionNotFound() elif "fastapi-slim" in dist: raise DistributionNotFound() else: + print("JEREVOSS get_distribution_without_fastapi ELSE: dist: %s" % dist) return None #TODO: may want to change to not found @@ -1092,7 +1096,7 @@ def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_ge # mock_get_distribution("fastapi ~= 0.58").return_value = None #Value does not matter. Only whether an exception is thrown mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts mock_get_dependency_conflicts.side_effect = get_dependency_conflicts - mock_get_distribution.side_affect = get_distribution_with_fastapi + mock_get_distribution.side_effect = get_distribution_with_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) @@ -1101,9 +1105,12 @@ def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_ge mock_get_distribution.assert_called_once_with("fastapi~=0.58") # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1) - call_args = mock_distro.load_instrumentor.call_args.args - print("JEREVOSS call_args: %s" % call_args) - ep = call_args[0] + call_args = mock_distro.load_instrumentor.call_args + call_args_args = call_args.args + print("JEREVOSS call_args: %s" % str(call_args)) + print(call_args) + print("JEREVOSS call_args_args: %s" % call_args_args) + ep = call_args_args[0] self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) @@ -1121,18 +1128,18 @@ def test_instruments_without_fastapi_installed(self, mock_get_distribution, mock # mock_get_distribution("fastapi ~= 0.58").side_effect = DistributionNotFound() mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts mock_get_dependency_conflicts.side_effect = get_dependency_conflicts - mock_get_distribution.side_affect = get_distribution_without_fastapi - with self.assertRaises(DistributionNotFound): - mock_get_distribution("fastapi~=0.58") + mock_get_distribution.side_effect = get_distribution_without_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) + print("JEREVOSS mock_get_distribution.side_effect: %s" % mock_get_distribution.side_effect) mock_get_distribution.assert_called_once_with("fastapi~=0.58") + print("-----\nJEREVOSS about to test error") + with self.assertRaises(DistributionNotFound): + mock_get_distribution("fastapi~=0.58") self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 0) - call_args = mock_distro.load_instrumentor.call_args.args - print("JEREVOSS call_args: %s" % call_args) # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") mock_distro.load_instrumentor.assert_not_called() From bd6254605adb2eff602243edd6ff6d6269dc0e36 Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 14:43:49 -0700 Subject: [PATCH 6/9] Clean up --- .../tests/test_fastapi_instrumentation.py | 54 ++----------------- 1 file changed, 4 insertions(+), 50 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 973ef87432..9a01908831 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -1038,32 +1038,13 @@ def client_response_hook(send_span, scope, message): def get_distribution_with_fastapi(*args, **kwargs): - print("JEREVOSS get_distribution_with_fastapi") - print("args: %s" % args) - print("kwargs: %s" % kwargs) dist = args[0] - print("JEREVOSS get_distribution_with_fastapi: dist: %s" % dist) if dist == "fastapi~=0.58": return None #Value does not matter. Only whether an exception is thrown - elif "fastapi-slim" in dist: - raise DistributionNotFound() - else: - return None #TODO: may want to change to not found + raise DistributionNotFound() def get_distribution_without_fastapi(*args, **kwargs): - print("JEREVOSS get_distribution_without_fastapi") - print("args: %s" % args) - print("kwargs: %s" % kwargs) - dist = args[0] - print("JEREVOSS get_distribution_without_fastapi: dist: %s" % dist) - if dist == "fastapi~=0.58": - print("JEREVOSS get_distribution_without_fastapi IF: dist: %s" % dist) - raise DistributionNotFound() - elif "fastapi-slim" in dist: - raise DistributionNotFound() - else: - print("JEREVOSS get_distribution_without_fastapi ELSE: dist: %s" % dist) - return None #TODO: may want to change to not found + raise DistributionNotFound() class TestAutoInstrumentation(TestBaseAutoFastAPI): @@ -1084,64 +1065,37 @@ def test_entry_point_exists(self): self.assertEqual(ep.name, 'fastapi') self.assertIsNone(next(eps, None)) - # @patch("pkg_resources.get_distribution", side_effect=get_distribution_with_fastapi) @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_distribution") def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): - # Need to use matchers - # mock_get_distribution.side_effect = TestAutoInstrumentationget_distribution_with_fastapi - # mock_get_distribution("foobar").side_effect = DistributionNotFound() - # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() - # mock_get_distribution("fastapi ~= 0.58").return_value = None #Value does not matter. Only whether an exception is thrown mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts mock_get_dependency_conflicts.side_effect = get_dependency_conflicts mock_get_distribution.side_effect = get_distribution_with_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) - print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) - print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) - print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) mock_get_distribution.assert_called_once_with("fastapi~=0.58") - # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1) - call_args = mock_distro.load_instrumentor.call_args - call_args_args = call_args.args - print("JEREVOSS call_args: %s" % str(call_args)) - print(call_args) - print("JEREVOSS call_args_args: %s" % call_args_args) - ep = call_args_args[0] + args = mock_distro.load_instrumentor.call_args.args + ep = args[0] self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) self.assertEqual(ep.name, 'fastapi') - # mock_distro.load_instrumentor.assert_called_once_with(self.entry_point, skip_dep_check=True) Doesn't work - # @patch("pkg_resources.get_distribution", side_effect=get_distribution_without_fastapi) @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_distribution") def test_instruments_without_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): - # mock_get_distribution.side_effect = get_distribution_without_fastapi - # mock_get_distribution("foobar").side_effect = DistributionNotFound() - # mock_get_distribution("fastapi-slim ~= 0.111").side_effect = DistributionNotFound() - # mock_get_distribution("fastapi ~= 0.58").side_effect = DistributionNotFound() mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts mock_get_dependency_conflicts.side_effect = get_dependency_conflicts mock_get_distribution.side_effect = get_distribution_without_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) - print("JEREVOSS mock_get_dist_dependency_conflicts.call_args_list: %s" % mock_get_dist_dependency_conflicts.call_args_list) - print("JEREVOSS mock_get_dependency_conflicts.call_args_list: %s" % mock_get_dependency_conflicts.call_args_list) - print("JEREVOSS mock_get_distribution.call_args_list: %s" % mock_get_distribution.call_args_list) - print("JEREVOSS mock_get_distribution.side_effect: %s" % mock_get_distribution.side_effect) mock_get_distribution.assert_called_once_with("fastapi~=0.58") - print("-----\nJEREVOSS about to test error") with self.assertRaises(DistributionNotFound): mock_get_distribution("fastapi~=0.58") self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 0) - # print("JEREVOSS mock_get_distribution().call_args_list: %s" % mock_get_distribution().call_args_list) - # mock_get_distribution.assert_called_once_with("fastapi ~= 0.58") mock_distro.load_instrumentor.assert_not_called() def _create_app(self): From 7e3003cca1f16d63018cad7525639ee12b429e25 Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 14:54:56 -0700 Subject: [PATCH 7/9] Clean up --- .../tests/test_fastapi_instrumentation.py | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 9a01908831..8cdd26af7a 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -16,15 +16,13 @@ from pkg_resources import ( DistributionNotFound, - EntryPoint, - get_distribution, - iter_entry_points + iter_entry_points, ) import unittest from timeit import default_timer from unittest.mock import ( Mock, - patch + patch, ) import fastapi from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware @@ -32,10 +30,6 @@ from fastapi.testclient import TestClient import opentelemetry.instrumentation.fastapi as otel_fastapi -from opentelemetry.instrumentation.dependencies import ( - get_dist_dependency_conflicts, - get_dependency_conflicts -) from opentelemetry import trace from opentelemetry.instrumentation.auto_instrumentation._load import _load_instrumentors from opentelemetry.instrumentation._semconv import ( @@ -1054,8 +1048,6 @@ class TestAutoInstrumentation(TestBaseAutoFastAPI): to both. """ - entry_point = EntryPoint.parse('fastapi = opentelemetry.instrumentation.fastapi:FastAPIInstrumentor') - def test_entry_point_exists(self): eps = iter_entry_points("opentelemetry_instrumentor") ep = next(eps) @@ -1065,12 +1057,8 @@ def test_entry_point_exists(self): self.assertEqual(ep.name, 'fastapi') self.assertIsNone(next(eps, None)) - @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") - @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_distribution") - def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): - mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts - mock_get_dependency_conflicts.side_effect = get_dependency_conflicts + def test_instruments_with_fastapi_installed(self, mock_get_distribution): mock_get_distribution.side_effect = get_distribution_with_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) @@ -1083,12 +1071,8 @@ def test_instruments_with_fastapi_installed(self, mock_get_distribution, mock_ge self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) self.assertEqual(ep.name, 'fastapi') - @patch("opentelemetry.instrumentation.auto_instrumentation._load.get_dist_dependency_conflicts") - @patch("opentelemetry.instrumentation.dependencies.get_dependency_conflicts") @patch("opentelemetry.instrumentation.dependencies.get_distribution") - def test_instruments_without_fastapi_installed(self, mock_get_distribution, mock_get_dependency_conflicts, mock_get_dist_dependency_conflicts): - mock_get_dist_dependency_conflicts.side_effect = get_dist_dependency_conflicts - mock_get_dependency_conflicts.side_effect = get_dependency_conflicts + def test_instruments_without_fastapi_installed(self, mock_get_distribution): mock_get_distribution.side_effect = get_distribution_without_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) From 1b913d2db4214c7127d7d5825d78e7ea4044dbed Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 15:01:14 -0700 Subject: [PATCH 8/9] lint --- .../tests/test_fastapi_instrumentation.py | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 8cdd26af7a..003ccbc1f8 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -31,7 +31,9 @@ import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry import trace -from opentelemetry.instrumentation.auto_instrumentation._load import _load_instrumentors +from opentelemetry.instrumentation.auto_instrumentation._load import ( + _load_instrumentors, +) from opentelemetry.instrumentation._semconv import ( OTEL_SEMCONV_STABILITY_OPT_IN, _OpenTelemetrySemanticConventionStability, @@ -1034,9 +1036,11 @@ def client_response_hook(send_span, scope, message): def get_distribution_with_fastapi(*args, **kwargs): dist = args[0] if dist == "fastapi~=0.58": - return None #Value does not matter. Only whether an exception is thrown + # Value does not matter. Only whether an exception is thrown + return None raise DistributionNotFound() + def get_distribution_without_fastapi(*args, **kwargs): raise DistributionNotFound() @@ -1051,10 +1055,12 @@ class TestAutoInstrumentation(TestBaseAutoFastAPI): def test_entry_point_exists(self): eps = iter_entry_points("opentelemetry_instrumentor") ep = next(eps) - self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') - self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') - self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) - self.assertEqual(ep.name, 'fastapi') + self.assertEqual(ep.dist.key, "opentelemetry-instrumentation-fastapi") + self.assertEqual( + ep.module_name, "opentelemetry.instrumentation.fastapi" + ) + self.assertEqual(ep.attrs, ("FastAPIInstrumentor",)) + self.assertEqual(ep.name, "fastapi") self.assertIsNone(next(eps, None)) @patch("opentelemetry.instrumentation.dependencies.get_distribution") @@ -1066,13 +1072,17 @@ def test_instruments_with_fastapi_installed(self, mock_get_distribution): self.assertEqual(len(mock_distro.load_instrumentor.call_args_list), 1) args = mock_distro.load_instrumentor.call_args.args ep = args[0] - self.assertEqual(ep.dist.key, 'opentelemetry-instrumentation-fastapi') - self.assertEqual(ep.module_name, 'opentelemetry.instrumentation.fastapi') - self.assertEqual(ep.attrs, ('FastAPIInstrumentor',)) - self.assertEqual(ep.name, 'fastapi') + self.assertEqual(ep.dist.key, "opentelemetry-instrumentation-fastapi") + self.assertEqual( + ep.module_name, "opentelemetry.instrumentation.fastapi" + ) + self.assertEqual(ep.attrs, ("FastAPIInstrumentor",)) + self.assertEqual(ep.name, "fastapi") @patch("opentelemetry.instrumentation.dependencies.get_distribution") - def test_instruments_without_fastapi_installed(self, mock_get_distribution): + def test_instruments_without_fastapi_installed( + self, mock_get_distribution + ): mock_get_distribution.side_effect = get_distribution_without_fastapi mock_distro = Mock() _load_instrumentors(mock_distro) From c1cc68fb19fd282f67a3df22558adfda69c9d5b6 Mon Sep 17 00:00:00 2001 From: jeremydvoss Date: Tue, 10 Sep 2024 15:40:22 -0700 Subject: [PATCH 9/9] lint --- CHANGELOG.md | 3 +++ .../tests/test_fastapi_instrumentation.py | 17 ++++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07510f643c..977f4eea8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- `opentelemetry-instrumentation-fastapi` Add autoinstrumentation mechanism tests. + ([#2860](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2860)) + ## Version 1.27.0/0.48b0 () ### Added diff --git a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py index 003ccbc1f8..b8a6ef010e 100644 --- a/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py @@ -14,26 +14,18 @@ # pylint: disable=too-many-lines -from pkg_resources import ( - DistributionNotFound, - iter_entry_points, -) import unittest from timeit import default_timer -from unittest.mock import ( - Mock, - patch, -) +from unittest.mock import Mock, patch + import fastapi from fastapi.middleware.httpsredirect import HTTPSRedirectMiddleware from fastapi.responses import JSONResponse from fastapi.testclient import TestClient +from pkg_resources import DistributionNotFound, iter_entry_points import opentelemetry.instrumentation.fastapi as otel_fastapi from opentelemetry import trace -from opentelemetry.instrumentation.auto_instrumentation._load import ( - _load_instrumentors, -) from opentelemetry.instrumentation._semconv import ( OTEL_SEMCONV_STABILITY_OPT_IN, _OpenTelemetrySemanticConventionStability, @@ -43,6 +35,9 @@ _server_duration_attrs_old, ) from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware +from opentelemetry.instrumentation.auto_instrumentation._load import ( + _load_instrumentors, +) from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, NumberDataPoint,