Skip to content

[public-api] Cleanup registration of metrics #9896

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions components/public-api-server/pkg/proxy/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
46 changes: 46 additions & 0 deletions components/public-api-server/pkg/proxy/conn_test.go
Original file line number Diff line number Diff line change
@@ -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
}

This file was deleted.

7 changes: 2 additions & 5 deletions components/public-api-server/pkg/server/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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()))
Expand Down Expand Up @@ -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)

Expand Down
15 changes: 7 additions & 8 deletions components/public-api-server/pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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{})
Expand Down