From 7f2e553dc825085fadfd428653d4820531f201a6 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 13 Apr 2022 11:43:49 -0600 Subject: [PATCH] Remove exceptions and show failed metric readers --- .../src/opentelemetry/util/_exceptions.py | 39 ------------------- .../opentelemetry/sdk/_metrics/__init__.py | 23 ++++++++--- .../tests/metrics/test_metrics.py | 17 ++++---- 3 files changed, 28 insertions(+), 51 deletions(-) delete mode 100644 opentelemetry-api/src/opentelemetry/util/_exceptions.py diff --git a/opentelemetry-api/src/opentelemetry/util/_exceptions.py b/opentelemetry-api/src/opentelemetry/util/_exceptions.py deleted file mode 100644 index f6b5f245b05..00000000000 --- a/opentelemetry-api/src/opentelemetry/util/_exceptions.py +++ /dev/null @@ -1,39 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from typing import List - - -class _Failure(Exception): - "Exception raised when a function fails" - - def __init__(self, method: str, exceptions: List[Exception]): - super().__init__() - self._method = method - self._exceptions = exceptions - - def __str__(self) -> str: - - exceptions = ", ".join( - [repr(exception) for exception in self._exceptions] - ) - - return ( - f"{self._method} failed with the following exceptions: " - f"{exceptions}" - ) - - -class _Timeout(Exception): - "Exception raised when a function times out" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py index 015f116af23..e78e69454ca 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py @@ -50,7 +50,6 @@ from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo from opentelemetry.util._once import Once -from opentelemetry.util._exceptions import _Failure _logger = getLogger(__name__) @@ -318,7 +317,7 @@ def _shutdown(): _logger.warning("shutdown can only be called once") return - metric_reader_errors = [] + metric_reader_error = {} for metric_reader in self._sdk_config.metric_readers: try: @@ -327,14 +326,28 @@ def _shutdown(): # pylint: disable=broad-except except Exception as error: - metric_reader_errors.append(error) + metric_reader_error[metric_reader] = error if self._atexit_handler is not None: unregister(self._atexit_handler) self._atexit_handler = None - if metric_reader_errors: - raise _Failure("MeterProvider.shutdown", metric_reader_errors) + if metric_reader_error: + + metric_reader_error_string = "\n".join( + [ + f"{metric_reader.__class__.__name__}: {repr(error)}" + for metric_reader, error in metric_reader_error.items() + ] + ) + + raise Exception( + ( + "MeterProvider.shutdown failed because the following " + "metric readers failed during shutdown:\n" + f"{metric_reader_error_string}" + ) + ) def get_meter( self, diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 70330b265a3..df51ee668c9 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -32,7 +32,6 @@ from opentelemetry.sdk._metrics.point import AggregationTemporality from opentelemetry.sdk.resources import Resource from opentelemetry.test.concurrency_test import ConcurrencyTestBase, MockFunc -from opentelemetry.util._exceptions import _Failure class DummyMetricReader(MetricReader): @@ -153,26 +152,30 @@ def test_shutdown(self): mock_metric_reader_0 = MagicMock( **{ "shutdown.side_effect": ZeroDivisionError(), - "__str__.return_value": "mock_metric_reader_0", } ) - mock_metric_reader_1 = Mock(**{"shutdown.return_value": True}) + mock_metric_reader_1 = MagicMock( + **{ + "shutdown.side_effect": AssertionError(), + } + ) meter_provider = MeterProvider( metric_readers=[mock_metric_reader_0, mock_metric_reader_1] ) - with self.assertRaises(_Failure) as error: + with self.assertRaises(Exception) as error: meter_provider.shutdown() error = error.exception - self.assertIsInstance(error, _Failure) self.assertEqual( str(error), ( - "MeterProvider.shutdown failed with the following exceptions:" - " ZeroDivisionError()" + "MeterProvider.shutdown failed because the following " + "metric readers failed during shutdown:\n" + "MagicMock: ZeroDivisionError()\n" + "MagicMock: AssertionError()" ), )