mgmt
mgmt copied to clipboard
[LOVE] Unify all the various debug flags under a single arg
Welcome:
This is a beginner to intermediate level patch for someone who is new or somewhat familiar with golang, and is new to mgmt. It is meant to encourage new programmers to get involved with mgmt. It will probably take you a few hours or so to complete.
Intro:
Mgmt has a number of different debug flags and booleans throughout the code base. We'd like to unify them all behind a single --debug flag parameter. To implement this, you'll need to either recommend an alternate design, or use the one provided below.
Design:
Some of the existing code was written without properly passing the correct flags through. There exist different constants and other sources for debug. In newer parts of the code, the flag is actually consumed and passed down to child processes. For this patch, you should first identify cases where debug is not using the value that comes from main.go. Secondly, you should switch the constant in main.go to be a command line arg which is used instead. Lastly you should update the README to correct the statement about the old usage of the debug flag.
Once you've decided on a design, present it here for discussion before you implement things.
Read the contributing tips: https://github.com/purpleidea/mgmt/blob/master/.github/PULL_REQUEST_TEMPLATE.md
Happy hacking, and ping me for questions!
Bonus/Future work:
- Add a debug flag to a section which does not have it, but needs it.
- Add the logf pattern to any part of the code which is currently using
log.Printf
, instead of a logf helper which gets a prefix passed in.
I believe that we could extend a bit the reach of this issue and go further and implement a nice logging solution which should help the project in the long run.
There is this project, which is the only project that I know that it is active which takes care of logging for golang and is being actively maintained: https://github.com/Sirupsen/logrus. It allows using different levels of logging as well as outputting the logs in several different formats if we wish for in the future.
This would involve not only adding a new library to the code, but rewrite possibly most of the strings being output currently in mgmt. Using logrus would also allow us to have several different levels of output all around the code, without using if obj.flags.debug
but write directly log.Debug("Fancy debug message")
. This way we can unify how we write log/output and also allow flexibility on where/how we want to do logging.
@dnegreira Hey,
I'm going to unfortunately have to NACK the idea of using a package like logrus at this time. The reason is that longer term, the amount of log output from mgmt (and in particular the fact that a lot of it will happen in parallel) will require something clever to display everything without overwhelming the user. As a result, using an existing log package won't really help here.
If you'd like to work on the approach I have in mind for future logging, then happy to discuss with you if you'd like to hack on it.
Note: a good deal of this was completed incidentally with the recent engine re-write. There is still some left to do. It's easy to find candidates by running an ack '"log"'
which looks for imports of the log
package. Only one or a few should exist in the core package by the time this is done.