[logging] Adds a method for using specific messaging for known errors #517
Conversation
Pull Request Test Coverage Report for Build 138755272
💛 - Coveralls |
| @@ -0,0 +1,45 @@ | |||
| package logging | |||
There was a problem hiding this comment.
May need to have copyright as multus/multus.go
logging/known_errors.go
Outdated
| "strings" | ||
| ) | ||
|
|
||
| // knownErrorPatterns Returns list of all known error patterns |
There was a problem hiding this comment.
How about to use variable instead of function?
var knownErrorPatterns = []string{
"error dialing DHCP daemon",
}
logging/known_errors.go
Outdated
| for _, eachstringer := range a { | ||
| for _, eachknownerror := range knownerrors { | ||
| if strings.Contains(fmt.Sprintf("%s", eachstringer), eachknownerror) { | ||
| knownerrormessage, _ = getKnownErrorMessage(eachknownerror) |
There was a problem hiding this comment.
How about to add error handling for getKnownErrorMessage() because getKnownErrorMessage() returns error.
| return val, nil | ||
| } | ||
|
|
||
| return "", fmt.Errorf("Known error key '" + patternkey + "' does not have a message") |
There was a problem hiding this comment.
We may be use lower case for error message
https://github.com/golang/go/wiki/CodeReviewComments#error-strings
logging/known_errors.go
Outdated
| if strings.Contains(fmt.Sprintf("%s", eachstringer), eachknownerror) { | ||
| knownerrormessage, _ = getKnownErrorMessage(eachknownerror) | ||
| knownerrormessage = " (" + knownerrormessage + ") " | ||
| break knownerrorloop |
There was a problem hiding this comment.
use continue instead of break?
dougbtv
force-pushed
the
known-errors-logging
branch
from
99b8ba8
to
488fdc5
Compare
|
Thank you very much for the thorough review! Quite helpful. I have a new commit up to address it 👍 |
|
This pull request is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This introduces a new set of "known errors" -- a series of known errors that may occur, especially related to usage.
The goal is to add some actionable instructions for users when they encounter something that we know to occur with some frequency. In this case, we start with a single known error -- that error being when the DHCP CNI daemon isn't running the DHCP IPAM CNI plugin returns an error stating so much. Here, we instruct the user to make sure the DHCP CNI daemon is running.
The downside is that this makes the error messages even longer for the time being. However, I'm hopeful that in the future this makes for a way to shorten at least some known error messages.