xperimental / netatmo-exporter

Prometheus exporter for Netatmo sensor data.
MIT License
45 stars 18 forks source link

Empty module name causes metrics collection to fail #15

Closed RichieB2B closed 1 year ago

RichieB2B commented 3 years ago

I have been giving guest access to another Netatmo Weather Station. When I accept the access, netatmo-exporter immediately stops working with these errors:

3 error(s) occurred:
* collected metric "netatmo_sensor_updated" { label:<name:"module" value:"" > label:<name:"station" value:"xxx (Office xyz)" > gauge:<value:1.634496735e+09 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_battery_percent" { label:<name:"module" value:"" > label:<name:"station" value:"xxx (xyz Rkerk)" > gauge:<value:100 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_rf_signal_strength" { label:<name:"module" value:"" > label:<name:"station" value:"xxx (Office xyz)" > gauge:<value:53 > } was collected before with the same name and label values

I have removed the guest station from my account and all is fine again. It would be nice if netatmo-exporter would just ignore guest stations altogether.

xperimental commented 3 years ago

The exporter operated under the assumption that both stations and modules have names. Judging from your error both your modules and the modules of the "guest station" do not have names (label:<name:"module" value:""). This should mean that your current metrics do not have a module label, because Prometheus ignores empty labels.

I'm labeling this as a bug, but the fix will probably be that the exporter will ignore modules/stations which have an empty name unless I have an idea how to automatically generate a stable name. If I remember correctly there's an address field in the data as well, but I usually want to avoid that because it's not nice to look at as a human.

RichieB2B commented 3 years ago

I understand your reasoning and agree with ignoring modules without a name. I wonder if this is a user error (station owner did not set a module name) or a result of the way Netatmo presents guest stations in the API.

Personally I am not interested at all in exporting the Netatmo data for guest stations. Having them visible in the Netatmo app but the data being ignored by netatmo-exporter would be the ideal situation.

xperimental commented 3 years ago

I don't know how guest stations are represented, because I don't have access to any guest stations. I had a look at the documentation of the NetAtmo API yesterday and it looks like there is a field to distinguish "owned" from guest stations, I just need to patch the client-library and netatmo-exporter to make use of this.

Would you be willing and able to test this with your configuration, if I push this to a branch of this repo? I'm currently planning to provide a "show metrics for guest stations" switch on the exporter. Have not decided on ignoring modules/stations with empty names yes.

Regarding the empty names: for me the error looks like your modules also do not have any names (because the error is that it tries to report a metric with the same labels twice). Is this the case or do you have names set for all your stations and modules?

RichieB2B commented 3 years ago

I've never compiled a Go program before. I'm running yours using Docker. :-) I can add you as a guest to my NetAtmo station though. Just PM me your E-mail address.

xperimental commented 3 years ago

@RichieB2B I can also build a Docker image for testing. Do you want to run it on amd64 or arm64?

RichieB2B commented 3 years ago

Ok, great. I'm on amd64.

xperimental commented 2 years ago

Sorry, forgot about it again. Can you try running the image xperimental/netatmo-exporter:1.3.0-8-g7cf6f8e. It is built using the code from this commit. The server contains a new endpoint called /debug/data which directly returns what the exporter can read from the NetAtmo API. I've updated the client library a bit to add new fields. You can comment with the full JSON here (anonymize it before) or just extract some relevant bits. I'm mostly interested in the new fields read_only, home_name, home_id and their relation to the old station_name and module_name. read_only should be true for the "guest station" if I read the documentation correctly, so that should be a way to filter those stations out.

RichieB2B commented 2 years ago

Thanks for the build! I have reconnected the guest station to my account (I can see it in the Netatmo iOS app) and pretty soon netatmo-exporter endpoint /metrics stopped working again:

An error has occurred while serving metrics:

