diff --git a/pkg/manager/internal.go b/pkg/manager/internal.go index 2f969fcd36..4f8f5add8e 100644 --- a/pkg/manager/internal.go +++ b/pkg/manager/internal.go @@ -50,6 +50,7 @@ const ( defaultReadinessEndpoint = "/readyz" defaultLivenessEndpoint = "/healthz" + defaultMetricsEndpoint = "/metrics" ) var log = logf.RuntimeLog.WithName("manager") @@ -95,6 +96,9 @@ type controllerManager struct { // metricsListener is used to serve prometheus metrics metricsListener net.Listener + // metricsExtraHandlers contains extra handlers to register on http server that serves metrics. + metricsExtraHandlers map[string]http.Handler + // healthProbeListener is used to serve liveness probe healthProbeListener net.Listener @@ -260,6 +264,25 @@ func (cm *controllerManager) SetFields(i interface{}) error { return nil } +// AddMetricsExtraHandler adds extra handler served on path to the http server that serves metrics. +func (cm *controllerManager) AddMetricsExtraHandler(path string, handler http.Handler) error { + if path == defaultMetricsEndpoint { + return fmt.Errorf("overriding builtin %s endpoint is not allowed", defaultMetricsEndpoint) + } + + cm.mu.Lock() + defer cm.mu.Unlock() + + _, found := cm.metricsExtraHandlers[path] + if found { + return fmt.Errorf("can't register extra handler by duplicate path %q on metrics http server", path) + } + + cm.metricsExtraHandlers[path] = handler + log.V(2).Info("Registering metrics http server extra handler", "path", path) + return nil +} + // AddHealthzCheck allows you to add Healthz checker func (cm *controllerManager) AddHealthzCheck(name string, check healthz.Checker) error { cm.mu.Lock() @@ -341,19 +364,28 @@ func (cm *controllerManager) GetWebhookServer() *webhook.Server { } func (cm *controllerManager) serveMetrics(stop <-chan struct{}) { - var metricsPath = "/metrics" handler := promhttp.HandlerFor(metrics.Registry, promhttp.HandlerOpts{ ErrorHandling: promhttp.HTTPErrorOnError, }) // TODO(JoelSpeed): Use existing Kubernetes machinery for serving metrics mux := http.NewServeMux() - mux.Handle(metricsPath, handler) + mux.Handle(defaultMetricsEndpoint, handler) + + func() { + cm.mu.Lock() + defer cm.mu.Unlock() + + for path, extraHandler := range cm.metricsExtraHandlers { + mux.Handle(path, extraHandler) + } + }() + server := http.Server{ Handler: mux, } // Run the server go func() { - log.Info("starting metrics server", "path", metricsPath) + log.Info("starting metrics server", "path", defaultMetricsEndpoint) if err := server.Serve(cm.metricsListener); err != nil && err != http.ErrServerClosed { cm.errSignal.SignalError(err) } @@ -367,6 +399,8 @@ func (cm *controllerManager) serveMetrics(stop <-chan struct{}) { } func (cm *controllerManager) serveHealthProbes(stop <-chan struct{}) { + // TODO(hypnoglow): refactor locking to use anonymous func in the similar way + // it's done in serveMetrics. cm.mu.Lock() mux := http.NewServeMux() diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 852eb787b9..df95c66397 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -19,6 +19,7 @@ package manager import ( "fmt" "net" + "net/http" "time" "github.com/go-logr/logr" @@ -54,6 +55,13 @@ type Manager interface { // interface - e.g. inject.Client. SetFields(interface{}) error + // AddMetricsExtraHandler adds an extra handler served on path to the http server that serves metrics. + // Might be useful to register some diagnostic endpoints e.g. pprof. Note that these endpoints meant to be + // sensitive and shouldn't be exposed publicly. + // If the simple path -> handler mapping offered here is not enough, a new http server/listener should be added as + // Runnable to the manager via Add method. + AddMetricsExtraHandler(path string, handler http.Handler) error + // AddHealthzCheck allows you to add Healthz checker AddHealthzCheck(name string, check healthz.Checker) error @@ -282,6 +290,9 @@ func New(config *rest.Config, options Options) (Manager, error) { return nil, err } + // By default we have no extra endpoints to expose on metrics http server. + metricsExtraHandlers := make(map[string]http.Handler) + // Create health probes listener. This will throw an error if the bind // address is invalid or already in use. healthProbeListener, err := options.newHealthProbeListener(options.HealthProbeBindAddress) @@ -302,6 +313,7 @@ func New(config *rest.Config, options Options) (Manager, error) { resourceLock: resourceLock, mapper: mapper, metricsListener: metricsListener, + metricsExtraHandlers: metricsExtraHandlers, internalStop: stop, internalStopper: stop, port: options.Port, diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 9b291d80cc..ee4ebe288b 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -412,7 +412,7 @@ var _ = Describe("manger.Manager", func() { Expect(resp.StatusCode).To(Equal(200)) }) - It("should not serve anything other than metrics endpoint", func(done Done) { + It("should not serve anything other than metrics endpoint by default", func(done Done) { opts.MetricsBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -469,6 +469,40 @@ var _ = Describe("manger.Manager", func() { ok := metrics.Registry.Unregister(one) Expect(ok).To(BeTrue()) }) + + It("should serve extra endpoints", func(done Done) { + opts.MetricsBindAddress = ":0" + m, err := New(cfg, opts) + Expect(err).NotTo(HaveOccurred()) + + err = m.AddMetricsExtraHandler("/debug", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, _ = w.Write([]byte("Some debug info")) + })) + Expect(err).NotTo(HaveOccurred()) + + // Should error when we add another extra endpoint on the already registered path. + err = m.AddMetricsExtraHandler("/debug", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, _ = w.Write([]byte("Another debug info")) + })) + Expect(err).To(HaveOccurred()) + + s := make(chan struct{}) + defer close(s) + go func() { + defer GinkgoRecover() + Expect(m.Start(s)).NotTo(HaveOccurred()) + close(done) + }() + + endpoint := fmt.Sprintf("http://%s/debug", listener.Addr().String()) + resp, err := http.Get(endpoint) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + + body, err := ioutil.ReadAll(resp.Body) + Expect(err).NotTo(HaveOccurred()) + Expect(string(body)).To(Equal("Some debug info")) + }) }) })