Re: [prometheus-developers] count_values string formatting

2021-06-04 Thread Julien Pivotto
On 06 May 16:53, Bjoern Rabenstein wrote:
> 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.

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-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/prometheus-developers/20210506145348.GN7498%40jahnn.

-- 
Julien Pivotto
@roidelapluie

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210604150730.GA714942%40oxygen.


Re: [prometheus-developers] count_values string formatting

2021-05-06 Thread 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.

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210506145348.GN7498%40jahnn.


Re: [prometheus-developers] count_values string formatting

2021-05-06 Thread 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-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/prometheus-developers/20210506120141.GH7498%40jahnn.

-- 
Julien Pivotto
@roidelapluie

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210506144138.GA691745%40oxygen.


Re: [prometheus-developers] count_values string formatting

2021-05-06 Thread 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:
-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210506120141.GH7498%40jahnn.


Re: [prometheus-developers] count_values string formatting

2021-05-04 Thread Julien Pivotto
On 04 May 15:49, Tristan Colgate wrote:
> On Tue, 4 May 2021 at 15:36, Julien Pivotto  
> 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-developers+unsubscr...@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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210504145448.GA694880%40oxygen.


Re: [prometheus-developers] count_values string formatting

2021-05-04 Thread Tristan Colgate
On Tue, 4 May 2021 at 15:36, Julien Pivotto  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.
-- 
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-developers+unsubscr...@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.


Re: [prometheus-developers] count_values string formatting

2021-05-04 Thread 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-developers+unsubscr...@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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210504143600.GA533764%40oxygen.


Re: [prometheus-developers] count_values string formatting

2021-05-04 Thread 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 "[%[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

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/20210504142026.GB2645%40jahnn.


[prometheus-developers] count_values string formatting

2021-05-04 Thread 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"

-- 
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-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/prometheus-developers/CAPGZSGKCw0-0Tp83i26M%2BJ70OLEF4W2dmMoX44zXOo2EoWypiA%40mail.gmail.com.