Export kubernetes client metrics from dns-controller. #4612
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign |
k8s-ci-robot
removed
the
needs-ok-to-test
|
Thanks for the PR @tomwilkie. Mind rebasing this one when you have a minute? |
k8s-ci-robot
added
the
needs-rebase
|
Sure, will give it a go! |
tomwilkie
force-pushed
the
dns-controller-metrics
branch
from
d90437d to
f8eaeb9
Compare
k8s-ci-robot
removed
the
needs-rebase
|
@robinpercy there you go. |
|
/retest |
|
Thanks again. I gave it a quick test and all looks good. |
k8s-ci-robot
added
the
lgtm
| @@ -74,6 +79,11 @@ func main() { | |||
| flags.AddGoFlagSet(flag.CommandLine) | |||
| flags.Parse(os.Args) | |||
|
|
|||
| go func() { | |||
| http.Handle("/metrics", promhttp.Handler()) | |||
There was a problem hiding this comment.
My only concern is that we have no way to shut this new feature off. That capability adds more complexity to this PR, but it we probably need the capability to control that functionality.
@robinpercy thoughts?
There was a problem hiding this comment.
@chrislovecnm It's not a particularly invasive feature. But after looking at how we do it elsewhere (eg calico), I think we should expose it and allow it to be disabled via the API, for consistency.
@tomwilkie We could add two variables to the ExternalDNS API, one to toggle the feature, and the other to override the port. It would be similar to how the WatchIngress flag was added in #2504
Let us know if you don't have time to add that in, and maybe we can merge this as-is and do the API stuff in another PR
There was a problem hiding this comment.
Yeah no problem, will do it tomorrow.
and the other to override the port
Port is configurable via -metrics-listen-addr.
There was a problem hiding this comment.
Great thanks.
Since kops generates the manifest for dns-controller at cluster-build time, we need to add a field for -metrics-listen-addr to the internal model and API in order for users to be able to specify it. Unfortunately, there are a few hoops to jump through :/. Let me know if you have any questions. #2504 is the best example I could find.
|
Thanks for the PR. I like this new feature. |
| @@ -74,6 +79,11 @@ func main() { | |||
| flags.AddGoFlagSet(flag.CommandLine) | |||
| flags.Parse(os.Args) | |||
|
|
|||
| go func() { | |||
| http.Handle("/metrics", promhttp.Handler()) | |||
| log.Fatal(http.ListenAndServe(metricsListen, nil)) | |||
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, robinpercy, tomwilkie The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
The metrics from the go-client are super useful when checking all your components can successfully talk to the API server.