feat(metrics): add auth support for Prometheus push gateway mode#20045
feat(metrics): add auth support for Prometheus push gateway mode#20045mablr wants to merge 6 commits intoparadigmxyz:mainfrom
Conversation
mablr
requested review from
Rjected,
gakonst,
klkvr and
mattsse
as code owners
| requires = "push_gateway_password", | ||
| env = "RETH_METRICS_PUSH_USERNAME" | ||
| )] | ||
| pub push_gateway_username: Option<String>, |
There was a problem hiding this comment.
should we perhaps just combine these two into one arg?
There was a problem hiding this comment.
Do you mean :
- a single arg "username:password" that will be parsed ?
- or an inner flatten optional Clap arg struct containing username and password fields ?
There was a problem hiding this comment.
I realize it should be possible to set username and leave password empty. (I'll fix that)
So the "username:password" argument to parse is definitely too complex compared to the benefit of having a single auth arg.
| default_value = "5", | ||
| value_parser = parse_duration_from_secs, | ||
| value_name = "SECONDS", | ||
| help_heading = "Metrics" |
There was a problem hiding this comment.
should we keep the heading here?
There was a problem hiding this comment.
As all fields of MetricArgs had this, I replaced them by adding to the struct #[command(next_help_heading = "Metrics")].
I can revert this btw, and set help_heading for each arg.
| self.node_config().metrics.push_gateway_username.clone(), | ||
| self.node_config().metrics.push_gateway_password.clone(), |
There was a problem hiding this comment.
if we're only using these together perhaps we can make this a single Option instead?
github-actions
bot
added
the
S-stale
github-actions
bot
removed
the
S-stale
|
Hey! We're doing some spring cleaning on our PR backlog 🧹 Closing old PRs to keep things tidy. If this is still relevant, please feel free to re-open — we appreciate your contribution! |
3 participants
#19243 (review) follow-up.
Add basic auth support for Prometheus push gateway mode.
It's unfortunately not practicable to use directly the built-in
PrometheusBuilder::with_push_gateway, see metrics-rs/metrics#634, so that's why I think PR may be relevant.