From 3197d3f01586739b9a6ddfe62c2a843e72265bc2 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 8 Nov 2024 11:17:33 +0100 Subject: [PATCH 1/2] opentelemetry-sdk: avoid recursion error with sdk disabled If the sdk is disable and our handler is added to the root logger we will recurse in order to log that SDK is disabled. Use the warnings facilities to print the message instead. --- .../sdk/_logs/_internal/__init__.py | 2 +- opentelemetry-sdk/tests/logs/test_handler.py | 24 ++++++++++++++++--- opentelemetry-sdk/tests/logs/test_logs.py | 7 +++++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py index ed4ec388011..2d52b1bc74c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_logs/_internal/__init__.py @@ -692,7 +692,7 @@ def get_logger( attributes: Optional[Attributes] = None, ) -> Logger: if self._disabled: - _logger.warning("SDK is disabled.") + warnings.warn("SDK is disabled.") return NoOpLogger( name, version=version, diff --git a/opentelemetry-sdk/tests/logs/test_handler.py b/opentelemetry-sdk/tests/logs/test_handler.py index e8f70d6f5c3..f6daa1b22cf 100644 --- a/opentelemetry-sdk/tests/logs/test_handler.py +++ b/opentelemetry-sdk/tests/logs/test_handler.py @@ -11,9 +11,12 @@ # 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. + import logging +import os import unittest -from unittest.mock import Mock +import warnings +from unittest.mock import Mock, patch from opentelemetry._logs import NoOpLoggerProvider, SeverityNumber from opentelemetry._logs import get_logger as APIGetLogger @@ -280,12 +283,27 @@ def test_log_body_is_always_string_with_formatter(self): log_record = processor.get_log_record(0) self.assertIsInstance(log_record.body, str) + @patch.dict(os.environ, {"OTEL_SDK_DISABLED": "true"}) + def test_handler_root_logger_with_disabled_sdk_does_not_go_into_recursion_error( + self, + ): + processor, logger = set_up_test_logging( + logging.NOTSET, root_logger=True + ) + with warnings.catch_warnings(record=True) as cw: + logger.warning("hello") + + self.assertEqual(len(cw), 1) + self.assertEqual("SDK is disabled.", str(cw[0].message)) + + self.assertEqual(processor.emit_count(), 0) + -def set_up_test_logging(level, formatter=None): +def set_up_test_logging(level, formatter=None, root_logger=False): logger_provider = LoggerProvider() processor = FakeProcessor() logger_provider.add_log_record_processor(processor) - logger = logging.getLogger("foo") + logger = logging.getLogger(None if root_logger else "foo") handler = LoggingHandler(level=level, logger_provider=logger_provider) if formatter: handler.setFormatter(formatter) diff --git a/opentelemetry-sdk/tests/logs/test_logs.py b/opentelemetry-sdk/tests/logs/test_logs.py index 4cb2a46c00c..0590669653b 100644 --- a/opentelemetry-sdk/tests/logs/test_logs.py +++ b/opentelemetry-sdk/tests/logs/test_logs.py @@ -15,6 +15,7 @@ # pylint: disable=protected-access import unittest +import warnings from unittest.mock import Mock, patch from opentelemetry.sdk._logs import LoggerProvider @@ -69,8 +70,12 @@ def test_get_logger(self): @patch.dict("os.environ", {OTEL_SDK_DISABLED: "true"}) def test_get_logger_with_sdk_disabled(self): - logger = LoggerProvider().get_logger(Mock()) + with warnings.catch_warnings(record=True) as cw: + logger = LoggerProvider().get_logger(Mock()) + self.assertIsInstance(logger, NoOpLogger) + self.assertEqual(len(cw), 1) + self.assertEqual("SDK is disabled.", str(cw[0].message)) @patch.object(Resource, "create") def test_logger_provider_init(self, resource_patch): From dbb2b47372a9b8643677c7e9a89ba6a2648f5e54 Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Fri, 8 Nov 2024 11:31:14 +0100 Subject: [PATCH 2/2] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 30060466904..092a13ce014 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix metrics export with exemplar and no context and filtering observable instruments ([#4251](https://github.com/open-telemetry/opentelemetry-python/pull/4251)) +- Fix recursion error with sdk disabled and handler added to root logger + ([#4259](https://github.com/open-telemetry/opentelemetry-python/pull/4259)) ## Version 1.28.0/0.49b0 (2024-11-05)