On Tue, Dec 22, 2020 at 3:10 PM William Dauchy <wdau...@gmail.com> wrote:
> > But in addition, it only uses the field for prometheus and not
> > for the rest of the stats, making a special case in the stats for
> > prometheus, which I don't like much.
> > So given that the version and release dates were already present there,
> > why not just add a new compiler_version field, fill it with the missing
> > info so that the consumer has all 3 info available ?
>
> I understand your comment; however from prometheus point of view, it
> is not so nice:
> My patch was producing something like:
>
> # HELP haproxy_process_build_info HAProxy build info.
> # TYPE haproxy_process_build_info gauge
> haproxy_process_build_info{version="2.4-dev4-7a58a2-1",releasedate="2020/12/22",gccversion="10.2.1
> 20201207"} 1
>
> And you propose to have:
> haproxy_process_version{version="2.4-dev4-7a58a2-1"} 1
> haproxy_process_relase_date{releasedate="2020/12/22"} 1
> haproxy_process_compiler_version{gccversion="10.2.1 20201207"} 1
>
> I understand it creates two different ways to handle that information
> but from a prometheus point of view, this is not the recommended way
> to do those things. With our current implementation there is no easy
> way to gather those 3 values within a single metric.What do we conclude here? Christopher, any thoughts about it? > > By the way I just had a look at the dump function in prometheus, I > > don't understand why it's reimplenting all the info filling by itself. > > We could seriously simplify things by calling stats_fill_info() once, > > then iterating over all the metrics, possibly performing adjustments > > for the rare ones that require an exception. What I'm seeing is that > > they use different names but we could possibly extend the struct > > associating the names and descriptions to make them match the two use > > cases. I hate to see that new fields are added to the global stats > > from time to time and that they don't show up in prometheus because > > the work has to be done there again. Otherwise if we want to keep it > > distinct, it must not use the current stats definitions. But that's > > more of a mid-term cleanup that is not directly related to your patch, > > though it certainly is the reason why it causes a colatteral impact > > on the stats. > > I agree the double implementation makes it error prone but I did not > look in more details in the implementation to see how we could > possibly simplify those things. As stated above, there are potentially > a few things which are different because of labels, and maybe the > conclusion is indeed to decorrelate the two stats lists. -- William
