rust-prometheus icon indicating copy to clipboard operation
rust-prometheus copied to clipboard

src/macros: Have `labels!` accept any type that implements `Display`

Open free opened this issue 4 years ago • 3 comments

This allows (among other things) histogram_opts! to take the same kinds of types as opts!. E.g. to replace this:

let status = String::from("200");
histogram_opts!(
    ...
    labels! {"status".to_string() => status.clone()}
)
// Do more stuff with `status`.

with this:

let status = String::from("200");
histogram_opts!(
    ...
    labels! {"status" => &status}
)
// Do more stuff with `status`.

And a lot more, such as using numbers or enums as label values directly.

Signed-off-by: Alin Sinpalean [email protected]

free avatar Oct 03 '20 10:10 free

I'm not sure whether this is a good idea.. @BusyJay PTAL

breezewish avatar Oct 09 '20 08:10 breezewish

I don't think it's a good idea to introduce avoidable clone behind a library interface. Current way allow callers to decide whether to create or reuse an owned string.

BusyJay avatar Oct 09 '20 08:10 BusyJay

I fully understand if this is not accepted as is, but I'd like to point out that AFAICT no one is going to be creating thousands upon thousands of metrics, so the cost is essentially constant.

The alternative would be to use into() instead. Or (likely by using a helper trait) try into()if the type implements Into<String> and fall back to ToString otherwise. I don't think it's worth the trouble, but I can give it a shot if you want.

free avatar Oct 09 '20 19:10 free