From c545bf29584fcc8b72b2df527312dc0250597a0d Mon Sep 17 00:00:00 2001 From: jerevoss Date: Tue, 14 Mar 2023 14:34:35 -0700 Subject: [PATCH 1/2] Add type validation for configuration arguments --- CHANGELOG.md | 2 ++ .../azure/monitor/opentelemetry/_configure.py | 28 +++++++++++++++++-- .../tests/configuration/test_configure.py | 16 +++++++---- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b40782b1..7d5d9bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ ([#259](https://github.com/microsoft/ApplicationInsights-Python/pull/259)) - Change interval params to use `_ms` as suffix ([#260](https://github.com/microsoft/ApplicationInsights-Python/pull/260)) +- Add type validation for configuration arguments + ([#258](https://github.com/microsoft/ApplicationInsights-Python/pull/258)) ## [1.0.0b10](https://github.com/microsoft/ApplicationInsights-Python/releases/tag/v1.0.0b10) - 2023-02-23 diff --git a/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py b/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py index dc4c92ec..bf3bca39 100644 --- a/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py +++ b/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py @@ -4,7 +4,7 @@ # license information. # -------------------------------------------------------------------------- from logging import NOTSET, getLogger -from typing import Any, Dict +from typing import Any, Dict, Sequence from azure.monitor.opentelemetry._types import ConfigurationValue from azure.monitor.opentelemetry.exporter import ( @@ -53,7 +53,7 @@ def configure_azure_monitor(**kwargs) -> None: end user to configure OpenTelemetry and Azure monitor components. The configuration can be done via arguments passed to this function. :keyword str connection_string: Connection string for your Application Insights resource. - :keyword Sequence[str] connection_string: Specifies the libraries with instrumentations to be enabled. + :keyword Sequence[str] exclude_instrumentations: Specifies instrumentations you do not want to enable. :keyword Resource resource: Specified the OpenTelemetry [resource][opentelemetry_spec_resource] associated with your application. :keyword bool disable_logging: If set to `True`, disables collection and export of logging telemetry. Defaults to `False`. :keyword bool disable_metrics: If set to `True`, disables collection and export of metric telemetry. Defaults to `False`. @@ -74,6 +74,7 @@ def configure_azure_monitor(**kwargs) -> None: """ configurations = _get_configurations(**kwargs) + _validate_configurations(configurations) disable_tracing = configurations.get("disable_tracing", False) disable_logging = configurations.get("disable_logging", False) @@ -199,3 +200,26 @@ def _setup_instrumentations(configurations: Dict[str, ConfigurationValue]): lib_name, exc_info=ex, ) + +def _is_instance_or_none(var, type): + return isinstance(var, type) or var is None + +def _validate_configurations(configurations): + assert(_is_instance_or_none(configurations.get("connection_string"), str)) + assert(_is_instance_or_none(configurations.get("exclude_instrumentations"), Sequence)) + assert(_is_instance_or_none(configurations.get("resource"), Resource)) + assert(_is_instance_or_none(configurations.get("disable_logging"), bool)) + assert(_is_instance_or_none(configurations.get("disable_metrics"), bool)) + assert(_is_instance_or_none(configurations.get("disable_tracing"), bool)) + assert(_is_instance_or_none(configurations.get("logging_level"), int)) + assert(_is_instance_or_none(configurations.get("logger_name"), str)) + assert(_is_instance_or_none(configurations.get("logging_export_interval_millis"), int)) + assert(_is_instance_or_none(configurations.get("metric_readers"), Sequence)) + assert(_is_instance_or_none(configurations.get("views"), Sequence)) + assert(_is_instance_or_none(configurations.get("sampling_ratio"), float)) + assert(_is_instance_or_none(configurations.get("tracing_export_interval_millis"), int)) + for library in _SUPPORTED_INSTRUMENTED_LIBRARIES: + assert(_is_instance_or_none(configurations.get(library + "_config"), Dict)) + assert(_is_instance_or_none(configurations.get("disable_offline_storage"), bool)) + assert(_is_instance_or_none(configurations.get("storage_directory"), str)) + diff --git a/azure-monitor-opentelemetry/tests/configuration/test_configure.py b/azure-monitor-opentelemetry/tests/configuration/test_configure.py index be264574..2a4ee012 100644 --- a/azure-monitor-opentelemetry/tests/configuration/test_configure.py +++ b/azure-monitor-opentelemetry/tests/configuration/test_configure.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from logging import WARN import unittest from unittest.mock import Mock, patch @@ -26,6 +27,9 @@ ) +TEST_LOGGING_LEVEL = WARN + + class TestConfigure(unittest.TestCase): @patch( "azure.monitor.opentelemetry._configure._setup_instrumentations", @@ -56,7 +60,7 @@ def test_configure_azure_monitor( "disable_logging": False, "disable_metrics": False, "logging_export_interval_ms": 10000, - "logging_level": "test_logging_level", + "logging_level": TEST_LOGGING_LEVEL, "logger_name": "test_logger_name", "metric_readers": "test_metric_readers", "service_name": "test_service_name", @@ -104,7 +108,7 @@ def test_configure_azure_monitor_disable_tracing( "disable_logging": False, "disable_metrics": False, "logging_export_interval_ms": 10000, - "logging_level": "test_logging_level", + "logging_level": TEST_LOGGING_LEVEL, "logger_name": "test_logger_name", "service_name": "test_service_name", "service_namespace": "test_namespace", @@ -151,7 +155,7 @@ def test_configure_azure_monitor_disable_logging( "disable_logging": True, "disable_metrics": False, "logging_export_interval_ms": 10000, - "logging_level": "test_logging_level", + "logging_level": TEST_LOGGING_LEVEL, "logger_name": "test_logger_name", "service_name": "test_service_name", "service_namespace": "test_namespace", @@ -198,7 +202,7 @@ def test_configure_azure_monitor_disable_metrics( "disable_logging": False, "disable_metrics": True, "logging_export_interval_ms": 10000, - "logging_level": "test_logging_level", + "logging_level": TEST_LOGGING_LEVEL, "service_name": "test_service_name", "service_namespace": "test_namespace", "service_instance_id": "test_id", @@ -337,7 +341,7 @@ def test_setup_logging( "connection_string": "test_cs", "disable_logging": False, "logging_export_interval_ms": 10000, - "logging_level": "test_logging_level", + "logging_level": TEST_LOGGING_LEVEL, "logger_name": "test_logger_name", } _setup_logging(resource_mock, configurations) @@ -353,7 +357,7 @@ def test_setup_logging( blrp_init_mock ) logging_handler_mock.assert_called_once_with( - level="test_logging_level", logger_provider=lp_init_mock + level=TEST_LOGGING_LEVEL, logger_provider=lp_init_mock ) get_logger_mock.assert_called_once_with("test_logger_name") logger_mock.addHandler.assert_called_once_with( From e0cd36ec90ab1671244afade2a4f1cc2e6422a1f Mon Sep 17 00:00:00 2001 From: jerevoss Date: Fri, 17 Mar 2023 11:24:48 -0700 Subject: [PATCH 2/2] lint --- .../azure/monitor/opentelemetry/_configure.py | 45 ++++++++++++------- .../tests/configuration/test_configure.py | 3 +- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py b/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py index bf3bca39..b632680c 100644 --- a/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py +++ b/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py @@ -201,25 +201,36 @@ def _setup_instrumentations(configurations: Dict[str, ConfigurationValue]): exc_info=ex, ) + def _is_instance_or_none(var, type): return isinstance(var, type) or var is None + def _validate_configurations(configurations): - assert(_is_instance_or_none(configurations.get("connection_string"), str)) - assert(_is_instance_or_none(configurations.get("exclude_instrumentations"), Sequence)) - assert(_is_instance_or_none(configurations.get("resource"), Resource)) - assert(_is_instance_or_none(configurations.get("disable_logging"), bool)) - assert(_is_instance_or_none(configurations.get("disable_metrics"), bool)) - assert(_is_instance_or_none(configurations.get("disable_tracing"), bool)) - assert(_is_instance_or_none(configurations.get("logging_level"), int)) - assert(_is_instance_or_none(configurations.get("logger_name"), str)) - assert(_is_instance_or_none(configurations.get("logging_export_interval_millis"), int)) - assert(_is_instance_or_none(configurations.get("metric_readers"), Sequence)) - assert(_is_instance_or_none(configurations.get("views"), Sequence)) - assert(_is_instance_or_none(configurations.get("sampling_ratio"), float)) - assert(_is_instance_or_none(configurations.get("tracing_export_interval_millis"), int)) + assert _is_instance_or_none(configurations.get("connection_string"), str) + assert _is_instance_or_none( + configurations.get("exclude_instrumentations"), Sequence + ) + assert _is_instance_or_none(configurations.get("resource"), Resource) + assert _is_instance_or_none(configurations.get("disable_logging"), bool) + assert _is_instance_or_none(configurations.get("disable_metrics"), bool) + assert _is_instance_or_none(configurations.get("disable_tracing"), bool) + assert _is_instance_or_none(configurations.get("logging_level"), int) + assert _is_instance_or_none(configurations.get("logger_name"), str) + assert _is_instance_or_none( + configurations.get("logging_export_interval_millis"), int + ) + assert _is_instance_or_none(configurations.get("metric_readers"), Sequence) + assert _is_instance_or_none(configurations.get("views"), Sequence) + assert _is_instance_or_none(configurations.get("sampling_ratio"), float) + assert _is_instance_or_none( + configurations.get("tracing_export_interval_millis"), int + ) for library in _SUPPORTED_INSTRUMENTED_LIBRARIES: - assert(_is_instance_or_none(configurations.get(library + "_config"), Dict)) - assert(_is_instance_or_none(configurations.get("disable_offline_storage"), bool)) - assert(_is_instance_or_none(configurations.get("storage_directory"), str)) - + assert _is_instance_or_none( + configurations.get(library + "_config"), Dict + ) + assert _is_instance_or_none( + configurations.get("disable_offline_storage"), bool + ) + assert _is_instance_or_none(configurations.get("storage_directory"), str) diff --git a/azure-monitor-opentelemetry/tests/configuration/test_configure.py b/azure-monitor-opentelemetry/tests/configuration/test_configure.py index 2a4ee012..0386a299 100644 --- a/azure-monitor-opentelemetry/tests/configuration/test_configure.py +++ b/azure-monitor-opentelemetry/tests/configuration/test_configure.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from logging import WARN import unittest +from logging import WARN from unittest.mock import Mock, patch from azure.monitor.opentelemetry._configure import ( @@ -26,7 +26,6 @@ configure_azure_monitor, ) - TEST_LOGGING_LEVEL = WARN