client_golang
client_golang copied to clipboard
Improve Exemplar Interface
Using exemplars is quite painful currently. There is a common pattern you want to do such as:
I would propose something more natively supported. Some options:
A) Bring ObserveWithExemplar
to Observer
, why not?
B) Create a separate package that creates natively observers with exemplars (sounds brittle and more to maintain for not a good reason)
C) Create a helper that does something like:
func ExemplarObserve(obs prometheus.Observer, val float64, traceID string) {
if traceID != "" {
obs.(prometheus.ExemplarObserver).ObserveWithExemplar(
val, map[string]string{"trace-id": traceID})
} else {
obs.Observe(val)
}
}
The opinionated state is simple:
- We want to observe with a trace ID.
- We want a consistent trace ID label.
- We want trace ID which is sampled, no point in any other (ofc won't work with tail sampling)
- Assumption: Empty Trace ID means it's not sampled.
Ideally we have opinionated helper as part of method to Observe itself with configurable trace ID label... 🙈
Having if else
when histogram is used is making client instrumentation too verbose IMO. Let's brainstorm this (:
Thoughts? @kakkoyun @juliusv @beorn7
A) Bring ObserveWithExemplar to Observer, why not?
Because it changes the Observer
interface. It breaks everyone who has implemented the Observer
interface outside of our control (most commonly, I guess, when creating mocks for testing, but there might be other use cases). And yes, this might be a fairly niche part of the user community, but since this package is used so widely, it will still create a fair amount of breakage and confusion and destroy the trust people have developed into this package.
This reason, BTW, was the most important one for picking the interface upgrade path when implementing exemplar support.
The other approaches are all valid options, I think. I would just generally suggest to put helpers into a separate package. My experience from the past is that users get really confused and frustrated if a package has too many top-level types. It's a lot of cognitive load when learning how to use the package. Therefore, I had spent a lot of effort in the past to reduce the number of top-level types in the prometheus
package. Note that the "group helpers for specific use cases in appropriate packages" pattern has been followed before (testutil
, collectors
, promhttp
, promauto
).
I would also vote for a dedicated package to contain new helper functions around exemplars. We can even have high-level interface that would conform span/context from popular tracing packages or OTel. (I haven't checked their types if it's even possible IsSampled
and TraceID
seems like good candidates for that imagined interface). What do you think?
MBOI we have been using this helper "CollectedRequest
" for a few years: it times the request, collects the histogram observation, and now adds exemplars:
https://github.com/weaveworks/common/blob/bd288de53d57de300fa286688ce2fc935687213f/http/client/client.go#L66-L67
Hello 👋 Looks like there was no activity on this issue for the last 3 months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next 4 weeks, this issue will be closed (we can always reopen an issue if we need!).
Closing for now as promised, let us know if you need this to be reopened! 🤗
Still relevant (: