Cut v1.4.0

[beorn7]

Note that adding a method to the Histogram and Counter interfaces is technically a breaking change. However, I cannot really think about any option that wouldn't be terribly convoluted. (It would be much easier if the constructor returned a concrete type that implements certain interfaces, but this mistake was made long ago.)

I would expect that very few will have implemented their own Histogram and Counter, and if they did, it will be easy to fix. So the impact will (hopefully) be quite limited.

[brian-brazil]

👍

[bwplotka]

Nice! LGTM 👍

Well, we could add just a separate interface e.g

type HistogramWithExemplars interface {
        // ObserveWithExemplar works like Observe but also replaces the
	// currently saved exemplar for the relevant bucket (possibly none) with
	// a new one, created from the provided value, the current time as
	// timestamp, and the provided Labels. Empty Labels will lead to a valid
	// (label-less) exemplar. But if Labels is nil, the current exemplar in
	// the relevant bucket is left in place. This method panics if any of
	// the provided labels are invalid or if the provided labels contain
	// more than 64 runes in total.
	ObserveWithExemplar(value float64, exemplar Labels)
}

and then:

• have constructor that returns Histogram like now, then user can try casting it to HistogramWithExamplar interface,
• have NewHistogramWithExemplar constructor as well.

This way technically we don't have any compatibility issue, but might be an overengineering. I can see people implementing their own Histogram implementation (eg wrappers), but fixing this incompatibility indeed might be not a huge issue.

The same path is kind of done by http.ResponseWriter when if you want to use custom methods you need to cast to certain interfaces.

Wonder what are you thoughts about separate interface idea @beorn7 (: But I think I am fine with this approach as well.

[beorn7]

@bwplotka : Yes, your thoughts are precisely what I considered as a strictly compatible solution, but then I thought it's too clunky, at least if the issue we are avoiding by it is not very relevant. But I'm on the fence about it. Or in other words: I could easily be convince to do it that way, for example if there is a lot of backlash with this release. But perhaps we shouldn't even risk it.

A related point is that the methods of a HistogramVec and a SummaryVec to retrieve a child return an Observer, which is the interface shared by Summary and Histogram. But now the Histogram has an ObserveWithExemplar, but the Summary does not. So the moment a vector is in the game, you have to do the interface upgrade anyway (and that has already occurred in practice at Grafana).

I think I'll create a proposal PR with an ExemplarObserver and an ExemplarAdder interface so that you can all look at it and decide what's the better trade-off.

[bwplotka]

Perfect thanks 👍 I guess this means we wait with the release then.

[beorn7]

I have rebased with #708 included.

The only CHANGE is now for the metric doc string, which is benign.

Please give final approvement or raise objections.

[simonpasquier]

I'm not sure why about the indirect dependencies. I think they shouldn't be there.

[beorn7]

They got introduced by the update of dependencies. go mod tidy doesn't remove them.

It seems that's the way it has to be.

Do you know a way I could try to get rid of them?