search

djjudas21 / ecowitt-exporter

EcoWitt exporter for Prometheus
4 stars 3 forks source link

please don't return zero values #20

Open anarcat opened 8 months ago

anarcat commented 8 months ago

Hi!

First off, thank you for this software. It's really nice! I was considering getting a weather station and the existence of your program is what made me find out and ultimately buy an Ecowitt, so yay! :)

One thing I have noticed though is that it seems like when the exporter is scraped but doesn't have data to present, it returns zero for the metrics it has no data on.

I think that's incorrect: normally, when you don't have a value to present, in prometheus, you just don't show that value at all in the exporter. For example, I don't have a second temperature gauge, but this is still shown:

# HELP temp1_c Temp 1
# TYPE temp1_c gauge
temp1_c 0.0

IMHO, that whole block should just not show up in the /metrics output if it is not available.

Now, I haven't read the code or debugged this in details: this might be a "garbage-in, garbage-out" case, where (for the above example) the ecowitt really is reporting a "zero" temperature for that sensor. But my hunch goes the other way because, during a slight configuration glitch here, all metrics zapped to zero: temperature, pressure, everything. You can see the dips in my graphs here:

image

If you agree, I might be able to take a look at that python of yours to see if i can figure something out. :)

Oh, and thanks for the grafana dashboard too, nice touch! I recommend you link to it from the README here as well, although it was easy enough to find on the blg post.

Again, awesome work, and thanks!

djjudas21 commented 8 months ago

Hey, thanks for reporting. I'm actually in bed sick at the moment so I'm just writing this on my phone off the top of my head, but the Prometheus exporter module is initialised when the script starts up, and that object persists across multiple submissions by the Ecowitt station and multiple scrapes by Prometheus.

There is a block of code that sets up each metric to be returned and these metrics could definitely be toggled. I did look at this and I found that it wasn't possible for the script to just set up metrics targets for each metric the Ecowitt sends, because each metric has to be initialised exactly once. You have to initialise the metrics handlers before you start receiving input from the Ecowitt.

In my case, it seemed easier to just initialise everything I could possibly need, and when I've added the additional temperature sensors they have just magically started working without needing to restart the exporter etc. Then it's just a case of editing the grafana dashboard to show the desired metrics and delete the rest.

I do actually have a more updated version of the grafana dashboard in use which I haven't checked into git yet - I'll do that when I'm alive again.

Also worth noting that these days I prefer using influxDB for my Ecowitt data. I only use Prometheus for actual IT monitoring and I keep the data for only 14 days, but I send Ecowitt data to influx and keep it forever, and I use the data compaction stuff.

djjudas21 commented 8 months ago

What I meant to say was I will gladly consider any suggestions about improving the module. Potentially adding config toggles to enable/disable specific metrics or sensors (like the air quality sensor) but I'm not sure if it will be easy to do this automatically because I think the Ecowitt always sends temps 1-8 even if you don't have those additional sensors.

anarcat commented 8 months ago

On 2024-02-03 01:45:24, Jonathan wrote:

What I meant to say was I will gladly consider any suggestions about improving the module. Potentially adding config toggles to enable/disable specific metrics or sensors (like the air quality sensor) but I'm not sure if it will be easy to do this automatically because I think the Ecowitt always sends temps 1-8 even if you don't have those additional sensors.

Oh, interesting. So I don't mind so much the extra metrics: i'm fine with the exporter relaying whatever it gets from the ecowitt.

What I'm less fine with is relaying data it didn't get from the ecowitt, namely "zero hPa" pressure for example, because that's really whacky. ;)

I'm pretty sure I can find a way to send only metrics when there's data to send, I'll take a look.

Also quite interested by InfluxDB now. :p We have long term storage issues with prometheus at work, and I didn't think of InfluxDB for that, but maybe it would make sense. We do need prometheus for alerting in the short term, but maybe we can remote write or something.. the other alternatives are either too complicated (Cortex, Thanos) or ... well, I need to look into VictoriaMetrics more. :)

anarcat commented 8 months ago

On 2024-02-03 01:40:29, Jonathan wrote:

Hey, thanks for reporting. I'm actually in bed sick at the moment

Also: oh dear, get well soon! :)