Open
Description
Bug
Ignition Version
2.0.0+
Summary
Ignition currently doesn't have any guidelines for how to report errors or when/how to log things. The only guiding principle is "if something fails, fail hard".
Some questions to answer:
- When do we wrap errors like:
if err := someFunc(foo); err != nil {
return fmt.Errorf("error someFunc-ing on %s: %v", foo.String(), err)
}
- When do we log? If we have functions
A, B, C, D
which all call each other likeA(B(C(D())))
do we log at all levels? Is it context dependent? - How do we determine what level to log at? Ignition currently has
Emergency
,Alert
,Critical
,Error
,Warning
,Notice
,Info
, andDebug
. Do we need that many? This was mostly inherited from https://golang.org/pkg/log/syslog/.
Proposals:
- Always wrap errors unless we have a strong reason not to. Consider using something like https://github.com/hashicorp/errwrap or https://godoc.org/github.com/pkg/errors to ensure a common structure.
- Drop
Emergency
,Alert
,Critical
andNotice
, from the logger interface. I don't see a distinction between the first 3 andError
orNotice
andInfo
. That leaves us with justError
,Warning
,Info
, andDebug
. - Only use
Error
for things that are fatal (which are most things, given the fail hard philosophy). UseWarning
for things that seem wrong,Info
for general logging andDebug
for things that would only seem useful to Ignition developers. - Use the
logger.PushPrefix
andlogger.PopPrefix
more. - Log actions that are long running (e.g. fetching resources over network), modify the system (i.e. writing a file), are significant events (e.g. subsections of stages starting/ending), or errors at the top level where they are handled (assuming we wrap them so they still have context).
- Debug logging can break these rules and be inserted wherever useful.