Edge case not handled in prometheus makeLabels function

[joe94]

The following function (https://github.com/go-kit/kit/blob/master/metrics/prometheus/prometheus.go#L161 ) will have an index out of bounds error when labelValues is of odd length.

In the odd length case, one approach is to ignore the last unmatched value:

func makeLabels(labelValues ...string) prometheus.Labels {
	labels := prometheus.Labels{}
	for i := 0; i < len(labelValues) - 1; i += 2 {
		labels[labelValues[i]] = labelValues[i+1]
	}
	return labels
}

[ifraixedes]

I prefer a crash and have a bug that having to find the bug when I'm inspecting the metrics and see that I'm missing one and have to find where I missed.

If that happens, it should be a bug, so I don't think that the library must be resilient about programmatic errors.

[joe94]

@ifraixedes, I think I see your point. However, from a user of the library's perspective at least a panic with a corresponding error message should be there such that the user wouldn't need to dig into library code logic to find what the mistake was.

[peterbourgon]

@joe94 What does the Prometheus client library do in this situation?

[joe94]

@peterbourgon, the Prometheus go-client does not run into this specific situation (building ... string -> prometheus.Labels) as far I see in their source code (https://github.com/prometheus/client_golang).

I did, however, notice that all the go-kit use cases for makeLabels() is simply to create a prometheus.Labels argument for each prometheus Metric Vector With(Labels) method (i.e. https://github.com/prometheus/client_golang/blob/master/prometheus/counter.go#L161 for Counter).

Unless I am misunderstanding the difference between labelValues and Labels in prometheus, I think we should instead be passing labelValues directly to WithLabelValues() Metric Vector functions (like : https://github.com/prometheus/client_golang/blob/master/prometheus/counter.go#L154) in these cases.

Finally, this is why the odd-length case would not happen: https://github.com/go-kit/kit/blob/988c05d06d8ee3a9c13782f0e49b2c6e4726388d/metrics/internal/lv/labelvalues.go#L10