-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Heartbeat] Fix status for TCP checks #10777
Conversation
Pinging @elastic/uptime |
@urso I'm requesting your review since Nicholas will be out, as will I next week. If this looks good please feel free to hit the merge button. |
737ec6b
to
ff0729b
Compare
As a note, we'll need to forward port this to 7.0 |
ff0729b
to
5143c74
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code LGTM but this will need a changelog entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a bug fix changelog.
LGTM.
The heartbeat checks for TCP send/receive have been broken for a long time, since at least 6.3 from my testing. An error was returned, but the status was still set to 'up'. This patch fixes that and adds tests.
TCP checks are not adding URL fields on NXDOMAIN endpoints. This fixes that issue. It does so by ensuring that URL metadata is added before executing the check, and not during, as done previously. A side effect of this is that we now perform DNS lookups once per `{hostname,port}` instead of once per `{hostname}`. This is worth the increased simplicity however, as the code would be quite convoluted otherwise, which would put us at risk for more bugs. Related (but different) 6.x issue: elastic#10777 Fix import formatting
TCP checks are not adding URL fields on NXDOMAIN endpoints. This fixes that issue. It does so by ensuring that URL metadata is added before executing the check, and not during, as done previously. A side effect of this is that we now perform DNS lookups once per `{hostname,port}` instead of once per `{hostname}`. This is worth the increased simplicity however, as the code would be quite convoluted otherwise, which would put us at risk for more bugs. Related (but different) 6.x issue: #10777 Fix import formatting
TCP checks are not adding URL fields on NXDOMAIN endpoints. This fixes that issue. It does so by ensuring that URL metadata is added before executing the check, and not during, as done previously. A side effect of this is that we now perform DNS lookups once per `{hostname,port}` instead of once per `{hostname}`. This is worth the increased simplicity however, as the code would be quite convoluted otherwise, which would put us at risk for more bugs. Related (but different) 6.x issue: elastic#10777 Fix import formatting (cherry picked from commit 821baec)
TCP checks are not adding URL fields on NXDOMAIN endpoints. This fixes that issue. It does so by ensuring that URL metadata is added before executing the check, and not during, as done previously. A side effect of this is that we now perform DNS lookups once per `{hostname,port}` instead of once per `{hostname}`. This is worth the increased simplicity however, as the code would be quite convoluted otherwise, which would put us at risk for more bugs. Related (but different) 6.x issue: #10777 Fix import formatting (cherry picked from commit 821baec)
The heartbeat checks for TCP send/receive have been broken for a long time, since at least 6.3 from my testing. An error was returned, but the status was still set to 'up'.
This patch fixes that and adds tests.