Reintroduce AddMetricsExtraHandler on manager

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

pkg/manager/internal.go

@@ -179,6 +179,26 @@ func (cm *controllerManager) add(r Runnable) error {
 	return cm.runnables.Add(r)
 }
 
+// AddMetricsExtraHandler adds extra handler served on path to the http server that serves metrics.
+func (cm *controllerManager) AddMetricsExtraHandler(path string, handler http.Handler) error {
+	cm.Lock()
+	defer cm.Unlock()
+
+	if cm.started {
+		return fmt.Errorf("unable to add new metrics handler because metrics endpoint has already been created")
+	}
+
+	if path == metricsserver.DefaultMetricsEndpoint {
+		return fmt.Errorf("overriding builtin %s endpoint is not allowed", metricsserver.DefaultMetricsEndpoint)
+	}
+
+	if err := cm.metricsServer.AddExtraHandler(path, handler); err != nil {
+		return err
+	}
+	cm.logger.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.Lock()

pkg/manager/manager.go

@@ -64,6 +64,15 @@ type Manager interface {
 	// election was configured.
 	Elected() <-chan struct{}
 
+	// 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
 

pkg/manager/manager_test.go

@@ -1312,6 +1312,12 @@ var _ = Describe("manger.Manager", func() {
 				m, err := New(cfg, opts)
 				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())
+
 				ctx, cancel := context.WithCancel(context.Background())
 				defer cancel()
 				go func() {

pkg/metrics/server/server.go

@@ -38,14 +38,17 @@ import (
 )
 
 const (
-	defaultMetricsEndpoint = "/metrics"
+	DefaultMetricsEndpoint = "/metrics"
 )
 
 // DefaultBindAddress is the default bind address for the metrics server.
 var DefaultBindAddress = ":8080"
 
 // Server is a server that serves metrics.
 type Server interface {
+	// AddExtraHandler adds extra handler served on path to the http server that serves metrics.
+	AddExtraHandler(path string, handler http.Handler) error
+
 	// NeedLeaderElection implements the LeaderElectionRunnable interface, which indicates
 	// the metrics server doesn't need leader election.
 	NeedLeaderElection() bool
@@ -120,8 +123,8 @@ func NewServer(o Options, config *rest.Config, httpClient *http.Client) (Server,
 
 	// Validate that ExtraHandlers is not overwriting the default /metrics endpoint.
 	if o.ExtraHandlers != nil {
-		if _, ok := o.ExtraHandlers[defaultMetricsEndpoint]; ok {
-			return nil, fmt.Errorf("overriding builtin %s endpoint is not allowed", defaultMetricsEndpoint)
+		if _, ok := o.ExtraHandlers[DefaultMetricsEndpoint]; ok {
+			return nil, fmt.Errorf("overriding builtin %s endpoint is not allowed", DefaultMetricsEndpoint)
 		}
 	}
 
@@ -182,6 +185,20 @@ func (*defaultServer) NeedLeaderElection() bool {
 	return false
 }
 
+// AddMetricsExtraHandler adds extra handler served on path to the http server that serves metrics.
+func (s *defaultServer) AddExtraHandler(path string, handler http.Handler) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.options.ExtraHandlers == nil {
+		s.options.ExtraHandlers = make(map[string]http.Handler)
+	}
+	if _, found := s.options.ExtraHandlers[path]; found {
+		return fmt.Errorf("can't register extra handler by duplicate path %q on metrics http server", path)
+	}
+	s.options.ExtraHandlers[path] = handler
+	return nil
+}
+
 // Start runs the server.
 // It will install the metrics related resources depend on the server configuration.
 func (s *defaultServer) Start(ctx context.Context) error {
@@ -202,15 +219,15 @@ func (s *defaultServer) Start(ctx context.Context) error {
 		ErrorHandling: promhttp.HTTPErrorOnError,
 	})
 	if s.metricsFilter != nil {
-		log := log.WithValues("path", defaultMetricsEndpoint)
+		log := log.WithValues("path", DefaultMetricsEndpoint)
 		var err error
 		handler, err = s.metricsFilter(log, handler)
 		if err != nil {
 			return fmt.Errorf("failed to start metrics server: failed to add metrics filter: %w", err)
 		}
 	}
 	// TODO(JoelSpeed): Use existing Kubernetes machinery for serving metrics
-	mux.Handle(defaultMetricsEndpoint, handler)
+	mux.Handle(DefaultMetricsEndpoint, handler)
 
 	for path, extraHandler := range s.options.ExtraHandlers {
 		if s.metricsFilter != nil {