work on better json logging

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)

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:
+        for standard in self.standards:
-            print(type(standard))
+            if type(self).__name__.lower() not in standard.types:
-            if type(candidate).__name__.lower() not in standard.types:
                 continue
-            if settings.standards_filter and standard.name not in settings.standards_filter:
+            result = standard.check(self, settings.config)
-                continue
-            result = standard.check(candidate, settings)
 
             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(
+                    LOG.warn("{id}Best practice '{name}' not met:\n{path}:{error}".format(
-                        id=standard.id, name=standard.name, path=candidate.path, error=err), settings)
+                        id=standard.id, name=standard.name, path=self.path, error=err), extra=err_labels)
-                elif LooseVersion(standard.version) > LooseVersion(candidate.version):
+                elif LooseVersion(standard.version) > LooseVersion(self.version):
-                    warn("{id}Future standard '{name}' not met:\n{path}:{error}".format(
+                    LOG.warn("{id}Future standard '{name}' not met:\n{path}:{error}".format(
-                        id=standard.id, name=standard.name, path=candidate.path, error=err), settings)
+                        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(
+                    LOG.error("{id}Standard '{name}' not met:\n{path}:{error}".format(
-                        id=standard.id, name=standard.name, path=candidate.path, error=err))
+                        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)
+                    LOG.info("Best practice '%s' met" % standard.name, extra=labels)
-                elif LooseVersion(standard.version) > LooseVersion(candidate.version):
+                elif LooseVersion(standard.version) > LooseVersion(self.version):
-                    info("Future standard '%s' met" % standard.name, settings)
+                    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")
+        parentdir = os.path.dirname(os.path.abspath(filename))
-        # if os.path.exists(role_modules):
+        while parentdir != os.path.dirname(parentdir):
-        #     module_loader.add_directory(role_modules)
+            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
-

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):

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):

ansiblelater/settings.py

@@ -83,6 +83,7 @@ class Settings(object):
             },
             "logging": {
                 "level": logging.WARN,
+                "json": False
             },
             "ansible": {
                 "custom_modules": [],

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:
+        # if "id" not in standard_dict:
-            standard_dict.update(id="")
+        #     standard_dict.update(id="")
-        else:
+        # else:
-            standard_dict.update(id="[{}] ".format(standard_dict.get("id")))
+        #     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")

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