Replace prometheus logging with simple email notifications (#258)

* Deletes duplicate test case * Revert "Adds prometheus reporting (#236)" This reverts commit 67e1c36. * Adds admin notification via email * Adds checks for SMTP connectivity * Updates application tests * Adds admin email notification on user notification failure * Check for admin emails before notification * Updates docs * Improves log message clarity
pitt-crc · Jun 14, 2023 · 0194ca2 · 0194ca2
1 parent 297aead
commit 0194ca2
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 67 deletions.
diff --git a/docs/source/overview/installation.rst b/docs/source/overview/installation.rst
@@ -3,13 +3,10 @@ Installation
 
 Follow the instructions below to install and configure the ``notifier`` utility.
 
-System Utilities
-----------------
+Installing System Utilities
+---------------------------
 
-The ``df`` command line utility must be installed for the quota notifier to
-process *generic* file systems. The ``beegfs-ctl`` utility is also required to
-support BeeGFS file systems. See :doc:`file_systems` for more details on how
-different file system types are expected to be configured.
+The ``df`` command line utility must be installed for the quota notifier to function properly.
 
 Installing the Package
 ----------------------
@@ -19,7 +16,7 @@ The ``notifier`` command line utility is installable via the `pip <https://pip.p
 
 .. code-block::
 
-   pipx install git+https://github.com/pitt-crc/quota_notifier.git
+   pip install quota-notifier
 
 Configuration
 -------------
@@ -46,7 +43,8 @@ The :doc:`file_systems` page provides an overview of supported file system types
            }
        ],
        "email_from": "[email protected]",
-       "email_domain": "@domain.com"
+       "email_domain": "@domain.com",
+       "email_admins": ["[email protected]"]
    }
 
 Once the application has been configured, you can check the configuration file is valid by running:

diff --git a/pyproject.toml b/pyproject.toml
@@ -30,7 +30,6 @@ notifier = "quota_notifier.cli:Application.execute"
 python = ">=3.8"
 pydantic = "1.10.9"
 sqlalchemy = "2.0.15"
-prometheus_client = "0.17.0"
 
 [tool.poetry.group.tests]
 optional = true

diff --git a/quota_notifier/cli.py b/quota_notifier/cli.py
@@ -13,6 +13,7 @@
 import logging.config
 from argparse import ArgumentParser
 from pathlib import Path
+from smtplib import SMTP
 from typing import List
 
 from . import __version__
@@ -108,11 +109,21 @@ def _configure_logging(cls, console_log_level: int) -> None:
                     'formatter': 'log_file_formatter',
                     'level': ApplicationSettings.get('log_level'),
                     'filename': ApplicationSettings.get('log_path')
+                },
+                'smtp_handler': {
+                    'class': 'logging.handlers.SMTPHandler',
+                    'formatter': 'log_file_formatter',
+                    'level': 'CRITICAL',
+                    'mailhost': ApplicationSettings.get('smtp_host'),
+                    'fromaddr': ApplicationSettings.get('email_from'),
+                    'toaddrs': ApplicationSettings.get('email_admins'),
+                    'subject': 'Quota Notifier - Admin Notification'
                 }
             },
             'loggers': {
                 'console_logger': {'handlers': ['console_handler'], 'level': 0, 'propagate': False},
                 'file_logger': {'handlers': ['log_file_handler'], 'level': 0, 'propagate': False},
+                'smtp_logger': {'handlers': ['smtp_handler'], 'level': 0, 'propagate': False},
                 '': {'handlers': ['console_handler', 'log_file_handler'], 'level': 0, 'propagate': False},
             }
         })
@@ -121,12 +132,30 @@ def _configure_logging(cls, console_log_level: int) -> None:
     def _configure_database(cls) -> None:
         """Configure the application database connection"""
 
+        logging.debug('Configuring database connection...')
         if ApplicationSettings.get('debug'):
             DBConnection.configure('sqlite:///:memory:')
 
         else:
             DBConnection.configure(ApplicationSettings.get('db_url'))
 
+    @classmethod
+    def _test_smtp_server(cls) -> None:
+        """Ensure the SMTP server can be reached"""
+
+        logging.debug('Testing SMTP server...')
+        host = ApplicationSettings.get('smtp_host')
+        port = ApplicationSettings.get('smtp_port')
+        server = SMTP(host=host, port=port)
+
+        try:
+            server.connect()
+
+        except Exception as caught:
+            raise ConnectionError(
+                f'Could not connect to SMTP server at {host}:{port}. Please check your application settings file.'
+            ) from caught
+
     @classmethod
     def run(cls, validate: bool = False, verbosity: int = 0, debug: bool = False) -> None:
         """Run the application using parsed commandline arguments
@@ -142,30 +171,31 @@ def run(cls, validate: bool = False, verbosity: int = 0, debug: bool = False) ->
         if validate:
             return
 
-        # Map number of verbosity flags to logging levels
-        log_levels = {
-            0: logging.ERROR,
-            1: logging.WARNING,
-            2: logging.INFO,
-            3: logging.DEBUG}
-
         # Configure application logging (to console and file)
-        cls._configure_logging(console_log_level=log_levels.get(verbosity, logging.DEBUG))
+        verbosity_to_log_level = {0: logging.ERROR, 1: logging.WARNING, 2: logging.INFO, 3: logging.DEBUG}
+        cls._configure_logging(console_log_level=verbosity_to_log_level.get(verbosity, logging.DEBUG))
+
+        # Test the SMTP server can be reached
         if ApplicationSettings.get('debug'):
             logging.warning('Running application in debug mode')
 
-        # Connect to the database and run the core application logic
+        else:
+            cls._test_smtp_server()
+
+        # Connect to the application database
         cls._configure_database()
+
+        # Run the core application logic
         UserNotifier().send_notifications()
 
     @classmethod
     def execute(cls, arg_list: List[str] = None) -> None:
         """Parse arguments and execute the application
 
-        Raised exceptions are passed to STDERR via the argument parser.
+        This method is equivalent to parsing arguments and passing them to the `run` method.
 
         Args:
-            arg_list: Run the application with the given arguments instead of parsing the command line
+            arg_list: Parse the given argument list instead of parsing the command line
         """
 
         parser = Parser()
@@ -177,9 +207,14 @@ def execute(cls, arg_list: List[str] = None) -> None:
                 verbosity=args.verbose,
                 debug=args.debug)
 
+        except ConnectionError as caught:
+            logging.getLogger('console_logger').critical(f'Error connecting to SMTP server - {caught}')
+
         except Exception as caught:
             logging.getLogger('file_logger').critical('Application crash', exc_info=caught)
             logging.getLogger('console_logger').critical(str(caught))
+            if ApplicationSettings.get('admin_emails'):
+                logging.getLogger('smtp_logger').critical(str(caught))
 
         else:
             logging.info('Exiting application gracefully')
diff --git a/quota_notifier/notify.py b/quota_notifier/notify.py
@@ -13,8 +13,8 @@
 from pathlib import Path
 from smtplib import SMTP
 from typing import Collection, Optional, Set, Union, Tuple, List
+from typing import Iterable
 
-from prometheus_client import Summary, CollectorRegistry, push_to_gateway
 from sqlalchemy import delete, insert, select
 from sqlalchemy.orm import Session
 
@@ -81,8 +81,8 @@ def send(self, address: str, smtp: Optional[SMTP] = None) -> EmailMessage:
             return email
 
         with smtp or SMTP(
-            host=ApplicationSettings.get('smtp_host'),
-            port=ApplicationSettings.get('smtp_port')
+                host=ApplicationSettings.get('smtp_host'),
+                port=ApplicationSettings.get('smtp_port')
         ) as smtp_server:
             smtp_server.send_message(email)
 
@@ -93,7 +93,7 @@ class UserNotifier:
     """Issue and manage user quota notifications"""
 
     @classmethod
-    def get_users(cls) -> List[User]:
+    def get_users(cls) -> Iterable[User]:
         """Return a collection of users to check quotas for
 
         Returns:
@@ -284,28 +284,18 @@ def send_notifications(self) -> None:
             logging.debug('No cachable system queries found')
 
         logging.info('Scanning user quotas...')
-        failures = 0
+        failure = False
         for user in users:
             try:
                 self.notify_user(user)
 
             except Exception as caught:
-                failures += 1
+                # Only include exception information in the logfile, not the console
                 logging.getLogger('file_logger').error(f'Error notifying {user}', exc_info=caught)
                 logging.getLogger('console_logger').error(f'Error notifying {user} - {caught}')
+                failure = True
 
-        # Check if prometheus reporting is enabled
-        prom_host = ApplicationSettings.get('prometheus_host')
-        if not prom_host:
-            return
-
-        registry = CollectorRegistry()
-        failed_summary = Summary('failed_users', 'Number of users with failed notifications', registry=registry)
-        failed_summary.observe(failures)
-
-        processed_summary = Summary('processed_users', 'Number of users scanned for notification', registry=registry)
-        processed_summary.observe(len(users))
-
-        prom_port = ApplicationSettings.get('prometheus_port')
-        prom_job = ApplicationSettings.get('prometheus_job')
-        push_to_gateway(f'{prom_host}:{prom_port}', job=prom_job, registry=registry)
+        if failure and ApplicationSettings.get('admin_emails'):
+            logging.getLogger('smtp_logger').critical(
+                'Email notifications failed for one or more user accounts. See the application logs for more details.'
+            )
diff --git a/quota_notifier/settings.py b/quota_notifier/settings.py
@@ -139,25 +139,6 @@ class SettingsSchema(BaseSettings):
         default_factory=lambda: Path(NamedTemporaryFile().name),
         description='Optionally log application events to a file.')
 
-    # Prometheus settings
-    prometheus_host: str = Field(
-        title='Prometheus Server Host Name',
-        default='',
-        description='Optional report metrics to a Prometheus server.'
-    )
-
-    prometheus_port: int = Field(
-        title='Prometheus Server Port Number',
-        default=9091,
-        description='Port for the Prometheus server'
-    )
-
-    prometheus_job: str = Field(
-        title='Prometheus Job Name',
-        default='notifier',
-        description='Job label attached to pushed metrics '
-    )
-
     # Settings for the smtp host/port
     smtp_host: str = Field(
         title='SMTP Server Host Name',
@@ -195,6 +176,12 @@ class SettingsSchema(BaseSettings):
         description=('String to append to usernames when generating user email addresses. '
                      'The leading `@` is optional.'))
 
+    email_admins: List[str] = Field(
+        title='Administrator Emails',
+        default=[],
+        description='Admin users to contact when the application encounters a critical issue.'
+    )
+
     # Settings for debug / dry-runs
     debug: bool = Field(
         title='Debug Mode',

diff --git a/tests/cli/test_application.py b/tests/cli/test_application.py
@@ -116,10 +116,3 @@ def test_db_in_memory(self) -> None:
 
         Application.execute(['--debug'])
         self.assertEqual('sqlite:///:memory:', DBConnection.url)
-
-    def test_db_matches_default_settings(self) -> None:
-        """Test the DB URL defaults to the default application settings"""
-
-        Application.execute([])
-        os.remove(ApplicationSettings.get('db_url').lstrip('sqlite:'))
-        self.assertEqual(ApplicationSettings.get('db_url'), DBConnection.url)