[In progress] Use python logging framework consistently#920
[In progress] Use python logging framework consistently#920hamiltont wants to merge 14 commits intoTechEmpower:masterfrom
Conversation
toolset/benchmark/benchmarker.py
Outdated
There was a problem hiding this comment.
https://docs.python.org/2.7/library/logging.html#logging.Logger.exception
log.exception("Error loading %s", config_file_name)
There was a problem hiding this comment.
Uh, I've missed following raise.
Traceback will show after.
log.error("Error loading '%s'", config_file_name) is enough.
|
This now adds real-time console output from __run_test and any setup.py files, and stores info on uncaught exceptions into log files. It needs some more testing before I'm sure work is done here, just wanted to share. I may update again to add automatic per-framework generation of installation log files (idea from @stefanocasazza ), in addition to the current console output |
|
How will the affect test output? Currently, we are just using a file and writing/flushing. If you look at the setup.py file for most tests, all their calls will pipe stdout and stderr to the appropriate log file, but I am not sure you can simply pass the python 'log' object around like that. |
|
Mike, As far as setup.py, this change is invisible. Start still gets stdout and
|
|
Here's a big chunk of output so it's more clear what the new changes add. In general, moving us to the logging framework means 1) no more "add then remove print statements" when debugging, and 2) ability to use filters/log levels/etc to have very selective logging. For example, you could turn on DEBUG just for one method or just for one file, you could send logs to a file with a few lines of code, etc PS - this is not merge-ready at the moment, I need to refine/test these changes From Console, Running Default --log From Console, Running --log DEBUG From ec2/latest/logs/go, Regardless of --log setting |
|
The output from the verification step (json, in your example) is included still, correct? We are showing the response from the /json test still? |
|
Yes, and not only in the logs but also on the console while the tests are running. I've just truncated this so I don't paste huge log files. |
|
This is hot! Thanks again for another excellent contribution @hamiltont. |
|
Ok, this is back to "good to merge" status (after your own tests, of course). See here for example output from a number of sources. A few minor changes happened:
Output from a normal run (e.g. no |
This lets the handlers set their own levels. Otherwise all handlers are limited to the messages that the root logger is willing to pass to them (e.g. they only get messages that pass root logger's level setting)
Instead of opening a file handler, just use the logging framework. In addition to the normal StreamHandler, register a FileHandler to save a log file from running each test. The setup.py files depend on out and err file objects, so we fake them with a new class WrapLogger. This allows us to see real-time output from those files as well. It is also easier for the setup.py files as they use subprocess heavily, and it uses parameters for errfile and outfile
- Adds a default StreamHandler inside of __run_test that can be used to send unformatted output to stdout. This basically emulates 'print' while allowing the messages to also be logged into the FileHandler - Docs at top of FrameworkTest - Removes some extra calls to basicConfig - Refines WrapLogger to support all log levels - Adds utils.py file to store usefulties
Our usage of textwrap was causing extra newlines at the prefix and suffix of each message. This fixes that issue and cleans up the codebase a bit
Changes this poor output:
benchmarker : INFO --------------------------------------------------------------------------------
Starting go
--------------------------------------------------------------------------------
Into this better output:
benchmarker : INFO
--------------------------------------------------------------------------------
Starting go
--------------------------------------------------------------------------------
…t, stop Thanks TechEmpower#920 for bringing this to my attention [ci skip]
|
This needs to be rebased on master and tested a bit more before it can be merged. Also, I should probably make sure my other logging improvement attempt branch and this work are in agreement. Leaving open to remind me to circle around to this when I have time... |
05fbfd0 to
d5b976a
Compare
|
Ugh, this is over a year old and DEFINITELY needs to be rebased off master. @ssmith-techempower See what you can do on a new branch with these changes... probably a lot of merge conflicts. Maybe it makes more sense to open a new PR with the ideas set forth here (using |
ddbc264 to
c948188
Compare
|
Okay, I'm closing this pull request. We are still using simple file handles for writing out/err logging, but this pull request is dead. |
This is basically a forward-looking commit. Currently some modules use logging, some dont, some use
print, etc. This just standardizes us on python's logging and provides enough logging code to allow
people to have an idea of what is expected from their additions
Replaces
--verboseflag with--logflagI'd propose some minimal guidelines for logging:
elsestatements just to logObviously these are just some general thoughts, feedback welcome