addressed review comment: removed loglevel and added logformat and re…

…factored setup_logging to leverage verbosity

wptagent.py

@@ -1059,63 +1059,58 @@ def get_browser_versions(browsers):
             browsers[browser]['version'] = get_file_version(exe)
 
 
-# Constant used for --loglevel command line parameter mapping
-# Limited on purpose to python logging levels to allow easily to change the
-# handler if needed.
-LOG_LEVEL_MAPPING = {
-    "critical": logging.CRITICAL,
-    "error": logging.ERROR,
-    "warning": logging.WARNING,
-    "info": logging.INFO,
-    "debug": logging.DEBUG
-}
-
-def setup_logging(verbosity=0, logging_level=None, log_file=None):
+# Constant used for --logformat command line parameter mapping
+LOG_FORMATS = ["syslog"]
+
+def setup_logging(verbosity=0, log_format=None, log_file=None):
     """Setup logging according to passed command line values"""
 
-    # logging_level has priority over other parameters.
-    # When specified the logging to file will be the only allowed type of logging.
-    # Must be used in conjunction with log_file.
-    if logging_level != None and log_file != None:
-        logger = logging.getLogger()
-        logger.setLevel(LOG_LEVEL_MAPPING[logging_level])
-        handler = Rfc5424SysLogHandler( \
-            address=None, \
-            socktype=None, \
-            framing=None, \
-            msg_as_utf8=True, \
-            appname="wptagent", \
-            # Currently used just for versioning
-            enterprise_id="1", \
-            utc_timestamp=True, \
-            file_name=log_file \
-            )
-
-        logger.addHandler(handler)
-        logger.debug("Rfc5424SysLogHandler initialized", extra=logging_context().as_extra({"msg_as_utf8": True}))
-    else:
-        # Set log level and legacy format
-        basic_log_level = logging.CRITICAL
-        if verbosity == None:
-            pass # default critical
-        elif verbosity == 1:
-            basic_log_level = logging.ERROR
-        elif verbosity == 2:
-            basic_log_level = logging.WARNING
-        elif verbosity == 3:
-            basic_log_level = logging.INFO
-        elif verbosity >= 4:
-            basic_log_level = logging.DEBUG
+    # Set log level and legacy format
+    basic_log_level = logging.CRITICAL
+    if verbosity is None:
+        pass # default critical
+    elif verbosity == 1:
+        basic_log_level = logging.ERROR
+    elif verbosity == 2:
+        basic_log_level = logging.WARNING
+    elif verbosity == 3:
+        basic_log_level = logging.INFO
+    elif verbosity >= 4:
+        basic_log_level = logging.DEBUG
+
+    if log_format is None:
+        # legacy behavior
         logging.basicConfig(level=basic_log_level, format="%(asctime)s.%(msecs)03d - %(message)s",
                             datefmt="%H:%M:%S")
 
-        # If file is specified without logging_level legacy behavior is to add
-        # a self rotating file with just errors level or above.
-        if log_file:
+        # If file is specified add self rotating file with just errors level or above.
+        if log_file is not None:
             err_log = logging.handlers.RotatingFileHandler(log_file, maxBytes=1000000,
                                                         backupCount=5, delay=True)
             err_log.setLevel(logging.ERROR)
             logging.getLogger().addHandler(err_log)
+    else:
+        if log_format == "syslog" and log_file is not None:
+            logger = logging.getLogger()
+            logger.setLevel(basic_log_level)
+            handler = Rfc5424SysLogHandler( \
+                address=None, \
+                socktype=None, \
+                framing=None, \
+                msg_as_utf8=True, \
+                appname="wptagent", \
+                # Currently used just for versioning
+                enterprise_id="1", \
+                utc_timestamp=True, \
+                file_name=log_file \
+                )
+
+            logger.addHandler(handler)
+            logger.debug("Rfc5424SysLogHandler initialized", extra=logging_context().as_extra({"msg_as_utf8": True}))
+        else:
+            # used default loggger
+            logging.critical("log_file must be specified if log_format is used.")
+            exit(1)
 
 def main():
     """Startup and initialization"""
@@ -1140,16 +1135,14 @@ def main():
                         help="Watchdog file to update when successfully connected.")
     parser.add_argument('--log',
                         help="Output the logs to the given file."
-                         "When used in conjunction with --loglevel option, the log entries will be in rfc5424 format "
-                         "Using UTC timestamps."
-                         "For backward compatibility if --loglevel is not specified only critical errors will be logged "
-                         "to the file with the historic non standard format")
-    parser.add_argument('--loglevel',
-                        help="Must be used in conjunction with --log otherwise takes no effect."
-                        "Controls the log level of the rfc5424 logging."
-                        "When specified no other logging will be enabled.",
+                         "For backward compatibility if --logformat is not specified only critical errors will be logged "
+                         "to the file with the legacy non standardized format")
+    parser.add_argument('--logformat',
+                        help="Change the log level format when using '--log' option."
+                        "When this option is specified will only print to the file specified with '--log'."
+                        "When this parameter is used verbosity will also take effect.",
                         type=str.lower,
-                        choices=LOG_LEVEL_MAPPING.keys())
+                        choices=LOG_FORMATS)
     parser.add_argument('--noidle', action='store_true', default=False,
                         help="Do not wait for system idle at any point.")
     parser.add_argument('--collectversion', action='store_true', default=False,
@@ -1282,7 +1275,7 @@ def main():
         exit(1)
 
     # Set up logging
-    setup_logging(options.verbose, options.loglevel, options.log)
+    setup_logging(options.verbose, options.logformat, options.log)
 
     if options.ec2 or options.gce:
         upgrade_pip_modules()