[public-api] Cleanup registration of metrics

Author: easyCZ

components/public-api-server/pkg/proxy/conn.go

@@ -19,19 +19,32 @@ type ServerConnectionPool interface {
 	Get(ctx context.Context, token string) (gitpod.APIInterface, error)
 }
 
-// NoConnectionPool is a simple version of the ServerConnectionPool which always creates a new connection.
-type NoConnectionPool struct {
+type connectionConstructor func(endpoint string, opts gitpod.ConnectToServerOpts) (gitpod.APIInterface, error)
+
+func NewAlwaysNewConnectionPool(address *url.URL) (*AlwaysNewConnectionPool, error) {
+	return &AlwaysNewConnectionPool{
+		ServerAPI: address,
+		constructor: func(endpoint string, opts gitpod.ConnectToServerOpts) (gitpod.APIInterface, error) {
+			return gitpod.ConnectToServer(endpoint, opts)
+		},
+	}, nil
+}
+
+// AlwaysNewConnectionPool is a simple version of the ServerConnectionPool which always creates a new connection.
+type AlwaysNewConnectionPool struct {
 	ServerAPI *url.URL
+
+	constructor connectionConstructor
 }
 
-func (p *NoConnectionPool) Get(ctx context.Context, token string) (gitpod.APIInterface, error) {
+func (p *AlwaysNewConnectionPool) Get(ctx context.Context, token string) (gitpod.APIInterface, error) {
 	logger := ctxlogrus.Extract(ctx)
 
 	start := time.Now()
 	defer func() {
 		reportConnectionDuration(time.Since(start))
 	}()
-	server, err := gitpod.ConnectToServer(p.ServerAPI.String(), gitpod.ConnectToServerOpts{
+	server, err := p.constructor(p.ServerAPI.String(), gitpod.ConnectToServerOpts{
 		Context: ctx,
 		Token:   token,
 		Log:     logger,
@@ -42,3 +55,20 @@ func (p *NoConnectionPool) Get(ctx context.Context, token string) (gitpod.APIInt
 
 	return server, nil
 }
+
+func WrapConnectionPoolWithMetrics(pool ServerConnectionPool) ServerConnectionPool {
+	return &connectionPoolWithMetrics{pool: pool}
+}
+
+type connectionPoolWithMetrics struct {
+	pool ServerConnectionPool
+}
+
+func (c *connectionPoolWithMetrics) Get(ctx context.Context, token string) (gitpod.APIInterface, error) {
+	start := time.Now()
+	defer func() {
+		reportConnectionDuration(time.Since(start))
+	}()
+
+	return c.pool.Get(ctx, token)
+}

components/public-api-server/pkg/proxy/conn_test.go

@@ -0,0 +1,46 @@
+// Copyright (c) 2022 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License-AGPL.txt in the project root for license information.
+
+package proxy
+
+import (
+	"context"
+	"errors"
+	gitpod "github.com/gitpod-io/gitpod/gitpod-protocol"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/stretchr/testify/require"
+	"net/url"
+	"testing"
+)
+
+func TestConnectionPoolWithMetrics(t *testing.T) {
+	registry := prometheus.NewRegistry()
+	RegisterMetrics(registry)
+
+	address, err := url.Parse("http://gitpod.test.address.io")
+	require.NoError(t, err)
+
+	_, err = WrapConnectionPoolWithMetrics(&AlwaysNewConnectionPool{
+		ServerAPI: address,
+		constructor: func(endpoint string, opts gitpod.ConnectToServerOpts) (gitpod.APIInterface, error) {
+			// We don't actually need the connection, so just returning an error
+			return nil, errors.New("failed to create connection pool")
+		},
+	}).Get(context.Background(), "some-token")
+	require.Error(t, err)
+
+	count, err := testutil.GatherAndCount(registry, "gitpod_public_api_proxy_connection_create_duration_seconds")
+	require.NoError(t, err)
+	require.Equal(t, 1, count)
+
+}
+
+type MockConnectionPool struct {
+	api gitpod.APIInterface
+}
+
+func (m *MockConnectionPool) Get(ctx context.Context, token string) (gitpod.APIInterface, error) {
+	return m.api, nil
+}

components/public-api-server/pkg/proxy/prometheusmetrics.go renamed to components/public-api-server/pkg/proxy/metrics.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"github.com/gitpod-io/gitpod/common-go/baseserver"
 	v1 "github.com/gitpod-io/gitpod/public-api/v1"
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
@@ -22,12 +21,11 @@ import (
 func TestPublicAPIServer_v1_WorkspaceService(t *testing.T) {
 	ctx := metadata.AppendToOutgoingContext(context.Background(), "authorization", "some-token")
 	srv := baseserver.NewForTests(t)
-	registry := prometheus.NewRegistry()
 
 	gitpodAPI, err := url.Parse("wss://main.preview.gitpod-dev.com/api/v1")
 	require.NoError(t, err)
 
-	require.NoError(t, register(srv, Config{GitpodAPI: gitpodAPI}, registry))
+	require.NoError(t, register(srv, Config{GitpodAPI: gitpodAPI}))
 	baseserver.StartServerForTests(t, srv)
 
 	conn, err := grpc.Dial(srv.GRPCAddress(), grpc.WithTransportCredentials(insecure.NewCredentials()))
@@ -69,12 +67,11 @@ func TestPublicAPIServer_v1_WorkspaceService(t *testing.T) {
 func TestPublicAPIServer_v1_PrebuildService(t *testing.T) {
 	ctx := context.Background()
 	srv := baseserver.NewForTests(t)
-	registry := prometheus.NewRegistry()
 
 	gitpodAPI, err := url.Parse("wss://main.preview.gitpod-dev.com/api/v1")
 	require.NoError(t, err)
 
-	require.NoError(t, register(srv, Config{GitpodAPI: gitpodAPI}, registry))
+	require.NoError(t, register(srv, Config{GitpodAPI: gitpodAPI}))
 
 	baseserver.StartServerForTests(t, srv)
 

components/public-api-server/pkg/proxy/prometheusmetrics_test.go

@@ -12,25 +12,21 @@ import (
 	"github.com/gitpod-io/gitpod/public-api-server/pkg/apiv1"
 	"github.com/gitpod-io/gitpod/public-api-server/pkg/proxy"
 	v1 "github.com/gitpod-io/gitpod/public-api/v1"
-	"github.com/prometheus/client_golang/prometheus"
 	"github.com/sirupsen/logrus"
 	"net/http"
 )
 
 func Start(logger *logrus.Entry, cfg Config) error {
-	registry := prometheus.NewRegistry()
-
 	srv, err := baseserver.New("public_api_server",
 		baseserver.WithLogger(logger),
 		baseserver.WithHTTPPort(cfg.HTTPPort),
 		baseserver.WithGRPCPort(cfg.GRPCPort),
-		baseserver.WithMetricsRegistry(registry),
 	)
 	if err != nil {
 		return fmt.Errorf("failed to initialize public api server: %w", err)
 	}
 
-	if registerErr := register(srv, cfg, registry); registerErr != nil {
+	if registerErr := register(srv, cfg); registerErr != nil {
 		return fmt.Errorf("failed to register services: %w", registerErr)
 	}
 
@@ -41,14 +37,17 @@ func Start(logger *logrus.Entry, cfg Config) error {
 	return nil
 }
 
-func register(srv *baseserver.Server, cfg Config, registry *prometheus.Registry) error {
-	proxy.RegisterMetrics(registry)
+func register(srv *baseserver.Server, cfg Config) error {
+	proxy.RegisterMetrics(srv.MetricsRegistry())
 
 	logger := log.New()
 	m := middleware.NewLoggingMiddleware(logger)
 	srv.HTTPMux().Handle("/", m(http.HandlerFunc(HelloWorldHandler)))
 
-	connPool := &proxy.NoConnectionPool{ServerAPI: cfg.GitpodAPI}
+	connPool, err := proxy.NewAlwaysNewConnectionPool(cfg.GitpodAPI)
+	if err != nil {
+		return fmt.Errorf("failed to create connection pool: %w", err)
+	}
 
 	v1.RegisterWorkspacesServiceServer(srv.GRPC(), apiv1.NewWorkspaceService(connPool))
 	v1.RegisterPrebuildsServiceServer(srv.GRPC(), v1.UnimplementedPrebuildsServiceServer{})