Skip to content

metrics: skip health metrics collection when healthchecking is disabled #41297

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

Jump to bottom
Merged
merged 1 commit into from
Aug 25, 2025
Merged

metrics: skip health metrics collection when healthchecking is disabled #41297

merged 1 commit into from
Aug 25, 2025

Conversation

HadrienPatte
Copy link
Member

@HadrienPatte HadrienPatte commented Aug 20, 2025 •
edited
Loading

Currently, disabling agent health checks with --enable-health-checking=false, causes a lot of error logs like

Error while getting cilium-health status: Get "http://localhost/v1beta/status": dial unix /var/run/cilium/health.sock: connect: no such file or directory

This PR updates the agent metrics collection logic to skip collecting health related metrics if health checking is disabled.

Fix bug that would cause error messages when disabling agent health checks

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 20, 2025
@HadrienPatte
Copy link
Member Author

/test

@HadrienPatte HadrienPatte marked this pull request as ready for review August 20, 2025 12:29
@HadrienPatte HadrienPatte requested a review from a team as a code owner August 20, 2025 12:29
@HadrienPatte HadrienPatte requested a review from derailed August 20, 2025 12:29
derailed
derailed requested changes Aug 21, 2025
View reviewed changes
Copy link
Contributor

@derailed derailed left a comment

Choose a reason for hiding this comment

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

@HadrienPatte Nice work!

@HadrienPatte
metrics: skip health metrics collection when healthchecking is disabled
2db9e65 
Currently, disabling agent health checks with
`--enable-health-checking=false`, causes a lot of error logs like

> Error while getting cilium-health status: Get "http://localhost/v1beta/status": dial unix /var/run/cilium/health.sock: connect: no such file or directory

This PR updates the agent metrics collection logic to skip collecting
health related metrics if health checking is disabled.

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
@HadrienPatte HadrienPatte force-pushed the pr/HadrienPatte/health-metrics branch from 319a687 to 2db9e65 Compare August 22, 2025 09:33
@HadrienPatte HadrienPatte requested a review from derailed August 22, 2025 09:34
@HadrienPatte
Copy link
Member Author

/test

@HadrienPatte HadrienPatte added area/metrics Impacts statistics / metrics gathering, eg via Prometheus. release-note/misc This PR makes changes that have no direct user impact. labels Aug 22, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 22, 2025
@pchaigno pchaigno enabled auto-merge August 22, 2025 18:59
@pchaigno
Copy link
Member

@HadrienPatte Could you add a small release note like Fix bug that would cause error messages when disabling agent health checks? We typically consider these user-visible bugs.

@pchaigno pchaigno added release-note/bug This PR fixes an issue in a previous release of Cilium. area/health Relates to the cilium-health component needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch and removed release-note/misc This PR makes changes that have no direct user impact. labels Aug 22, 2025
@pchaigno pchaigno added this pull request to the merge queue Aug 25, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 25, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 25, 2025
@pchaigno pchaigno added this pull request to the merge queue Aug 25, 2025
Merged via the queue into main with commit 1a2bcb1 Aug 25, 2025
423 of 427 checks passed
@pchaigno pchaigno deleted the pr/HadrienPatte/health-metrics branch August 25, 2025 16:32
@viktor-kurchenko viktor-kurchenko mentioned this pull request Sep 2, 2025
Merged
18 tasks
@viktor-kurchenko viktor-kurchenko added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Sep 2, 2025
@github-actions github-actions bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derailed derailed derailed approved these changes

Assignees
No one assigned
Labels
area/health Relates to the cilium-health component area/metrics Impacts statistics / metrics gathering, eg via Prometheus. backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@HadrienPatte @pchaigno @derailed @viktor-kurchenko