Update dependencies for 2.40 #11524
Update dependencies for 2.40 #11524
Conversation
codesome
requested review from
roidelapluie,
juliusv,
beorn7 and
LeviHarrison
|
CI looks like some types in dependencies have changed... |
codesome
force-pushed
the
updeps
branch
2 times, most recently
from
6aae35d
to
6ce1267
Compare
| @@ -251,7 +250,10 @@ func main() { | |||
| a.Flag("web.listen-address", "Address to listen on for UI, API, and telemetry."). | |||
| Default("0.0.0.0:9090").StringVar(&cfg.web.ListenAddress) | |||
|
|
|||
| webConfig := toolkit_webflag.AddFlags(a) | |||
| webConfig := a.Flag( | |||
There was a problem hiding this comment.
The toolkit_webflag.AddFlags dependency introduced a new flag and registers web.listen-address flag to be a slice of address with different message. So I just copied ther web config registration here to keep it same.
There was a problem hiding this comment.
Hmm, I think just updating to the new AddFlags would be better, or would that break anything else I'm not aware of?
There was a problem hiding this comment.
- It adds a new flag
web.systemd-socketwhich is not used in Prometheus but still will be visible in the help text. - In Prometheus
web.listen-addressis a single address, but the AddFlags overrides it to be a list of address and help text saying you can add more than one address, so we can't use that in Prometheus directly. (the code)
There was a problem hiding this comment.
Ah yeah, I was aware of the multiple addresses but thought we could just allow listening on multiple in Prometheus as well.
For the systemd socket, by skimming prometheus/exporter-toolkit#95 it looks like it'd just work for Prometheus as well, but of course there's limited use for that in Prometheus, which should always be running. I think in the long run it'd be good to modify the exporter toolkit to make that flag optional (that way we have to duplicate less boiler plate here), but this PR is fine for now.
There was a problem hiding this comment.
As noted in that pull, the purpose of the systemd socket is so that the service can use a privileged port when running as non-root without any additional messy configuration on the operating system side. It also prevents other unprivileged processes from stealing the unprivileged port. In general it provides an optionally more secure way to listen on systemd-based systems. The benefits of this are more thoroughly explained in prometheus/node_exporter#2393. There was not much interest in jumpstarting a service that should always be running.
In general the pull allows for listening in multiple places and aims to reduce boilerplate everywhere else by urging exporter maintainers to not use the config struct directly. Possibly there is a way to make use of the same code without duplicating it here? It simplifies listening on multiple addresses or systemd sockets to only a few lines:
https://github.com/prometheus/node_exporter/pull/2393/files#diff-7907251477470326df65250bf7fa699e0756ce5d6282e27b34c112132a7ef6b5R193-R197
Signed-off-by: Ganesh Vernekar <ganeshvern@gmail.com>
|
@juliusv should be all good now |
|
I'm a bit confused that there are so many updated dependencies. Shouldn't they update automatically via dependabot? |
Not sure how the bot works :/ |
In prepartion for v2.40.0