Expose metrics http server for extra endpoints

This allows users to register extra http endpoints on the http server
that serves metrics.

Author: hypnoglow

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()
 

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
 
@@ -291,6 +299,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)
@@ -311,6 +322,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,

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"))
+			})
 		})
 	})