nifikop
nifikop copied to clipboard
Rewrite logging levels and content
Bug Report
The current nifikop logging can be pretty noisy, with varied usefulness.
We should rewriting the log content/levels to be more helpful to users
A few examples that i've come across working on nifikop:
-
When logs are created about a resource, include the name of the relevant resource. When there are many like resources, it's not clear which the log is about.
- Example:
r.Log.Info(fmt.Sprintf("nifi cluster communication error while removing node id: %s", nodeId))
- Example:
-
Re-evaluate verbosity (e.g. log level) of some logs. Maybe some general guidelines?
- These seem to be good guidelines: https://github.com/uber-go/zap/blob/v1.21.0/zapcore/level.go#L34-L52
- IMO,
Info
logs should be the actions taken by the operator (creations, deletions, modifications, etc).Debug
should be anything more (starting/ending reconciliation, etc). Perhaps there are cases i'm not considering here.
-
The events that nifikop generates in Kubernetes are also quite verbose and not all together useful. Lots of "Reconciling resource x..." events that drown the event feed in the rancher UI, for example. These should drastically be reduced as well to be major resource state changes or errors.
Apart from #121, the below is a major area i've seen that could use some attention.
The below link refers to this RequeueWithError()
method, which is called 500+ times as a result of an error but it logs all of the error messages at the Info
level. There's no context for what the error was, so it's not possible to know what context should be added to the log. Callers right now provide a parameter that's formatted with fmt.Sprintf("...")
, but we can make use of the zap lib to consistently log errors. Callers should log the context for the error and then requeue the request.
https://github.com/konpyutaika/nifikop/blob/v0.11.0-release/controllers/controller_common.go#L24-L28