From 91031cbbc818e46a41fc5b2aee3746202a7a70fd Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 9 Feb 2017 17:16:56 -0500 Subject: [PATCH 1/5] modularize logging config in proper Django fashion --- awx/conf/apps.py | 10 ++---- awx/main/tasks.py | 25 +++---------- awx/main/utils/handlers.py | 72 +++++++++++++++++++++++++++++++++++--- 3 files changed, 74 insertions(+), 33 deletions(-) diff --git a/awx/conf/apps.py b/awx/conf/apps.py index 9ae459fb35..a70d21326c 100644 --- a/awx/conf/apps.py +++ b/awx/conf/apps.py @@ -2,7 +2,7 @@ from django.apps import AppConfig # from django.core import checks from django.utils.translation import ugettext_lazy as _ -from django.utils.log import configure_logging +from awx.main.utils.handlers import configure_external_logger from django.conf import settings @@ -15,10 +15,4 @@ class ConfConfig(AppConfig): self.module.autodiscover() from .settings import SettingsWrapper SettingsWrapper.initialize() - if settings.LOG_AGGREGATOR_ENABLED: - LOGGING_DICT = settings.LOGGING - LOGGING_DICT['handlers']['http_receiver']['class'] = 'awx.main.utils.handlers.HTTPSHandler' - if 'awx' in settings.LOG_AGGREGATOR_LOGGERS: - if 'http_receiver' not in LOGGING_DICT['loggers']['awx']['handlers']: - LOGGING_DICT['loggers']['awx']['handlers'] += ['http_receiver'] - configure_logging(settings.LOGGING_CONFIG, LOGGING_DICT) + configure_external_logger(settings) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 0cd0dcf6c8..08242cac7c 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -32,7 +32,7 @@ import pexpect # Celery from celery import Task, task -from celery.signals import celeryd_init, worker_ready +from celery.signals import celeryd_init, worker_process_init from celery import current_app # Django @@ -54,6 +54,7 @@ from awx.main.task_engine import TaskEnhancer from awx.main.utils import (get_ansible_version, get_ssh_version, decrypt_field, update_scm_url, check_proot_installed, build_proot_temp_dir, wrap_args_with_proot, get_system_task_capacity, OutputEventFilter, parse_yaml_or_json) +from awx.main.utils.handlers import configure_external_logger from awx.main.consumers import emit_channel_notification __all__ = ['RunJob', 'RunSystemJob', 'RunProjectUpdate', 'RunInventoryUpdate', @@ -86,26 +87,10 @@ def celery_startup(conf=None, **kwargs): logger.error("Failed to rebuild schedule {}: {}".format(sch, e)) -def _setup_tower_logger(): - global logger - from django.utils.log import configure_logging - LOGGING_DICT = settings.LOGGING - if settings.LOG_AGGREGATOR_ENABLED: - LOGGING_DICT['handlers']['http_receiver']['class'] = 'awx.main.utils.handlers.HTTPSHandler' - LOGGING_DICT['handlers']['http_receiver']['async'] = False - if 'awx' in settings.LOG_AGGREGATOR_LOGGERS: - if 'http_receiver' not in LOGGING_DICT['loggers']['awx']['handlers']: - LOGGING_DICT['loggers']['awx']['handlers'] += ['http_receiver'] - configure_logging(settings.LOGGING_CONFIG, LOGGING_DICT) - logger = logging.getLogger('awx.main.tasks') - - -@worker_ready.connect +@worker_process_init.connect def task_set_logger_pre_run(*args, **kwargs): cache.close() - if settings.LOG_AGGREGATOR_ENABLED: - _setup_tower_logger() - logger.debug('Custom Tower logger configured for worker process.') + configure_external_logger(settings, is_startup=False) def _uwsgi_reload(): @@ -121,7 +106,7 @@ def _uwsgi_reload(): def _reset_celery_logging(): - # Worker logger reloaded, now send signal to restart pool + # Send signal to restart thread pool app = current_app._get_current_object() app.control.broadcast('pool_restart', arguments={'reload': True}, destination=['celery@{}'.format(settings.CLUSTER_HOST_ID)], reply=False) diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index 69f556ddba..1a667d2ff5 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -12,8 +12,11 @@ import traceback from requests_futures.sessions import FuturesSession -# custom -from django.conf import settings as django_settings +# AWX +from awx.main.utils.formatters import LogstashFormatter + + +__all__ = ['HTTPSNullHandler', 'BaseHTTPSHandler', 'HTTPSHandler', 'configure_external_logger'] # AWX external logging handler, generally designed to be used # with the accompanying LogstashHandler, derives from python-logstash library @@ -40,7 +43,7 @@ def unused_callback(sess, resp): class HTTPSNullHandler(logging.NullHandler): "Placeholder null handler to allow loading without database access" - def __init__(self, host, **kwargs): + def __init__(self, *args, **kwargs): return super(HTTPSNullHandler, self).__init__() @@ -167,5 +170,64 @@ class BaseHTTPSHandler(logging.Handler): class HTTPSHandler(object): - def __new__(cls, *args, **kwargs): - return BaseHTTPSHandler.from_django_settings(django_settings, *args, **kwargs) + def __new__(cls, settings_module, *args, **kwargs): + return BaseHTTPSHandler.from_django_settings(settings_module, *args, **kwargs) + + +def add_or_remove_logger(address, instance, adding=True): + specific_logger = logging.getLogger(address) + i_occurance = None + for i in range(len(specific_logger.handlers)): + if isinstance(specific_logger.handlers[i], (HTTPSNullHandler, BaseHTTPSHandler)): + i_occurance = i + break + + if i_occurance is None and not adding: + return + elif i_occurance is None: + specific_logger.handlers.append(instance) + else: + specific_logger.handlers[i_occurance] = instance + + +def configure_external_logger(settings_module, async_flag=True, is_startup=True): + + is_enabled = settings_module.LOG_AGGREGATOR_ENABLED + if is_startup and (not is_enabled): + # Pass-through if external logging not being used + return + + if is_enabled: + instance = HTTPSHandler(settings_module, async=async_flag) + instance.setFormatter(LogstashFormatter()) + else: + instance = HTTPSNullHandler() + + add_or_remove_logger('awx.analytics', instance, adding=is_enabled) + add_or_remove_logger('awx', instance, adding=(is_enabled and 'awx' in settings_module.LOG_AGGREGATOR_LOGGERS)) + + +def configure_external_logger_old(settings_module, async_flag=True, is_startup=True): + + from django.utils.log import configure_logging + + is_enabled = settings_module.LOG_AGGREGATOR_ENABLED + if is_startup and (not is_enabled): + # Pass-through if external logging not being used + return + + LOGGING_DICT = settings_module.LOGGING + if is_enabled: + LOGGING_DICT['handlers']['http_receiver']['class'] = 'awx.main.utils.handlers.BaseHTTPSHandler' + if not async_flag: + LOGGING_DICT['handlers']['http_receiver']['async'] = False + else: + LOGGING_DICT['handlers']['http_receiver']['async'] = True + for param, django_setting_name in PARAM_NAMES.items(): + LOGGING_DICT['handlers']['http_receiver'][param] = getattr(settings_module, django_setting_name, None) + if 'awx' in settings_module.LOG_AGGREGATOR_LOGGERS: + if 'http_receiver' not in LOGGING_DICT['loggers']['awx']['handlers']: + LOGGING_DICT['loggers']['awx']['handlers'] += ['http_receiver'] + # External logging not enabled, but removal of existing configuration needed + configure_logging(settings_module.LOGGING_CONFIG, LOGGING_DICT) + From cdf28f1bca59d87bca1eaf1e339969f4196a3de3 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 10 Feb 2017 11:37:50 -0500 Subject: [PATCH 2/5] remove old logger reconfig method --- awx/main/tasks.py | 2 +- awx/main/utils/handlers.py | 28 +--------------------------- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 08242cac7c..d3d12b6259 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -90,7 +90,7 @@ def celery_startup(conf=None, **kwargs): @worker_process_init.connect def task_set_logger_pre_run(*args, **kwargs): cache.close() - configure_external_logger(settings, is_startup=False) + configure_external_logger(settings, async_flag=False, is_startup=False) def _uwsgi_reload(): diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index 1a667d2ff5..08005f7ee4 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -181,7 +181,7 @@ def add_or_remove_logger(address, instance, adding=True): if isinstance(specific_logger.handlers[i], (HTTPSNullHandler, BaseHTTPSHandler)): i_occurance = i break - + if i_occurance is None and not adding: return elif i_occurance is None: @@ -205,29 +205,3 @@ def configure_external_logger(settings_module, async_flag=True, is_startup=True) add_or_remove_logger('awx.analytics', instance, adding=is_enabled) add_or_remove_logger('awx', instance, adding=(is_enabled and 'awx' in settings_module.LOG_AGGREGATOR_LOGGERS)) - - -def configure_external_logger_old(settings_module, async_flag=True, is_startup=True): - - from django.utils.log import configure_logging - - is_enabled = settings_module.LOG_AGGREGATOR_ENABLED - if is_startup and (not is_enabled): - # Pass-through if external logging not being used - return - - LOGGING_DICT = settings_module.LOGGING - if is_enabled: - LOGGING_DICT['handlers']['http_receiver']['class'] = 'awx.main.utils.handlers.BaseHTTPSHandler' - if not async_flag: - LOGGING_DICT['handlers']['http_receiver']['async'] = False - else: - LOGGING_DICT['handlers']['http_receiver']['async'] = True - for param, django_setting_name in PARAM_NAMES.items(): - LOGGING_DICT['handlers']['http_receiver'][param] = getattr(settings_module, django_setting_name, None) - if 'awx' in settings_module.LOG_AGGREGATOR_LOGGERS: - if 'http_receiver' not in LOGGING_DICT['loggers']['awx']['handlers']: - LOGGING_DICT['loggers']['awx']['handlers'] += ['http_receiver'] - # External logging not enabled, but removal of existing configuration needed - configure_logging(settings_module.LOGGING_CONFIG, LOGGING_DICT) - From d912013db759edc4ec56006fcec3790c811f9e66 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 10 Feb 2017 15:57:24 -0500 Subject: [PATCH 3/5] fix some bugs with the logging reload approach --- awx/main/utils/handlers.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index 08005f7ee4..d115df11c7 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -174,7 +174,7 @@ class HTTPSHandler(object): return BaseHTTPSHandler.from_django_settings(settings_module, *args, **kwargs) -def add_or_remove_logger(address, instance, adding=True): +def add_or_remove_logger(address, instance): specific_logger = logging.getLogger(address) i_occurance = None for i in range(len(specific_logger.handlers)): @@ -182,12 +182,14 @@ def add_or_remove_logger(address, instance, adding=True): i_occurance = i break - if i_occurance is None and not adding: - return - elif i_occurance is None: - specific_logger.handlers.append(instance) + if i_occurance is None: + if instance is not None: + specific_logger.handlers.append(instance) else: - specific_logger.handlers[i_occurance] = instance + if instance is None: + specific_logger.handlers[i_occurance] = HTTPSNullHandler() + else: + specific_logger.handlers[i_occurance] = instance def configure_external_logger(settings_module, async_flag=True, is_startup=True): @@ -197,11 +199,13 @@ def configure_external_logger(settings_module, async_flag=True, is_startup=True) # Pass-through if external logging not being used return + instance = None if is_enabled: instance = HTTPSHandler(settings_module, async=async_flag) instance.setFormatter(LogstashFormatter()) - else: - instance = HTTPSNullHandler() + awx_logger_instance = instance + if is_enabled and 'awx' not in settings_module.LOG_AGGREGATOR_LOGGERS: + awx_logger_instance = None - add_or_remove_logger('awx.analytics', instance, adding=is_enabled) - add_or_remove_logger('awx', instance, adding=(is_enabled and 'awx' in settings_module.LOG_AGGREGATOR_LOGGERS)) + add_or_remove_logger('awx.analytics', instance) + add_or_remove_logger('awx', awx_logger_instance) From 603dfea58082e22ef01ec23d1c2bbf1c271aded7 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 13 Feb 2017 14:54:02 -0500 Subject: [PATCH 4/5] get rid of HTTPSHandler subclass which is not needed --- awx/main/utils/handlers.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index d115df11c7..cc27823751 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -16,7 +16,7 @@ from requests_futures.sessions import FuturesSession from awx.main.utils.formatters import LogstashFormatter -__all__ = ['HTTPSNullHandler', 'BaseHTTPSHandler', 'HTTPSHandler', 'configure_external_logger'] +__all__ = ['HTTPSNullHandler', 'BaseHTTPSHandler', 'configure_external_logger'] # AWX external logging handler, generally designed to be used # with the accompanying LogstashHandler, derives from python-logstash library @@ -168,16 +168,10 @@ class BaseHTTPSHandler(logging.Handler): **self.get_post_kwargs(payload)) -class HTTPSHandler(object): - - def __new__(cls, settings_module, *args, **kwargs): - return BaseHTTPSHandler.from_django_settings(settings_module, *args, **kwargs) - - def add_or_remove_logger(address, instance): specific_logger = logging.getLogger(address) i_occurance = None - for i in range(len(specific_logger.handlers)): + for i, _ in enumerate(specific_logger.handlers): if isinstance(specific_logger.handlers[i], (HTTPSNullHandler, BaseHTTPSHandler)): i_occurance = i break @@ -201,7 +195,7 @@ def configure_external_logger(settings_module, async_flag=True, is_startup=True) instance = None if is_enabled: - instance = HTTPSHandler(settings_module, async=async_flag) + instance = BaseHTTPSHandler.from_django_settings(settings_module, async=async_flag) instance.setFormatter(LogstashFormatter()) awx_logger_instance = instance if is_enabled and 'awx' not in settings_module.LOG_AGGREGATOR_LOGGERS: From fbe712dcd14dcda9099993e71c9be6537830d7f1 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 13 Feb 2017 15:45:05 -0500 Subject: [PATCH 5/5] refactor adding logger loop from PR review --- awx/main/utils/handlers.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/awx/main/utils/handlers.py b/awx/main/utils/handlers.py index cc27823751..f3e555c4b5 100644 --- a/awx/main/utils/handlers.py +++ b/awx/main/utils/handlers.py @@ -170,20 +170,13 @@ class BaseHTTPSHandler(logging.Handler): def add_or_remove_logger(address, instance): specific_logger = logging.getLogger(address) - i_occurance = None - for i, _ in enumerate(specific_logger.handlers): - if isinstance(specific_logger.handlers[i], (HTTPSNullHandler, BaseHTTPSHandler)): - i_occurance = i + for i, handler in enumerate(specific_logger.handlers): + if isinstance(handler, (HTTPSNullHandler, BaseHTTPSHandler)): + specific_logger.handlers[i] = instance or HTTPSNullHandler() break - - if i_occurance is None: + else: if instance is not None: specific_logger.handlers.append(instance) - else: - if instance is None: - specific_logger.handlers[i_occurance] = HTTPSNullHandler() - else: - specific_logger.handlers[i_occurance] = instance def configure_external_logger(settings_module, async_flag=True, is_startup=True):