client_golang
client_golang copied to clipboard
Consider SummaryVec and HistogramVec methods to return concrete types rather than Observer/ObserverVec, or merge Summary and Histogram into one type
When applying labels to SummaryVec or HistogramVec, a "generic" Observer interface is returned. This is inconsistent with other *Vec types and does not appear to serve any useful practical purpose.
It will be substantially more convenient (especially, when all kinds of automation via reflection scenarios are concerned) if SummaryVec and HistogramVec simply returned the corresponding concrete interfaces implementing Summary and Histogram types.
Perhaps you are right. This is a spot in the Go type system where there really is no ideal solution. Whatever we do, it is bad in the one way or the other. Let's look at the With(Labels) Observer and MustCurryWith(Labels) ObserverVec methods as a representatives. (The same applies to all the other methods returning Observer or ObserverVec.)
What you are suggesting is that those methods should look like With(Labels) Histogram and MustCurryWith(Labels) HistogramVec for a HistogramVec and With(Labels) Summary and MustCurryWith(Labels) SummaryVec for a SummaryVec.
At first glance, that totally makes sense. The returned Histogram and Summary both still implement the Observer interface. However, now we cannot anymore define an interface that both HistogramVec and SummaryVec implement. The Go type system is not "transitive" enough: The method With(Labels) Histogram does not implement the function defined in an interface like With(Labels) Observer even if Histogram implements Observer. We needed yet another level of "transitiveness" to make MustCurryWith(Labels) HistogramVec implement MustCurryWith(Labels) ObserverVec. The Go type system is just not powerful enough for that. (This is somewhat similar to the often suffered problem that []ConcretTypeImplementingInterfaceFoo is not assignable to []InterfaceFoo. You could also claim that generics would solve the problem. And now you are looking into a very deep rabbit hole…)
Now have a look at the very handy promhttp.InstrumentHandlerDuration and friends: It takes an ObserverVec as a parameter. Following your suggestion, there would be no ObserverVec anymore. The package promhttp needed to define all those helper functions separately for SummaryVec and HistogramVec, and once we introduce more Observer implementations (e.g. the improved sparse histograms I'm currently working on), we have to replicate all that code again.
And there you have the "useful practical purpose" you didn't see when filing this issue.
Having said all that, this is a trade-off, and I'm happy to look at evidence that returning concrete types is overall better than the current solution. However, it would in any case be a breaking change and could only happen in v2 of this package. For v2, we might anyway restructure things more fundamentally. For example, I could see to have just one concrete "distribution" type that will be rendered on the wire as a Prometheus Summary or Histogram, depending on its inner configuration.
I'll leave this issue open within the v2 milestone so that we remember to consider it when the time has come.
I fully agree with you that Go type system is to blame. :)