improvements: clean code #32
Conversation
| rpcs := cfg.GetRPCsMap() | ||
| rpcs, err := cfg.GetRPCsMap(paths) | ||
| if err != nil { | ||
| log.Fatal(err.Error()) |
There was a problem hiding this comment.
Validating a config is a nice feature but what if someone wants to use the exporter only for some of the paths? Maybe we should introduce a flag to validate and terminate exporter by default but give also an option to log a warning and continue if some RPCs are missing in the config. It will make an exporter useful for someone using it only for cosmos_wallet_balance metric
There was a problem hiding this comment.
Might be a good idea to have a flag but perhaps warrants a separate PR when the need arrives? I would like to keep the scope of this PR limited to current usecase
|
|
||
| log.Debug("Stop collecting", zap.String("metric", walletBalanceMetricName)) | ||
| } | ||
| var ctx = context.Background() |
There was a problem hiding this comment.
Why to create a package global variable? Isn't it better to add ctx property to each collector struct which needs it?
There was a problem hiding this comment.
did this to please the linter :) I did consider collector level context but at the end the separation is only code files but the other variables are also global to package level e.g. clientExpiry. Linter liked this way better, is it a blocker? I am fine either way
There was a problem hiding this comment.
up to you, doesn't matter much
|
|
||
| port := urlParsed.Port() | ||
|
|
||
| // Port must be a iny <= 65535. |
fixes netx-145