From bf3c90d40e74b927dded0e933ab902f39d34a958 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 22 May 2025 15:00:30 +0200 Subject: [PATCH 1/2] opentelemetry-instrumentation-system-metrics: fix running on Google Cloud Run psutil fails reading context switches fails on Google Cloud Run, so before setting up the observable counter trying to read the values check we are actually being able to do so. --- CHANGELOG.md | 2 + .../system_metrics/__init__.py | 18 ++++++++- .../tests/test_system_metrics.py | 39 +++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ecdc6d396..3caf0cdf40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `opentelemetry-instrumentation-system-metrics`: fix loading on Google Cloud Run + ([#3533](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3533)) - `opentelemetry-instrumentation-fastapi`: fix wrapping of middlewares ([#3012](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3012)) diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py b/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py index e0407d8ab4..ff716502fe 100644 --- a/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-system-metrics/src/opentelemetry/instrumentation/system_metrics/__init__.py @@ -395,7 +395,10 @@ def _instrument(self, **kwargs: Any): self._meter, callbacks=[self._get_cpu_utilization] ) - if "process.context_switches" in self._config: + if ( + "process.context_switches" in self._config + and self._can_read_context_switches() + ): self._meter.create_observable_counter( name="process.context_switches", callbacks=[self._get_context_switches], @@ -482,7 +485,10 @@ def _instrument(self, **kwargs: Any): unit="1", ) - if "process.runtime.context_switches" in self._config: + if ( + "process.runtime.context_switches" in self._config + and self._can_read_context_switches() + ): self._meter.create_observable_counter( name=f"process.runtime.{self._python_implementation}.context_switches", callbacks=[self._get_runtime_context_switches], @@ -493,6 +499,14 @@ def _instrument(self, **kwargs: Any): def _uninstrument(self, **kwargs: Any): pass + def _can_read_context_switches(self) -> bool: + """On Google Cloud Run psutil is not able to read context switches, catch it before creating the observable instrument""" + try: + self._proc.num_ctx_switches() + return True + except NotImplementedError: + return False + def _get_open_file_descriptors( self, options: CallbackOptions ) -> Iterable[Observation]: diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py index c5a600218b..c09916ff4e 100644 --- a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py @@ -855,6 +855,24 @@ def test_context_switches(self, mock_process_num_ctx_switches): ] self._test_metrics("process.context_switches", expected) + @mock.patch("psutil.Process.num_ctx_switches") + def test_context_switches_not_implemented_error( + self, mock_process_num_ctx_switches + ): + mock_process_num_ctx_switches.side_effect = NotImplementedError + + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + system_metrics = SystemMetricsInstrumentor() + system_metrics.instrument(meter_provider=meter_provider) + + seen_metrics = set() + for resource_metrics in reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + for metric in scope_metrics.metrics: + seen_metrics.add(metric.name) + self.assertNotIn("process.context_switches", seen_metrics) + @mock.patch("psutil.Process.num_threads") def test_thread_count(self, mock_process_thread_num): mock_process_thread_num.configure_mock(**{"return_value": 42}) @@ -947,6 +965,27 @@ def test_runtime_context_switches(self, mock_process_num_ctx_switches): f"process.runtime.{self.implementation}.context_switches", expected ) + @mock.patch("psutil.Process.num_ctx_switches") + def test_runtime_context_switches_not_implemented_error( + self, mock_process_num_ctx_switches + ): + mock_process_num_ctx_switches.side_effect = NotImplementedError + + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + system_metrics = SystemMetricsInstrumentor() + system_metrics.instrument(meter_provider=meter_provider) + + seen_metrics = set() + for resource_metrics in reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + for metric in scope_metrics.metrics: + seen_metrics.add(metric.name) + self.assertNotIn( + f"process.runtime.{self.implementation}.context_switches", + seen_metrics, + ) + @mock.patch("psutil.Process.num_threads") def test_runtime_thread_count(self, mock_process_thread_num): mock_process_thread_num.configure_mock(**{"return_value": 42}) From a6a222f79ef72fc1a7b7969414d1124b99af52f8 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Thu, 22 May 2025 15:36:54 +0200 Subject: [PATCH 2/2] Please pylint --- .../tests/test_system_metrics.py | 43 ++++++++----------- 1 file changed, 18 insertions(+), 25 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py index c09916ff4e..975f9b907c 100644 --- a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -# pylint: disable=protected-access +# pylint: disable=protected-access,too-many-lines import sys from collections import namedtuple @@ -235,14 +235,28 @@ def _assert_metrics(self, observer_name, reader, expected): assertions += 1 self.assertEqual(len(expected), assertions) - def _test_metrics(self, observer_name, expected): + @staticmethod + def _setup_instrumentor() -> InMemoryMetricReader: reader = InMemoryMetricReader() meter_provider = MeterProvider(metric_readers=[reader]) system_metrics = SystemMetricsInstrumentor() system_metrics.instrument(meter_provider=meter_provider) + return reader + + def _test_metrics(self, observer_name, expected): + reader = self._setup_instrumentor() self._assert_metrics(observer_name, reader, expected) + def _assert_metrics_not_found(self, observer_name): + reader = self._setup_instrumentor() + seen_metrics = set() + for resource_metrics in reader.get_metrics_data().resource_metrics: + for scope_metrics in resource_metrics.scope_metrics: + for metric in scope_metrics.metrics: + seen_metrics.add(metric.name) + self.assertNotIn(observer_name, seen_metrics) + # This patch is added here to stop psutil from raising an exception # because we're patching cpu_times # pylint: disable=unused-argument @@ -861,17 +875,7 @@ def test_context_switches_not_implemented_error( ): mock_process_num_ctx_switches.side_effect = NotImplementedError - reader = InMemoryMetricReader() - meter_provider = MeterProvider(metric_readers=[reader]) - system_metrics = SystemMetricsInstrumentor() - system_metrics.instrument(meter_provider=meter_provider) - - seen_metrics = set() - for resource_metrics in reader.get_metrics_data().resource_metrics: - for scope_metrics in resource_metrics.scope_metrics: - for metric in scope_metrics.metrics: - seen_metrics.add(metric.name) - self.assertNotIn("process.context_switches", seen_metrics) + self._assert_metrics_not_found("process.context_switches") @mock.patch("psutil.Process.num_threads") def test_thread_count(self, mock_process_thread_num): @@ -971,19 +975,8 @@ def test_runtime_context_switches_not_implemented_error( ): mock_process_num_ctx_switches.side_effect = NotImplementedError - reader = InMemoryMetricReader() - meter_provider = MeterProvider(metric_readers=[reader]) - system_metrics = SystemMetricsInstrumentor() - system_metrics.instrument(meter_provider=meter_provider) - - seen_metrics = set() - for resource_metrics in reader.get_metrics_data().resource_metrics: - for scope_metrics in resource_metrics.scope_metrics: - for metric in scope_metrics.metrics: - seen_metrics.add(metric.name) - self.assertNotIn( + self._assert_metrics_not_found( f"process.runtime.{self.implementation}.context_switches", - seen_metrics, ) @mock.patch("psutil.Process.num_threads")