sdk logging file size & rotation (#3495)

* sdk logging file size & rotation

Author: kobymeir

Diff for: .circleci/config.yml

@@ -27,6 +27,10 @@ parameters:
 
 
 references:
+    environment: &environment
+      environment:
+        DEMISTO_SDK_LOG_FILE_SIZE: "1073741824"  # 1GB
+
     version_tag_regex: &version_tag_regex
       /^v\d+\.\d+\.\d+$/ # version regex vx.x.x (i.e. v1.2.3)
 
@@ -75,6 +79,7 @@ references:
 
     install_poetry: &install_poetry
       - run:
+          <<: *environment
           name: Install Poetry
           command: |
             # in old images we need to remove existing poetry
@@ -120,6 +125,7 @@ jobs:
           type: string
       docker:
         - image: cimg/python:<< parameters.pythonversion >>-node
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -135,7 +141,7 @@ jobs:
             command: |
               shopt -u globstar
               # The first sed expression is to remove test files from under the demisto_sdk/commands/init/templates
-              # directory from the globbed list of files. Need to manually remove them since the pytest 'ignore'
+              # directory from the globed list of files. Need to manually remove them since the pytest 'ignore'
               # argument in pytest.ini is itself ignored when explicitly passing tests to pytest. The second is to
               # remove the integration tests (integration tests as opposed to unit tests) from the list because those
               # tests shouldn't be parallelized.
@@ -184,6 +190,7 @@ jobs:
           type: string
       docker:
         - image: cimg/python:<< parameters.pythonversion >>-node
+      <<: *environment
       steps:
         - checkout
         - <<: *install_neo4j
@@ -230,6 +237,7 @@ jobs:
   precommit-checks:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -262,6 +270,7 @@ jobs:
   checkout-content:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
         - checkout
         - run:
@@ -279,6 +288,7 @@ jobs:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
       resource_class: large
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -317,6 +327,7 @@ jobs:
   test-lint:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -359,6 +370,7 @@ jobs:
   create-content-artifacts:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -381,6 +393,7 @@ jobs:
     docker:
       - image: << pipeline.parameters.global-docker-image >>
     resource_class: xlarge
+    <<: *environment
     steps:
       - checkout
       - <<: *install_poetry
@@ -418,10 +431,11 @@ jobs:
   build:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
           - checkout
           - run:
-              name: Build Distrubution
+              name: Build Distribution
               command: poetry build
           - persist_to_workspace:
               root: ~/project
@@ -433,6 +447,7 @@ jobs:
   deploy:
       docker:
         - image: << pipeline.parameters.global-docker-image >>
+      <<: *environment
       steps:
         - checkout
         - <<: *install_poetry
@@ -445,6 +460,7 @@ jobs:
   waiter:
     docker:
       - image: circleci/node
+    <<: *environment
     steps:
       - run: |
           while [[ $(curl --location --request GET "https://circleci.com/api/v2/workflow/$CIRCLE_WORKFLOW_ID/job" | jq -r '.items[]|select(.name != "waiter")|.status' | grep -c "running") -gt 0 ]]
@@ -457,6 +473,7 @@ jobs:
   slack-notifier:
     docker:
       - image: cimg/python:<< pipeline.parameters.pythonversion_latest >>-node
+    <<: *environment
     steps:
       - checkout
       - <<: *install_poetry

Diff for: CHANGELOG.md

@@ -2,6 +2,9 @@
 ## Unreleased
 * Added a period at the end of lines produced by the **generate-docs** command that state the tested version of the product.
 * Update `RN112` validation's docs reference link.
+* Added support to control the maximum file size and log rotation files count in the sdk logger.
+* Fixed an issue with where passing the deprecated logging arguments to any command presented an incorrect recommendation for argument substitution.
+* Fixed an issue where the documentation of logging arguments was incorrect.
 * Fixed an issue in calculating content graph hash when creating or updating it.
 * Calling **graph create** or **graph update** now run the commands with default arguments, instead of showing the command help.
 * Removed the use of chunks when calculating content relationships.

Diff for: demisto_sdk/__main__.py

@@ -214,7 +214,7 @@ def main(ctx, config, version, release_notes, **kwargs):
         except DistributionNotFound:
             __version__ = "dev"
             logger.info(
-                "[yellow]Cound not find the version of the demisto-sdk. This usually happens when running in a development environment.[/yellow]"
+                "[yellow]Could not find the version of the demisto-sdk. This usually happens when running in a development environment.[/yellow]"
             )
         else:
             last_release = ""

Diff for: demisto_sdk/commands/common/logger.py

@@ -4,6 +4,7 @@
 import sys
 from logging.handlers import RotatingFileHandler
 from pathlib import Path
+from typing import Any, Optional, Union
 
 from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH
 from demisto_sdk.commands.common.tools import string_to_bool
@@ -25,18 +26,42 @@
 DATE_FORMAT = "%Y-%m-%dT%H:%M:%S"
 
 DEPRECATED_PARAMETERS = {
-    "-v": "--console-log-threshold or --file-log-threshold",
-    "-vv": "--console-log-threshold or --file-log-threshold",
-    "-vvv": "--console-log-threshold or --file-log-threshold",
-    "--verbose": "--console-log-threshold or --file-log-threshold",
-    "-q": "--console-log-threshold or --file-log-threshold",
-    "--quiet": "--console-log-threshold or --file-log-threshold",
-    "-ln": "--log-path",
-    "--log-name": "--log-path",
-    "no_logging": "--console-log-threshold or --file-log-threshold",
+    "-v": "--console_log_threshold or --file_log_threshold",
+    "-vv": "--console_log_threshold or --file_log_threshold",
+    "-vvv": "--console_log_threshold or --file_log_threshold",
+    "--verbose": "--console_log_threshold or --file_log_threshold",
+    "-q": "--console_log_threshold or --file_log_threshold",
+    "--quiet": "--console_log_threshold or --file_log_threshold",
+    "-ln": "--log_file_path",
+    "--log-name": "--log_file_path",
+    "no_logging": "--console_log_threshold or --file_log_threshold",
 }
 
+
+def parse_int_or_default(value: Any, default: int) -> int:
+    """
+    Parse int or return default value
+    Args:
+        value: value to parse
+        default: default value to return if parsing failed
+
+    Returns:
+        int: parsed value or default value
+
+    """
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return default
+
+
 SUCCESS_LEVEL: int = 25
+DEMISTO_SDK_LOG_FILE_SIZE = parse_int_or_default(
+    os.getenv("DEMISTO_SDK_LOG_FILE_SIZE"), 1_048_576  # 1MB
+)
+DEMISTO_SDK_LOG_FILE_COUNT = parse_int_or_default(
+    os.getenv("DEMISTO_SDK_LOG_FILE_COUNT"), 10
+)
 
 
 def handle_deprecated_args(input_args):
@@ -97,10 +122,14 @@ def handle_deprecated_args(input_args):
 
 
 def get_handler_by_name(logger: logging.Logger, handler_name: str):
-    for current_handler in logger.handlers:
-        if current_handler.get_name == handler_name:
-            return current_handler
-    return None
+    return next(
+        (
+            current_handler
+            for current_handler in logger.handlers
+            if current_handler.get_name == handler_name
+        ),
+        None,
+    )
 
 
 def set_demisto_logger(demisto_logger: logging.Logger):
@@ -124,7 +153,7 @@ def _add_logging_level(
     `logging` module or if the method name is already present
     Example
     -------
-    >>> addLoggingLevel('TRACE', logging.DEBUG - 5)
+    >>> _add_logging_level('TRACE', logging.DEBUG - 5)
     >>> logging.getLogger(__name__).setLevel("TRACE")
     >>> logging.getLogger(__name__).trace('that worked')
     >>> logging.trace('so did this')
@@ -144,17 +173,17 @@ def _add_logging_level(
     # This method was inspired by the answers to Stack Overflow post
     # http://stackoverflow.com/q/2183233/2988730, especially
     # http://stackoverflow.com/a/13638084/2988730
-    def logForLevel(self, message, *args, **kwargs):
+    def log_for_level(self, message, *args, **kwargs):
         if self.isEnabledFor(level_num):
             self._log(level_num, message, args, **kwargs)
 
-    def logToRoot(message, *args, **kwargs):
+    def log_to_root(message, *args, **kwargs):
         logging.log(level_num, message, *args, **kwargs)
 
     logging.addLevelName(level_num, level_name)
     setattr(logging, level_name, level_num)
-    setattr(logging.getLoggerClass(), method_name, logForLevel)
-    setattr(logging, method_name, logToRoot)
+    setattr(logging.getLoggerClass(), method_name, log_for_level)
+    setattr(logging, method_name, log_to_root)
 
 
 class ColorConsoleFormatter(logging.Formatter):
@@ -180,10 +209,7 @@ def __init__(
     @staticmethod
     def _record_contains_escapes(record: logging.LogRecord) -> bool:
         message = record.getMessage()
-        for key in escapes:
-            if not key.startswith("[/]") and key in message:
-                return True
-        return False
+        return any(not key.startswith("[/]") and key in message for key in escapes)
 
     @staticmethod
     def _string_starts_with_escapes(string: str) -> bool:
@@ -203,7 +229,7 @@ def _get_start_escapes(record: logging.LogRecord) -> str:
         while ColorConsoleFormatter._string_starts_with_escapes(current_message):
 
             # Record starts with escapes - Extract them
-            current_escape = current_message[0 : current_message.find("]") + 1]
+            current_escape = current_message[: current_message.find("]") + 1]
             ret_value += current_escape
             current_message = current_message[
                 len(current_escape) : current_message.find("]", len(current_escape)) + 1
@@ -263,16 +289,17 @@ def replace_escapes(self, message):
 
 
 def logging_setup(
-    console_log_threshold=logging.INFO,
-    file_log_threshold=logging.DEBUG,
-    log_file_path=LOG_FILE_PATH,
+    console_log_threshold: Union[int, str] = logging.INFO,
+    file_log_threshold: Union[int, str] = logging.DEBUG,
+    log_file_path: Optional[Union[str, Path]] = LOG_FILE_PATH,
 ) -> logging.Logger:
     """Init logger object for logging in demisto-sdk
         For more info - https://docs.python.org/3/library/logging.html
 
     Args:
-        console_log_threshold(int): Minimum console log threshold. Defaults to logging.INFO
-        file_log_threshold(int): Minimum console log threshold. Defaults to logging.INFO
+        console_log_threshold: Minimum console log threshold. Defaults to logging.INFO
+        file_log_threshold: Minimum console log threshold. Defaults to logging.INFO
+        log_file_path: Path to log file. Defaults to LOG_FILE_PATH
 
     Returns:
         logging.Logger: logger object
@@ -287,24 +314,22 @@ def logging_setup(
 
     console_handler = logging.StreamHandler()
     console_handler.set_name(CONSOLE_HANDLER)
-    console_handler.setLevel(
-        console_log_threshold if console_log_threshold else logging.INFO
-    )
+    console_handler.setLevel(console_log_threshold or logging.INFO)
 
     if custom_log_path := os.getenv("DEMISTO_SDK_LOG_FILE_PATH"):
         current_log_file_path = Path(custom_log_path)
     else:
-        current_log_file_path = log_file_path or LOG_FILE_PATH
-        if Path(current_log_file_path).is_dir():
+        current_log_file_path = Path(log_file_path or LOG_FILE_PATH)
+        if current_log_file_path.is_dir():
             current_log_file_path = current_log_file_path / LOG_FILE_NAME
     file_handler = RotatingFileHandler(
         filename=current_log_file_path,
         mode="a",
-        maxBytes=1048576,
-        backupCount=10,
+        maxBytes=DEMISTO_SDK_LOG_FILE_SIZE,
+        backupCount=DEMISTO_SDK_LOG_FILE_COUNT,
     )
     file_handler.set_name(FILE_HANDLER)
-    file_handler.setLevel(file_log_threshold if file_log_threshold else logging.DEBUG)
+    file_handler.setLevel(file_log_threshold or logging.DEBUG)
 
     if string_to_bool(os.getenv("DEMISTO_SDK_LOG_NO_COLORS", "False")):
         console_handler.setFormatter(fmt=NoColorFileFormatter())
@@ -335,9 +360,10 @@ def logging_setup(
     demisto_logger.debug(f"Platform: {platform.system()}")
 
     if not log_file_name_notified:
-        demisto_logger.info(
-            f"[yellow]Log file location: {current_log_file_path}[/yellow]"
-        )
+        if string_to_bool(os.getenv("DEMISTO_SDK_LOG_NOTIFY_PATH", "True")):
+            demisto_logger.info(
+                f"[yellow]Log file location: {current_log_file_path}[/yellow]"
+            )
         log_file_name_notified = True
 
     logger = demisto_logger

Diff for: demisto_sdk/commands/content_graph/README.md

@@ -71,7 +71,7 @@ When the graph creation is completed, it will be available in http://localhost:7
 
     Quiet output, only output results in the end.
 
-* **-lp, --log-path**
+* **-lp, --log_file_path**
 
     Path to store all levels of logs.
 
@@ -124,7 +124,7 @@ When the graph update is completed, it will be available in http://localhost:747
 
     Quiet output, only output results in the end.
 
-* **-lp, --log-path**
+* **-lp, --log_file_path**
 
     Path to store all levels of logs.
 

Diff for: demisto_sdk/commands/generate_modeling_rules/README.md

@@ -14,10 +14,9 @@ $ generate-modeling-rules [OPTIONS]
 * `-o, --output PATH`: A path to the folder you want to generate the modeling rules in. Best practice is to put the working pack path here.  [required]
 * `-ve, --vendor TEXT`: The vendor name of the product in snake_case.
 * `-p, --product TEXT`: The name of the product in snake_case.
-* `-v, --verbose INTEGER RANGE`: Verbosity level -v / -vv / .. / -vvv  [Default: 0; x<=3]
-* `--quiet / --no-quiet`: Quiet output - sets verbosity to default.  [Default: no-quiet]
-* `-lp, --log-path PATH`: Path of the directory in which you would like to store all log levels. If not given, then the "log_file_name" command line option will be disregarded, and the log output will be to stdout.
-* `-ln, --log-name TEXT`: The file name (including extension) where the log output should be saved to.  [Default: generate_modeling_rules.log]
+* `-clt, --console_log_threshold`: Minimum logging threshold for the console logger.  [default: INFO]
+* `-flt --file_log_threshold`: Minimum logging threshold for the file logger. [default: DEBUG]
+* `-lp, --log_file_path`:  Path to the log file. Default: ./demisto_sdk_debug.log. [default: ./demisto_sdk_debug.log]
 * `--install-completion`: Install completion for the current shell.
 * `--show-completion`: Show completion for the current shell, to copy it or customize the installation.
 * `--help`: Show this message and exit.

Diff for: demisto_sdk/commands/generate_unit_tests/README.md

@@ -13,12 +13,12 @@ Also supports generating unit tests for specific commands.
   Specific commands name to generate unit test for (e.g. xdr-get-incidents).
 * *-o, --output_dir*
   Directory to store the command output (generated test file) in (default is the input integration directory).
-* *-v, --verbose*
-  Verbose output - mainly for debugging purposes, logging level will be displayed accordingly.
-* *-q, --quiet*
-  Quiet output, only output results in the end.
-* *-lp, --log-path*
-  Path to store all levels of logs.
+* *-clt, --console_log_threshold*
+  Minimum logging threshold for the console logger.  [default: INFO]
+* *-flt --file_log_threshold*
+  Minimum logging threshold for the file logger. [default: DEBUG]
+* *-lp, --log_file_path*
+  Path to the log file. Default: ./demisto_sdk_debug.log. [default: ./demisto_sdk_debug.log]
 * *-e, --examples*
   One of the following:
   - A path for a file containing Integration command examples. Each command example should be in a separate line.