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.

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

Take host label from the pinger for consistency #141

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

tomhughes
Copy link
Contributor

The update of pro-bing from 0.3.0 to 0.4.0 in #140 pulled in prometheus-community/pro-bing#65 which has changed the meaning of pkt.Addr and causes metrics to be recorded with the IP address as both the host and ip labels.

This causes mismatches with the request metric which still has the host name, and duplicate response metrics as the initial ones are added with the correct host label (but never increment) while the callbacks created duplicate metrics with the IP address as the host label.

This change ensures that the host name from the pinger is used consistently when recording metrics.

Older versions of pro-bing return the host name as pkt.Addr but
newer versions return the stringified version of IPAddr instead.

Signed-off-by: Tom Hughes <tom@compton.nu>
tomhughes added a commit to openstreetmap/prometheus-exporters that referenced this pull request Mar 10, 2024
@SuperQ
Copy link
Owner

SuperQ commented Mar 10, 2024

Hmm, I wonder if we should fix this upstream.

Copy link
Owner

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ SuperQ merged commit 2d93b04 into SuperQ:master Mar 10, 2024
2 checks passed
@tomhughes tomhughes deleted the host-label branch March 10, 2024 16:19
SuperQ added a commit that referenced this pull request Mar 10, 2024
* [BUGFIX] Fix incorrect label setting #141 #142

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ mentioned this pull request Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants