count_values string formatting

[Tristan Colgate]

Bringing the discussion from github to here to get more feedback.

https://github.com/prometheus/prometheus/issues/8763

At present the format used by count_values isn't specified in
documentation. (It ends up using %f formatting). There are times when
it is useful for count_values resulting label to be compatible with
the labels produced for histogram "le" labels. (one such case is
documented here,
https://medium.com/@tristan_96324/prometheus-apdex-alerting-d17a065e39d0)

While in the provided example, it is possible to produce the "le"
label for matching via other means), it would still be convenient to
offer a method of controlling the format used by count_values.

In the github issue referenced about, it has been suggested that
count_values arguments could be used to control the value format.
We've suggested allowing a fmt.Printf style (actually
strconv.FormatFloat), specification to be appended to the label name,
perhaps via a comma. e.g.

count_values("le,g.2")

In addition, to support the bucket labels produced for OpenMetrics
histograms, we could use 'o' to facilitate OpenMetrics compatible
formatting (this is %g with s potential additional .0 appended to
exact integer values). 'o' is unused by FormatFloat.

I'm willing to undertake the work, if people agree, but appreciate
that the method of specifying a formatting here probably warrants
wider discussion.
--
Tristan Colgate-McFarlane
----
"You can get all your daily vitamins from 52 pints of guiness, and a
glass of milk"

[Bjoern Rabenstein]

As you can guess from my comments on the issue, I like the
idea. Formatting is a problem if you put a number into a string. And
while I doubt that `FormatFloat` has any guarantees of always
formatting in the same way across Go versions and platforms, at least
we can help the user a bit here.

On 04.05.21 09:38, Tristan Colgate wrote:
>
> In the github issue referenced about, it has been suggested that
> count_values arguments could be used to control the value format.
> We've suggested allowing a fmt.Printf style (actually
> strconv.FormatFloat), specification to be appended to the label name,
> perhaps via a comma. e.g.
>
> count_values("le,g.2")

I think that would work. We just need any separator that is not a
legal charactor of a label name. Which one we pick is matter of
taste. How about `%` to connect it to the printf-style formatting?

So the format could be "<labelname>[%[number]char]".

`number` becomes the `prec` parameter of `FormatFloat` (with default
value -1), and `char` becomes the `fmt` parameter of `FormatFloat`.

If nothing is specified, we fall back to `%f`, which is the current
behavior, so wo don't change current usage.

> In addition, to support the bucket labels produced for OpenMetrics
> histograms, we could use 'o' to facilitate OpenMetrics compatible
> formatting (this is %g with s potential additional .0 appended to
> exact integer values). 'o' is unused by FormatFloat.

+1

--
Björn Rabenstein
[PGP-ID] 0x851C3DA17D748D03
[email] bjo...@rabenste.in

[Julien Pivotto]

On 04 May 09:38, Tristan Colgate wrote:
> Bringing the discussion from github to here to get more feedback.
>
> https://github.com/prometheus/prometheus/issues/8763
>
> At present the format used by count_values isn't specified in
> documentation. (It ends up using %f formatting). There are times when
> it is useful for count_values resulting label to be compatible with
> the labels produced for histogram "le" labels. (one such case is
> documented here,
> https://medium.com/@tristan_96324/prometheus-apdex-alerting-d17a065e39d0)
>
> While in the provided example, it is possible to produce the "le"
> label for matching via other means), it would still be convenient to
> offer a method of controlling the format used by count_values.
>
> In the github issue referenced about, it has been suggested that
> count_values arguments could be used to control the value format.
> We've suggested allowing a fmt.Printf style (actually
> strconv.FormatFloat), specification to be appended to the label name,
> perhaps via a comma. e.g.
>
> count_values("le,g.2")

I would suggest using an optional argument instead:

count_values("le", "%.2f")

>
> In addition, to support the bucket labels produced for OpenMetrics
> histograms, we could use 'o' to facilitate OpenMetrics compatible
> formatting (this is %g with s potential additional .0 appended to
> exact integer values). 'o' is unused by FormatFloat.
>
> I'm willing to undertake the work, if people agree, but appreciate
> that the method of specifying a formatting here probably warrants
> wider discussion.
> --
> Tristan Colgate-McFarlane
> ----
> "You can get all your daily vitamins from 52 pints of guiness, and a
> glass of milk"
>

> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAPGZSGKCw0-0Tp83i26M%2BJ70OLEF4W2dmMoX44zXOo2EoWypiA%40mail.gmail.com.

--
Julien Pivotto
@roidelapluie

[Tristan Colgate]

On Tue, 4 May 2021 at 15:36, Julien Pivotto <roidel...@prometheus.io> wrote:
> > count_values("le,g.2")
>
> I would suggest using an optional argument instead:
>
> count_values("le", "%.2f")

There are a couple of issues with that approach. we would either have
to modify the evaluator.aggregation() function to accept multiple
params, or split COUNT_VALUES out into its own path. Also, using a
standalone % string might mislead people into thinking a full Printf
format string was an option (there are practical reasons why accepting
an arbitrary printf string would have problems).
count_values ("le%2g",...) feels less prone to that misunderstanding.

[Julien Pivotto]

On 04 May 15:49, Tristan Colgate wrote:
> On Tue, 4 May 2021 at 15:36, Julien Pivotto <roidel...@prometheus.io> wrote:
> > > count_values("le,g.2")
> >
> > I would suggest using an optional argument instead:
> >
> > count_values("le", "%.2f")
>
> There are a couple of issues with that approach. we would either have
> to modify the evaluator.aggregation() function to accept multiple
> params, or split COUNT_VALUES out into its own path. Also, using a

I am fine with splitting count_values slightly in its path or
reconciliating before the aggregator, but from an end user perspective,
I would still prefer `count_values("le", ".2f")`.

> standalone % string might mislead people into thinking a full Printf
> format string was an option (there are practical reasons why accepting
> an arbitrary printf string would have problems).
> count_values ("le%2g",...) feels less prone to that misunderstanding.
> --
> Tristan Colgate-McFarlane
> ----
> "You can get all your daily vitamins from 52 pints of guiness, and a
> glass of milk"
>

> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAPGZSGKMzaaUyVxY-Ssi4iOSYV_EN%2Bb%3DX-GNvbJMPRaxGXocyg%40mail.gmail.com.

--
Julien Pivotto
@roidelapluie

[Bjoern Rabenstein]

Initially, I intuitively thought we should do what Julien has now
proposed, too. However, in the course of the discussion, I then
convinced myself that Tristan's approach makes more sense.

So maybe we all have to go through these stages. (o:

[Julien Pivotto]

On 06 May 14:01, Bjoern Rabenstein wrote:
> Initially, I intuitively thought we should do what Julien has now
> proposed, too. However, in the course of the discussion, I then
> convinced myself that Tristan's approach makes more sense.
>
> So maybe we all have to go through these stages. (o:

I am splitting the end user experience from the implementation.

We could pass the labelname + "\0" + formatting to the aggr function,
which would simulate Tristan's approach without being complex for the
user.

> --
> Björn Rabenstein
> [PGP-ID] 0x851C3DA17D748D03
> [email] bjo...@rabenste.in
>

> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/20210506120141.GH7498%40jahnn.

--
Julien Pivotto
@roidelapluie

[Bjoern Rabenstein]

On 06.05.21 16:41, Julien Pivotto wrote:
> On 06 May 14:01, Bjoern Rabenstein wrote:
> > Initially, I intuitively thought we should do what Julien has now
> > proposed, too. However, in the course of the discussion, I then
> > convinced myself that Tristan's approach makes more sense.
> >
> > So maybe we all have to go through these stages. (o:
>
>
> I am splitting the end user experience from the implementation.
>
> We could pass the labelname + "\0" + formatting to the aggr function,
> which would simulate Tristan's approach without being complex for the
> user.

I think Tristan's approach is easier for the user.

A separate formatting string suggests to be, well, a formatting
string. But we only want a number and a single character. Plus, it's
pretty much a power-user feature, so an additional optional parameter
is a bit too prominent for my taste.

[Julien Pivotto]

Okay, let's go for Tristan's approach.

>
> --
> Björn Rabenstein
> [PGP-ID] 0x851C3DA17D748D03
> [email] bjo...@rabenste.in
>

> --
> You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/20210506145348.GN7498%40jahnn.

--
Julien Pivotto
@roidelapluie