rocket-pool / smartnode

The CLI package for Rocket Pool smart nodes.
GNU General Public License v3.0
150 stars 112 forks source link

NodeMetricsPort and WatchtowerMetricsPort are ignored #689

Open jakubgs opened 1 month ago

jakubgs commented 1 month ago

It appears that both WatchtowerMetricsPort and NodeMetricsPort settings are ignored by the nodes: https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/shared/services/config/rocket-pool-config.go#L81-L83 And ExporterMetricsPort probably also.

Based on reading the code it appears the value for smartnode metrics port and address comes from CLI context: https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/rocketpool/node/metrics-exporter.go#L17 https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/rocketpool/node/metrics-exporter.go#L95-L96 Which as far as I can tell does not include settings from user-settings.yml file. The same goes for watchtower: https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/rocketpool/watchtower/metrics-exporter.go#L17 https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/rocketpool/watchtower/metrics-exporter.go#L42-L43 I base this off of the fact that urfave/cli provides the GlobalString() method, which to me excludes the possibility that the configurations are merged.

Also, the metrics ports settings are defined using your own type for parameters: https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/shared/services/config/rocket-pool-config.go#L427-L429 https://github.com/rocket-pool/smartnode/blob/4969a80aa3910d7adcb3ab834561c11a2a796e6c/shared/types/config/parameter.go#L10-L11 Which as far as I can tell is not merged with CLI flags.

Based on this I conclude that both of these settings in user config file are not used.