Re: [PATCH] BUG/MINOR: contrib/prometheus-exporter: consistent per-server metrics
On Mon, Dec 21, 2020 at 01:05:50PM +0100, William Dauchy wrote:
> > I'm not opposed at all to merging your patch, I'm just worried that
> > some users will then report that these metrics are missing. Or should
> > we duplicate them so that they appear with the two names in older
> > versions maybe ?
>
> There is definitely a (hard) choice to be made here. I'm trying to
> address a case where we collect metrics from different haproxy
> versions and test whether the metric is available or not. Adding more
> metrics along versions is supported but I did not expect to have a
> rename. Duplicating metrics might seem a good middle point but it also
> introduces some confusion for the users looking at available metrics.
> So after a second thought, I realise how things were done with the
> stats socket where we check the version. My conclusion is therefore as
> follow: ignore this patch, and I will most likely propose another one
> which expose the haproxy version in the reported metrics. From that I
> should be able to implement the logic on my side and change the list
> of expected metrics depending on the version.
> This could look like:
> haproxy_process_build_info{version="2.3.2"} 1
> (advices taken from
> https://www.robustperception.io/exposing-the-software-version-to-prometheus)
>
> Does that sound better to you?
This sounds like an excellent idea indeed! I didn't know the version was
not exposed, and for sure sooner or later this version will be needed,
even to distinguish certain metrics whose semantics could have slightly
changed over time for example.
Thanks!
Willy
Re: [PATCH] BUG/MINOR: contrib/prometheus-exporter: consistent per-server metrics
Hi Willy,
Thank you for your answer.
On Mon, Dec 21, 2020 at 11:08 AM Willy Tarreau wrote:
> What impact do you think this renaming can have on stable versions ?
> I suspect the reason it was not backported was to avoid breakage. But
> actually maybe the difference between older and newer versions makes
> the situation even worse.
>
> I'm not opposed at all to merging your patch, I'm just worried that
> some users will then report that these metrics are missing. Or should
> we duplicate them so that they appear with the two names in older
> versions maybe ?
There is definitely a (hard) choice to be made here. I'm trying to
address a case where we collect metrics from different haproxy
versions and test whether the metric is available or not. Adding more
metrics along versions is supported but I did not expect to have a
rename. Duplicating metrics might seem a good middle point but it also
introduces some confusion for the users looking at available metrics.
So after a second thought, I realise how things were done with the
stats socket where we check the version. My conclusion is therefore as
follow: ignore this patch, and I will most likely propose another one
which expose the haproxy version in the reported metrics. From that I
should be able to implement the logic on my side and change the list
of expected metrics depending on the version.
This could look like:
haproxy_process_build_info{version="2.3.2"} 1
(advices taken from
https://www.robustperception.io/exposing-the-software-version-to-prometheus)
Does that sound better to you?
--
William
Re: [PATCH] BUG/MINOR: contrib/prometheus-exporter: consistent per-server metrics
Hi William,
On Thu, Dec 17, 2020 at 11:55:18AM +0100, William Dauchy wrote:
> commit c55a626217a7e676e1cc ("MINOR: contrib/prometheus-exporter: Add
> missing global and per-server metrics") is renaming two metrics:
> server_idle_connections_current
> server_idle_connections_limit
>
> This patch to propose to backport this change in 2.2, 2.1. and 2.0 to
> make it consitent accross versions. The main point being, it makes it
> painful to integrate in some automated testing accross versions, which
> make use of those metrics.
What impact do you think this renaming can have on stable versions ?
I suspect the reason it was not backported was to avoid breakage. But
actually maybe the difference between older and newer versions makes
the situation even worse.
I'm not opposed at all to merging your patch, I'm just worried that
some users will then report that these metrics are missing. Or should
we duplicate them so that they appear with the two names in older
versions maybe ?
Thanks,
Willy