3 error(s) occurred:
* collected metric "netatmo_sensor_updated" { label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:1.636931172e+09 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_battery_percent" { label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:100 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_rf_signal_strength" { label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:61 > } was collected before with the same name and label values

The /debug/data indeed shows the guest station:

$ jq '.Body.devices[] | select(.read_only == true)'  debug-data.json 
{
  "_id": "70:ee:50:00:xx:xx",
  "module_name": "Office Rkerk",
  "home_id": "xxxx",
  "home_name": "Ridderkerk",
  "station_name": "Ridderkerk (Office Rkerk)",
  "wifi_status": 11,
  "Type": "NAMain",
  "read_only": true,
  "dashboard_data": {
    "Temperature": 22.2,
    "Humidity": 51,
    "CO2": 1005,
    "Noise": 36,
    "Pressure": 1027.3,
    "AbsolutePressure": 1027.3,
    "time_utc": 1636931217
  },
  "modules": [
    {
      "_id": "03:00:00:00:xx:xx",
      "module_name": "Bedroom Rkerk",
      "home_id": "",
      "home_name": "",
      "station_name": "",
      "battery_percent": 91,
      "rf_status": 68,
      "Type": "NAModule4",
      "read_only": false,
      "dashboard_data": {
        "Temperature": 19,
        "Humidity": 62,
        "CO2": 1040,
        "time_utc": 1636931185
      },
      "modules": null
    },
    {
      "_id": "03:00:00:00:xx:xx",
      "module_name": "Huiskamer Rkerk",
      "home_id": "",
      "home_name": "",
      "station_name": "",
      "battery_percent": 91,
      "rf_status": 74,
      "Type": "NAModule4",
      "read_only": false,
      "dashboard_data": {
        "Temperature": 20.7,
        "Humidity": 57,
        "CO2": 828,
        "time_utc": 1636931210
      },
      "modules": null
    },
    {
      "_id": "05:00:00:00:xx:xx",
      "module_name": "",
      "home_id": "",
      "home_name": "",
      "station_name": "",
      "battery_percent": 100,
      "rf_status": 57,
      "Type": "NAModule3",
      "read_only": false,
      "dashboard_data": {
        "Rain": 0,
        "sum_rain_1": 0,
        "sum_rain_24": 0,
        "time_utc": 1636931210
      },
      "modules": null
    },
    {
      "_id": "02:00:00:00:xx:xx",
      "module_name": "",
      "home_id": "",
      "home_name": "",
      "station_name": "",
      "battery_percent": 100,
      "rf_status": 61,
      "Type": "NAModule1",
      "read_only": false,
      "dashboard_data": {
        "Temperature": 9.6,
        "Humidity": 90,
        "time_utc": 1636931172
      },
      "modules": null
    }
  ]
}

As you can see there are 2 modules without home_name or home_id. I think they were modules that have been removed and are no longer in use.

I see you are now using your own repo for netatmo-api-go which is a great idea. Please consider merging this PR as well.

xperimental commented 2 years ago

Sorry for the long silence, I forgot about this issue.

I noticed that there are a few stations which have neither station_name nor module_name. They seem to be your stations (as read_only is false). Do you have stations which are unnamed or is this a different issue (like the "removed stations" you mentioned)?

For now, I would suggest ignoring all stations which have empty values for the names and introducing a flag to hide "read only" stations. Would that still produce valid results for you?

RichieB2B commented 2 years ago

There are no unnamed stations, so I'm assuming this is due to the stations being removed. These are all guest stations (not mine). I don't know why read_only is false. I'm not sure that is even an option when inviting guests.

RichieB2B commented 2 years ago

I double checked on Netatmo.com and it is possible to add multiple administrators. I will check with the owner of the station which type he enabled for me.

xperimental commented 2 years ago

Sorry for the long silence again, this has been open for a long time now. I'm not sure where we left off here and I will probably not work on this the next weeks. Once I continue I'll try to implement a fix for the empty station names and then we'll have another look at the "guest stations".

scottlaird commented 2 years ago

There's a chance that #24 will fix this, although perhaps not in the best possible way. I was having somewhat similar issues with I have 2 different Netatmo stations on the same account. I didn't have empty names, but I had 2 different modules named "Outdoors".

RichieB2B commented 2 years ago

Using the mac address as a label makes sense and should fix the issue I was seeing.

xperimental commented 2 years ago

@RichieB2B @scottlaird This took a while again. I left out the MAC address / ID on purpose in the exporter because I'd rather like "human" descriptions for my stations.

I have now switched the master branch to a fork of the API which contains an update to the response definition. One field was deprecated a while ago and I suspect that the change in which field is used is the cause for both of your issues. Can you try the new (unreleased, use latest) version of the exporter and tell me if this fixes your issue without having the MAC address?

If this still leads to conflicts between different stations, could you enable the new "debug handler" and post the JSON here?

scottlaird commented 2 years ago

I just checked out a new copy from git, and it seems to be working fine. My labels changed (of course--no MAC), but it's not crashing, and everything that I graph seems to still be there.

RichieB2B commented 2 years ago

The solution of adding home as a label doesn't help me since the home_name is empty for the misbehaving stations. I upgraded to the latest version and added the guest station again. This gives these errors:

An error has occurred while serving metrics:

3 error(s) occurred:
* collected metric "netatmo_sensor_updated" { label:<name:"home" value:"Ridderkerk" > label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:1.664133878e+09 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_battery_percent" { label:<name:"home" value:"Ridderkerk" > label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:84 > } was collected before with the same name and label values
* collected metric "netatmo_sensor_rf_signal_strength" { label:<name:"home" value:"Ridderkerk" > label:<name:"module" value:"" > label:<name:"station" value:"Ridderkerk (Office Rkerk)" > gauge:<value:67 > } was collected before with the same name and label values
xperimental commented 2 years ago

@RichieB2B What do you use "in the Netatmo interface" to identify the modules in your account? It looks like you have a few modules without a name in the same "home". I have given all my modules a name ("living room", "office", ...) so I don't know what the app/website would display if I had not done that. Maybe it generates some default human-readable name if the user does not have one set and we could do something similar.

xperimental commented 1 year ago

@RichieB2B The newly-released 1.5.0 version should fix your issue.

It does not have the new authentication yet, this will be a future release. The old username/password authentication still seems to work fine for me though. :shrug:

xperimental commented 1 year ago

@RichieB2B Have you tried your setup with one of the new versions (post 1.5.0) already?

RichieB2B commented 1 year ago

I have been running 1.5.0 since December 6th but did not get around testing the guest access with the empty names yet. Will try to do so this weekend.

xperimental commented 1 year ago

@RichieB2B I'm closing this as done, feel free to reopen in case I am wrong :slightly_smiling_face: