Skip to content
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

feat: Track metrics around usage of the GitHub API #3273

Merged
merged 8 commits into from
May 20, 2024
4 changes: 3 additions & 1 deletion pkg/clientpool/ingester_client_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ func (f *ingesterPoolFactory) FromInstance(inst ring.InstanceDesc) (ring_client.
if err != nil {
return nil, err
}

httpClient := util.InstrumentedDefaultHTTPClient(util.WithTracingTransport())
return &ingesterPoolClient{
IngesterServiceClient: ingesterv1connect.NewIngesterServiceClient(util.InstrumentedHTTPClient(), "http://"+inst.Addr, f.options...),
IngesterServiceClient: ingesterv1connect.NewIngesterServiceClient(httpClient, "http://"+inst.Addr, f.options...),
HealthClient: grpc_health_v1.NewHealthClient(conn),
Closer: conn,
}, nil
Expand Down
4 changes: 3 additions & 1 deletion pkg/clientpool/store_gateway_client_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ func (f *storeGatewayPoolFactory) FromInstance(inst ring.InstanceDesc) (ring_cli
if err != nil {
return nil, err
}

httpClient := util.InstrumentedDefaultHTTPClient(util.WithTracingTransport())
return &storeGatewayPoolClient{
StoreGatewayServiceClient: storegatewayv1connect.NewStoreGatewayServiceClient(util.InstrumentedHTTPClient(), "http://"+inst.Addr, f.options...),
StoreGatewayServiceClient: storegatewayv1connect.NewStoreGatewayServiceClient(httpClient, "http://"+inst.Addr, f.options...),
HealthClient: grpc_health_v1.NewHealthClient(conn),
Closer: conn,
}, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func New(params *NewQuerierParams) (*Querier, error) {
params.IngestersRing,
),
storeGatewayQuerier: storeGatewayQuerier,
VCSServiceHandler: vcs.New(params.Logger),
VCSServiceHandler: vcs.New(params.Logger, params.Reg),
storageBucket: params.StorageBucket,
tenantConfigProvider: params.CfgProvider,
limits: params.Overrides,
Expand Down
12 changes: 9 additions & 3 deletions pkg/querier/vcs/client/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import (
)

// GithubClient returns a github client.
func GithubClient(ctx context.Context, token *oauth2.Token) (*githubClient, error) {
func GithubClient(ctx context.Context, token *oauth2.Token, client *http.Client) (*githubClient, error) {
return &githubClient{
client: github.NewClient(nil).WithAuthToken(token.AccessToken),
client: github.NewClient(client).WithAuthToken(token.AccessToken),
}, nil
}

Expand All @@ -30,6 +30,7 @@ func (gh *githubClient) GetCommit(ctx context.Context, owner, repo, ref string)
if err != nil {
return nil, err
}

return &vcsv1.GetCommitResponse{
Sha: toString(commit.SHA),
Message: toString(commit.Commit.Message),
Expand All @@ -45,25 +46,30 @@ func (gh *githubClient) GetFile(ctx context.Context, req FileRequest) (File, err
// We could abstract away git provider using git protocol
// git clone https://x-access-token:<token>@github.com/owner/repo.git
// For now we use the github client.

file, _, _, err := gh.client.Repositories.GetContents(ctx, req.Owner, req.Repo, req.Path, &github.RepositoryContentGetOptions{Ref: req.Ref})
if err != nil {
var githubErr *github.ErrorResponse
if ok := errors.As(err, &githubErr); ok && githubErr.Response.StatusCode == http.StatusNotFound {
if errors.As(err, &githubErr) && githubErr.Response.StatusCode == http.StatusNotFound {
return File{}, fmt.Errorf("%w: %s", ErrNotFound, err)
}
return File{}, err
}

if file == nil {
return File{}, ErrNotFound
}

// We only support files retrieval.
if file.Type != nil && *file.Type != "file" {
return File{}, connect.NewError(connect.CodeInvalidArgument, errors.New("path is not a file"))
}

content, err := file.GetContent()
if err != nil {
return File{}, err
}

return File{
Content: content,
URL: toString(file.HTMLURL),
Expand Down
84 changes: 84 additions & 0 deletions pkg/querier/vcs/client/metrics.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package client

import (
"fmt"
"net/http"
"regexp"
"time"

"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"

"github.com/grafana/pyroscope/pkg/util"
)

var (
githubRouteMatchers = map[string]*regexp.Regexp{
// Get repository contents.
// https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28#get-repository-content
"/repos/{owner}/{repo}/contents/{path}": regexp.MustCompile(`^\/repos\/\S+\/\S+\/contents\/\S+$`),

// Get a commit.
// https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#get-a-commit
"/repos/{owner}/{repo}/commits/{ref}": regexp.MustCompile(`^\/repos\/\S+\/\S+\/commits\/\S+$`),

// Refresh auth token.
// https://docs.github.com/en/apps/creating-github-apps/authenticating-with-a-github-app/refreshing-user-access-tokens#refreshing-a-user-access-token-with-a-refresh-token
"/login/oauth/access_token": regexp.MustCompile(`^\/login\/oauth\/access_token$`),
}
)

func InstrumentedHTTPClient(logger log.Logger, reg prometheus.Registerer) *http.Client {
apiDuration := promauto.With(reg).NewHistogramVec(
prometheus.HistogramOpts{
Namespace: "pyroscope",
Name: "vcs_github_request_duration",
Help: "Duration of GitHub API requests in seconds",
Buckets: prometheus.ExponentialBucketsRange(0.1, 10, 8),
},
[]string{"method", "route", "status_code"},
)

defaultClient := &http.Client{
Timeout: 10 * time.Second,
Transport: http.DefaultTransport,
}
client := util.InstrumentedHTTPClient(defaultClient, withGitHubMetricsTransport(logger, apiDuration))
return client
}

// withGitHubMetricsTransport wraps a transport with a client to track GitHub
// API usage.
func withGitHubMetricsTransport(logger log.Logger, hv *prometheus.HistogramVec) util.RoundTripperInstrumentFunc {
return func(next http.RoundTripper) http.RoundTripper {
return util.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
route := matchGitHubAPIRoute(req.URL.Path)
statusCode := ""
start := time.Now()

res, err := next.RoundTrip(req)
if err == nil {
statusCode = fmt.Sprintf("%d", res.StatusCode)
}

if route == "unknown_route" {
level.Warn(logger).Log("path", req.URL.Path, "msg", "unknown GitHub API route")
}
hv.WithLabelValues(req.Method, route, statusCode).Observe(time.Since(start).Seconds())

return res, err
})
}
}

func matchGitHubAPIRoute(path string) string {
for route, regex := range githubRouteMatchers {
if regex.MatchString(path) {
return route
}
}

return "unknown_route"
}
78 changes: 78 additions & 0 deletions pkg/querier/vcs/client/metrics_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package client

import (
"testing"

"github.com/stretchr/testify/require"
)

func Test_matchGitHubAPIRoute(t *testing.T) {
tests := []struct {
Name string
Path string
Want string
}{
{
Name: "GetContents",
Path: "/repos/grafana/pyroscope/contents/pkg/querier/querier.go",
Want: "/repos/{owner}/{repo}/contents/{path}",
},
{
Name: "GetContents with dash",
Path: "/repos/connectrpc/connect-go/contents/protocol.go",
Want: "/repos/{owner}/{repo}/contents/{path}",
},
{
Name: "GetContents without path",
Path: "/repos/grafana/pyroscope/contents/",
Want: "unknown_route",
},
{
Name: "GetContents with whitespace in path",
Path: "/repos/grafana/pyroscope/contents/path with spaces",
Want: "unknown_route",
},
{
Name: "GetCommit",
Path: "/repos/grafana/pyroscope/commits/abcdef1234567890",
Want: "/repos/{owner}/{repo}/commits/{ref}",
},
{
Name: "GetCommit with lowercase and uppercase ref",
Path: "/repos/grafana/pyroscope/commits/abcdefABCDEF1234567890",
Want: "/repos/{owner}/{repo}/commits/{ref}",
},
{
Name: "GetCommit with non-hexadecimal ref",
Path: "/repos/grafana/pyroscope/commits/HEAD",
Want: "/repos/{owner}/{repo}/commits/{ref}",
},
{
Name: "GetCommit without commit",
Path: "/repos/grafana/pyroscope/commits/",
Want: "unknown_route",
},
{
Name: "Refresh",
Path: "/login/oauth/access_token",
Want: "/login/oauth/access_token",
},
{
Name: "empty path",
Path: "",
Want: "unknown_route",
},
{
Name: "unmapped path",
Path: "/some/random/path",
Want: "unknown_route",
},
}

for _, tt := range tests {
t.Run(tt.Name, func(t *testing.T) {
got := matchGitHubAPIRoute(tt.Path)
require.Equal(t, tt.Want, got)
})
}
}
5 changes: 1 addition & 4 deletions pkg/querier/vcs/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ func githubOAuthConfig() (*oauth2.Config, error) {

// refreshGithubToken sends a request configured for the GitHub API and marshals
// the response into a githubAuthToken.
func refreshGithubToken(req *http.Request) (*githubAuthToken, error) {
client := http.Client{
Timeout: 10 * time.Second,
}
func refreshGithubToken(req *http.Request, client *http.Client) (*githubAuthToken, error) {
res, err := client.Do(req)
if err != nil {
return nil, fmt.Errorf("failed to make request: %w", err)
Expand Down
41 changes: 0 additions & 41 deletions pkg/querier/vcs/github.md

This file was deleted.

2 changes: 1 addition & 1 deletion pkg/querier/vcs/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func Test_refreshGithubToken(t *testing.T) {
req, err := http.NewRequest("POST", fakeGithubAPI.URL, nil)
require.NoError(t, err)

got, err := refreshGithubToken(req)
got, err := refreshGithubToken(req, http.DefaultClient)
require.NoError(t, err)
require.Equal(t, want, *got)
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/querier/vcs/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/go-kit/log"
giturl "github.com/kubescape/go-git-url"
"github.com/kubescape/go-git-url/apis"
"github.com/prometheus/client_golang/prometheus"
"golang.org/x/oauth2"

vcsv1 "github.com/grafana/pyroscope/api/gen/proto/go/vcs/v1"
Expand All @@ -22,12 +23,16 @@ import (
var _ vcsv1connect.VCSServiceHandler = (*Service)(nil)

type Service struct {
logger log.Logger
logger log.Logger
httpClient *http.Client
}

func New(logger log.Logger) *Service {
func New(logger log.Logger, reg prometheus.Registerer) *Service {
httpClient := client.InstrumentedHTTPClient(logger, reg)

return &Service{
logger: logger,
logger: logger,
httpClient: httpClient,
}
}

Expand Down Expand Up @@ -81,7 +86,7 @@ func (q *Service) GithubRefresh(ctx context.Context, req *connect.Request[vcsv1.
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to refresh token"))
}

githubToken, err := refreshGithubToken(githubRequest)
githubToken, err := refreshGithubToken(githubRequest, q.httpClient)
if err != nil {
q.logger.Log("err", err, "msg", "failed to refresh token with GitHub")
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("failed to refresh token"))
Expand Down Expand Up @@ -130,7 +135,7 @@ func (q *Service) GetFile(ctx context.Context, req *connect.Request[vcsv1.GetFil
}

// todo: we can support multiple provider: bitbucket, gitlab, etc.
ghClient, err := client.GithubClient(ctx, token)
ghClient, err := client.GithubClient(ctx, token, q.httpClient)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -173,7 +178,7 @@ func (q *Service) GetCommit(ctx context.Context, req *connect.Request[vcsv1.GetC
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("only GitHub repositories are supported"))
}

ghClient, err := client.GithubClient(ctx, token)
ghClient, err := client.GithubClient(ctx, token, q.httpClient)
if err != nil {
return nil, err
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/storegateway/clientpool/client_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ func (f *poolFactory) FromInstance(inst ring.InstanceDesc) (ring_client.PoolClie
if err != nil {
return nil, err
}

httpClient := util.InstrumentedDefaultHTTPClient(util.WithTracingTransport())
return &storeGatewayPoolClient{
StoreGatewayServiceClient: storegatewayv1connect.NewStoreGatewayServiceClient(util.InstrumentedHTTPClient(), "http://"+inst.Addr, f.options...),
StoreGatewayServiceClient: storegatewayv1connect.NewStoreGatewayServiceClient(httpClient, "http://"+inst.Addr, f.options...),
HealthClient: grpc_health_v1.NewHealthClient(conn),
Closer: conn,
}, nil
Expand Down