move proxy metrics to a separate server

cmd/main.go

@@ -112,6 +112,9 @@ func main() {
 		informerShutdown <- struct{}{}
 	})
 
+	// Start metrics server
+	metricsSrv := metrics.StartMetricsServer()
+
 	srv := server.New(app)
 
 	err = srv.SetupRoutes()
@@ -141,7 +144,7 @@ func main() {
 		}
 	}()
 
-	gracefulShutdown(configuration.GracefulTimeout, srv.HTTPServer(), proxySrv)
+	gracefulShutdown(configuration.GracefulTimeout, srv.HTTPServer(), proxySrv, metricsSrv)
 }
 
 func gracefulShutdown(timeout time.Duration, hs ...*http.Server) {

deploy/registration-service.yaml

@@ -98,6 +98,7 @@ objects:
               ports:
                 - containerPort: 8080
                 - containerPort: 8081
+                - containerPort: 8082
               command:
                 - registration-service
               imagePullPolicy: IfNotPresent
@@ -214,6 +215,24 @@ objects:
       tls:
         termination: edge
       wildcardPolicy: None
+  - kind: Service
+    apiVersion: v1
+    metadata:
+      name: proxy-metrics-service
+      namespace: ${NAMESPACE}
+      labels:
+        provider: codeready-toolchain
+        run: registration-service
+    spec:
+      ports:
+        - name: "8082"
+          protocol: TCP
+          port: 80
+          targetPort: 8082
+      selector:
+        run: registration-service
+      type: ClusterIP
+      sessionAffinity: null
 parameters:
   - name: NAMESPACE
     value: 'toolchain-host-operator'

pkg/metrics/metrics.go

@@ -1,8 +1,13 @@
 package metrics
 
 import (
+	"github.com/labstack/echo/v4"
+	glog "github.com/labstack/gommon/log"
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"net/http"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"time"
 )
 
 var log = logf.Log.WithName("registration_metrics")
@@ -12,6 +17,7 @@
 	MetricLabelRejected  = "Rejected"
 	MetricsLabelVerbGet  = "Get"
 	MetricsLabelVerbList = "List"
+	MetricsPort          = "8082"
 )
 
 // histogram with labels
@@ -58,9 +64,37 @@
 // RegisterCustomMetrics registers the custom metrics
 func RegisterCustomMetrics() {
 	Reg = prometheus.NewRegistry()
+	if len(allHistogramVecs) == 0 {
+		log.Info("No Histograms to register")
+	}
 	// register metrics
 	for _, v := range allHistogramVecs {
 		Reg.MustRegister(v)
 	}
 	log.Info("custom metrics registered")
 }
+
+//nolint:unparam
+func prometheusHandler(ctx echo.Context) error {
+	h := promhttp.HandlerFor(Reg, promhttp.HandlerOpts{DisableCompression: true, Registry: Reg})
+	h.ServeHTTP(ctx.Response().Writer, ctx.Request())
+	return nil
+}
+
+func StartMetricsServer() *http.Server {
+	// start server
+	router := echo.New()
+	router.Logger.SetLevel(glog.INFO)
+	router.GET("/metrics", prometheusHandler)
+
+	log.Info("Starting the Registration-Service Metrics server...")
+	srv := &http.Server{Addr: ":" + MetricsPort, Handler: router, ReadHeaderTimeout: 2 * time.Second}
+	// listen concurrently to allow for graceful shutdown
+	go func() {
+		if err := srv.ListenAndServe(); err != nil {
+			log.Error(err, err.Error())
+		}
+	}()
+
+	return srv
+}

pkg/metrics/metrics_test.go

@@ -1,10 +1,13 @@
 package metrics
 
 import (
+	"github.com/labstack/echo/v4"
 	promtestutil "github.com/prometheus/client_golang/prometheus/testutil"
 	clientmodel "github.com/prometheus/client_model/go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
@@ -64,6 +67,31 @@ func TestRegisterCustomMetrics(t *testing.T) {
 	}
 }
 
