-
Notifications
You must be signed in to change notification settings - Fork 180
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
Add TCP checks on top of HTTP checks #47
Comments
Hey, glad you are finding it useful ! We started with HTTP, because it was the most common use case at the time, and it tests the underlying layers too. From SRE perspective, it's probably more interesting to know HTTP connectivity goes through, rather than ICMP, but it of course depends on what you're doing. Someone has suggested doing actual ICMP pings, and it's likely to appear at some point in time. Feel free to contribute, it should be pretty straightforward to add as an option 👍 |
I see one more advantage of using HTTP here. Goldpinger could be improved such a way to return some custom health-related metrics in the response, and HTTP (comparing to ICMP) makes it easier :) |
That all makes sense, thanks for the quick response. I was going to suggest timing the response when receiving the first byte, but if ICMP pings appear at some point anyway I suppose that won't matter. Plus maybe getting the timing after the whole response gives a more realistic idea of how the network is behaving at the http level? |
Yup, yup and yup, respectively. |
Hi folks, I understand the reasoning behind the HTTP checks, however it is mentioned the intention of supporting ICMPs in the future too. May I humbly suggest making TCP the next supported protocol instead? TCP is often offloaded to hardware acceleration, whilst ICMP isn't (and sometimes blocked or deprioritised). In my opinion ICMP will not always give you a good view of your network health (it would also have less problems with things like asymmetric routing, giving you a wrong impression!). TCP being offloaded certainly means HTTP is also offloaded (as it's TCP+ L7 payload), but on an HTTP check there could be a lot of moving parts depending on what you're querying (e.g. the HTTP check might trigger -and wait for- backend DB queries!). I've got the feeling HTTP is good for an overall health check of the infra at the app layer, but it does not help separating issues between network and application layers. TCP is perfect for that. Disclaimer: I am not a dev by any stretch, but time permitting I might have a look at the code and see if that's something I can add myself. At first sight it does not seem like the product is ready to support more than one type of check though, so it may be a bit daunting for someone like me with limited coding skills. |
OK, so maybe let's do it this way. We'll implement TCP checks as soon as this issue reaches 10 thumbs up. Fair ? |
Thumbs up to that! 👍 😄 |
@seeker89 there you go, I'm number 10 😄 |
I've been trying Goldpinger out recently, and have been finding it useful already. I was looking through the code, and was curious why you chose to do http monitoring, rather than tcp or icmp pings?
The text was updated successfully, but these errors were encountered: