From 4f33afff3e2fb38da509dabe6171d68df3e3520a Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 3 Apr 2019 17:42:46 +0200 Subject: [PATCH] work on better json logging --- ansiblelater/__main__.py | 12 ++-- ansiblelater/command/candidates.py | 88 ++++++++++++++++++------------ ansiblelater/command/review.py | 13 ++++- ansiblelater/logger.py | 50 +++++++++++++---- ansiblelater/settings.py | 1 + ansiblelater/standard.py | 10 ++-- ansiblelater/utils/rulehelper.py | 4 +- 7 files changed, 119 insertions(+), 59 deletions(-) diff --git a/ansiblelater/__main__.py b/ansiblelater/__main__.py index b62005e..1930227 100755 --- a/ansiblelater/__main__.py +++ b/ansiblelater/__main__.py @@ -6,6 +6,7 @@ import logging from ansiblelater import __version__ from ansiblelater import LOG +from ansiblelater import logger from ansiblelater.command import base from ansiblelater.command import candidates @@ -29,12 +30,15 @@ def main(): args = parser.parse_args().__dict__ settings = base.get_settings(args) + config = settings.config # print(json.dumps(settings.config, indent=4, sort_keys=True)) - LOG.setLevel(settings.config["logging"]["level"]) - files = settings.config["rules"]["files"] - standards = base.get_standards(settings.config["rules"]["standards"]) - errors = [] + logger.update_logger(LOG, config["logging"]["level"], config["logging"]["json"]) + + files = config["rules"]["files"] + standards = base.get_standards(config["rules"]["standards"]) + + errors = 0 for filename in files: lines = None candidate = candidates.classify(filename, settings, standards) diff --git a/ansiblelater/command/candidates.py b/ansiblelater/command/candidates.py index 79f96a8..b6d81c9 100644 --- a/ansiblelater/command/candidates.py +++ b/ansiblelater/command/candidates.py @@ -2,6 +2,7 @@ import codecs +import copy import os import re import sys @@ -10,7 +11,8 @@ from distutils.version import LooseVersion import ansible from ansiblelater import LOG -from ansiblelater.utils import (get_property, is_line_in_ranges, lines_ranges, standards_latest) +from ansiblelater import utils +from ansiblelater.command.review import Error from ansiblelater.exceptions import ( # noqa LaterError, LaterAnsibleError ) @@ -37,10 +39,10 @@ class Candidate(object): self.path = filename self.binary = False self.vault = False - self.standards = standards self.filetype = type(self).__name__.lower() self.expected_version = True - self.version = self._find_version(settings) + self.standards = self._get_standards(settings, standards) + self.version = self._get_version(settings) try: with codecs.open(filename, mode="rb", encoding="utf-8") as f: @@ -49,13 +51,14 @@ class Candidate(object): except UnicodeDecodeError: self.binary = True - def _find_version(self, settings): + def _get_version(self, settings): if isinstance(self, RoleFile): parentdir = os.path.dirname(os.path.abspath(self.path)) while parentdir != os.path.dirname(parentdir): meta_file = os.path.join(parentdir, "meta", "main.yml") if os.path.exists(meta_file): path = meta_file + break parentdir = os.path.dirname(parentdir) else: path = self.path @@ -70,7 +73,7 @@ class Candidate(object): version = match.group(1) if not version: - version = standards_latest(self.standards) + version = utils.standards_latest(self.standards) if self.expected_version: if isinstance(self, RoleFile): LOG.warn("%s %s is in a role that contains a meta/main.yml without a declared " @@ -87,40 +90,58 @@ class Candidate(object): return version + def _get_standards(self, settings, standards): + target_standards = [] + limits = settings.config["rules"]["filter"] + + if limits: + for standard in standards: + if standard.id in limits: + target_standards.append(standard) + else: + target_standards = standards + + # print(target_standards) + return target_standards + def review(self, settings, lines=None): errors = 0 - for standard in standards.standards: - print(type(standard)) - if type(candidate).__name__.lower() not in standard.types: + for standard in self.standards: + if type(self).__name__.lower() not in standard.types: continue - if settings.standards_filter and standard.name not in settings.standards_filter: - continue - result = standard.check(candidate, settings) + result = standard.check(self, settings.config) if not result: - abort("Standard '%s' returns an empty result object." % + utils.sysexit_with_message("Standard '%s' returns an empty result object." % (standard.check.__name__)) + labels = {"tag": "review", "standard": standard.name, "file": self.path, "passed": True} + for err in [err for err in result.errors - if not err.lineno or is_line_in_ranges(err.lineno, lines_ranges(lines))]: + if not err.lineno or utils.is_line_in_ranges(err.lineno, utils.lines_ranges(lines))]: + err_labels = copy.copy(labels) + err_labels["passed"] = False + if isinstance(err, Error): + err_labels.update(err.to_dict()) + if not standard.version: - warn("{id}Best practice '{name}' not met:\n{path}:{error}".format( - id=standard.id, name=standard.name, path=candidate.path, error=err), settings) - elif LooseVersion(standard.version) > LooseVersion(candidate.version): - warn("{id}Future standard '{name}' not met:\n{path}:{error}".format( - id=standard.id, name=standard.name, path=candidate.path, error=err), settings) + LOG.warn("{id}Best practice '{name}' not met:\n{path}:{error}".format( + id=standard.id, name=standard.name, path=self.path, error=err), extra=err_labels) + elif LooseVersion(standard.version) > LooseVersion(self.version): + LOG.warn("{id}Future standard '{name}' not met:\n{path}:{error}".format( + id=standard.id, name=standard.name, path=self.path, error=err), extra=err_labels) else: - error("{id}Standard '{name}' not met:\n{path}:{error}".format( - id=standard.id, name=standard.name, path=candidate.path, error=err)) + LOG.error("{id}Standard '{name}' not met:\n{path}:{error}".format( + id=standard.id, name=standard.name, path=self.path, error=err), extra=err_labels) errors = errors + 1 if not result.errors: if not standard.version: - info("Best practice '%s' met" % standard.name, settings) - elif LooseVersion(standard.version) > LooseVersion(candidate.version): - info("Future standard '%s' met" % standard.name, settings) + LOG.info("Best practice '%s' met" % standard.name, extra=labels) + elif LooseVersion(standard.version) > LooseVersion(self.version): + LOG.info("Future standard '%s' met" % standard.name, extra=labels) else: - info("Standard '%s' met" % standard.name, settings) + LOG.info("Standard '%s' met" % standard.name) return errors @@ -134,18 +155,14 @@ class Candidate(object): class RoleFile(Candidate): def __init__(self, filename, settings={}, standards=[]): super(RoleFile, self).__init__(filename, settings, standards) - self.version = None - # parentdir = os.path.dirname(os.path.abspath(filename)) - # while parentdir != os.path.dirname(parentdir): - # meta_file = os.path.join(parentdir, "meta", "main.yml") - # if os.path.exists(meta_file): - # self.version = self._find_version(meta_file) - # if self.version: - # break - # role_modules = os.path.join(parentdir, "library") - # if os.path.exists(role_modules): - # module_loader.add_directory(role_modules) + parentdir = os.path.dirname(os.path.abspath(filename)) + while parentdir != os.path.dirname(parentdir): + role_modules = os.path.join(parentdir, "library") + if os.path.exists(role_modules): + module_loader.add_directory(role_modules) + break + parentdir = os.path.dirname(parentdir) class Playbook(Candidate): @@ -257,4 +274,3 @@ def classify(filename, settings={}, standards=[]): if "README" in basename: return Doc(filename, settings, standards) return None - diff --git a/ansiblelater/command/review.py b/ansiblelater/command/review.py index 2455d46..b746898 100644 --- a/ansiblelater/command/review.py +++ b/ansiblelater/command/review.py @@ -3,11 +3,13 @@ import os import sys +from six import iteritems + class Error(object): """Default error object created if a rule failed.""" - def __init__(self, lineno, message): + def __init__(self, lineno, message, error_type=None, **kwargs): """ Initialize a new error object and returns None. @@ -17,6 +19,9 @@ class Error(object): """ self.lineno = lineno self.message = message + self.kwargs = kwargs + for (key, value) in iteritems(kwargs): + setattr(self, key, value) def __repr__(self): # noqa if self.lineno: @@ -24,6 +29,12 @@ class Error(object): else: return " %s" % (self.message) + def to_dict(self): + result = dict(lineno=self.lineno, message=self.message) + for (key, value) in iteritems(self.kwargs): + result[key] = value + return result + class Result(object): def __init__(self, candidate, errors=None): diff --git a/ansiblelater/logger.py b/ansiblelater/logger.py index ab2142c..538c41b 100644 --- a/ansiblelater/logger.py +++ b/ansiblelater/logger.py @@ -8,6 +8,9 @@ import colorama from ansible.module_utils.parsing.convert_bool import boolean as to_bool from pythonjsonlogger import jsonlogger +CONSOLE_FORMAT = "%(levelname)s: %(message)s" +JSON_FORMAT = "(levelname) (message) (asctime)" + def _should_do_markup(): @@ -21,6 +24,19 @@ def _should_do_markup(): colorama.init(autoreset=True, strip=not _should_do_markup()) +def OverwriteMakeRecord(self, name, level, fn, lno, msg, args, exc_info, func=None, extra=None): + """ + A factory method which can be overridden in subclasses to create + specialized LogRecords. + """ + rv = logging.LogRecord(name, level, fn, lno, msg, args, exc_info, func) + if extra is not None: + for key in extra: + rv.__dict__[key] = extra[key] + print("xxx", rv.__dict__) + return rv + + class LogFilter(object): """A custom log filter which excludes log messages above the logged level.""" @@ -50,6 +66,7 @@ def get_logger(name=None, level=logging.DEBUG, json=False): """ logger = logging.getLogger(name) + logger.makeRecord(OverwriteMakeRecord) logger.setLevel(level) logger.addHandler(_get_error_handler(json=json)) logger.addHandler(_get_warn_handler(json=json)) @@ -60,14 +77,25 @@ def get_logger(name=None, level=logging.DEBUG, json=False): return logger +def update_logger(logger, level=None, json=None): + for handler in logger.handlers[:]: + logger.removeHandler(handler) + + logger.setLevel(level) + logger.addHandler(_get_error_handler(json=json)) + logger.addHandler(_get_warn_handler(json=json)) + logger.addHandler(_get_info_handler(json=json)) + logger.addHandler(_get_critical_handler(json=json)) + + def _get_error_handler(json=False): handler = logging.StreamHandler(sys.stderr) handler.setLevel(logging.ERROR) handler.addFilter(LogFilter(logging.ERROR)) - handler.setFormatter(logging.Formatter(error("%(message)s"))) + handler.setFormatter(logging.Formatter(error(CONSOLE_FORMAT))) if json: - handler.setFormatter(jsonlogger.JsonFormatter("%(message)s")) + handler.setFormatter(jsonlogger.JsonFormatter(JSON_FORMAT)) return handler @@ -76,10 +104,10 @@ def _get_warn_handler(json=False): handler = logging.StreamHandler(sys.stdout) handler.setLevel(logging.WARN) handler.addFilter(LogFilter(logging.WARN)) - handler.setFormatter(logging.Formatter(warn("%(message)s"))) + handler.setFormatter(logging.Formatter(warn(CONSOLE_FORMAT))) if json: - handler.setFormatter(jsonlogger.JsonFormatter("%(message)s")) + handler.setFormatter(jsonlogger.JsonFormatter(JSON_FORMAT)) return handler @@ -91,7 +119,7 @@ def _get_info_handler(json=False): handler.setFormatter(logging.Formatter(info("%(message)s"))) if json: - handler.setFormatter(jsonlogger.JsonFormatter("%(message)s")) + handler.setFormatter(jsonlogger.JsonFormatter(JSON_FORMAT)) return handler @@ -100,32 +128,32 @@ def _get_critical_handler(json=False): handler = logging.StreamHandler(sys.stderr) handler.setLevel(logging.CRITICAL) handler.addFilter(LogFilter(logging.CRITICAL)) - handler.setFormatter(logging.Formatter(critical("%(message)s"))) + handler.setFormatter(logging.Formatter(critical(CONSOLE_FORMAT))) if json: - handler.setFormatter(jsonlogger.JsonFormatter("%(message)s")) + handler.setFormatter(jsonlogger.JsonFormatter(JSON_FORMAT)) return handler def critical(message): """Format critical messages and return string.""" - return color_text(colorama.Fore.RED, "FATAL: {}".format(message)) + return color_text(colorama.Fore.RED, "{}".format(message)) def error(message): """Format error messages and return string.""" - return color_text(colorama.Fore.RED, "ERROR: {}".format(message)) + return color_text(colorama.Fore.RED, "{}".format(message)) def warn(message): """Format warn messages and return string.""" - return color_text(colorama.Fore.YELLOW, "WARN: {}".format(message)) + return color_text(colorama.Fore.YELLOW, "{}".format(message)) def info(message): """Format info messages and return string.""" - return color_text(colorama.Fore.BLUE, "INFO: {}".format(message)) + return color_text(colorama.Fore.BLUE, "{}".format(message)) def color_text(color, msg): diff --git a/ansiblelater/settings.py b/ansiblelater/settings.py index 8e018f5..7f00fcb 100644 --- a/ansiblelater/settings.py +++ b/ansiblelater/settings.py @@ -83,6 +83,7 @@ class Settings(object): }, "logging": { "level": logging.WARN, + "json": False }, "ansible": { "custom_modules": [], diff --git a/ansiblelater/standard.py b/ansiblelater/standard.py index 070bad8..0026b86 100644 --- a/ansiblelater/standard.py +++ b/ansiblelater/standard.py @@ -13,12 +13,12 @@ class Standard(object): :param standard_dict: Dictionary object containing all neseccary attributes """ - if "id" not in standard_dict: - standard_dict.update(id="") - else: - standard_dict.update(id="[{}] ".format(standard_dict.get("id"))) + # if "id" not in standard_dict: + # standard_dict.update(id="") + # else: + # standard_dict.update(id="[{}] ".format(standard_dict.get("id"))) - self.id = standard_dict.get("id") + self.id = standard_dict.get("id", "") self.name = standard_dict.get("name") self.version = standard_dict.get("version") self.check = standard_dict.get("check") diff --git a/ansiblelater/utils/rulehelper.py b/ansiblelater/utils/rulehelper.py index 6bafd34..9ee87d8 100644 --- a/ansiblelater/utils/rulehelper.py +++ b/ansiblelater/utils/rulehelper.py @@ -51,7 +51,7 @@ def get_normalized_task(task, candidate, settings): normalized = None errors = [] try: - normalized = normalize_task(task, candidate.path, settings.custom_modules) + normalized = normalize_task(task, candidate.path, settings["ansible"]["custom_modules"]) except LaterError as ex: e = ex.original errors.append(Error(e.problem_mark.line + 1, "syntax error: %s" % (e.problem))) @@ -77,7 +77,7 @@ def get_normalized_tasks(candidate, settings): if 'skip_ansible_lint' in (task.get('tags') or []): # No need to normalize_task if we are skipping it. continue - normalized.append(normalize_task(task, candidate.path, settings.custom_modules)) + normalized.append(normalize_task(task, candidate.path, settings["ansible"]["custom_modules"])) except LaterError as ex: e = ex.original