client_rust
client_rust copied to clipboard
Using `()` as label set generates invalid format
I tried using ()
as the label set for a family, like Family<(), Histogram, ...>
. Unfortunately this resulted in the following line in my metrics output:
histogram_name_bucket{,le="1.0"} 44
This results in a expected label name, got "COMMA"
error when Prometheus tries to collect the metrics.
I would expect this to "just work" and just not insert any label (i.e. basically not have the leading comma in {,le="1.0"}
)
I tried using
()
as the label set for a family, likeFamily<(), Histogram, ...>
.
What is your reasoning for not using a plain Histogram
?
I've built a small wrapping library around client_rust
for internal use at my job, to streamline and standardize our usage. Basically I'm not using a plain Histogram as this is using some generic code which allows you to use any label type you want. I figured that in the case you didn't want any labels, you could just use ()
- but that turned out not to work :sweat_smile:
One solution could be to only add a ,
here in case the previous label has a length larger than 0
.
https://github.com/prometheus/client_rust/blob/6b7a3458b47a406d15f2ae0a678c611969c4645d/src/encoding/text.rs#L277-L285
Tangentially related, but I noticed that &str
also implements encode - does that assume that you must have the &str
be equal to "label_name=\"label_value\"
? I'm assuming that if you just set it equal to "foobar"
then prometheus will complain that the label has no value (must labels have values?).
But yea that might be one solution I suppose. Alternatively, maybe it should be the responsibility of the Encode
implementation to insert the comma?
Tangentially related, but I noticed that
&str
also implements encode - does that assume that you must have the&str
be equal to"label_name=\"label_value\"
? I'm assuming that if you just set it equal to"foobar"
then prometheus will complain that the label has no value (must labels have values?).
That assumption is correct.
But yea that might be one solution I suppose. Alternatively, maybe it should be the responsibility of the
Encode
implementation to insert the comma?
That would as well be a viable solution.
I honestly find it a bit surprising that &str
implements Encode
in that case. I guess it's the same with f32
and the other primitives that implement Encode
. encode
should at least return Err
when it doesn't match the expected format, I would say.
I feel like (&str, &str)
should implement Encode
but not &str
. Currently the tuple impl requires the items to also implement Encode
- maybe it should just implement Encode
for (K, V)
where K: Display, V: Display
?
I agree. Today I am using Encode
to encode the following:
- Label pair
- Label key
- Label value
- Metric value
Each of these use-cases should probably use their own EncodeXXX
trait instead of a single shared one.