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.

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

Feature/adding logs to monkey island #139

Merged
merged 7 commits into from Jun 5, 2018

Conversation

Projects
None yet
2 participants
@MaorCore
Contributor

MaorCore commented May 30, 2018

Feature

This feature adds internal logging to the Monkey Island system, and allow the user to download it via the Log tab in the web app.

Changes

  • Replaced prints with logging functions
  • The logger is on the highest level and can be modified via JSON configuration
  • Added a download button in the Log tab on the web app that downloads the log file.

Handles #101

MaorCore added some commits May 17, 2018

Added a logging system to the monkey_island module.
Added a main function in main.py
Inserted a few logs to test the log system

@MaorCore MaorCore self-assigned this May 30, 2018

"class": "logging.StreamHandler",
"level": "DEBUG",
"formatter": "simple",
"stream": "ext://sys.stdout"

This comment has been minimized.

@danielguardicore

danielguardicore May 31, 2018

Contributor

Why not stderr?

This comment has been minimized.

@MaorCore

MaorCore May 31, 2018

Contributor

its just common practice

This comment has been minimized.

@MaorCore

MaorCore May 31, 2018

Contributor

It can be confusing to output both debug and errors alongside INFO level to stderr, thats why the split is being made on a higher level design. also splitting the streams is a bit of an overkill.

try:
return IslandLogService.get_log_file()
except Exception as e:
logger.error('Monkey Island logs failed to download', exc_info=True)

This comment has been minimized.

@danielguardicore

danielguardicore May 31, 2018

Contributor

I'd rather in this case return some sort of error message to the user, maybe indicating the exception, because this is a rare case where you can't debug the problem! 🤕

This comment has been minimized.

@MaorCore

MaorCore May 31, 2018

Contributor

Ok good idea

MaorCore added some commits May 31, 2018

I've added logs to cover these situations and modules:
    Configuration reset
    Configuration Insert
    Configuration Update
    Report steps
    Monkey downloads
    Env startup logs

Also I've changed the logging init position so it covers every functions from main, some functions and vars are being called and init from import level, in order to log those situations I had to init the log system right on the beginning of the module.

@danielguardicore danielguardicore merged commit d77704b into develop Jun 5, 2018

@danielguardicore danielguardicore deleted the feature/Adding_logs_to_monkey_island branch Jun 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment