From 49eac0d1fc711d1391754397fe9339a53381c054 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 23 Jan 2023 14:03:30 -0800 Subject: [PATCH 1/5] Disabling diagnostics for diagnostic module. Added duplicate handler prevention --- .../distro/_diagnostics/_diagnostic_logging.py | 15 +++++++++------ .../tests/diagnostics/test_diagnostic_logging.py | 8 ++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py index e0f3ad4c..dd670eaf 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py @@ -18,7 +18,6 @@ ) from azure.monitor.opentelemetry.distro._version import VERSION -_OPENTELEMETRY_DIAGNOSTIC_LOGGER_NAME = "opentelemetry" _DIAGNOSTIC_LOGGER_FILE_NAME = "applicationinsights-extension.log" _SITE_NAME = _env_var_or_default("WEBSITE_SITE_NAME") _SUBSCRIPTION_ID_ENV_VAR = _env_var_or_default("WEBSITE_OWNER_NAME") @@ -68,14 +67,18 @@ def _initialize(): fmt=format, datefmt="%Y-%m-%dT%H:%M:%S" ) AzureDiagnosticLogging._f_handler.setFormatter(formatter) - _logger.addHandler(AzureDiagnosticLogging._f_handler) AzureDiagnosticLogging._initialized = True _logger.info("Initialized Azure Diagnostic Logger.") def enable(logger: logging.Logger): AzureDiagnosticLogging._initialize() if AzureDiagnosticLogging._initialized: - logger.addHandler(AzureDiagnosticLogging._f_handler) - _logger.info( - "Added Azure diagnostics logging to %s." % logger.name - ) + if AzureDiagnosticLogging._f_handler in logger.handlers: + _logger.info( + "Azure diagnostics already enabled for %s logger." % logger.name + ) + else: + logger.addHandler(AzureDiagnosticLogging._f_handler) + _logger.info( + "Added Azure diagnostics logging to %s." % logger.name + ) diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py index d231d07c..6f874566 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py @@ -151,6 +151,14 @@ def test_warning(self): TEST_LOGGER_SUB_MODULE.warning(MESSAGE2) check_file_for_messages("WARNING", (MESSAGE1, MESSAGE2)) + def test_warning_multiple_enable(self): + set_up(is_diagnostics_enabled=True) + diagnostic_logger.AzureDiagnosticLogging.enable(TEST_LOGGER) + diagnostic_logger.AzureDiagnosticLogging.enable(TEST_LOGGER) + TEST_LOGGER_SUB_MODULE.warning(MESSAGE1) + TEST_LOGGER_SUB_MODULE.warning(MESSAGE2) + check_file_for_messages("WARNING", (MESSAGE1, MESSAGE2)) + def test_error(self): set_up(is_diagnostics_enabled=True) TEST_LOGGER_SUB_MODULE.error(MESSAGE1) From 28355d2541e578366c5f742a1f3cb084eb093985 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 23 Jan 2023 16:46:11 -0800 Subject: [PATCH 2/5] changlog --- CHANGELOG.md | 2 ++ .../opentelemetry/distro/_diagnostics/_diagnostic_logging.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 469b88e0..fe802ade 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ ([#217](https://github.com/microsoft/ApplicationInsights-Python/pull/217)) - Add Logging configuration to Distro API ([#218](https://github.com/microsoft/ApplicationInsights-Python/pull/218)) +- Removing diagnostic logging from its module's logger. Preventing duplicate handlers. + ([#225](https://github.com/microsoft/ApplicationInsights-Python/pull/225)) ## [1.0.0b8](https://github.com/microsoft/ApplicationInsights-Python/releases/tag/v1.0.0b8) - 2022-09-26 diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py index dd670eaf..8694712e 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py @@ -75,7 +75,8 @@ def enable(logger: logging.Logger): if AzureDiagnosticLogging._initialized: if AzureDiagnosticLogging._f_handler in logger.handlers: _logger.info( - "Azure diagnostics already enabled for %s logger." % logger.name + "Azure diagnostics already enabled for %s logger." + % logger.name ) else: logger.addHandler(AzureDiagnosticLogging._f_handler) From 4d1c2545d2f4d1c6a49095c54687916ebb8dcb0e Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 23 Jan 2023 16:19:31 -0800 Subject: [PATCH 3/5] Constants tests pass for conn str revamp --- .../monitor/opentelemetry/distro/__init__.py | 7 ++ .../opentelemetry/distro/_constants.py | 42 +++++-- .../_diagnostics/_diagnostic_logging.py | 12 +- .../distro/_diagnostics/_status_logger.py | 14 ++- .../diagnostics/test_diagnostic_logging.py | 4 +- .../tests/diagnostics/test_status_logger.py | 108 +++++++++--------- .../tests/test_constants.py | 35 +++++- 7 files changed, 145 insertions(+), 77 deletions(-) diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py index 3154d187..3007e67e 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py @@ -6,6 +6,7 @@ from logging import NOTSET, getLogger from azure.monitor.opentelemetry.distro.util import get_configurations +from azure.monitor.opentelemetry.distro._constants import ConnectionStringConstants from azure.monitor.opentelemetry.exporter import ( ApplicationInsightsSampler, AzureMonitorLogExporter, @@ -33,6 +34,12 @@ def configure_azure_monitor(**kwargs): """ configurations = get_configurations(**kwargs) + conn_str = configurations.get("connection_string", "") + if conn_str is None: + # TODO: JEREVOSS: We could levae this for the exporter to determine. + configurations["connection_string"] = ConnectionStringConstants.get_conn_str() + else: + ConnectionStringConstants.set_conn_str(conn_str) service_name = configurations.get("service_name", "") service_namespace = configurations.get("service_namespace", "") service_instance_id = configurations.get("service_instance_id", "") diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py index 810c500c..85dd41af 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py @@ -1,3 +1,9 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License in the project root for +# license information. +# -------------------------------------------------------------------------- + import logging import platform from os import environ @@ -7,12 +13,6 @@ ConnectionStringParser, ) -# ------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License in the project root for -# license information. -# -------------------------------------------------------------------------- - _LOG_PATH_LINUX = "/var/log/applicationinsights" _LOG_PATH_WINDOWS = "\\LogFiles\\ApplicationInsights" _IS_ON_APP_SERVICE = "WEBSITE_SITE_NAME" in environ @@ -22,12 +22,6 @@ # _EXPORTER_DIAGNOSTICS_ENABLED_ENV_VAR = ( # "AZURE_MONITOR_OPENTELEMETRY_DISTRO_ENABLE_EXPORTER_DIAGNOSTICS" # ) -logger = logging.getLogger(__name__) -_CUSTOMER_IKEY = "unknown" -try: - _CUSTOMER_IKEY = ConnectionStringParser().instrumentation_key -except ValueError as e: - logger.error("Failed to parse Instrumentation Key: %s" % e) def _get_log_path(status_log_path=False): @@ -64,3 +58,27 @@ def _env_var_or_default(var_name, default_val=""): ) # TODO: Enabled when duplciate logging issue is solved # _EXPORTER_DIAGNOSTICS_ENABLED = _is_exporter_diagnostics_enabled() + + +class ConnectionStringConstants: + _conn_str_parser = None + + + def set_conn_str_from_env_var(): + ConnectionStringConstants._conn_str_parser = ConnectionStringParser() + + + def set_conn_str(conn_str): + ConnectionStringConstants._conn_str_parser = ConnectionStringParser(conn_str) + + + def get_conn_str(): + if ConnectionStringConstants._conn_str_parser is None: + return None + return ConnectionStringConstants._conn_str_parser._conn_str + + + def get_customer_ikey(): + if ConnectionStringConstants._conn_str_parser is None: + return None + return ConnectionStringConstants._conn_str_parser.instrumentation_key diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py index 8694712e..df0905fe 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py @@ -10,7 +10,7 @@ from os.path import exists, join from azure.monitor.opentelemetry.distro._constants import ( - _CUSTOMER_IKEY, + ConnectionStringConstants, _EXTENSION_VERSION, _IS_DIAGNOSTICS_ENABLED, _env_var_or_default, @@ -39,6 +39,14 @@ def _initialize(): with AzureDiagnosticLogging._lock: if not AzureDiagnosticLogging._initialized: if _IS_DIAGNOSTICS_ENABLED and _DIAGNOSTIC_LOG_PATH: + customer_ikey = ConnectionStringConstants.get_customer_ikey() + if customer_ikey is None: + try: + ConnectionStringConstants.set_conn_str_from_env_var() + customer_ikey = ConnectionStringConstants.get_customer_ikey() + except ValueError as e: + _logger.error("Failed to parse Instrumentation Key: %s" % e) + customer_ikey = "unknown" format = ( "{" + '"time":"%(asctime)s.%(msecs)03d", ' @@ -48,7 +56,7 @@ def _initialize(): + '"properties":{' + '"operation":"Startup", ' + f'"sitename":"{_SITE_NAME}", ' - + f'"ikey":"{_CUSTOMER_IKEY}", ' + + f'"ikey":"{customer_ikey}", ' + f'"extensionVersion":"{_EXTENSION_VERSION}", ' + f'"sdkVersion":"{VERSION}", ' + f'"subscriptionId":"{_SUBSCRIPTION_ID}", ' diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py index 574ec64f..2d44e1a2 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py @@ -10,26 +10,36 @@ from platform import node from azure.monitor.opentelemetry.distro._constants import ( - _CUSTOMER_IKEY, + ConnectionStringConstants, _EXTENSION_VERSION, _IS_DIAGNOSTICS_ENABLED, _get_log_path, ) from azure.monitor.opentelemetry.distro._version import VERSION +import logging _MACHINE_NAME = node() _STATUS_LOG_PATH = _get_log_path(status_log_path=True) +_logger = logging.getLogger(__name__) class AzureStatusLogger: def _get_status_json(agent_initialized_successfully, pid, reason=None): + customer_ikey = ConnectionStringConstants.get_customer_ikey() + if customer_ikey is None: + try: + ConnectionStringConstants.set_conn_str_from_env_var() + customer_ikey = ConnectionStringConstants.get_customer_ikey() + except ValueError as e: + _logger.error("Failed to parse Instrumentation Key: %s" % e) + customer_ikey = "unknown" status_json = { "AgentInitializedSuccessfully": agent_initialized_successfully, "AppType": "python", "MachineName": _MACHINE_NAME, "PID": pid, "SdkVersion": VERSION, - "Ikey": _CUSTOMER_IKEY, + "Ikey": customer_ikey, "ExtensionVersion": _EXTENSION_VERSION, } if reason: diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py index 6f874566..1aa0dccc 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py @@ -96,8 +96,8 @@ def set_up( TEST_DIAGNOSTIC_LOGGER_FILE_NAME, ).start() patch( - "azure.monitor.opentelemetry.distro._diagnostics._diagnostic_logging._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, + "azure.monitor.opentelemetry.distro._diagnostics._diagnostic_logging.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, ).start() patch( "azure.monitor.opentelemetry.distro._diagnostics._diagnostic_logging._EXTENSION_VERSION", diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py index 0df79142..3f651cd8 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py @@ -72,10 +72,6 @@ def setUp(self) -> None: "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -88,15 +84,19 @@ def setUp(self) -> None: "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", True, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_log_status_success(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_log_status_success(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True, MESSAGE2) check_file_for_messages(True, MESSAGE2) @@ -105,10 +105,6 @@ def test_log_status_success(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -121,15 +117,19 @@ def test_log_status_success(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", True, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_log_status_failed_initialization(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_log_status_failed_initialization(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(True, MESSAGE1) AzureStatusLogger.log_status(False, MESSAGE2) check_file_for_messages(False, MESSAGE2) @@ -138,10 +138,6 @@ def test_log_status_failed_initialization(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -154,15 +150,19 @@ def test_log_status_failed_initialization(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", True, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_log_status_no_reason(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_log_status_no_reason(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True) check_file_for_messages(True) @@ -171,10 +171,6 @@ def test_log_status_no_reason(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -187,15 +183,19 @@ def test_log_status_no_reason(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", False, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_disabled_log_status_success(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_disabled_log_status_success(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True, MESSAGE2) check_file_is_empty() @@ -204,10 +204,6 @@ def test_disabled_log_status_success(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -220,15 +216,19 @@ def test_disabled_log_status_success(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", False, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_disabled_log_status_failed_initialization(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_disabled_log_status_failed_initialization(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(True, MESSAGE1) AzureStatusLogger.log_status(False, MESSAGE2) check_file_is_empty() @@ -237,10 +237,6 @@ def test_disabled_log_status_failed_initialization(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._STATUS_LOG_PATH", TEST_LOGGER_PATH, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger._CUSTOMER_IKEY", - TEST_CUSTOMER_IKEY, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._EXTENSION_VERSION", TEST_EXTENSION_VERSION, @@ -253,15 +249,19 @@ def test_disabled_log_status_failed_initialization(self, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger._IS_DIAGNOSTICS_ENABLED", False, ) - @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", - return_value=TEST_PID, - ) @patch( "azure.monitor.opentelemetry.distro._diagnostics._status_logger._MACHINE_NAME", TEST_MACHINE_NAME, ) - def test_disabled_log_status_no_reason(self, mock_getpid): + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + return_value=TEST_CUSTOMER_IKEY, + ) + @patch( + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", + return_value=TEST_PID, + ) + def test_disabled_log_status_no_reason(self, mock_get_customer_ikey, mock_getpid): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True) check_file_is_empty() diff --git a/azure-monitor-opentelemetry-distro/tests/test_constants.py b/azure-monitor-opentelemetry-distro/tests/test_constants.py index d887435e..cfbf9f38 100644 --- a/azure-monitor-opentelemetry-distro/tests/test_constants.py +++ b/azure-monitor-opentelemetry-distro/tests/test_constants.py @@ -7,7 +7,7 @@ from importlib import reload from os import environ from unittest import TestCase -from unittest.mock import patch +from unittest.mock import patch, Mock from azure.monitor.opentelemetry.distro import _constants @@ -38,14 +38,39 @@ def test_extension_version_default(self): @patch.dict( "os.environ", {"APPLICATIONINSIGHTS_CONNECTION_STRING": TEST_CONN_STR} ) - def test_ikey(self): + @patch( + "azure.monitor.opentelemetry.exporter._connection_string_parser.ConnectionStringParser" + ) + def test_set_conn_str_from_env_var(self, mock_csp): + reload(_constants) + mock_csp_init = Mock() + mock_csp.return_value = mock_csp_init + mock_csp_init.instrumentation_key = TEST_IKEY + self.assertIsNone(_constants.ConnectionStringConstants._conn_str_parser) + _constants.ConnectionStringConstants.set_conn_str_from_env_var() + mock_csp.assert_called_once() + self.assertEqual(_constants.ConnectionStringConstants._conn_str_parser, mock_csp_init) + self.assertEqual(_constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY) + + @patch( + "azure.monitor.opentelemetry.exporter._connection_string_parser.ConnectionStringParser" + ) + def test_set_conn_str(self, mock_csp): reload(_constants) - self.assertEqual(_constants._CUSTOMER_IKEY, TEST_IKEY) + mock_csp_init = Mock() + mock_csp.return_value = mock_csp_init + mock_csp_init.instrumentation_key = TEST_IKEY + _constants.ConnectionStringConstants.set_conn_str(TEST_CONN_STR) + self.assertEqual(_constants.ConnectionStringConstants._conn_str_parser, mock_csp_init) + self.assertEqual(_constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY) - def test_ikey_defaults(self): + @patch( + "azure.monitor.opentelemetry.exporter._connection_string_parser.ConnectionStringParser" + ) + def test_ikey_defaults(self, mock_csp): clear_env_var("APPLICATIONINSIGHTS_CONNECTION_STRING") reload(_constants) - self.assertEqual(_constants._CUSTOMER_IKEY, "unknown") + self.assertIsNone(_constants.ConnectionStringConstants.get_customer_ikey()) # TODO: Enabled when duplciate logging issue is solved # @patch.dict( From d7d29d0dd31454e80fe998dc3f5503833582c483 Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 23 Jan 2023 16:21:52 -0800 Subject: [PATCH 4/5] All tests pass --- .../tests/diagnostics/test_diagnostic_logging.py | 2 +- .../tests/diagnostics/test_status_logger.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py index 1aa0dccc..3547d53d 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_diagnostic_logging.py @@ -96,7 +96,7 @@ def set_up( TEST_DIAGNOSTIC_LOGGER_FILE_NAME, ).start() patch( - "azure.monitor.opentelemetry.distro._diagnostics._diagnostic_logging.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._diagnostic_logging.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ).start() patch( diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py index 3f651cd8..e874b93e 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py @@ -89,7 +89,7 @@ def setUp(self) -> None: TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( @@ -122,7 +122,7 @@ def test_log_status_success(self, mock_get_customer_ikey, mock_getpid): TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( @@ -155,7 +155,7 @@ def test_log_status_failed_initialization(self, mock_get_customer_ikey, mock_get TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( @@ -188,7 +188,7 @@ def test_log_status_no_reason(self, mock_get_customer_ikey, mock_getpid): TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( @@ -221,7 +221,7 @@ def test_disabled_log_status_success(self, mock_get_customer_ikey, mock_getpid): TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( @@ -254,7 +254,7 @@ def test_disabled_log_status_failed_initialization(self, mock_get_customer_ikey, TEST_MACHINE_NAME, ) @patch( - "azure.monitor.opentelemetry.distro._diagnostics._status_logger.get_customer_ikey", + "azure.monitor.opentelemetry.distro._diagnostics._status_logger.ConnectionStringConstants.get_customer_ikey", return_value=TEST_CUSTOMER_IKEY, ) @patch( From 6865f41c50301fc80757230c5ba9e7460fe166af Mon Sep 17 00:00:00 2001 From: jerevoss Date: Mon, 23 Jan 2023 16:49:54 -0800 Subject: [PATCH 5/5] changelog --- CHANGELOG.md | 4 ++- .../monitor/opentelemetry/distro/__init__.py | 8 ++++-- .../opentelemetry/distro/_constants.py | 9 ++---- .../_diagnostics/_diagnostic_logging.py | 14 +++++++--- .../distro/_diagnostics/_status_logger.py | 4 +-- .../tests/diagnostics/test_status_logger.py | 16 ++++++++--- .../tests/test_constants.py | 28 ++++++++++++++----- 7 files changed, 57 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe802ade..9e7b4b50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,10 @@ ([#217](https://github.com/microsoft/ApplicationInsights-Python/pull/217)) - Add Logging configuration to Distro API ([#218](https://github.com/microsoft/ApplicationInsights-Python/pull/218)) -- Removing diagnostic logging from its module's logger. Preventing duplicate handlers. +- Removing diagnostic logging from its module's logger. Preventing duplicate handlers ([#225](https://github.com/microsoft/ApplicationInsights-Python/pull/225)) +- Redesdigning connection string constants to work for configuration and diagnostics + ([#226](https://github.com/microsoft/ApplicationInsights-Python/pull/226)) ## [1.0.0b8](https://github.com/microsoft/ApplicationInsights-Python/releases/tag/v1.0.0b8) - 2022-09-26 diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py index 3007e67e..3b9b5f52 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/__init__.py @@ -5,8 +5,10 @@ # -------------------------------------------------------------------------- from logging import NOTSET, getLogger +from azure.monitor.opentelemetry.distro._constants import ( + ConnectionStringConstants, +) from azure.monitor.opentelemetry.distro.util import get_configurations -from azure.monitor.opentelemetry.distro._constants import ConnectionStringConstants from azure.monitor.opentelemetry.exporter import ( ApplicationInsightsSampler, AzureMonitorLogExporter, @@ -37,7 +39,9 @@ def configure_azure_monitor(**kwargs): conn_str = configurations.get("connection_string", "") if conn_str is None: # TODO: JEREVOSS: We could levae this for the exporter to determine. - configurations["connection_string"] = ConnectionStringConstants.get_conn_str() + configurations[ + "connection_string" + ] = ConnectionStringConstants.get_conn_str() else: ConnectionStringConstants.set_conn_str(conn_str) service_name = configurations.get("service_name", "") diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py index 85dd41af..4f5b3bd9 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_constants.py @@ -4,7 +4,6 @@ # license information. # -------------------------------------------------------------------------- -import logging import platform from os import environ from pathlib import Path @@ -63,21 +62,19 @@ def _env_var_or_default(var_name, default_val=""): class ConnectionStringConstants: _conn_str_parser = None - def set_conn_str_from_env_var(): ConnectionStringConstants._conn_str_parser = ConnectionStringParser() - def set_conn_str(conn_str): - ConnectionStringConstants._conn_str_parser = ConnectionStringParser(conn_str) - + ConnectionStringConstants._conn_str_parser = ConnectionStringParser( + conn_str + ) def get_conn_str(): if ConnectionStringConstants._conn_str_parser is None: return None return ConnectionStringConstants._conn_str_parser._conn_str - def get_customer_ikey(): if ConnectionStringConstants._conn_str_parser is None: return None diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py index df0905fe..53502584 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_diagnostic_logging.py @@ -10,9 +10,9 @@ from os.path import exists, join from azure.monitor.opentelemetry.distro._constants import ( - ConnectionStringConstants, _EXTENSION_VERSION, _IS_DIAGNOSTICS_ENABLED, + ConnectionStringConstants, _env_var_or_default, _get_log_path, ) @@ -39,13 +39,19 @@ def _initialize(): with AzureDiagnosticLogging._lock: if not AzureDiagnosticLogging._initialized: if _IS_DIAGNOSTICS_ENABLED and _DIAGNOSTIC_LOG_PATH: - customer_ikey = ConnectionStringConstants.get_customer_ikey() + customer_ikey = ( + ConnectionStringConstants.get_customer_ikey() + ) if customer_ikey is None: try: ConnectionStringConstants.set_conn_str_from_env_var() - customer_ikey = ConnectionStringConstants.get_customer_ikey() + customer_ikey = ( + ConnectionStringConstants.get_customer_ikey() + ) except ValueError as e: - _logger.error("Failed to parse Instrumentation Key: %s" % e) + _logger.error( + "Failed to parse Instrumentation Key: %s" % e + ) customer_ikey = "unknown" format = ( "{" diff --git a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py index 2d44e1a2..cb6c477e 100644 --- a/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py +++ b/azure-monitor-opentelemetry-distro/azure/monitor/opentelemetry/distro/_diagnostics/_status_logger.py @@ -4,19 +4,19 @@ # license information. # -------------------------------------------------------------------------- +import logging from json import dumps from os import getpid, makedirs from os.path import exists, join from platform import node from azure.monitor.opentelemetry.distro._constants import ( - ConnectionStringConstants, _EXTENSION_VERSION, _IS_DIAGNOSTICS_ENABLED, + ConnectionStringConstants, _get_log_path, ) from azure.monitor.opentelemetry.distro._version import VERSION -import logging _MACHINE_NAME = node() _STATUS_LOG_PATH = _get_log_path(status_log_path=True) diff --git a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py index e874b93e..1829f70a 100644 --- a/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py +++ b/azure-monitor-opentelemetry-distro/tests/diagnostics/test_status_logger.py @@ -129,7 +129,9 @@ def test_log_status_success(self, mock_get_customer_ikey, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", return_value=TEST_PID, ) - def test_log_status_failed_initialization(self, mock_get_customer_ikey, mock_getpid): + def test_log_status_failed_initialization( + self, mock_get_customer_ikey, mock_getpid + ): AzureStatusLogger.log_status(True, MESSAGE1) AzureStatusLogger.log_status(False, MESSAGE2) check_file_for_messages(False, MESSAGE2) @@ -195,7 +197,9 @@ def test_log_status_no_reason(self, mock_get_customer_ikey, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", return_value=TEST_PID, ) - def test_disabled_log_status_success(self, mock_get_customer_ikey, mock_getpid): + def test_disabled_log_status_success( + self, mock_get_customer_ikey, mock_getpid + ): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True, MESSAGE2) check_file_is_empty() @@ -228,7 +232,9 @@ def test_disabled_log_status_success(self, mock_get_customer_ikey, mock_getpid): "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", return_value=TEST_PID, ) - def test_disabled_log_status_failed_initialization(self, mock_get_customer_ikey, mock_getpid): + def test_disabled_log_status_failed_initialization( + self, mock_get_customer_ikey, mock_getpid + ): AzureStatusLogger.log_status(True, MESSAGE1) AzureStatusLogger.log_status(False, MESSAGE2) check_file_is_empty() @@ -261,7 +267,9 @@ def test_disabled_log_status_failed_initialization(self, mock_get_customer_ikey, "azure.monitor.opentelemetry.distro._diagnostics._status_logger.getpid", return_value=TEST_PID, ) - def test_disabled_log_status_no_reason(self, mock_get_customer_ikey, mock_getpid): + def test_disabled_log_status_no_reason( + self, mock_get_customer_ikey, mock_getpid + ): AzureStatusLogger.log_status(False, MESSAGE1) AzureStatusLogger.log_status(True) check_file_is_empty() diff --git a/azure-monitor-opentelemetry-distro/tests/test_constants.py b/azure-monitor-opentelemetry-distro/tests/test_constants.py index cfbf9f38..756bf115 100644 --- a/azure-monitor-opentelemetry-distro/tests/test_constants.py +++ b/azure-monitor-opentelemetry-distro/tests/test_constants.py @@ -7,7 +7,7 @@ from importlib import reload from os import environ from unittest import TestCase -from unittest.mock import patch, Mock +from unittest.mock import Mock, patch from azure.monitor.opentelemetry.distro import _constants @@ -46,11 +46,18 @@ def test_set_conn_str_from_env_var(self, mock_csp): mock_csp_init = Mock() mock_csp.return_value = mock_csp_init mock_csp_init.instrumentation_key = TEST_IKEY - self.assertIsNone(_constants.ConnectionStringConstants._conn_str_parser) + self.assertIsNone( + _constants.ConnectionStringConstants._conn_str_parser + ) _constants.ConnectionStringConstants.set_conn_str_from_env_var() mock_csp.assert_called_once() - self.assertEqual(_constants.ConnectionStringConstants._conn_str_parser, mock_csp_init) - self.assertEqual(_constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY) + self.assertEqual( + _constants.ConnectionStringConstants._conn_str_parser, + mock_csp_init, + ) + self.assertEqual( + _constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY + ) @patch( "azure.monitor.opentelemetry.exporter._connection_string_parser.ConnectionStringParser" @@ -61,8 +68,13 @@ def test_set_conn_str(self, mock_csp): mock_csp.return_value = mock_csp_init mock_csp_init.instrumentation_key = TEST_IKEY _constants.ConnectionStringConstants.set_conn_str(TEST_CONN_STR) - self.assertEqual(_constants.ConnectionStringConstants._conn_str_parser, mock_csp_init) - self.assertEqual(_constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY) + self.assertEqual( + _constants.ConnectionStringConstants._conn_str_parser, + mock_csp_init, + ) + self.assertEqual( + _constants.ConnectionStringConstants.get_customer_ikey(), TEST_IKEY + ) @patch( "azure.monitor.opentelemetry.exporter._connection_string_parser.ConnectionStringParser" @@ -70,7 +82,9 @@ def test_set_conn_str(self, mock_csp): def test_ikey_defaults(self, mock_csp): clear_env_var("APPLICATIONINSIGHTS_CONNECTION_STRING") reload(_constants) - self.assertIsNone(_constants.ConnectionStringConstants.get_customer_ikey()) + self.assertIsNone( + _constants.ConnectionStringConstants.get_customer_ikey() + ) # TODO: Enabled when duplciate logging issue is solved # @patch.dict(