Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added log config and replaced print with log #235

Closed
wants to merge 6 commits into from
Closed

added log config and replaced print with log #235

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
78cd8d6
added log config and replaced print with log
rjarun8235 Jul 1, 2023
eb38052
removed redundant logger config
rjarun8235 Jul 3, 2023
06c9a0a
resolved pylint error
rjarun8235 Jul 14, 2023
f474808
PR comment - added new line at end of file
rjarun8235 Jul 17, 2023
cb25ffb
PR comment - added new line at end of file
rjarun8235 Jul 17, 2023
88a59b4
review commentes : removed old commented code
rjarun8235 Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/gcp_scanner/scanner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
from pathlib import Path
from typing import List, Tuple, Dict, Optional, Union

# Configure the logging module
logging.basicConfig(format='%(asctime)s %(message)s', level=logging.INFO, datefmt='%Y-%m-%d %H:%M:%S')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have it already here:

gcp_scanner/src/gcp_scanner/scanner.py

Line 529 in b8c75d6

logging.basicConfig(level=getattr(logging, args.log_level.upper(), None),


from google.auth.exceptions import MalformedError
from google.cloud import container_v1
from google.cloud import iam_credentials
Expand Down Expand Up @@ -166,7 +169,7 @@ def crawl_loop(initial_sa_tuples: List[Tuple[str, Credentials, List[str]]],

project_id = project['projectId']
project_number = project['projectNumber']
print(f'Inspecting project {project_id}')
logging.info(f'Inspecting project {project_id}')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually want this to be printed in console so user understand what's going on.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using logging for everything makes sense here and have the default log level be info. The output is metadata about the process, not the result of the process itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using logging for everything makes sense here and have the default log level be info. The output is metadata about the process, not the result of the process itself.

Will there always be a logging file as part of the arguments? coz I was thinking if that file name is optional and not mandatory we can separate the logging as a filehandle for file logging and console handler for console printing

something like this

import os
import logging

def main():
    log_level = getattr(logging, 'INFO', None)
    log_format = '%(asctime)s - %(levelname)s - %(message)s'
    date_format = '%Y-%m-%d %H:%M:%S'
    
    # Create a formatter
    formatter = logging.Formatter(fmt=log_format, datefmt=date_format)

    # Get the root logger
    logger = logging.getLogger()
    logger.setLevel(log_level)

    # Check if the log file exists
    if os.path.exists('my_log_file.log'):
        # If the log file exists, create a handler for the log file
        file_handler = logging.FileHandler(filename='my_log_file.log', mode='a')
        file_handler.setFormatter(formatter)
        logger.addHandler(file_handler)
    else:
        # If the log file does not exist, create a handler for the console
        console_handler = logging.StreamHandler()
        console_handler.setFormatter(formatter)
        logger.addHandler(console_handler)

    logger.info("this is a new line")

main()  # Call the main function

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using logging for everything makes sense here and have the default log level be info. The output is metadata about the process, not the result of the process itself.

Yea, probably that's how it should be. I was thinking about having INFO to be default but in some cases it is just a lot of data in terminal. IMO, it would be great to allow user to specify verbosity level instead of just ERROR, WARNING, INFO, with default (verbosity 0 or 1) to be minimum required to understand that scanner is working and scanning projects (probably print now) and maximum (e.g. 3) where we print everything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using logging for everything makes sense here and have the default log level be info. The output is metadata about the process, not the result of the process itself.

Will there always be a logging file as part of the arguments? coz I was thinking if that file name is optional and not mandatory we can separate the logging as a filehandle for file logging and console handler for console printing

something like this

import os
import logging

def main():
    log_level = getattr(logging, 'INFO', None)
    log_format = '%(asctime)s - %(levelname)s - %(message)s'
    date_format = '%Y-%m-%d %H:%M:%S'
    
    # Create a formatter
    formatter = logging.Formatter(fmt=log_format, datefmt=date_format)

    # Get the root logger
    logger = logging.getLogger()
    logger.setLevel(log_level)

    # Check if the log file exists
    if os.path.exists('my_log_file.log'):
        # If the log file exists, create a handler for the log file
        file_handler = logging.FileHandler(filename='my_log_file.log', mode='a')
        file_handler.setFormatter(formatter)
        logger.addHandler(file_handler)
    else:
        # If the log file does not exist, create a handler for the console
        console_handler = logging.StreamHandler()
        console_handler.setFormatter(formatter)
        logger.addHandler(console_handler)

    logger.info("this is a new line")

main()  # Call the main function

The log filename argument is optional. If it is set, we print in the file and if not the argument is None and it is simply console printing as far as I remember.

project_result = sa_results['projects'][project_id]

project_result['project_info'] = project
Expand Down