Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Status should not be a gauge? #19

Open
yegle opened this issue Dec 22, 2019 · 1 comment
Open

Status should not be a gauge? #19

yegle opened this issue Dec 22, 2019 · 1 comment

Comments

@yegle
Copy link

yegle commented Dec 22, 2019

I'm new to Prometheus so my understanding might be wrong. I don't think the status should be a gauge? Code that I'm referring to (note the conversion from t.Status to float):

ch <- prometheus.MustNewConstMetric(
tc.Status,
prometheus.GaugeValue,
float64(t.Status),
id, t.Name,
)

The status field in the Transmission rpc spec: https://github.com/transmission/transmission/blob/46b3e6c8dae02531b1eb8907b51611fb9229b54a/extras/rpc-spec.txt#L221

Reading transmission.h, it looks like this is an enum that corresponding to https://github.com/transmission/transmission/blob/46b3e6c8dae02531b1eb8907b51611fb9229b54a/libtransmission/transmission.h#L1814-L1824

So if my reading is correct, what needs to be changed is: The descriptor for the torrent status metric should have a status field which is the string representation of the tr_torrent_activity enum, and the value for the gauge should be 1.

		Status: prometheus.NewDesc(
			namespace+collectorNamespace+"status",
			"Status of a torrent",
			[]string{"id", "name", "status"},
			nil,
		ch <- prometheus.MustNewConstMetric(
			tc.Status,
			prometheus.GaugeValue,
			1,
			id, t.Name, enumToString(t.Status)
		)

Does it make sense? If yes I can probably send a PR to fix it.

@metalmatze
Copy link
Owner

It makes sense both ways I'd say. I don't really have a strong opinion on this very metric.
However, keep in mind that the current way it is implemented results in n time series for n statuses whereas the approach you propose results in 7*n. It might make a bigger difference for people having a lot of torrents. As the number of statuses is limited to 7, I'd still be fine with changing it to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants