Skip to content

Commit

Permalink
opentelemetry-sdk: avoid recursion error with sdk disabled (#4259)
Browse files Browse the repository at this point in the history
* 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.

* Add changelog
  • Loading branch information
xrmx authored Nov 8, 2024
1 parent c4fa8d7 commit f194bc3
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 21 additions & 3 deletions opentelemetry-sdk/tests/logs/test_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion opentelemetry-sdk/tests/logs/test_logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# pylint: disable=protected-access

import unittest
import warnings
from unittest.mock import Mock, patch

from opentelemetry.sdk._logs import LoggerProvider
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit f194bc3

Please sign in to comment.