-
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
Extend DNS resolver to include TCP and HTTP checks for external targets #115
Conversation
0b7bf0f
to
26bfd31
Compare
Hey @kitfoman - sorry for the delay, I finally got around to looking at this. It's great, but I'm a little bit concerned about the backwards compatibility. How would you feel about keeping the old flags with -ms postfix for now, marking them as deprecated, and using those, unless the new ones (duration) are used? Ideally we wouldn't break anyone. The other thing I'm wondering is how to best reflect the new probes in the prometheus metrics. |
Hey @seeker89 I just saw this.
That sounds reasonable to me. I'll make those changes.
Do you mean in addition to these? We can include a histogram as well if we want to expose latency information. Is there anything else that would be useful? |
@seeker89 does this seem okay? https://github.com/kitfoman/goldpinger/blob/add-external-probes/cmd/goldpinger/main.go#L147 |
Yup, that's what I was thinking. |
Looks like the DCO bot is unhappy again - would you mind re-pushing with |
75e0951
to
5279838
Compare
Hey, 69708ba is still missing the |
ad6de0e
to
e837ac4
Compare
only check node IPs when determining hostIP IP_VERSIONS not IP_FAMILIES Signed-off-by: Tyler Lloyd <tyler.lloyd@microsoft.com>
Signed-off-by: kitfoman <thaddeusgetachew@gmail.com> make timeout flags backwards compatible Signed-off-by: kitfoman <thaddeusgetachew@gmail.com>
e837ac4
to
14ea969
Compare
sorry I confused myself there. It should be good now |
Is this going to be merged? |
@seeker89 The new version hasn't been pushed yet to docker hub. Is it planned? |
Pushed v3.6.1 just now - not sure how the CI didn’t fail before, but here it goes. |
Goldpinger can currently do DNS checks via
HOSTS_TO_RESOLVE
. This aims to extend that functionality so it can also do L4/L7 tcp and http/s checks.Breaking Changes
Timeout types were changed from
int64
totime.Duration
in the configuration here.The flags
ping-timeout-ms
,check-timeout-ms
andcheck-all-timeout-ms
were also changed accordingly.Backward compatibility was considered but there doesn't seem to be a nice way of accomplishing this.