sfudeus / homematic_exporter

Prometheus exporter for homematic ccu3
Apache License 2.0
23 stars 17 forks source link

Enabled ENUM sensors #7

Closed kremers closed 4 years ago

kremers commented 4 years ago

Signed-off-by: Martin Kremers info@martinkremers.de

Fixes #6

sfudeus commented 4 years ago

Hi Martin - thanks for your contribution. Does your proposed patch really solve your problem? Adding the enumset as a label seems a bit awkward to me. What is the metric value in that case? Is that supposed to be the index in the enumset? Unfortunately I have no device here which would show an enum different then index 0 currently.

Regarding the labels of the metrics, I'd sooner add a label with the name of the enum item and value 1 Example: Instead of

homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",param_desc="['NORMAL', 'OVERFLOW', 'UNDERFLOW']",parent_device_type="HMIP-PSM"} 0.0

it would be

homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="NORMAL",parent_device_type="HMIP-PSM"} 1.0
homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="OVERFLOW",parent_device_type="HMIP-PSM"} 0.0
homematic_current_status{ccu="hostname",device="xxxxxxxxxxxx:6",device_type="ENERGIE_METER_TRANSMITTER",mapped_name="xxxxxxxxxx:6",status="UNDERFLOW",parent_device_type="HMIP-PSM"} 0.0

What do you think?

kremers commented 4 years ago

Does your proposed patch really solve your problem?

Yes, I am using it already with an own docker image on my rpi

Is that supposed to be the index in the enumset?

Yes

What do you think?

Having all enum values and the selected index will allow queries to be way simpler using enum/value mapping in grafana later on. One con is that it won't adapt well for changing enums, but I consider that a very rare case.

As far as I understand StateSetMetricFamily has to be used to generate your intended result?

kremers commented 4 years ago

Alternative Version for review: #8

kremers commented 4 years ago

Closed #8 in favour of the most up to date master, container your suggestions

sfudeus commented 4 years ago

Sorry for the delay, looks good, thanks.