From d444f5e43dc27883a261f9de476816c4adea155e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 18 2018 13:21:07 +0000 Subject: Fix logging and the SMTPHandler With this, we now have the SMTPHandler handler properly configured and running. It sends email on exception both for the flask application as well as for the different workers. Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 035a481..a2a2f76 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -646,6 +646,95 @@ See the `SSH hostkeys/Fingerprints page on pagure.io ` and `` must be replaced by your values. +LOGGING +~~~~~~~ + +This configuration key allows you to set up the logging of the application. +It relies on the standard `python logging module +`_. + +The default value is: + +:: + + LOGGING = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'standard': { + 'format': '%(asctime)s [%(levelname)s] %(name)s: %(message)s' + }, + 'email_format': { + 'format': MSG_FORMAT + } + }, + 'filters': { + 'myfilter': { + '()': ContextInjector, + } + }, + 'handlers': { + 'console': { + 'level': 'INFO', + 'formatter': 'standard', + 'class': 'logging.StreamHandler', + 'stream': 'ext://sys.stdout', + }, + 'email': { + 'level': 'ERROR', + 'formatter': 'email_format', + 'class': 'logging.handlers.SMTPHandler', + 'mailhost': 'localhost', + 'fromaddr': 'pagure@localhost', + 'toaddrs': 'root@localhost', + 'subject': 'ERROR on pagure', + 'filters': ['myfilter'], + }, + }, + # The root logger configuration; this is a catch-all configuration + # that applies to all log messages not handled by a different logger + 'root': { + 'level': 'INFO', + 'handlers': ['console'], + }, + 'loggers': { + 'pagure': { + 'handlers': ['console'], + 'level': 'DEBUG', + 'propagate': True + }, + 'flask': { + 'handlers': ['console'], + 'level': 'INFO', + 'propagate': False + }, + 'sqlalchemy': { + 'handlers': ['console'], + 'level': 'WARN', + 'propagate': False + }, + 'binaryornot': { + 'handlers': ['console'], + 'level': 'WARN', + 'propagate': True + }, + 'pagure.lib.encoding_utils': { + 'handlers': ['console'], + 'level': 'WARN', + 'propagate': False + }, + } + } + +.. note:: as you can see there is an ``email`` handler defined. It's not used + anywhere by default but you can use it to get report of errors by email + and thus monitor your pagure instance. + To do this the easiest is to set, on the ``root`` logger: + :: + + 'handlers': ['console', 'email'], + + ITEM_PER_PAGE ~~~~~~~~~~~~~ diff --git a/pagure/default_config.py b/pagure/default_config.py index 23ea386..d1275ed 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -11,6 +11,8 @@ import os from datetime import timedelta +from pagure.mail_logging import ContextInjector, MSG_FORMAT + # Set the time after which the admin session expires ADMIN_SESSION_LIFETIME = timedelta(minutes=20) @@ -360,6 +362,14 @@ LOGGING = { 'standard': { 'format': '%(asctime)s [%(levelname)s] %(name)s: %(message)s' }, + 'email_format': { + 'format': MSG_FORMAT + } + }, + 'filters': { + 'myfilter': { + '()': ContextInjector, + } }, 'handlers': { 'console': { @@ -368,6 +378,16 @@ LOGGING = { 'class': 'logging.StreamHandler', 'stream': 'ext://sys.stdout', }, + 'email': { + 'level': 'ERROR', + 'formatter': 'email_format', + 'class': 'logging.handlers.SMTPHandler', + 'mailhost': 'localhost', + 'fromaddr': 'pagure@localhost', + 'toaddrs': 'root@localhost', + 'subject': 'ERROR on pagure', + 'filters': ['myfilter'], + }, }, # The root logger configuration; this is a catch-all configuration # that applies to all log messages not handled by a different logger diff --git a/pagure/flask_app.py b/pagure/flask_app.py index 75d558a..8637b26 100644 --- a/pagure/flask_app.py +++ b/pagure/flask_app.py @@ -11,7 +11,6 @@ import datetime import gc import logging -import logging.config import time import os @@ -38,8 +37,6 @@ else: perfrepo = None -logging.basicConfig() -logging.config.dictConfig(pagure_config.get('LOGGING') or {'version': 1}) logger = logging.getLogger(__name__) REDIS = None @@ -69,8 +66,7 @@ def create_app(config=None): import flask_session flask_session.Session(app) - logging.basicConfig() - logging.config.dictConfig(app.config.get('LOGGING') or {'version': 1}) + pagure.utils.set_up_logging(app=app) app.jinja_env.trim_blocks = True app.jinja_env.lstrip_blocks = True @@ -120,16 +116,6 @@ def create_app(config=None): oidc.init_app(app) app.before_request(fas_user_from_oidc) - # Report error by email - if not app.debug and not pagure_config.get('DEBUG', False): - app.logger.addHandler(pagure.mail_logging.get_mail_handler( - smtp_server=pagure_config.get('SMTP_SERVER', '127.0.0.1'), - mail_admin=pagure_config.get( - 'MAIL_ADMIN', pagure_config['EMAIL_ERROR']), - from_email=pagure_config.get( - 'FROM_EMAIL', 'pagure@fedoraproject.org') - )) - # Support proxy app.wsgi_app = pagure.proxy.ReverseProxied(app.wsgi_app) diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index a9ecf4d..8e57001 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -12,7 +12,6 @@ import collections import datetime import gc import hashlib -import logging import os import os.path import shutil @@ -27,6 +26,8 @@ import six from celery import Celery from celery.result import AsyncResult +from celery.signals import after_setup_task_logger +from celery.utils.log import get_task_logger from sqlalchemy.exc import SQLAlchemyError import pagure.lib @@ -39,7 +40,7 @@ from pagure.config import config as pagure_config from pagure.utils import get_parent_repo_path # logging.config.dictConfig(pagure_config.get('LOGGING') or {'version': 1}) -_log = logging.getLogger(__name__) +_log = get_task_logger(__name__) if os.environ.get('PAGURE_BROKER_URL'): @@ -53,6 +54,11 @@ conn = Celery('tasks', broker=broker_url, backend=broker_url) conn.conf.update(pagure_config['CELERY_CONFIG']) +@after_setup_task_logger.connect +def augment_celery_log(**kwargs): + pagure.utils.set_up_logging(force=True) + + def pagure_task(function): """ Simple decorator that is responsible for: * Adjusting the status of the task when it starts diff --git a/pagure/lib/tasks_services.py b/pagure/lib/tasks_services.py index e666432..6afd928 100644 --- a/pagure/lib/tasks_services.py +++ b/pagure/lib/tasks_services.py @@ -12,7 +12,6 @@ import datetime import hashlib import hmac import json -import logging import os import os.path import time @@ -22,6 +21,8 @@ import requests import six from celery import Celery +from celery.signals import after_setup_task_logger +from celery.utils.log import get_task_logger from kitchen.text.converters import to_bytes from sqlalchemy.exc import SQLAlchemyError @@ -30,10 +31,10 @@ from pagure.config import config as pagure_config from pagure.lib.tasks import pagure_task from pagure.mail_logging import format_callstack from pagure.lib.lib_ci import trigger_jenkins_build -from pagure.utils import split_project_fullname +from pagure.utils import split_project_fullname, set_up_logging # logging.config.dictConfig(pagure_config.get('LOGGING') or {'version': 1}) -_log = logging.getLogger(__name__) +_log = get_task_logger(__name__) _i = 0 @@ -48,6 +49,11 @@ conn = Celery('tasks', broker=broker_url, backend=broker_url) conn.conf.update(pagure_config['CELERY_CONFIG']) +@after_setup_task_logger.connect +def augment_celery_log(**kwargs): + set_up_logging(force=True) + + def call_web_hooks(project, topic, msg, urls): ''' Sends the web-hook notification. ''' _log.info( diff --git a/pagure/utils.py b/pagure/utils.py index fd97fa8..70448ea 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -8,6 +8,8 @@ """ +import logging +import logging.config import os import re import urlparse @@ -21,6 +23,22 @@ import werkzeug from pagure.config import config as pagure_config +_log = logging.getLogger(__name__) +LOGGER_SETUP = False + + +def set_up_logging(app=None, force=False): + global LOGGER_SETUP + if LOGGER_SETUP and not force: + _log.info('logging already setup') + return + + logging.basicConfig() + logging.config.dictConfig(pagure_config.get('LOGGING') or {'version': 1}) + + LOGGER_SETUP = True + + def authenticated(): ''' Utility function checking if the current user is logged in or not. '''