On Mon, Jan 04, 2021 at 10:03:00AM +0100, Christopher Faulet wrote:
> Le 04/01/2021 à 08:58, William Dauchy a écrit :
> > 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?
> > 
> 
> If exposing a build_info metric with labels is the Prometheus way to provide
> these info, we should do it this way. It may be a special case for the
> INF_VERSION metrics. This info is not exposed yet. So we may use it to
> provide build info with the version as label. For now, the labels are not
> generics. This part should probably be rework a little to be extensible. In
> the William's patch, the build_info labels are built statically and it is
> probably good enough at short term.

We must not start to create diverging info between prometheus and stats
otherwise it will be impossible to make them converge again later. This
is why I disagreed with merging the patch as-is. I prefer that we add a
new field with that info and that we expose only the required ones in
prometheus. This way when we decide to make the two outputs converge we
won't have to deal with impossible situations. In the worst case we'll
just have extra unneeded entries.

Willy

Reply via email to