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.

Sign up for GitHub

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

Add a diagnostic refresh_age metric and corresponding container healthcheck #50

Merged
3 commits merged into from
Jun 1, 2024
Merged

Add a diagnostic refresh_age metric and corresponding container healthcheck #50

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c47a05a
Add `refresh_age` metric
michaelosthege May 12, 2024
463a2e8
Add a healthcheck
michaelosthege May 12, 2024
36c4135
Include curl to fix healthcheck
michaelosthege May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ COPY requirements.txt /tmp
RUN pip3 install --no-cache-dir -r /tmp/requirements.txt

COPY exporter.py /usr/local/bin/homematic_exporter
COPY healthcheck.sh /usr/local/bin/healthcheck.sh

ENTRYPOINT [ "/usr/local/bin/homematic_exporter" ]

EXPOSE 8010
HEALTHCHECK --interval=20s --timeout=3s \
CMD bash /usr/local/bin/healthcheck.sh
6 changes: 6 additions & 0 deletions exporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ def run(self):
try:
with generate_metrics_summary.labels(self.ccu_host).time():
self.generate_metrics()
self.refresh_time = time.time()
except OSError as os_error:
logging.info("Failed to generate metrics: {0}".format(os_error))
error_counter.labels(self.ccu_host).inc()
Expand Down Expand Up @@ -183,6 +184,11 @@ def __init__(self, ccu_host, ccu_port, auth, gathering_interval, reload_names_in
self.gathering_interval = int(gathering_interval)
self.reload_names_interval = int(reload_names_interval)
self.devicecount = Gauge('devicecount', 'Number of processed/supported devices', labelnames=['ccu'], namespace=self.METRICS_NAMESPACE)
# Upon request export the seconds since the last successful update.
# This is robust against internal crashes and can be used by the healthcheck.
self.refresh_time = time.time()
sfudeus marked this conversation as resolved.
Show resolved Hide resolved
self.refresh_age = Gauge("refresh_age_seconds", "Seconds since the last successful refresh.", labelnames=["ccu"], namespace=self.METRICS_NAMESPACE)
self.refresh_age.labels(self.ccu_host).set_function(lambda: time.time() - self.refresh_time)

def generate_metrics(self):
logging.info("Gathering metrics")
Expand Down
14 changes: 14 additions & 0 deletions healthcheck.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash

# First argument is the maximum age in seconds (default: 60)
maxage=${1:-60}

# Get the age of the last successful metrics refresh in seconds
age=$(curl -s http://localhost:9040/metrics | grep 'homematic_refresh_age{' | cut -d ' ' -f2 | cut -d '.' -f1)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no curl in the docker image yet - and the scripts lacks an execute bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, I should have checked the healthcheck logs!

I didn't need to change the file permissions though. It's now doing 0-exit on my deployment..

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

your healthcheck command included the bash-invocation and only gave the script as argument, that is why you didn't nee the bit. But it would make sense to do so to be able to call it natively.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please feel free to suggest a change or make a commit. I don't know what exactly you'd like to see here

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 5f580ed, it is just about the permissions bits on the healthcheck.sh. I can add that later myself as well, I cannot push to your branch.

The most recent docker build contains that already and looks good so far in my cluster. I did not set a liveness check using the healthcheck yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Linux file permissions at the git level? I didn't know these were copied into the docker image.


if [[ $age -lt $maxage ]]; then
exit 0
else
# Maximum age exceeded → unhealthy
exit 1
fi
Loading