-
Notifications
You must be signed in to change notification settings - Fork 561
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
Use common logging in crypto and others #447
Use common logging in crypto and others #447
Conversation
This is slightly more complicated since olevba has its own json-print functions that need to be made compatible with log_helper's json formatting
Make log_helper compatible with olevba which has its own print_json function and does not (yet?) use logging to create json.
Do not need the last meta-information return code json output from olevba any more
So glad we have the unittests, otherwise would never have found this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. However, I see you removed the function enable_logging() from olevba: in that case, how can another python app which imports olevba control whether olevba logging is enabled or not? (for example ViperMonkey).
In the way I design my tools (to be used either as modules or CLI apps), I would like logging to be disabled by default when imported as a module. Then it's up to the main function (in CLI mode) or the calling app to enable logging in the module if needed.
That's why the olevba logger was set by default with loglevel=CRITICAL+1 (i.e. all logging is ignored), and if enable_logging() is called, loglevel becomes NOTSET (which means the main app controls the loglevel).
Is it still working that way with log_helper? (I have not checked the code yet)
Logging uses the default StreamHandler, which per default sends its output to sys.stderr. However, the surrounding '[', ']' are printed to stdout. Fix that.
The common logger sends its output to stderr when in json mode. Do the same here
Found some trouble with logging, mixing between stdout and stderr. Fixing that first.... |
I will check and ensure that is possible |
Use common logging framework in crypto
Since crypto is used from olevba, this was the perfect opportunity to use the logging also in olevba. However, there is so much custom json code in olevba and many great plans on how to improve result handling there, that I did not do big rework of json-output in olevba. Instead, I made the log_helper compatible with olevba-json-output using an additional flag and made only small changes to olevba json output.
Also use log_helper in record_base (easy one)