Add possibility to dynamically get label values for http instrumentation

[Okhoshi]

Signed-off-by: Quentin Devos 4972091+Okhoshi@users.noreply.github.com

I need to partition the metrics generated by promhttp by the value coming in a specific header, and while I appreciate the design of promhttp is made to be simple and cover only the base use cases, it would be great if we could have a bit more of flexibility.

It would also give a way to solve #491 elegantly while letting the user control the cardinality of the label they add.

@bwplotka @kakkoyun

[Okhoshi]

Gently repinging @bwplotka and @kakkoyun for a review of this PR ?

Let me know if it doesn't have any chance to be merged, so that I don't wait for it 👍

Also, I saw it's quite similar to #1104, probably share the same concern (that user can now add as many label values as it wants) but I also have the same thought than @chradcliffe on the fact that a user is knowingly added a high cardinality label by using the option, and therefore it's not promhttp responsibility.

Note that I could work with #1104 even though using context would force me to add another middleware to push request/response value into the context.

[Okhoshi]

I added a way to constraint the values any dynamic label can take, such that the cardinality has to be known in advance. Would that address your concerns @bwplotka ?

[bwplotka]

Thanks for this!

I think with more and more requests to add dynamic labels to promhttp - we should do it.

I like your idea for enforcing label values, but it might be better to have something built-in metric so the same enforcing or categorization logic can be reused across anyone who uses prometheus.Observer or prometheus.Adder

With this direction, I would be happy with solutions like yours or #1104 to add dynamic labels without verification.

As per ctx vs pointers structs, the ctx will be safer for this library in some sense but tedious for users... I think I would favour your idea (but perhaps separate for client and server?) Read only thing is nice, but there is more risk for users to add new middleware, makes they you don't forget to add it and put the correct context value that does not conflict and test etc, than risk of someone modifying the request or response in their e.g. dynLabelValues(req, resp) []string

[bwplotka]

Looks good, just some ideas for more explicit naming, WDYT?

Also tests fail 🤗

[Okhoshi]

Looks good, just some ideas for more explicit naming, WDYT?

That naming is way better than mine 💯 Changed.

Also tests fail 🤗

Yes, something weird about not being able to find some git references 🤷

[bwplotka]

LGTM, solid work. Thanks for patience 💪🏽 (:

[jcass8695]

Hey gang, I was wondering if there was any estimate on when there could be a release cut with this PR included? My team are keen to use this great feature. TIA 😌

[kakkoyun]

Hey gang, I was wondering if there was any estimate on when there could be a release cut with this PR included? My team are keen to use this great feature. TIA 😌

Hey @jcass8695, I think it's been a while since the last time we released it. Considering there's also a new Golang version, I think we can update our language support and then take a release. So it will be soon. I think it's better to wait for the Go 1.20.1 release, though.