From facd889f749482c3dfce7709e2cbfa218ac92382 Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Wed, 19 Jun 2024 11:48:15 +0200 Subject: [PATCH] feat: export http server metrics expose metrics for registration service on a separate server/port to avoid making them available from the current Route include a unit test to verify that the new `sandbox_promhttp_*` metrics are exposed on the `/metrics` endpoint Signed-off-by: Xavier Coulon Signed-off-by: Xavier Coulon --- cmd/main.go | 38 +++--- deploy/registration-service.yaml | 81 +++++++++---- go.mod | 2 +- .../{middleware.go => jwt_middleware.go} | 0 ...dleware_test.go => jwt_middleware_test.go} | 8 +- pkg/middleware/promhttp_middleware.go | 49 ++++++++ pkg/middleware/promhttp_middleware_test.go | 113 ++++++++++++++++++ pkg/proxy/handlers/spacelister.go | 2 +- pkg/proxy/handlers/spacelister_get.go | 2 +- pkg/proxy/handlers/spacelister_get_test.go | 2 +- pkg/proxy/handlers/spacelister_list.go | 2 +- pkg/proxy/handlers/spacelister_list_test.go | 2 +- .../metrics/proxy_metrics.go} | 35 +----- .../metrics/proxy_metrics_test.go} | 58 --------- pkg/proxy/metrics_server.go | 35 ++++++ pkg/proxy/metrics_server_test.go | 71 +++++++++++ pkg/proxy/proxy.go | 2 +- pkg/proxy/proxy_test.go | 2 +- pkg/server/metrics_server.go | 44 +++++++ pkg/server/routes.go | 54 +++++++-- pkg/server/server_test.go | 3 +- 21 files changed, 451 insertions(+), 154 deletions(-) rename pkg/middleware/{middleware.go => jwt_middleware.go} (100%) rename pkg/middleware/{middleware_test.go => jwt_middleware_test.go} (98%) create mode 100644 pkg/middleware/promhttp_middleware.go create mode 100644 pkg/middleware/promhttp_middleware_test.go rename pkg/{metrics/metrics.go => proxy/metrics/proxy_metrics.go} (58%) rename pkg/{metrics/metrics_test.go => proxy/metrics/proxy_metrics_test.go} (78%) create mode 100644 pkg/proxy/metrics_server.go create mode 100644 pkg/proxy/metrics_server_test.go create mode 100644 pkg/server/metrics_server.go diff --git a/cmd/main.go b/cmd/main.go index baf30732..3b13965f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -10,27 +10,26 @@ import ( "syscall" "time" - "github.com/codeready-toolchain/registration-service/pkg/metrics" - "github.com/codeready-toolchain/toolchain-common/pkg/cluster" - "github.com/prometheus/client_golang/prometheus" - controllerlog "sigs.k8s.io/controller-runtime/pkg/log" - toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/auth" "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/informers" "github.com/codeready-toolchain/registration-service/pkg/log" "github.com/codeready-toolchain/registration-service/pkg/proxy" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/server" + "github.com/codeready-toolchain/toolchain-common/pkg/cluster" commonconfig "github.com/codeready-toolchain/toolchain-common/pkg/configuration" errs "github.com/pkg/errors" + "github.com/prometheus/client_golang/prometheus" "go.uber.org/zap/zapcore" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" + controllerlog "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) @@ -108,12 +107,15 @@ func main() { panic(errs.Wrap(err, "failed to init default token parser")) } - // Register metrics - proxyMetrics := metrics.NewProxyMetrics(prometheus.NewRegistry()) - // Start metrics server - metricsSrv := proxyMetrics.StartMetricsServer() + // --------------------------------------------- + // API Proxy + // --------------------------------------------- - // Start the proxy server + // API Proxy metrics server + proxyRegistry := prometheus.NewRegistry() + proxyMetrics := metrics.NewProxyMetrics(proxyRegistry) + proxyMetricsSrv := proxy.StartMetricsServer(proxyRegistry, proxy.ProxyMetricsPort) + // Proxy API server p, err := proxy.NewProxy(app, proxyMetrics, cluster.GetMemberClusters) if err != nil { panic(errs.Wrap(err, "failed to create proxy")) @@ -125,21 +127,25 @@ func main() { informerShutdown <- struct{}{} }) - srv := server.New(app) - - err = srv.SetupRoutes(proxy.DefaultPort) + // --------------------------------------------- + // Registration Service + // --------------------------------------------- + regsvcRegistry := prometheus.NewRegistry() + regsvcMetricsSrv, _ := server.StartMetricsServer(regsvcRegistry, server.RegSvcMetricsPort) + regsvcSrv := server.New(app) + err = regsvcSrv.SetupRoutes(proxy.DefaultPort, regsvcRegistry) if err != nil { panic(err.Error()) } - routesToPrint := srv.GetRegisteredRoutes() + routesToPrint := regsvcSrv.GetRegisteredRoutes() log.Infof(nil, "Configured routes: %s", routesToPrint) // listen concurrently to allow for graceful shutdown go func() { log.Infof(nil, "Service Revision %s built on %s", configuration.Commit, configuration.BuildTime) log.Infof(nil, "Listening on %q...", configuration.HTTPAddress) - if err := srv.HTTPServer().ListenAndServe(); err != nil { + if err := regsvcSrv.HTTPServer().ListenAndServe(); err != nil { if errors.Is(err, http.ErrServerClosed) { log.Info(nil, fmt.Sprintf("%s - this is expected when server shutdown has been initiated", err.Error())) } else { @@ -158,7 +164,7 @@ func main() { } }() - gracefulShutdown(configuration.GracefulTimeout, srv.HTTPServer(), proxySrv, metricsSrv) + gracefulShutdown(configuration.GracefulTimeout, regsvcSrv.HTTPServer(), regsvcMetricsSrv, proxySrv, proxyMetricsSrv) } func gracefulShutdown(timeout time.Duration, hs ...*http.Server) { diff --git a/deploy/registration-service.yaml b/deploy/registration-service.yaml index f6cf8c3a..8122fc51 100644 --- a/deploy/registration-service.yaml +++ b/deploy/registration-service.yaml @@ -96,10 +96,12 @@ objects: - name: registration-service image: ${IMAGE} ports: - - containerPort: 8080 - - containerPort: 8081 - - containerPort: 8082 + - containerPort: 8080 # registration service + - containerPort: 8081 # proxy + - containerPort: 8082 # proxy metrics name: metrics + - containerPort: 8083 # registration service metrics + name: registration-service-metrics command: - registration-service imagePullPolicy: IfNotPresent @@ -140,24 +142,8 @@ objects: requests: cpu: "50m" memory: "100M" - - kind: Service - apiVersion: v1 - metadata: - name: registration-service - namespace: ${NAMESPACE} - labels: - provider: codeready-toolchain - run: registration-service - spec: - ports: - - name: "8080" - protocol: TCP - port: 80 - targetPort: 8080 - selector: - run: registration-service - type: ClusterIP - sessionAffinity: null + + # route for the registration service - kind: Route apiVersion: v1 metadata: @@ -177,24 +163,48 @@ objects: tls: termination: edge wildcardPolicy: None + + # service associated with the registration service route - kind: Service apiVersion: v1 metadata: - name: api + name: registration-service namespace: ${NAMESPACE} labels: provider: codeready-toolchain run: registration-service spec: ports: - - name: "8081" + - name: "8080" protocol: TCP port: 80 - targetPort: 8081 + targetPort: 8080 + selector: + run: registration-service + type: ClusterIP + sessionAffinity: null + + # internal service for the registration service, used by Prometheus to scrape the metrics + - kind: Service + apiVersion: v1 + metadata: + name: registration-service-metrics + namespace: ${NAMESPACE} + labels: + provider: codeready-toolchain + run: registration-service + spec: + ports: + - name: registration-service-metrics + protocol: TCP + port: 80 + targetPort: registration-service-metrics selector: run: registration-service type: ClusterIP sessionAffinity: null + + # route for the proxy - kind: Route apiVersion: v1 metadata: @@ -206,7 +216,6 @@ objects: name: api namespace: ${NAMESPACE} spec: - host: '' port: targetPort: "8081" to: @@ -216,6 +225,28 @@ objects: tls: termination: edge wildcardPolicy: None + + # service associated with the proxy route + - kind: Service + apiVersion: v1 + metadata: + name: api + namespace: ${NAMESPACE} + labels: + provider: codeready-toolchain + run: registration-service + spec: + ports: + - name: "8081" + protocol: TCP + port: 80 + targetPort: 8081 + selector: + run: registration-service + type: ClusterIP + sessionAffinity: null + + # internal service for the proxy, used by Prometheus to scrape the metrics - kind: Service apiVersion: v1 metadata: diff --git a/go.mod b/go.mod index b046d122..01c806dd 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/nyaruka/phonenumbers v1.1.1 github.com/prometheus/client_golang v1.14.0 github.com/prometheus/client_model v0.3.0 + github.com/prometheus/common v0.40.0 github.com/spf13/pflag v1.0.5 go.uber.org/zap v1.21.0 gopkg.in/square/go-jose.v2 v2.3.0 @@ -73,7 +74,6 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/migueleliasweb/go-github-mock v0.0.18 // indirect - github.com/prometheus/common v0.40.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect diff --git a/pkg/middleware/middleware.go b/pkg/middleware/jwt_middleware.go similarity index 100% rename from pkg/middleware/middleware.go rename to pkg/middleware/jwt_middleware.go diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/jwt_middleware_test.go similarity index 98% rename from pkg/middleware/middleware_test.go rename to pkg/middleware/jwt_middleware_test.go index bf8309d7..15290bf8 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/jwt_middleware_test.go @@ -8,22 +8,22 @@ import ( "testing" "time" - "github.com/codeready-toolchain/registration-service/test/fake" - "gopkg.in/h2non/gock.v1" - "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/middleware" "github.com/codeready-toolchain/registration-service/pkg/proxy" "github.com/codeready-toolchain/registration-service/pkg/server" "github.com/codeready-toolchain/registration-service/test" + "github.com/codeready-toolchain/registration-service/test/fake" "github.com/codeready-toolchain/toolchain-common/pkg/status" authsupport "github.com/codeready-toolchain/toolchain-common/pkg/test/auth" testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "gopkg.in/h2non/gock.v1" ) type TestAuthMiddlewareSuite struct { @@ -86,7 +86,7 @@ func (s *TestAuthMiddlewareSuite) TestAuthMiddlewareService() { assert.Equal(s.T(), keysEndpointURL, cfg.Auth().AuthClientPublicKeysURL(), "key url not set correctly") // Setting up the routes. - err = srv.SetupRoutes(proxy.DefaultPort) + err = srv.SetupRoutes(proxy.DefaultPort, prometheus.NewRegistry()) require.NoError(s.T(), err) // Check that there are routes registered. diff --git a/pkg/middleware/promhttp_middleware.go b/pkg/middleware/promhttp_middleware.go new file mode 100644 index 00000000..7e681684 --- /dev/null +++ b/pkg/middleware/promhttp_middleware.go @@ -0,0 +1,49 @@ +package middleware + +import ( + "strconv" + "time" + + "github.com/gin-gonic/gin" + "github.com/prometheus/client_golang/prometheus" +) + +// see https://pkg.go.dev/github.com/prometheus/client_golang/prometheus/promhttp#example-InstrumentRoundTripperDuration + +func InstrumentRoundTripperInFlight(gauge prometheus.Gauge) gin.HandlerFunc { + return func(c *gin.Context) { + gauge.Inc() + defer func() { + gauge.Dec() + }() + c.Next() + } +} + +func InstrumentRoundTripperCounter(counter *prometheus.CounterVec) gin.HandlerFunc { + return func(c *gin.Context) { + defer func() { + counter.With(prometheus.Labels{ + "code": strconv.Itoa(c.Writer.Status()), + "method": c.Request.Method, + "path": c.Request.URL.Path, + }).Inc() + }() + c.Next() + } +} + +func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) gin.HandlerFunc { + return func(c *gin.Context) { + start := time.Now() + defer func() { + duration := time.Since(start) + histVec.With(prometheus.Labels{ + "code": strconv.Itoa(c.Writer.Status()), + "method": c.Request.Method, + "path": c.Request.URL.Path, + }).Observe(float64(duration.Milliseconds())) + }() + c.Next() + } +} diff --git a/pkg/middleware/promhttp_middleware_test.go b/pkg/middleware/promhttp_middleware_test.go new file mode 100644 index 00000000..f94b3813 --- /dev/null +++ b/pkg/middleware/promhttp_middleware_test.go @@ -0,0 +1,113 @@ +package middleware_test + +import ( + "bytes" + "net/http" + "net/http/httptest" + "testing" + + "github.com/codeready-toolchain/registration-service/pkg/configuration" + "github.com/codeready-toolchain/registration-service/pkg/proxy" + "github.com/codeready-toolchain/registration-service/pkg/server" + "github.com/codeready-toolchain/registration-service/test" + "github.com/codeready-toolchain/registration-service/test/fake" + authsupport "github.com/codeready-toolchain/toolchain-common/pkg/test/auth" + testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" + + "github.com/prometheus/client_golang/prometheus" + prommodel "github.com/prometheus/client_model/go" + promcommon "github.com/prometheus/common/expfmt" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type PromHTTPMiddlewareSuite struct { + test.UnitTestSuite +} + +func TestPromHTTPMiddlewareSuite(t *testing.T) { + suite.Run(t, &PromHTTPMiddlewareSuite{test.UnitTestSuite{}}) +} + +func (s *PromHTTPMiddlewareSuite) TestPromHTTPMiddleware() { + // given + tokengenerator := authsupport.NewTokenManager() + srv := server.New(fake.NewMockableApplication(nil)) + keysEndpointURL := tokengenerator.NewKeyServer().URL + s.SetConfig(testconfig.RegistrationService(). + Environment(configuration.UnitTestsEnvironment). + Auth().AuthClientPublicKeysURL(keysEndpointURL)) + + cfg := configuration.GetRegistrationServiceConfig() + assert.Equal(s.T(), keysEndpointURL, cfg.Auth().AuthClientPublicKeysURL(), "key url not set correctly") + reg := prometheus.NewRegistry() + err := srv.SetupRoutes(proxy.DefaultPort, reg) + require.NoError(s.T(), err) + + // making a call on an HTTP endpoint + resp := httptest.NewRecorder() + req, err := http.NewRequest(http.MethodGet, "/api/v1/segment-write-key", nil) + require.NoError(s.T(), err) + + // when + srv.Engine().ServeHTTP(resp, req) + + // then + assert.Equal(s.T(), http.StatusOK, resp.Code, "request returned wrong status code") + + s.Run("check metrics", func() { + // setup the metrics server to access the Prometheus registry contents + _, router := server.StartMetricsServer(reg, server.RegSvcMetricsPort) + + resp = httptest.NewRecorder() + req, err = http.NewRequest(http.MethodGet, "/metrics", nil) + require.NoError(s.T(), err) + + // when + router.ServeHTTP(resp, req) + + // then + assert.Equal(s.T(), http.StatusOK, resp.Code, "request returned wrong status code") + require.NoError(s.T(), err) + + assertMetricExists(s.T(), resp.Body.Bytes(), "sandbox_promhttp_client_in_flight_requests", nil) + assertMetricExists(s.T(), resp.Body.Bytes(), "sandbox_promhttp_client_api_requests_total", map[string]string{ + "code": "200", + "method": "GET", + "path": "/api/v1/segment-write-key", + }) + assertMetricExists(s.T(), resp.Body.Bytes(), "sandbox_promhttp_request_duration_seconds", map[string]string{ + "code": "200", + "method": "GET", + "path": "/api/v1/segment-write-key", + }) + }) +} + +func assertMetricExists(t *testing.T, data []byte, name string, labels map[string]string) { + p := &promcommon.TextParser{} + metrics, err := p.TextToMetricFamilies(bytes.NewReader(data)) + require.NoError(t, err) + + metric, found := metrics[name] + require.True(t, found, "unable to find metric '%s'", name) + for _, m := range metric.GetMetric() { + if matchesLabels(m.GetLabel(), labels) { + return + } + } + assert.Fail(t, "unable to find metric '%s' with labels '%v'", name, labels) +} + +func matchesLabels(x []*prommodel.LabelPair, y map[string]string) bool { + if len(x) != len(y) { + return false + } + for _, l := range x { + if v, found := y[l.GetName()]; !found || l.GetValue() != v { + return false + } + } + return true +} diff --git a/pkg/proxy/handlers/spacelister.go b/pkg/proxy/handlers/spacelister.go index bbf152a6..b2b182d2 100644 --- a/pkg/proxy/handlers/spacelister.go +++ b/pkg/proxy/handlers/spacelister.go @@ -7,7 +7,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/application" "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/metrics" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" commonproxy "github.com/codeready-toolchain/toolchain-common/pkg/proxy" "github.com/gin-gonic/gin" diff --git a/pkg/proxy/handlers/spacelister_get.go b/pkg/proxy/handlers/spacelister_get.go index 1d54cb33..53b98930 100644 --- a/pkg/proxy/handlers/spacelister_get.go +++ b/pkg/proxy/handlers/spacelister_get.go @@ -9,7 +9,7 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" regsercontext "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/metrics" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" commonproxy "github.com/codeready-toolchain/toolchain-common/pkg/proxy" diff --git a/pkg/proxy/handlers/spacelister_get_test.go b/pkg/proxy/handlers/spacelister_get_test.go index 5b0f9382..6195cf92 100644 --- a/pkg/proxy/handlers/spacelister_get_test.go +++ b/pkg/proxy/handlers/spacelister_get_test.go @@ -12,8 +12,8 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/application/service" rcontext "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/metrics" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" proxytest "github.com/codeready-toolchain/registration-service/pkg/proxy/test" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/test/fake" diff --git a/pkg/proxy/handlers/spacelister_list.go b/pkg/proxy/handlers/spacelister_list.go index f0776d9c..bc2af8fe 100644 --- a/pkg/proxy/handlers/spacelister_list.go +++ b/pkg/proxy/handlers/spacelister_list.go @@ -8,7 +8,7 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/metrics" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/labstack/echo/v4" errs "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" diff --git a/pkg/proxy/handlers/spacelister_list_test.go b/pkg/proxy/handlers/spacelister_list_test.go index 22e4b4a7..c07b4a09 100644 --- a/pkg/proxy/handlers/spacelister_list_test.go +++ b/pkg/proxy/handlers/spacelister_list_test.go @@ -18,8 +18,8 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/application/service" rcontext "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/codeready-toolchain/registration-service/pkg/metrics" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" "github.com/codeready-toolchain/registration-service/test/fake" ) diff --git a/pkg/metrics/metrics.go b/pkg/proxy/metrics/proxy_metrics.go similarity index 58% rename from pkg/metrics/metrics.go rename to pkg/proxy/metrics/proxy_metrics.go index aea88277..6f99ce60 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/proxy/metrics/proxy_metrics.go @@ -1,23 +1,13 @@ package metrics import ( - "errors" - "net/http" - - "github.com/labstack/echo/v4" - glog "github.com/labstack/gommon/log" "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promhttp" - logf "sigs.k8s.io/controller-runtime/pkg/log" ) -var log = logf.Log.WithName("registration_metrics") - const ( MetricLabelRejected = "Rejected" MetricsLabelVerbGet = "Get" MetricsLabelVerbList = "List" - MetricsPort = "8082" ) type ProxyMetrics struct { @@ -33,14 +23,13 @@ const metricsPrefix = "sandbox_" func NewProxyMetrics(reg *prometheus.Registry) *ProxyMetrics { regServProxyAPIHistogramVec := newHistogramVec("proxy_api_http_request_time", "time taken by proxy to route to a target cluster", "status_code", "route_to") regServWorkspaceHistogramVec := newHistogramVec("proxy_workspace_http_request_time", "time for response of a request to proxy ", "status_code", "kube_verb") - metrics := &ProxyMetrics{ + reg.MustRegister(regServProxyAPIHistogramVec) + reg.MustRegister(regServWorkspaceHistogramVec) + return &ProxyMetrics{ RegServWorkspaceHistogramVec: regServWorkspaceHistogramVec, RegServProxyAPIHistogramVec: regServProxyAPIHistogramVec, Reg: reg, } - metrics.Reg.MustRegister(metrics.RegServProxyAPIHistogramVec) - metrics.Reg.MustRegister(metrics.RegServWorkspaceHistogramVec) - return metrics } func newHistogramVec(name, help string, labels ...string) *prometheus.HistogramVec { @@ -51,21 +40,3 @@ func newHistogramVec(name, help string, labels ...string) *prometheus.HistogramV }, labels) return v } - -func (p *ProxyMetrics) StartMetricsServer() *http.Server { - // start server - srv := echo.New() - srv.Logger.SetLevel(glog.INFO) - srv.GET("/metrics", echo.WrapHandler(promhttp.HandlerFor(p.Reg, promhttp.HandlerOpts{DisableCompression: true, Registry: p.Reg}))) - srv.DisableHTTP2 = true // disable HTTP/2 for now - - log.Info("Starting the Registration-Service Metrics server...") - // listen concurrently to allow for graceful shutdown - go func() { - if err := srv.Start(":" + MetricsPort); !errors.Is(err, http.ErrServerClosed) { - log.Error(err, err.Error()) - } - }() - - return srv.Server -} diff --git a/pkg/metrics/metrics_test.go b/pkg/proxy/metrics/proxy_metrics_test.go similarity index 78% rename from pkg/metrics/metrics_test.go rename to pkg/proxy/metrics/proxy_metrics_test.go index 7a4424b0..a71c1de8 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/proxy/metrics/proxy_metrics_test.go @@ -1,9 +1,6 @@ package metrics import ( - "bytes" - "io" - "net/http" "strings" "testing" "time" @@ -60,56 +57,6 @@ func TestHistogramVec(t *testing.T) { } -func TestMetricsServer(t *testing.T) { - reg := prometheus.NewRegistry() - testMetrics := NewProxyMetrics(reg) - server := testMetrics.StartMetricsServer() - require.NotNil(t, server) - // Wait up to N seconds for the Metrics server to start - ready := false - sec := 10 - for i := 0; i < sec; i++ { - req, err := http.NewRequest("GET", "http://localhost:8082/metrics", nil) - require.NoError(t, err) - require.NotNil(t, req) - resp, err := http.DefaultClient.Do(req) - if err != nil { - time.Sleep(time.Second) - continue - } - _, _ = io.Copy(io.Discard, resp.Body) - _ = resp.Body.Close() - if resp.StatusCode != http.StatusOK { - // The server may be running but still not fully ready to accept requests - time.Sleep(time.Second) - continue - } - // Server is up and running! - ready = true - break - } - require.True(t, ready, "Metrics Server is not ready after %d seconds", sec) - defer func() { - _ = server.Close() - }() - - req, err := http.NewRequest("GET", "http://localhost:8082/metrics", nil) - require.NoError(t, err) - require.NotNil(t, req) - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - require.NotNil(t, resp) - require.Equal(t, http.StatusOK, resp.StatusCode) - require.Equal(t, "text/plain; version=0.0.4; charset=utf-8", resp.Header.Get("Content-Type")) - // compare the body of the response as well - defer resp.Body.Close() - buf := new(bytes.Buffer) - _, err = buf.ReadFrom(resp.Body) - require.NoError(t, err) - assert.Equal(t, expectedServerBlankResponse, buf.String()) - -} - var expectedResponseMetadata = ` # HELP sandbox_test_histogram_vec test histogram description # TYPE sandbox_test_histogram_vec histogram` @@ -155,11 +102,6 @@ var expectedResponse = ` sandbox_test_histogram_vec_sum{kube_verb="list",status_code="500"} 3.001 sandbox_test_histogram_vec_count{kube_verb="list",status_code="500"} 2 ` -var expectedServerBlankResponse = `# HELP promhttp_metric_handler_errors_total Total number of internal errors encountered by the promhttp metric handler. -# TYPE promhttp_metric_handler_errors_total counter -promhttp_metric_handler_errors_total{cause="encoding"} 0 -promhttp_metric_handler_errors_total{cause="gathering"} 0 -` func compareLabelPairValues(t *testing.T, expected []clientmodel.LabelPair, labelPairs []*clientmodel.LabelPair) { for i := range labelPairs { diff --git a/pkg/proxy/metrics_server.go b/pkg/proxy/metrics_server.go new file mode 100644 index 00000000..f919fc24 --- /dev/null +++ b/pkg/proxy/metrics_server.go @@ -0,0 +1,35 @@ +package proxy + +import ( + "errors" + "fmt" + "net/http" + + "github.com/labstack/echo/v4" + glog "github.com/labstack/gommon/log" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +const ProxyMetricsPort = 8082 + +// StartMetricsServer start server with a single `/metrics` endpoint to server the Prometheus metrics +// Uses echo web framework +func StartMetricsServer(reg *prometheus.Registry, port int) *http.Server { + log := logf.Log.WithName("proxy_metrics") + srv := echo.New() + srv.Logger.SetLevel(glog.INFO) + srv.GET("/metrics", echo.WrapHandler(promhttp.HandlerFor(reg, promhttp.HandlerOpts{DisableCompression: true, Registry: reg}))) + srv.DisableHTTP2 = true // disable HTTP/2 for now + + log.Info("Starting the proxy metrics server...") + // listen concurrently to allow for graceful shutdown + go func() { + if err := srv.Start(fmt.Sprintf(":%d", port)); !errors.Is(err, http.ErrServerClosed) { + log.Error(err, err.Error()) + } + }() + + return srv.Server +} diff --git a/pkg/proxy/metrics_server_test.go b/pkg/proxy/metrics_server_test.go new file mode 100644 index 00000000..634d1fbc --- /dev/null +++ b/pkg/proxy/metrics_server_test.go @@ -0,0 +1,71 @@ +package proxy_test + +import ( + "bytes" + "io" + "net/http" + "testing" + "time" + + "github.com/codeready-toolchain/registration-service/pkg/proxy" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProxyMetricsServer(t *testing.T) { + reg := prometheus.NewRegistry() + _ = metrics.NewProxyMetrics(reg) + server := proxy.StartMetricsServer(reg, proxy.ProxyMetricsPort) + require.NotNil(t, server) + // Wait up to N seconds for the Metrics server to start + ready := false + sec := 10 + for i := 0; i < sec; i++ { + req, err := http.NewRequest("GET", "http://localhost:8082/metrics", nil) + require.NoError(t, err) + require.NotNil(t, req) + resp, err := http.DefaultClient.Do(req) + if err != nil { + time.Sleep(time.Second) + continue + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + // The server may be running but still not fully ready to accept requests + time.Sleep(time.Second) + continue + } + // Server is up and running! + ready = true + break + } + require.True(t, ready, "Metrics Server is not ready after %d seconds", sec) + defer func() { + _ = server.Close() + }() + + req, err := http.NewRequest("GET", "http://localhost:8082/metrics", nil) + require.NoError(t, err) + require.NotNil(t, req) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.NotNil(t, resp) + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "text/plain; version=0.0.4; charset=utf-8", resp.Header.Get("Content-Type")) + // compare the body of the response as well + defer resp.Body.Close() + buf := new(bytes.Buffer) + _, err = buf.ReadFrom(resp.Body) + require.NoError(t, err) + assert.Equal(t, expectedServerBlankResponse, buf.String()) + +} + +var expectedServerBlankResponse = `# HELP promhttp_metric_handler_errors_total Total number of internal errors encountered by the promhttp metric handler. +# TYPE promhttp_metric_handler_errors_total counter +promhttp_metric_handler_errors_total{cause="encoding"} 0 +promhttp_metric_handler_errors_total{cause="gathering"} 0 +` diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index fc574ebe..d950c1bb 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -23,9 +23,9 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/context" crterrors "github.com/codeready-toolchain/registration-service/pkg/errors" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/codeready-toolchain/registration-service/pkg/metrics" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" commoncluster "github.com/codeready-toolchain/toolchain-common/pkg/cluster" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 249ca590..d465cfda 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -19,9 +19,9 @@ import ( appservice "github.com/codeready-toolchain/registration-service/pkg/application/service" "github.com/codeready-toolchain/registration-service/pkg/auth" - "github.com/codeready-toolchain/registration-service/pkg/metrics" "github.com/codeready-toolchain/registration-service/pkg/proxy/access" "github.com/codeready-toolchain/registration-service/pkg/proxy/handlers" + "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/proxy/service" proxytest "github.com/codeready-toolchain/registration-service/pkg/proxy/test" "github.com/codeready-toolchain/registration-service/pkg/signup" diff --git a/pkg/server/metrics_server.go b/pkg/server/metrics_server.go new file mode 100644 index 00000000..0c625985 --- /dev/null +++ b/pkg/server/metrics_server.go @@ -0,0 +1,44 @@ +package server + +import ( + "crypto/tls" + "fmt" + "net/http" + "time" + + "github.com/gin-gonic/gin" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promhttp" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +const RegSvcMetricsPort = 8083 + +var log = logf.Log.WithName("registration_metrics") + +// return a new Gin server exposing the `/metrics` endpoint associated with the given Prometheus registry +func StartMetricsServer(reg *prometheus.Registry, port int) (*http.Server, *gin.Engine) { + router := gin.Default() + router.GET("/metrics", gin.WrapH(promhttp.InstrumentMetricHandler( + reg, promhttp.HandlerFor(reg, promhttp.HandlerOpts{ + DisableCompression: true, + }), + ))) + log.Info("Starting the registration-service metrics server...") + srv := &http.Server{ + Addr: fmt.Sprintf(":%d", port), + Handler: router.Handler(), + ReadHeaderTimeout: 2 * time.Second, + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + NextProtos: []string{"http/1.1"}, // disable HTTP/2 for now + }, + } + go func() { + // service connections + if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { + log.Error(err, err.Error()) + } + }() + return srv, router +} diff --git a/pkg/server/routes.go b/pkg/server/routes.go index b68774b8..b8000926 100644 --- a/pkg/server/routes.go +++ b/pkg/server/routes.go @@ -3,24 +3,50 @@ package server import ( "github.com/codeready-toolchain/registration-service/pkg/assets" "github.com/codeready-toolchain/registration-service/pkg/auth" - "github.com/gin-contrib/static" - "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/controller" "github.com/codeready-toolchain/registration-service/pkg/middleware" + "github.com/gin-contrib/static" errs "github.com/pkg/errors" + "github.com/prometheus/client_golang/prometheus" ) // SetupRoutes registers handlers for various URL paths. // proxyPort is the API Proxy Server port to be used to setup a route for the health checker for the proxy. -func (srv *RegistrationServer) SetupRoutes(proxyPort string) error { +func (srv *RegistrationServer) SetupRoutes(proxyPort string, reg *prometheus.Registry) error { var err error _, err = auth.InitializeDefaultTokenParser() if err != nil { return err } + inFlightGauge := prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "sandbox_promhttp_client_in_flight_requests", + Help: "A gauge of in-flight requests for the wrapped client.", + }) + + counter := prometheus.NewCounterVec( + prometheus.CounterOpts{ + Name: "sandbox_promhttp_client_api_requests_total", + Help: "A counter for requests from the wrapped client.", + }, + []string{"code", "method", "path"}, + ) + + // histVec has no labels, making it a zero-dimensional ObserverVec. + histVec := prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Name: "sandbox_promhttp_request_duration_seconds", + Help: "A histogram of request latencies.", + Buckets: prometheus.DefBuckets, + }, + []string{"code", "method", "path"}, + ) + + // Register all of the metrics in the standard registry. + reg.MustRegister(counter, histVec, inFlightGauge) + srv.routesSetup.Do(func() { // creating the controllers healthCheckCtrl := controller.NewHealthCheck(controller.NewHealthChecker(proxyPort)) @@ -29,6 +55,16 @@ func (srv *RegistrationServer) SetupRoutes(proxyPort string) error { signupCtrl := controller.NewSignup(srv.application) usernamesCtrl := controller.NewUsernames(srv.application) + // unsecured routes + unsecuredV1 := srv.router.Group("/api/v1") + unsecuredV1.Use( + middleware.InstrumentRoundTripperInFlight(inFlightGauge), + middleware.InstrumentRoundTripperCounter(counter), + middleware.InstrumentRoundTripperDuration(histVec)) + unsecuredV1.GET("/health", healthCheckCtrl.GetHandler) // TODO: move to root (`/`)? + unsecuredV1.GET("/authconfig", authConfigCtrl.GetHandler) + unsecuredV1.GET("/segment-write-key", analyticsCtrl.GetDevSpacesSegmentWriteKey) //expose the devspaces segment key + // create the auth middleware var authMiddleware *middleware.JWTMiddleware authMiddleware, err = middleware.NewAuthMiddleware() @@ -36,15 +72,13 @@ func (srv *RegistrationServer) SetupRoutes(proxyPort string) error { err = errs.Wrapf(err, "failed to init auth middleware") return } - - // unsecured routes - unsecuredV1 := srv.router.Group("/api/v1") - unsecuredV1.GET("/health", healthCheckCtrl.GetHandler) - unsecuredV1.GET("/authconfig", authConfigCtrl.GetHandler) - unsecuredV1.GET("/segment-write-key", analyticsCtrl.GetDevSpacesSegmentWriteKey) //expose the devspaces segment key // secured routes securedV1 := srv.router.Group("/api/v1") - securedV1.Use(authMiddleware.HandlerFunc()) + securedV1.Use( + middleware.InstrumentRoundTripperInFlight(inFlightGauge), + middleware.InstrumentRoundTripperCounter(counter), + middleware.InstrumentRoundTripperDuration(histVec), + authMiddleware.HandlerFunc()) securedV1.POST("/signup", signupCtrl.PostHandler) // requires a ctx body containing the country_code and phone_number securedV1.PUT("/signup/verification", signupCtrl.InitVerificationHandler) diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 330164e9..8ab5559e 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -9,6 +9,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/server" "github.com/codeready-toolchain/registration-service/test" "github.com/codeready-toolchain/registration-service/test/fake" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,7 +38,7 @@ func (s *TestServerSuite) TestServer() { fake.MockKeycloakCertsCall(s.T()) // Setting up the routes. - err := srv.SetupRoutes("8091") // uses a different proxy port than the default one to avoid collision with other concurrent tests + err := srv.SetupRoutes("8091", prometheus.NewRegistry()) // uses a different proxy port than the default one to avoid collision with other concurrent tests require.NoError(s.T(), err) gock.OffAll()