+func TestMetricsHandler(t *testing.T) {
+	// Create a request to pass to our handler. We don't have any query parameters, so we'll
+	// pass 'nil' as the third parameter.
+	req, err := http.NewRequest(http.MethodGet, "/metrics", nil)
+	require.NoError(t, err)
+
+	// Create handler instance.
+	RegisterCustomMetrics()
+
+	t.Run("valid metrics json", func(t *testing.T) {
+		// We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response.
+		e := echo.New()
+		rec := httptest.NewRecorder()
+		ctx := e.NewContext(req, rec)
+
+		//when
+		err := prometheusHandler(ctx)
+		require.NoError(t, err)
+		// then
+		// check the status code is what we expect, and the content-type
+		require.Equal(t, http.StatusOK, rec.Code)
+		require.Equal(t, "text/plain; version=0.0.4; charset=utf-8", rec.Header().Get("Content-Type"))
+	})
+}
+
 var expectedResponseMetadata = `
 		# HELP sandbox_test_histogram_vec test histogram description
 		# TYPE sandbox_test_histogram_vec histogram`

pkg/proxy/proxy.go

@@ -45,16 +45,14 @@ const (
 	bearerProtocolPrefix = "base64url.bearer.authorization.k8s.io." //nolint:gosec
 
 	proxyHealthEndpoint = "/proxyhealth"
-
-	pluginsEndpoint = "/plugins/"
+	pluginsEndpoint     = "/plugins/"
 )
 
 type Proxy struct {
 	app         application.Application
 	cl          client.Client
 	tokenParser *auth.TokenParser
 	spaceLister *handlers.SpaceLister
-	metrics     *handlers.Metrics
 }
 
 func NewProxy(app application.Application) (*Proxy, error) {
@@ -77,13 +75,11 @@ func newProxyWithClusterClient(app application.Application, cln client.Client) (
 
 	// init handlers
 	spaceLister := handlers.NewSpaceLister(app)
-	metrics := handlers.NewMetrics()
 	return &Proxy{
 		app:         app,
 		cl:          cln,
 		tokenParser: tokenParser,
 		spaceLister: spaceLister,
-		metrics:     metrics,
 	}, nil
 }
 
@@ -132,7 +128,6 @@ func (p *Proxy) StartProxy() *http.Server {
 	wg.GET("/:workspace", p.spaceLister.HandleSpaceGetRequest)
 	wg.GET("", p.spaceLister.HandleSpaceListRequest)
 	router.GET(proxyHealthEndpoint, p.health)
-	router.GET("/metrics", p.metrics.PrometheusHandler)
 	router.Any("/*", p.handleRequestAndRedirect)
 
 	// Insert the CORS preflight middleware

pkg/proxy/proxy_test.go

@@ -120,21 +120,6 @@ func (s *TestProxySuite) TestProxy() {
 				s.assertResponseBody(resp, `{"alive": true}`)
 			})
 
-			s.Run("check metrics not ok", func() {
-				req, err := http.NewRequest("GET", "http://localhost:8081/metrics", nil)
-				require.NoError(s.T(), err)
-				require.NotNil(s.T(), req)
-
-				// when
-				resp, err := http.DefaultClient.Do(req)
-
-				// then
-				require.NoError(s.T(), err)
-				require.NotNil(s.T(), resp)
-				defer resp.Body.Close()
-				assert.Equal(s.T(), 401, resp.StatusCode)
-			})
-
 			s.Run("plain http error", func() {
 				s.Run("unauthorized if no token present", func() {
 					req, err := http.NewRequest("GET", "http://localhost:8081/api/mycoolworkspace/pods", nil)
@@ -219,6 +204,22 @@ func (s *TestProxySuite) TestProxy() {
 					assert.Equal(s.T(), http.StatusInternalServerError, resp.StatusCode)
 					s.assertResponseBody(resp, "unable to get target cluster: some-error")
 				})
+
+				s.Run("internal error if accessing incorrect url", func() {
+					// given
+					req := s.request()
+					req.URL.Path = "http://localhost:8081/metrics"
+					require.NotNil(s.T(), req)
+
+					// when
+					resp, err := http.DefaultClient.Do(req)
+
+					// then
+					require.NoError(s.T(), err)
+					require.NotNil(s.T(), resp)
+					defer resp.Body.Close()
+					assert.Equal(s.T(), http.StatusInternalServerError, resp.StatusCode)
+				})
 			})
 
 			s.Run("websockets error", func() {