diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 7a636ecf..3279aedf 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -97,7 +97,7 @@ def __init__( "reporter": "", # the reporter mode, either 'udp' or 'ssl'. "log_type": apm_logging.ApmLoggingType.default_type(), "debug_level": apm_logging.ApmLoggingLevel.default_level(), - "logname": "", + "log_filepath": "", "service_key": "", "hostname_alias": "", "trustedpath": "", @@ -135,10 +135,17 @@ def __init__( self.__config["service_key"], self.service_name, ) - self.update_log_type() - # update logging level of Python logging logger + + # Update and apply logging settings to Python logger + self.update_log_settings() + apm_logging.set_sw_log_type( + self.__config["log_type"], + self.__config["log_filepath"], + ) apm_logging.set_sw_log_level(self.__config["debug_level"]) + self.update_log_filepath_for_reporter() + # Calculate c-lib extension usage ( self.extension, @@ -463,21 +470,65 @@ def _update_service_key_name( # Else no need to update service_key when not reporting return service_key + def update_log_settings( + self, + ) -> None: + """Update log_filepath and log type""" + self.update_log_filepath() + self.update_log_type() + + def update_log_filepath( + self, + ) -> None: + """Checks SW_APM_LOG_FILEPATH path to file else fileHandler will fail. + If invalid, create path to match Boost.log behaviour. + If not possible, switch to default log settings. + """ + log_filepath = os.path.dirname(self.__config["log_filepath"]) + if log_filepath: + if not os.path.exists(log_filepath): + try: + os.makedirs(log_filepath) + logger.debug( + "Created directory path from provided SW_APM_LOG_FILEPATH." + ) + except FileNotFoundError: + logger.error( + "Could not create log file directory path from provided SW_APM_LOG_FILEPATH. Using default log settings." + ) + self.__config["log_filepath"] = "" + self.__config[ + "log_type" + ] = apm_logging.ApmLoggingType.default_type() + def update_log_type( self, ) -> None: """Updates agent log type depending on other configs. SW_APM_DEBUG_LEVEL -1 will set c-lib log_type to disabled (4). - SW_APM_LOGNAME not None will set c-lib log_type to file (2). + SW_APM_LOG_FILEPATH not None will set c-lib log_type to file (2). """ if self.__config["debug_level"] == -1: self.__config[ "log_type" ] = apm_logging.ApmLoggingType.disabled_type() - elif self.__config["logname"]: + elif self.__config["log_filepath"]: self.__config["log_type"] = apm_logging.ApmLoggingType.file_type() + def update_log_filepath_for_reporter( + self, + ) -> None: + """Updates log_filepath for extension Reporter to avoid conflict""" + orig_log_filepath = self.__config["log_filepath"] + if orig_log_filepath: + log_filepath, log_filepath_ext = os.path.splitext( + orig_log_filepath + ) + self.__config[ + "log_filepath" + ] = f"{log_filepath}_ext{log_filepath_ext}" + def _calculate_metric_format(self) -> int: """Return one of 1 (TransactionResponseTime only) or 2 (default; ResponseTime only). Note: 0 (both) is no longer supported. Based on collector URL which may have a port e.g. foo-collector.com:443""" metric_format = 2 diff --git a/solarwinds_apm/apm_logging.py b/solarwinds_apm/apm_logging.py index 2bbc6b15..f90bc184 100644 --- a/solarwinds_apm/apm_logging.py +++ b/solarwinds_apm/apm_logging.py @@ -36,6 +36,7 @@ import logging import os +from logging.handlers import RotatingFileHandler class ApmLoggingType: @@ -188,6 +189,41 @@ def disable_logger(disable=True): logger.propagate = True +def set_sw_log_type(log_type, log_filepath=""): + """Set the logging 'type' of the agent-internal logger. + + This function expects the log_type to be one of the integer + representations of the levels defined in ApmLoggingType.log_types. + If an invalid type has been provided, the logging handler will not be + changed but a warning message will be emitted. + + If log_filepath is provided and log_type is file_type, logger stream handler + is swapped with a rotating file handler. + """ + if not ApmLoggingType.is_valid_log_type(log_type): + logger.warning( + "set_sw_log_type: Ignore attempt to set solarwinds_apm logger with invalid logging handler." + ) + return + + if log_type == ApmLoggingType.file_type() and log_filepath: + try: + # no rollover to match oboe logs + file_hander = RotatingFileHandler( + filename=log_filepath, + maxBytes=0, + backupCount=0, + ) + logger.addHandler(file_hander) + # stop logging to stream + logger.removeHandler(_stream_handler) + except FileNotFoundError: + # path should be checked first by ApmConfig.update_log_filepath + logger.error( + "Could not write logs to file; please check configured SW_APM_LOG_FILEPATH." + ) + + def set_sw_log_level(level): """Set the logging level of the agent-internal logger to the provided level. This function expects the level to be one of the integer representations of the levels defined in ApmLoggingLevel.debug_levels. diff --git a/solarwinds_apm/configurator.py b/solarwinds_apm/configurator.py index 7fc85178..3c0ac751 100644 --- a/solarwinds_apm/configurator.py +++ b/solarwinds_apm/configurator.py @@ -465,7 +465,7 @@ def _initialize_solarwinds_reporter( "hostname_alias": apm_config.get("hostname_alias"), "log_type": apm_config.get("log_type"), "log_level": apm_config.get("debug_level"), - "log_file_path": apm_config.get("logname"), + "log_file_path": apm_config.get("log_filepath"), "max_transactions": apm_config.get("max_transactions"), "max_flush_wait_time": apm_config.get("max_flush_wait_time"), "events_flush_interval": apm_config.get("events_flush_interval"), diff --git a/tests/unit/test_apm_config/fixtures/cnf_dict.py b/tests/unit/test_apm_config/fixtures/cnf_dict.py index 99506799..b2dc26ff 100644 --- a/tests/unit/test_apm_config/fixtures/cnf_dict.py +++ b/tests/unit/test_apm_config/fixtures/cnf_dict.py @@ -9,7 +9,7 @@ def fixture_cnf_dict(): "collector": "foo-bar", "reporter": "udp", "debugLevel": 6, - "logname": "foo-bar", + "logFilepath": "foo-bar", "serviceKey": "not-good-to-put-here:still-could-be-used", "hostnameAlias": "foo-bar", "trustedpath": "foo-bar", @@ -37,7 +37,7 @@ def fixture_cnf_dict_enabled_false(): "collector": "foo-bar", "reporter": "udp", "debugLevel": 6, - "logname": "foo-bar", + "logFilepath": "foo-bar", "serviceKey": "not-good-to-put-here:still-could-be-used", "hostnameAlias": "foo-bar", "trustedpath": "foo-bar", @@ -65,7 +65,7 @@ def fixture_cnf_dict_enabled_false_mixed_case(): "collector": "foo-bar", "reporter": "udp", "debugLevel": 6, - "logname": "foo-bar", + "logFilepath": "foo-bar", "serviceKey": "not-good-to-put-here:still-could-be-used", "hostnameAlias": "foo-bar", "trustedpath": "foo-bar", diff --git a/tests/unit/test_apm_config/test_apm_config.py b/tests/unit/test_apm_config/test_apm_config.py index c5b19742..258feae7 100644 --- a/tests/unit/test_apm_config/test_apm_config.py +++ b/tests/unit/test_apm_config/test_apm_config.py @@ -297,6 +297,9 @@ def test__init_oboe_api_and_options_configured_valid(self, mocker): mock_oboe_api_options_swig, ) ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) apm_config.SolarWindsApmConfig() mock_oboe_api_options_swig.assert_called_once() @@ -763,45 +766,149 @@ def test__update_service_key_name_agent_enabled_and_service_name_ok(self): ) assert result == "valid_key_with:bar-service" + def test_update_log_settings(self, mocker): + mock_log_filepath = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_filepath" + ) + mock_log_type = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_type" + ) + # init includes update_log_settings() + apm_config.SolarWindsApmConfig() + mock_log_filepath.assert_called_once() + mock_log_type.assert_called_once() + + def test_update_log_filepath_none(self, mocker): + mock_exists = mocker.patch("solarwinds_apm.apm_config.os.path.exists") + mock_makedirs = mocker.patch("solarwinds_apm.apm_config.os.makedirs") + + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "") + test_config._set_config_value("log_type", 2) + test_config.update_log_filepath() + mock_exists.assert_not_called() + mock_makedirs.assert_not_called() + assert test_config.get("log_filepath") == "" + assert test_config.get("log_type") == 2 + + def test_update_log_filepath_no_parent_path(self, mocker): + mock_exists = mocker.patch("solarwinds_apm.apm_config.os.path.exists") + mock_makedirs = mocker.patch("solarwinds_apm.apm_config.os.makedirs") + + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "foo") + test_config._set_config_value("log_type", 2) + test_config.update_log_filepath() + mock_exists.assert_not_called() + mock_makedirs.assert_not_called() + assert test_config.get("log_filepath") == "foo" + assert test_config.get("log_type") == 2 + + def test_update_log_filepath_path_exists(self, mocker): + mock_exists = mocker.patch("solarwinds_apm.apm_config.os.path.exists", return_value=True) + mock_makedirs = mocker.patch("solarwinds_apm.apm_config.os.makedirs") + + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo") + test_config._set_config_value("log_type", 2) + test_config.update_log_filepath() + mock_exists.assert_called_once_with("/path/to") + mock_makedirs.assert_not_called() + assert test_config.get("log_filepath") == "/path/to/foo" + assert test_config.get("log_type") == 2 + + def test_update_log_filepath_create_path(self, mocker): + mock_exists = mocker.patch("solarwinds_apm.apm_config.os.path.exists", return_value=False) + mock_makedirs = mocker.patch("solarwinds_apm.apm_config.os.makedirs") + + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo") + test_config._set_config_value("log_type", 2) + test_config.update_log_filepath() + mock_exists.assert_called_once_with("/path/to") + mock_makedirs.assert_called_once_with("/path/to") + assert test_config.get("log_filepath") == "/path/to/foo" + assert test_config.get("log_type") == 2 + + def test_update_log_filepath_cannot_create_reset_settings(self, mocker): + mock_exists = mocker.patch("solarwinds_apm.apm_config.os.path.exists", return_value=False) + mock_makedirs = mocker.patch( + "solarwinds_apm.apm_config.os.makedirs", + side_effect=FileNotFoundError("mock error") + ) + + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo") + test_config._set_config_value("log_type", 2) + test_config.update_log_filepath() + mock_exists.assert_called_once_with("/path/to") + mock_makedirs.assert_called_once_with("/path/to") + assert test_config.get("log_filepath") == "" + assert test_config.get("log_type") == 0 + def test_update_log_type_no_change(self): test_config = apm_config.SolarWindsApmConfig() test_config._set_config_value("debug_level", 2) test_config._set_config_value("log_type", 0) - test_config._set_config_value("logname", "") + test_config._set_config_value("log_filepath", "") test_config.update_log_type() assert test_config.get("debug_level") == 2 assert test_config.get("log_type") == 0 - assert test_config.get("logname") == "" + assert test_config.get("log_filepath") == "" def test_update_log_type_disabled(self): test_config = apm_config.SolarWindsApmConfig() test_config._set_config_value("debug_level", -1) test_config._set_config_value("log_type", 0) - test_config._set_config_value("logname", "") + test_config._set_config_value("log_filepath", "") test_config.update_log_type() assert test_config.get("debug_level") == -1 assert test_config.get("log_type") == 4 - assert test_config.get("logname") == "" + assert test_config.get("log_filepath") == "" - def test_update_log_type_logname(self): + def test_update_log_type_log_filepath(self): test_config = apm_config.SolarWindsApmConfig() test_config._set_config_value("debug_level", 1) test_config._set_config_value("log_type", 0) - test_config._set_config_value("logname", "some-file-path") + test_config._set_config_value("log_filepath", "some-file-path") test_config.update_log_type() assert test_config.get("debug_level") == 1 assert test_config.get("log_type") == 2 - assert test_config.get("logname") == "some-file-path" + assert test_config.get("log_filepath") == "some-file-path" - def test_update_log_type_logname_but_disabled(self): + def test_update_log_type_log_filepath_but_disabled(self): test_config = apm_config.SolarWindsApmConfig() test_config._set_config_value("debug_level", -1) test_config._set_config_value("log_type", 0) - test_config._set_config_value("logname", "some-file-path") + test_config._set_config_value("log_filepath", "some-file-path") test_config.update_log_type() assert test_config.get("debug_level") == -1 assert test_config.get("log_type") == 4 - assert test_config.get("logname") == "some-file-path" + assert test_config.get("log_filepath") == "some-file-path" + + def test_update_log_filepath_for_reporter_empty(self): + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "") + test_config.update_log_filepath_for_reporter() + assert test_config.get("log_filepath") == "" + + def test_update_log_filepath_for_reporter_no_ext(self): + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo") + test_config.update_log_filepath_for_reporter() + assert test_config.get("log_filepath") == "/path/to/foo_ext" + + def test_update_log_filepath_for_reporter_ext(self): + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo.log") + test_config.update_log_filepath_for_reporter() + assert test_config.get("log_filepath") == "/path/to/foo_ext.log" + + def test_update_log_filepath_for_reporter_ext_multiple_dots(self): + test_config = apm_config.SolarWindsApmConfig() + test_config._set_config_value("log_filepath", "/path/to/foo.abc.def.xyz") + test_config.update_log_filepath_for_reporter() + assert test_config.get("log_filepath") == "/path/to/foo.abc.def_ext.xyz" def test_convert_to_bool_bool_true(self): test_config = apm_config.SolarWindsApmConfig() diff --git a/tests/unit/test_apm_config/test_apm_config_agent_enabled.py b/tests/unit/test_apm_config/test_apm_config_agent_enabled.py index 3463487f..7f67700e 100644 --- a/tests/unit/test_apm_config/test_apm_config_agent_enabled.py +++ b/tests/unit/test_apm_config/test_apm_config_agent_enabled.py @@ -33,6 +33,20 @@ def test_calculate_agent_enabled_service_key_missing(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) + resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -70,6 +84,20 @@ def test_calculate_agent_enabled_service_key_env_var_set_cnf_file_ignored( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) + resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config.get("service_key") == "valid:key-will-be-used" assert resulting_config.agent_enabled @@ -112,6 +140,20 @@ def test_calculate_agent_enabled_service_key_env_var_not_set_cnf_file_used( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) + resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config.get("service_key") == "not-good-to-put-here:still-could-be-used" assert resulting_config.agent_enabled @@ -137,6 +179,19 @@ def test_calculate_agent_enabled_service_key_bad_format(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -153,6 +208,19 @@ def test_calculate_agent_enabled_service_key_ok(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "key" @@ -170,6 +238,19 @@ def test_calculate_agent_enabled_env_var_true(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "key" @@ -187,6 +268,19 @@ def test_calculate_agent_enabled_env_var_false(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -204,6 +298,19 @@ def test_calculate_agent_enabled_env_var_false_mixed_case(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -231,6 +338,19 @@ def test_calculate_agent_enabled_env_var_not_set_cnf_file_false( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict_enabled_false ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config.agent_enabled assert resulting_config.service_name == "" @@ -258,6 +378,19 @@ def test_calculate_agent_enabled_env_var_not_set_cnf_file_false_mixed_case( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict_enabled_false_mixed_case ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config.agent_enabled assert resulting_config.service_name == "" @@ -285,6 +418,19 @@ def test_calculate_agent_enabled_env_var_true_cnf_file_false( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict_enabled_false ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config.agent_enabled assert resulting_config.service_name == "key" @@ -312,6 +458,20 @@ def test_calculate_agent_enabled_env_var_false_cnf_file_true( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) + # cnf file fixture has "agentEnabled": True resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config.agent_enabled @@ -332,6 +492,19 @@ def test_calculate_agent_enabled_ok_all_env_vars(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "key" @@ -341,6 +514,19 @@ def test_calculate_agent_enabled_no_sw_propagator(self, mocker): "OTEL_PROPAGATORS": "tracecontext,baggage", "SW_APM_SERVICE_KEY": "valid:key", }) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -350,6 +536,19 @@ def test_calculate_agent_enabled_no_tracecontext_propagator(self, mocker): "OTEL_PROPAGATORS": "solarwinds_propagator", "SW_APM_SERVICE_KEY": "valid:key", }) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -359,6 +558,19 @@ def test_calculate_agent_enabled_sw_before_tracecontext_propagator(self, mocker) "OTEL_PROPAGATORS": "solarwinds_propagator,tracecontext", "SW_APM_SERVICE_KEY": "valid:key", }) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -368,6 +580,19 @@ def test_calculate_agent_enabled_sw_before_baggage_propagator(self, mocker): "OTEL_PROPAGATORS": "tracecontext,solarwinds_propagator,baggage", "SW_APM_SERVICE_KEY": "valid:key", }) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -377,6 +602,19 @@ def test_calculate_agent_enabled_sw_after_baggage_propagator(self, mocker): "OTEL_PROPAGATORS": "tracecontext,baggage,solarwinds_propagator", "SW_APM_SERVICE_KEY": "valid:key", }) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "key" @@ -392,6 +630,19 @@ def test_calculate_agent_enabled_sw_but_no_such_other_exporter(self, mocker): mock_iter_entry_points.configure_mock( side_effect=StopIteration("mock error") ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert not resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "" @@ -409,6 +660,19 @@ def test_calculate_agent_enabled_sw_and_two_other_valid_exporters(self, mocker): mock_iter_entry_points.configure_mock( return_value=mock_points ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() assert resulting_config._calculate_agent_enabled() assert resulting_config.service_name == "key" diff --git a/tests/unit/test_apm_config/test_apm_config_cnf_file.py b/tests/unit/test_apm_config/test_apm_config_cnf_file.py index 6e1f02bf..debc90e2 100644 --- a/tests/unit/test_apm_config/test_apm_config_cnf_file.py +++ b/tests/unit/test_apm_config/test_apm_config_cnf_file.py @@ -103,6 +103,20 @@ def test_update_with_cnf_file_all_valid( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) + # use key from env var (Python APM only uses key from here), # agent enabled, nothing has errored resulting_config = apm_config.SolarWindsApmConfig() @@ -116,8 +130,7 @@ def test_update_with_cnf_file_all_valid( assert resulting_config.get("collector") == "foo-bar" assert resulting_config.get("reporter") == "udp" assert resulting_config.get("debug_level") == 6 - assert resulting_config.get("log_type") == 2 # because logname not none - assert resulting_config.get("logname") == "foo-bar" + assert resulting_config.get("log_filepath") == "foo-bar_ext" assert resulting_config.get("hostname_alias") == "foo-bar" assert resulting_config.get("trustedpath") == "foo-bar" assert resulting_config.get("events_flush_interval") == 2 @@ -161,7 +174,7 @@ def test_update_with_cnf_file_mostly_invalid( "collector": False, "reporter": "not-ssl-or-anything", "debugLevel": "foo", - "logname": False, + "logFilepath": False, "serviceKey": "not-good-to-put-here-and-wont-be-used", "hostnameAlias": False, "trustedpath": False, @@ -184,6 +197,19 @@ def test_update_with_cnf_file_mostly_invalid( mock_get_cnf_dict.configure_mock( return_value=mostly_invalid_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) # use key from env var (Python APM only uses key from here), # agent enabled, nothing has errored resulting_config = apm_config.SolarWindsApmConfig() @@ -195,7 +221,6 @@ def test_update_with_cnf_file_mostly_invalid( assert resulting_config.get("trigger_trace") == 1 assert resulting_config.get("reporter") == "" assert resulting_config.get("debug_level") == 2 - assert resulting_config.get("log_type") == 2 # because logname not none assert resulting_config.get("events_flush_interval") == -1 assert resulting_config.get("max_request_size_bytes") == -1 assert resulting_config.get("ec2_metadata_timeout") == 1000 @@ -212,7 +237,7 @@ def test_update_with_cnf_file_mostly_invalid( assert resulting_config.get("collector") == "False" assert resulting_config.get("hostname_alias") == "False" assert resulting_config.get("trustedpath") == "False" - assert resulting_config.get("logname") == "False" + assert resulting_config.get("log_filepath") == "False_ext" # update_transaction_filters was called mock_update_txn_filters.assert_called_once_with(mostly_invalid_cnf_dict) @@ -239,7 +264,7 @@ def test_update_with_cnf_file_and_all_validenv_vars( "SW_APM_COLLECTOR": "other-foo-bar", "SW_APM_REPORTER": "file", "SW_APM_DEBUG_LEVEL": "5", - "SW_APM_LOGNAME": "other-foo-bar", + "SW_APM_LOG_FILEPATH": "other-foo-bar", "SW_APM_HOSTNAME_ALIAS": "other-foo-bar", "SW_APM_TRUSTEDPATH": "other-foo-bar", "SW_APM_EVENTS_FLUSH_INTERVAL": "3", @@ -264,6 +289,19 @@ def test_update_with_cnf_file_and_all_validenv_vars( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() # update_transaction_filters was called mock_update_txn_filters.assert_called_once_with(fixture_cnf_dict) @@ -278,7 +316,6 @@ def test_update_with_cnf_file_and_all_validenv_vars( assert resulting_config.get("collector") == "other-foo-bar" assert resulting_config.get("reporter") == "file" assert resulting_config.get("debug_level") == 5 - assert resulting_config.get("log_type") == 2 # because logname not none assert resulting_config.get("hostname_alias") == "other-foo-bar" assert resulting_config.get("trustedpath") == "other-foo-bar" assert resulting_config.get("events_flush_interval") == 3 @@ -286,7 +323,7 @@ def test_update_with_cnf_file_and_all_validenv_vars( assert resulting_config.get("ec2_metadata_timeout") == 2222 assert resulting_config.get("max_flush_wait_time") == 3 assert resulting_config.get("max_transactions") == 3 - assert resulting_config.get("logname") == "other-foo-bar" + assert resulting_config.get("log_filepath") == "other-foo-bar_ext" assert resulting_config.get("trace_metrics") == 3 assert resulting_config.get("token_bucket_capacity") == 3 assert resulting_config.get("token_bucket_rate") == 3 @@ -318,7 +355,7 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( "SW_APM_COLLECTOR": "False", "SW_APM_REPORTER": "other-foo-bar", "SW_APM_DEBUG_LEVEL": "other-foo-bar", - "SW_APM_LOGNAME": "False", + "SW_APM_LOG_FILEPATH": "False", "SW_APM_HOSTNAME_ALIAS": "False", "SW_APM_TRUSTEDPATH": "False", "SW_APM_EVENTS_FLUSH_INTERVAL": "other-foo-bar", @@ -343,6 +380,19 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( mock_get_cnf_dict.configure_mock( return_value=fixture_cnf_dict ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.update_log_settings" + ) + mock_apm_logging = mocker.patch( + "solarwinds_apm.apm_config.apm_logging" + ) + mock_apm_logging.configure_mock( + **{ + "set_sw_log_type": mocker.Mock(), + "set_sw_log_level": mocker.Mock(), + "ApmLoggingLevel.default_level": mocker.Mock(return_value=2) + } + ) resulting_config = apm_config.SolarWindsApmConfig() # update_transaction_filters was called mock_update_txn_filters.assert_called_once_with(fixture_cnf_dict) @@ -357,7 +407,6 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( assert resulting_config.get("trigger_trace") == 1 assert resulting_config.get("reporter") == "udp" assert resulting_config.get("debug_level") == 6 - assert resulting_config.get("log_type") == 2 # because logname not none assert resulting_config.get("events_flush_interval") == 2 assert resulting_config.get("max_request_size_bytes") == 2 assert resulting_config.get("ec2_metadata_timeout") == 1234 @@ -375,7 +424,7 @@ def test_update_with_cnf_file_and_several_invalid_env_vars( assert resulting_config.get("collector") == "False" assert resulting_config.get("hostname_alias") == "False" assert resulting_config.get("trustedpath") == "False" - assert resulting_config.get("logname") == "False" + assert resulting_config.get("log_filepath") == "False_ext" # Restore old collector if old_collector: diff --git a/tests/unit/test_apm_logging.py b/tests/unit/test_apm_logging.py index 8a09dd2d..7510211f 100644 --- a/tests/unit/test_apm_logging.py +++ b/tests/unit/test_apm_logging.py @@ -54,3 +54,98 @@ def test_is_valid_level_int_out_of_range(self): assert not apm_logging.ApmLoggingLevel.is_valid_level(-2) assert not apm_logging.ApmLoggingLevel.is_valid_level(7) assert not apm_logging.ApmLoggingLevel.is_valid_level(9999) + + +class TestSetSwLog: + def get_mock_logger_and_rfhandler( + self, + mocker, + error=False, + ): + mock_apm_logger = mocker.patch( + "solarwinds_apm.apm_logging.logger" + ) + mock_warning = mocker.Mock() + mock_error = mocker.Mock() + mock_addhandler = mocker.Mock() + mock_rmhandler = mocker.Mock() + mock_apm_logger.configure_mock( + **{ + "addHandler": mock_addhandler, + "error": mock_error, + "removeHandler": mock_rmhandler, + "warning": mock_warning, + } + ) + if error: + mock_rfhandler = mocker.patch( + "solarwinds_apm.apm_logging.RotatingFileHandler", + side_effect=FileNotFoundError("mock error") + ) + else: + mock_rfhandler = mocker.patch( + "solarwinds_apm.apm_logging.RotatingFileHandler" + ) + + return mock_apm_logger, mock_rfhandler + + def test_set_sw_log_type_invalid(self, mocker): + mock_apm_logger, mock_rfhandler = self.get_mock_logger_and_rfhandler(mocker) + + apm_logging.set_sw_log_type(9999, "foo") + mock_rfhandler.assert_not_called() + mock_apm_logger.addHandler.assert_not_called() + mock_apm_logger.removeHandler.assert_not_called() + mock_apm_logger.error.assert_not_called() + mock_apm_logger.warning.assert_called_once_with( + "set_sw_log_type: Ignore attempt to set solarwinds_apm logger with invalid logging handler." + ) + + def test_set_sw_log_type_not_file(self, mocker): + mock_apm_logger, mock_rfhandler = self.get_mock_logger_and_rfhandler(mocker) + + apm_logging.set_sw_log_type(0, "foo") + mock_rfhandler.assert_not_called() + mock_apm_logger.addHandler.assert_not_called() + mock_apm_logger.removeHandler.assert_not_called() + mock_apm_logger.error.assert_not_called() + mock_apm_logger.warning.assert_not_called() + + def test_set_sw_log_type_no_log_filepath(self, mocker): + mock_apm_logger, mock_rfhandler = self.get_mock_logger_and_rfhandler(mocker) + + apm_logging.set_sw_log_type(2, "") + mock_rfhandler.assert_not_called() + mock_apm_logger.addHandler.assert_not_called() + mock_apm_logger.removeHandler.assert_not_called() + mock_apm_logger.error.assert_not_called() + mock_apm_logger.warning.assert_not_called() + + def test_set_sw_log_type_filenotfounderror(self, mocker): + mock_apm_logger, mock_rfhandler = self.get_mock_logger_and_rfhandler( + mocker, + error=True, + ) + + apm_logging.set_sw_log_type(2, "foo") + mock_rfhandler.assert_called_once_with( + filename='foo', maxBytes=0, backupCount=0 + ) + mock_apm_logger.addHandler.assert_not_called() + mock_apm_logger.removeHandler.assert_not_called() + mock_apm_logger.error.assert_called_once_with( + "Could not write logs to file; please check configured SW_APM_LOG_FILEPATH." + ) + mock_apm_logger.warning.assert_not_called() + + def test_set_sw_log_type_update_handlers(self, mocker): + mock_apm_logger, mock_rfhandler = self.get_mock_logger_and_rfhandler(mocker) + + apm_logging.set_sw_log_type(2, "foo") + mock_rfhandler.assert_called_once_with( + filename='foo', maxBytes=0, backupCount=0 + ) + mock_apm_logger.addHandler.assert_called_once() + mock_apm_logger.removeHandler.assert_called_once() + mock_apm_logger.error.assert_not_called() + mock_apm_logger.warning.assert_not_called()