From cd1ab75c77abb4254a59f51dbb7746f360e3ac06 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 14 Nov 2024 11:50:08 -0800 Subject: [PATCH 1/3] duplicate --- .../azure-monitor-opentelemetry/CHANGELOG.md | 3 + .../azure/monitor/opentelemetry/_configure.py | 9 ++- .../tests/test_configure.py | 57 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md b/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md index a314e2090ab1..afca13dd66c0 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md +++ b/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md @@ -4,6 +4,9 @@ ### Features Added +- Only add OpenTelemetry LoggingHandler if current logger does not have it + ([#38371](https://github.com/Azure/azure-sdk-for-python/pull/38371)) + ### Breaking Changes ### Bugs Fixed diff --git a/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py b/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py index 18e349913678..dc17cce8aedc 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py +++ b/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py @@ -168,9 +168,14 @@ def _setup_logging(configurations: Dict[str, ConfigurationValue]): ) logger_provider.add_log_record_processor(log_record_processor) set_logger_provider(logger_provider) - handler = LoggingHandler(logger_provider=logger_provider) logger_name: str = configurations[LOGGER_NAME_ARG] # type: ignore - getLogger(logger_name).addHandler(handler) + logger = getLogger(logger_name) + # Only add OpenTelemetry LoggingHandler if logger does not already have the handler + # This is to prevent most duplicate logging telemetry + if not any(isinstance(handler, LoggingHandler) for handler in logger.handlers): + handler = LoggingHandler(logger_provider=logger_provider) + logger_name: str = configurations[LOGGER_NAME_ARG] # type: ignore + logger.addHandler(handler) def _setup_metrics(configurations: Dict[str, ConfigurationValue]): diff --git a/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py b/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py index e084c5205c26..bc2a4a7d3576 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py +++ b/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py @@ -14,6 +14,7 @@ import unittest from unittest.mock import Mock, call, patch +from opentelemetry.sdk._logs import LoggingHandler from opentelemetry.sdk.resources import Resource from azure.core.tracing.ext.opentelemetry_span import OpenTelemetrySpan @@ -340,6 +341,7 @@ def test_setup_logging( logging_handler_init_mock = Mock() logging_handler_mock.return_value = logging_handler_init_mock logger_mock = Mock() + logger_mock.handlers = [] get_logger_mock.return_value = logger_mock configurations = { @@ -360,6 +362,61 @@ def test_setup_logging( get_logger_mock.assert_called_once_with("test") logger_mock.addHandler.assert_called_once_with(logging_handler_init_mock) + + @patch( + "azure.monitor.opentelemetry._configure.getLogger", + ) + @patch( + "azure.monitor.opentelemetry._configure.BatchLogRecordProcessor", + ) + @patch( + "azure.monitor.opentelemetry._configure.AzureMonitorLogExporter", + ) + @patch( + "azure.monitor.opentelemetry._configure.set_logger_provider", + ) + @patch( + "azure.monitor.opentelemetry._configure.LoggerProvider", + autospec=True, + ) + def test_setup_logging_duplicate_logger( + self, + lp_mock, + set_logger_provider_mock, + log_exporter_mock, + blrp_mock, + get_logger_mock, + ): + lp_init_mock = Mock() + lp_mock.return_value = lp_init_mock + log_exp_init_mock = Mock() + log_exporter_mock.return_value = log_exp_init_mock + blrp_init_mock = Mock() + blrp_mock.return_value = blrp_init_mock + logging_handler_init_mock = Mock(spec=LoggingHandler) + logger_mock = Mock() + logger_mock.handlers = [logging_handler_init_mock] + get_logger_mock.return_value = logger_mock + + configurations = { + "connection_string": "test_cs", + "logger_name": "test", + "resource": TEST_RESOURCE, + } + _setup_logging(configurations) + + lp_mock.assert_called_once_with(resource=TEST_RESOURCE) + set_logger_provider_mock.assert_called_once_with(lp_init_mock) + log_exporter_mock.assert_called_once_with(**configurations) + blrp_mock.assert_called_once_with( + log_exp_init_mock, + ) + lp_init_mock.add_log_record_processor.assert_called_once_with(blrp_init_mock) + # logging_handler_mock.assert_not_called() + get_logger_mock.assert_called_once_with("test") + logger_mock.addHandler.assert_not_called() + + @patch( "azure.monitor.opentelemetry._configure.PeriodicExportingMetricReader", ) From bb383f753fbafa0853482b5cf5f43ff4d64acef3 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Thu, 14 Nov 2024 12:06:25 -0800 Subject: [PATCH 2/3] cl --- sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md | 2 +- .../azure/monitor/opentelemetry/_configure.py | 1 - .../samples/logging/basic.py | 12 ++++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md b/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md index afca13dd66c0..eedf7583daa3 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md +++ b/sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md @@ -5,7 +5,7 @@ ### Features Added - Only add OpenTelemetry LoggingHandler if current logger does not have it - ([#38371](https://github.com/Azure/azure-sdk-for-python/pull/38371)) + ([#38549](https://github.com/Azure/azure-sdk-for-python/pull/38549)) ### Breaking Changes diff --git a/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py b/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py index dc17cce8aedc..c4863d2ab85d 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py +++ b/sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py @@ -174,7 +174,6 @@ def _setup_logging(configurations: Dict[str, ConfigurationValue]): # This is to prevent most duplicate logging telemetry if not any(isinstance(handler, LoggingHandler) for handler in logger.handlers): handler = LoggingHandler(logger_provider=logger_provider) - logger_name: str = configurations[LOGGER_NAME_ARG] # type: ignore logger.addHandler(handler) diff --git a/sdk/monitor/azure-monitor-opentelemetry/samples/logging/basic.py b/sdk/monitor/azure-monitor-opentelemetry/samples/logging/basic.py index 59e2beaa36b4..1c17dcb69701 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/samples/logging/basic.py +++ b/sdk/monitor/azure-monitor-opentelemetry/samples/logging/basic.py @@ -29,12 +29,12 @@ logger.warning("warning log") logger.error("error log") -logger.info("info log") -logger.warning("warning log") -logger.error("error log") +logger_child.info("Child: info log") +logger_child.warning("Child: warning log") +logger_child.error("Child: error log") -logger_not_tracked.info("info log2") -logger_not_tracked.warning("warning log2") -logger_not_tracked.error("error log2") +logger_not_tracked.info("Not tracked: info log") +logger_not_tracked.warning("Not tracked: warning log") +logger_not_tracked.error("Not tracked: error log") input() From 7b97d6da7e87cd2e82e86caaa6d168f731c88452 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Fri, 15 Nov 2024 13:27:41 -0800 Subject: [PATCH 3/3] black --- sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py b/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py index 24e7fef7b638..04d66ec6a8e5 100644 --- a/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py +++ b/sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py @@ -375,7 +375,6 @@ def test_setup_logging( elp_mock.assert_called_once_with(lp_init_mock) set_elp_mock.assert_called_once_with(elp_init_mock) - @patch( "azure.monitor.opentelemetry._configure.getLogger", ) @@ -429,7 +428,6 @@ def test_setup_logging_duplicate_logger( get_logger_mock.assert_called_once_with("test") logger_mock.addHandler.assert_not_called() - @patch( "azure.monitor.opentelemetry._configure.PeriodicExportingMetricReader", )