Unexpected behavior of Collect for curried vector

[palage4a]

Hi there. I am expecting that after currying a metric vector, i would able to collect metrics only for curried version of vector.
Here is representation of my expectations in test.

Library version: v1.19.0.

func TestMustCurryWithCollect(t *testing.T) {
	req := prometheus.NewRegistry()

	c := promauto.With(req).NewCounterVec(prometheus.CounterOpts{
		Name: "counter_total",
		Help: "help",
	}, []string{"success"})

	expectedFailed := float64(5)
	expectedSuccessed := float64(10)

	c.WithLabelValues("false").Add(expectedFailed)
	c.WithLabelValues("true").Add(expectedSuccessed)

	failedCh := make(chan prometheus.Metric)
	go func() {
		c.MustCurryWith(prometheus.Labels{"success": "false"}).Collect(failedCh)
		close(failedCh)
	}()

	successedCh := make(chan prometheus.Metric)
	go func() {
		c.MustCurryWith(prometheus.Labels{"success": "true"}).Collect(successedCh)
		close(successedCh)
	}()

	s := float64(0)
	f := float64(0)

	for metric := range failedCh {
		var m dto.Metric
		err := metric.Write(&m)
		assert.Nil(t, err)
		f += m.GetCounter().GetValue()
	}

	for metric := range successedCh {
		var m dto.Metric
		err := metric.Write(&m)
		assert.Nil(t, err)
		s += m.GetCounter().GetValue()
	}

	assert.Equal(t, expectedSuccessed, s)
	assert.Equal(t, expectedFailed, f)
}

Mb i am trying the wrong stuff in the first place? My general goal - get count of successed/failed requests independently to response code (i deleted "code" label for in favor of test example).

[dosubot]

Hello, @palage4a! I'm here to help with any coding bugs, questions, or contributions. Let's tackle this issue together!

The behavior you're observing is due to how the Collect method works with curried metric vectors in the Prometheus Go client library. When you use MustCurryWith, it creates a new vector with the specified labels pre-set, but it doesn't isolate the metrics for collection. Both the curried and uncurried vectors share the same underlying metrics, meaning that calling Collect on a curried vector will still collect all metrics from the original vector, not just those with the curried labels ^[1].

If your goal is to collect metrics independently based on the curried labels, you might need to filter the collected metrics manually after collection. The Collect method does not inherently filter metrics based on the curried labels; it simply pre-sets those labels for operations like GetMetricWithLabelValues.

To achieve your goal, you can iterate over the collected metrics and filter them based on the label values you're interested in. This way, you can separate the metrics for "success" and "failure" as needed.

To continue talking to Dosu, mention @dosu.

---

^{Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Other}

[bwplotka]

👋🏽

So Dosubot was kind of correct, it's an intended and documented behavior of CurryWith method.

Curried and uncurried vectors behave identically in terms of collection.

For your test indeed GetMetricWithLabelValues would work, but not if you want to collect from the pre-filtered vector that also contain the code.

I think I would do what Dosu suggests, so not curry, but write your own helper to sum all codes e.g. using ToFloat64

I do agree however, that it can be non-intuitive -- I initially expected collection to be "curried" too and I don't see immediate reason why not, maybe @beorn7 recall. If we think it's a bug I think it's doable to implement Collect that respects currying. Question is, is it breaking change or not, could users have depend on this behaviour 🤔 I don't think they should, but we got some compatibility complains in the past.

It's also true, that you should never call Collect on your own - it's usually Registry role, so if you want that only for your tests, than I would recommend testutil. We already discussed improvements for those along sum-ing and grouping etc, see: #1639

WDYT?

[palage4a]

Hi @bwplotka. Thank you for the explanation.
Actually, i did my own helper to sum all codes in the first place, but after some research i tried to use Collect to simplify my code and got this enexpected behavior.

It's also true, that you should never call Collect on your own - it's usually Registry role, so if you want that only for your tests, than I would recommend testutil.

Actually, i am trying to do weird stuff: i use prometheus as metric registry for my load testing tool not only for collecting it with prometheus server, but i also need print some output in terminal. That's why i have to collect data with Collect method.

So my goal in this issue: highlight the unexpected behavior and get maintainer's opinion on this case. I understand this use of Collect unexpected and undocumented, so is ok if this issue will be closed as "not planned" or "wont fix".

[beorn7]

I don't think there could be an “intuitive” behavior of the Collect method for a curried vector. Would you like to see the curried labels, or would you like them to be excluded? Which precise subset of metrics in the vector would you like to see? Just the one created (or merely accessed?) through the curried vector, or also those that happen to have the “correct” label set coinciding with the curried one, but created (or accessed?) through the base vector?

The decision to make the currying just some kind of facade modifying the access to metric elements (but not the collection) also allows a simpler and low-overhead implementation. Otherwise, separate pools of metric elements needed to be maintained to deliver the desired behavior (which isn't even clear, see above).

I tried to be concise yet clear in the documentation:

The metrics contained in the MetricVec are shared between the curried and uncurried vectors. They are just accessed
differently. Curried and uncurried vectors behave identically in terms of collection. Only one must be registered with a given registry (usually the uncurried version). The Reset method deletes all metrics, even if called on a curried vector.

[stale]

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!).