Skip to content

Conversation

foogod
Copy link
Contributor

@foogod foogod commented Dec 2, 2021

[Note, this change is dependent on upstream repo changes (digineo/go-ping/pull/19) which haven't been merged yet, so probably should not be merged until that happens]

This PR adds some new functionality:

  • It adds the ability to define multiple ping monitors with different parameters (so, for example, you can ping some targets at a different interval than others, etc).
  • Allows setting the ICMP echo identifier field differently for different monitors, so they will generally be considered different "sessions" by NAT gateways, etc.
  • Adds the ability to change the identifier for pings periodically, or under certain conditions (high packet loss, etc).

I mostly added these features to deal with some situations in my own networking setup which the existing code could not really handle:

I have a NAT gateway router with redundant links to the internet (a main connection, and a backup DSL connection). Because the two outbound links use different NAT addresses, each outbound ping session gets tied to a particular interface, and all packets for that session will go out that interface, regardless of their destination, so I was unable to correctly ping both remote gateways at the same time, for example (because packets for one or the other would go out the interface for the other one).

Additionally, I discovered if something happened to the primary connection, and the gateway automatically failed over to using the backup, all the overall internet connection continued to work for everything else, but any ping targets beyond the gateway would show up as not responding (because the NAT session for that ping ID was still tied to the failed connection, and would not fail over when everything else did). With these changes, I can now configure the ping_exporter to essentially initiate a new ping session whenever packet loss indicates something has happened to the primary connection, or just periodically change it after a certain amount of time (to account for fail-back, etc).

(Sorry this is such a big PR, but it all kinda evolved as a bunch of interrelated changes..)

@corny
Copy link
Contributor

corny commented Apr 6, 2024

I've just accepted your pull request in go-ping.

@czerwonk czerwonk self-requested a review April 9, 2024 07:15
@czerwonk czerwonk added this to the 1.2 milestone Jun 19, 2024
@Cuthbert286
Copy link

Cuthbert286 commented Aug 27, 2025

Hi @foogod - just checking in on this. It's been a while, and there are now some merge conflicts.

I recently ran into an issue where ping_exporter was experiencing high packet loss when running behind an AWS NAT Gateway. I’ve tested the feature in this PR, which changes the identifier for pings periodically (e.g. under high packet loss), and it seems to be working well.

Are you planning to revisit this? If not I'd be happy to raise a new PR based on your solution, with the merge conflicts resolved. I believe this could also help address #87

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.

4 participants