From edee3c37883f46d53ec6e9dae26f781109a0fa23 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 5 Apr 2025 16:11:42 -0700 Subject: [PATCH] Fix broken monitoring verbosity (#793). --- borgmatic/commands/borgmatic.py | 2 +- borgmatic/hooks/monitoring/logs.py | 8 ++++++-- .../hooks/monitoring/test_apprise.py | 14 +++++++++++--- .../hooks/monitoring/test_healthchecks.py | 13 +++++++++++-- .../unit/hooks/monitoring/test_healthchecks.py | 17 +++++------------ tests/unit/hooks/monitoring/test_logs.py | 15 ++++++++++++++- tests/unit/hooks/monitoring/test_pagerduty.py | 13 +++---------- 7 files changed, 51 insertions(+), 31 deletions(-) diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index bcd3df92..c8d82e72 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -95,7 +95,7 @@ class Monitoring_hooks: self.config_filename = config_filename self.config = config self.dry_run = global_arguments.dry_run - self.monitoring_log_level = verbosity_to_log_level(global_arguments.monitoring_verbosity) + self.monitoring_log_level = verbosity_to_log_level(config.get('monitoring_verbosity')) self.monitoring_hooks_are_activated = ( using_primary_action and self.monitoring_log_level != DISABLED ) diff --git a/borgmatic/hooks/monitoring/logs.py b/borgmatic/hooks/monitoring/logs.py index a2e2f39f..5b8a49ce 100644 --- a/borgmatic/hooks/monitoring/logs.py +++ b/borgmatic/hooks/monitoring/logs.py @@ -38,11 +38,13 @@ class Forgetful_buffering_handler(logging.Handler): self.forgot = True -def add_handler(handler): # pragma: no cover +def add_handler(handler): ''' Add the given handler to the global logger. ''' - logging.getLogger().addHandler(handler) + logger = logging.getLogger() + logger.addHandler(handler) + logger.setLevel(min(handler.level for handler in logger.handlers)) def get_handler(identifier): @@ -90,3 +92,5 @@ def remove_handler(identifier): logger.removeHandler(get_handler(identifier)) except ValueError: pass + + logger.setLevel(min(handler.level for handler in logger.handlers)) diff --git a/tests/integration/hooks/monitoring/test_apprise.py b/tests/integration/hooks/monitoring/test_apprise.py index 158e697b..f0f45c13 100644 --- a/tests/integration/hooks/monitoring/test_apprise.py +++ b/tests/integration/hooks/monitoring/test_apprise.py @@ -8,13 +8,17 @@ from borgmatic.hooks.monitoring import apprise as module def test_destroy_monitor_removes_apprise_handler(): logger = logging.getLogger() original_handlers = list(logger.handlers) + + # Don't mess with the actual log level because it can impact downstream tests. + flexmock(logger).should_receive('setLevel') module.borgmatic.hooks.monitoring.logs.add_handler( module.borgmatic.hooks.monitoring.logs.Forgetful_buffering_handler( identifier=module.HANDLER_IDENTIFIER, byte_capacity=100, log_level=1 ) ) - - module.destroy_monitor(flexmock(), flexmock(), flexmock(), flexmock()) + module.destroy_monitor( + hook_config=flexmock(), config=flexmock(), monitoring_log_level=1, dry_run=False + ) assert logger.handlers == original_handlers @@ -23,6 +27,10 @@ def test_destroy_monitor_without_apprise_handler_does_not_raise(): logger = logging.getLogger() original_handlers = list(logger.handlers) - module.destroy_monitor(flexmock(), flexmock(), flexmock(), flexmock()) + # Don't mess with the actual log level because it can impact downstream tests. + flexmock(logger).should_receive('setLevel') + module.destroy_monitor( + hook_config=flexmock(), config=flexmock(), monitoring_log_level=1, dry_run=False + ) assert logger.handlers == original_handlers diff --git a/tests/integration/hooks/monitoring/test_healthchecks.py b/tests/integration/hooks/monitoring/test_healthchecks.py index da9981b1..73a7fb60 100644 --- a/tests/integration/hooks/monitoring/test_healthchecks.py +++ b/tests/integration/hooks/monitoring/test_healthchecks.py @@ -8,13 +8,18 @@ from borgmatic.hooks.monitoring import healthchecks as module def test_destroy_monitor_removes_healthchecks_handler(): logger = logging.getLogger() original_handlers = list(logger.handlers) + + # Don't mess with the actual log level because it can impact downstream tests. + flexmock(logger).should_receive('setLevel') module.borgmatic.hooks.monitoring.logs.add_handler( module.borgmatic.hooks.monitoring.logs.Forgetful_buffering_handler( identifier=module.HANDLER_IDENTIFIER, byte_capacity=100, log_level=1 ) ) - module.destroy_monitor(flexmock(), flexmock(), flexmock(), flexmock()) + module.destroy_monitor( + hook_config=flexmock(), config=flexmock(), monitoring_log_level=1, dry_run=False + ) assert logger.handlers == original_handlers @@ -23,6 +28,10 @@ def test_destroy_monitor_without_healthchecks_handler_does_not_raise(): logger = logging.getLogger() original_handlers = list(logger.handlers) - module.destroy_monitor(flexmock(), flexmock(), flexmock(), flexmock()) + # Don't mess with the actual log level because it can impact downstream tests. + flexmock(logger).should_receive('setLevel') + module.destroy_monitor( + hook_config=flexmock(), config=flexmock(), monitoring_log_level=1, dry_run=False + ) assert logger.handlers == original_handlers diff --git a/tests/unit/hooks/monitoring/test_healthchecks.py b/tests/unit/hooks/monitoring/test_healthchecks.py index a8f703dc..1ae02d56 100644 --- a/tests/unit/hooks/monitoring/test_healthchecks.py +++ b/tests/unit/hooks/monitoring/test_healthchecks.py @@ -3,18 +3,10 @@ from flexmock import flexmock from borgmatic.hooks.monitoring import healthchecks as module -def mock_logger(): - logger = flexmock() - logger.should_receive('addHandler') - logger.should_receive('removeHandler') - flexmock(module.logging).should_receive('getLogger').and_return(logger) - - def test_initialize_monitor_creates_log_handler_with_ping_body_limit(): ping_body_limit = 100 monitoring_log_level = 1 - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).with_args( @@ -22,6 +14,7 @@ def test_initialize_monitor_creates_log_handler_with_ping_body_limit(): ping_body_limit - len(module.borgmatic.hooks.monitoring.logs.PAYLOAD_TRUNCATION_INDICATOR), monitoring_log_level, ).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'ping_body_limit': ping_body_limit}, {}, 'test.yaml', monitoring_log_level, dry_run=False @@ -31,7 +24,6 @@ def test_initialize_monitor_creates_log_handler_with_ping_body_limit(): def test_initialize_monitor_creates_log_handler_with_default_ping_body_limit(): monitoring_log_level = 1 - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).with_args( @@ -40,6 +32,7 @@ def test_initialize_monitor_creates_log_handler_with_default_ping_body_limit(): - len(module.borgmatic.hooks.monitoring.logs.PAYLOAD_TRUNCATION_INDICATOR), monitoring_log_level, ).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor({}, {}, 'test.yaml', monitoring_log_level, dry_run=False) @@ -48,10 +41,10 @@ def test_initialize_monitor_creates_log_handler_with_zero_ping_body_limit(): ping_body_limit = 0 monitoring_log_level = 1 - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).with_args(module.HANDLER_IDENTIFIER, ping_body_limit, monitoring_log_level).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'ping_body_limit': ping_body_limit}, {}, 'test.yaml', monitoring_log_level, dry_run=False @@ -59,10 +52,10 @@ def test_initialize_monitor_creates_log_handler_with_zero_ping_body_limit(): def test_initialize_monitor_creates_log_handler_when_send_logs_true(): - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'send_logs': True}, {}, 'test.yaml', monitoring_log_level=1, dry_run=False @@ -70,10 +63,10 @@ def test_initialize_monitor_creates_log_handler_when_send_logs_true(): def test_initialize_monitor_bails_when_send_logs_false(): - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).never() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'send_logs': False}, {}, 'test.yaml', monitoring_log_level=1, dry_run=False diff --git a/tests/unit/hooks/monitoring/test_logs.py b/tests/unit/hooks/monitoring/test_logs.py index 11073802..085c781c 100644 --- a/tests/unit/hooks/monitoring/test_logs.py +++ b/tests/unit/hooks/monitoring/test_logs.py @@ -37,6 +37,16 @@ def test_forgetful_buffering_handler_emit_forgets_log_records_when_capacity_reac assert handler.forgot +def test_add_handler_does_not_raise(): + logger = flexmock(handlers=[flexmock(level=0)]) + flexmock(module.logging).should_receive('getLogger').and_return(logger) + flexmock(logger).should_receive('addHandler') + flexmock(logger).should_receive('removeHandler') + flexmock(logger).should_receive('setLevel') + + module.add_handler(flexmock()) + + def test_get_handler_matches_by_identifier(): handlers = [ flexmock(), @@ -92,7 +102,10 @@ def test_format_buffered_logs_for_payload_without_handler_produces_empty_payload def test_remove_handler_with_matching_handler_does_not_raise(): flexmock(module).should_receive('get_handler').and_return(flexmock()) - flexmock(module.logging.getLogger()).should_receive('removeHandler') + logger = flexmock(handlers=[flexmock(level=0)]) + flexmock(module.logging).should_receive('getLogger').and_return(logger) + flexmock(logger).should_receive('removeHandler') + flexmock(logger).should_receive('setLevel') module.remove_handler('test') diff --git a/tests/unit/hooks/monitoring/test_pagerduty.py b/tests/unit/hooks/monitoring/test_pagerduty.py index 34c55d95..12266c32 100644 --- a/tests/unit/hooks/monitoring/test_pagerduty.py +++ b/tests/unit/hooks/monitoring/test_pagerduty.py @@ -3,29 +3,22 @@ from flexmock import flexmock from borgmatic.hooks.monitoring import pagerduty as module -def mock_logger(): - logger = flexmock() - logger.should_receive('addHandler') - logger.should_receive('removeHandler') - flexmock(module.logging).should_receive('getLogger').and_return(logger) - - def test_initialize_monitor_creates_log_handler(): monitoring_log_level = 1 - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor({}, {}, 'test.yaml', monitoring_log_level, dry_run=False) def test_initialize_monitor_creates_log_handler_when_send_logs_true(): - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).once() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'send_logs': True}, {}, 'test.yaml', monitoring_log_level=1, dry_run=False @@ -33,10 +26,10 @@ def test_initialize_monitor_creates_log_handler_when_send_logs_true(): def test_initialize_monitor_bails_when_send_logs_false(): - mock_logger() flexmock(module.borgmatic.hooks.monitoring.logs).should_receive( 'Forgetful_buffering_handler' ).never() + flexmock(module.borgmatic.hooks.monitoring.logs).should_receive('add_handler') module.initialize_monitor( {'send_logs': False}, {}, 'test.yaml', monitoring_log_level=1, dry_run=False