search

mbugert / connectbox-prometheus

Prometheus exporter for Compal CH7465LG cable modems, commonly sold as "Connect Box"
Apache License 2.0
49 stars 9 forks source link

Add source label to all exported metrics #15

Closed ressu closed 3 years ago

ressu commented 3 years ago

Having a source label is a more reliable way to run multiple instances to collect from multiple devices. This also allows more effective dashboards, which no longer rely on exporter configuration.

ressu commented 3 years ago

This change was inspired by the current dashboard which is relatively painful to use if the exporter address changes from time to time. This makes maintaining the exporter in Kubernetes slightly painful.

I have an updated dashboard that I use, which replaces the instance filter with source filter. I've uploaded the JSON to gist https://gist.github.com/ressu/63a6af7cd241da035b86a17461cb0154 (the only changes are in variables and filters)

mbugert commented 3 years ago

Hi, thanks for the input. I see your point regarding the dashboard; having two variables host and port for identifying a single target is not helpful since these bits of information cannot be enumerated independent of each other with prometheus.

The suggested source label would fix this, but its values would be quite similar to the instance label prometheus ships for all targets. Could you elaborate on the benefit of having the source label in addition to instance? The changes in this PR are quite extensive for an issue which (unless I missed something) only concerns the dashboard.

ressu commented 3 years ago

There are 2 issues at play here. Instance is not an identifier that determines the source of the information and the fact that instance can be dynamic. Let's start with the latter.

Instance is just an internal field in prometheus that can be used to distinguish multiple collectors. The intent is to allow redundancy and better scalability. It's not intended as an identifier for the source of the information. Instance will tell us where the exporter was listening, but not where it was collecting from. What makes things worse is that instance can be dynamic, like it is when running the exporter in environments like Kubernetes. It would be possible to artificially force the instance to stay stable, but that's still just shifting the problem.

The other problem is that the instance is not identifying the source of the data. Let's say we wanted to collect information from 2 devices into the same prometheus instance. Sure it would take a bit of setup, but there is no reason why that wouldn't work. Except that now one would need to figure out which instance maps to which device. By adding a source label, we are signalling which device is the source of the metrics. Generally there are 2 patterns how this is resolved, one is by the exporter labeling the metrics with the source and the other is via the scrape configuration. Latter solution is usually used when the target is provided in the same configuration. For example in the blackbox exporter the targets are provided in the scraper configuration and indeed the instance variable is being overwritten by the value of target.

Since this exporter has a static configuration file, I opted for the additional label solution instead.

mbugert commented 3 years ago
  1. I don't entirely agree - instance is the main way of identifying targets in prometheus, and it's also being used as such in the official node_exporter dashboard. Regarding the dynamic aspect of hostnames in Kubernetes I can't comment. docker-compose has a feature where hostnames correspond to container names which, if it exists for Kubernetes too, could be a workaround.
  2. Like you say, the proper way of implementing an exporter which scrapes multiple target would be the blackbox/snmp exporter way of relabeling targets. When I wrote this, I selfishly assumed users would only have one such device at home, so I built it the easy way. I would highly prefer this solution over adding extra labels, but I realize that it would require significant code changes...
ressu commented 3 years ago

Finally had some time to spend on this.. It turns out that the Python prometheus client library doesn't support multi-target probing, so there is no real way of getting around this limitation. Unless the architecture is changed drastically. So I gave up.. 😆

I think you still have a strong argument in rewriting the instance name instead. In fact I ended up refactoring my setup to simply overwrite the instance name. So I extracted the dashboard from this PR into https://github.com/mbugert/connectbox-prometheus/pull/18 so we can close this one.