merge checker to use the core/httpclient with baseurl transport

example/server/orchestrator/main.go

@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"net"
-	"net/http"
 	"os"
 	"os/signal"
 	"sync"
@@ -193,7 +192,10 @@ func run() error {
 	c := consumer.New(logger.Sugar(), scope.SubScope("consumer"), registry)
 
 	// Create merge checker
-	mc := newMergeChecker(logger, scope)
+	mc, err := newMergeChecker(logger, scope)
+	if err != nil {
+		return fmt.Errorf("failed to create merge checker: %w", err)
+	}
 
 	// Create change provider
 	cp, err := newChangeProvider(logger, scope)
@@ -547,28 +549,29 @@ func parseTimeout(envVal string, defaultVal time.Duration) time.Duration {
 }
 
 // newMergeChecker creates a MergeChecker for GitHub (github.com).
-// Configured via GITHUB_TOKEN and GITHUB_GRAPHQL_URL environment variables.
-func newMergeChecker(logger *zap.Logger, scope tally.Scope) mergechecker.MergeChecker {
-	graphQLURL := os.Getenv("GITHUB_GRAPHQL_URL")
-	if graphQLURL == "" {
-		graphQLURL = "https://api.github.com/graphql"
+// Configured via GITHUB_BASE_URL, GITHUB_TOKEN, and GITHUB_TIMEOUT environment variables.
+func newMergeChecker(logger *zap.Logger, scope tally.Scope) (mergechecker.MergeChecker, error) {
+	client, err := httpclient.NewClient(getEnv("GITHUB_BASE_URL", "https://api.github.com"))
+	if err != nil {
+		return nil, fmt.Errorf("failed to build GitHub HTTP client: %w", err)
 	}
 
-	httpClient := &http.Client{}
 	if token := os.Getenv("GITHUB_TOKEN"); token != "" {
-		httpClient.Transport = &bearerTransport{token: token}
+		ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
+		client.Transport = &oauth2.Transport{Source: ts, Base: client.Transport}
 	}
 
+	client.Timeout = parseTimeout(os.Getenv("GITHUB_TIMEOUT"), 30*time.Second)
+
 	github := githubchecker.NewMergeChecker(githubchecker.Params{
-		HTTPClient:   httpClient,
-		GraphQLURL:   graphQLURL,
+		HTTPClient:   client,
 		Logger:       logger.Sugar(),
 		MetricsScope: scope.SubScope("mergechecker"),
 	})
 
 	return mergechecker.NewMultiChecker(map[string]mergechecker.MergeChecker{
 		"github": github,
-	})
+	}), nil
 }
 
 // newChangeProvider creates a ChangeProvider for GitHub (github.com).
@@ -592,14 +595,3 @@ func newChangeProvider(logger *zap.Logger, scope tally.Scope) (changeprovider.Ch
 		MetricsScope: scope.SubScope("changeprovider"),
 	}), nil
 }
-
-// bearerTransport is an http.RoundTripper that adds a Bearer token to requests.
-type bearerTransport struct {
-	token string
-}
-
-func (t *bearerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
-	req = req.Clone(req.Context())
-	req.Header.Set("Authorization", "Bearer "+t.token)
-	return http.DefaultTransport.RoundTrip(req)
-}

extension/mergechecker/github/BUILD.bazel

@@ -27,6 +27,7 @@ go_test(
     ],
     embed = [":github"],
     deps = [
+        "//core/httpclient",
         "//entity",
         "//entity/github",
         "//extension/mergechecker",

extension/mergechecker/github/checker.go

@@ -31,12 +31,9 @@ import (
 
 // Params holds the dependencies for the GitHub MergeChecker.
 type Params struct {
-	// HTTPClient is a pre-configured HTTP client with auth (bearer token, GitHub App JWT, etc.).
-	// Auth is the caller's responsibility via HTTP transport/round-tripper.
+	// HTTPClient is a pre-configured HTTP client. The caller is responsible for
+	// configuring the base URL (via BaseURLTransport) and auth (via a transport layer).
 	HTTPClient *http.Client
-	// GraphQLURL is the GitHub GraphQL API endpoint
-	// (e.g., "https://api.github.com/graphql" or "https://ghe.example.com/api/graphql").
-	GraphQLURL string
 	// Logger is the structured logger.
 	Logger *zap.SugaredLogger
 	// MetricsScope is the metrics scope for instrumentation.
@@ -46,7 +43,6 @@ type Params struct {
 // mergeChecker implements the mergechecker.MergeChecker interface using the GitHub GraphQL API.
 type mergeChecker struct {
 	httpClient   *http.Client
-	graphQLURL   string
 	logger       *zap.SugaredLogger
 	metricsScope tally.Scope
 }
@@ -58,7 +54,6 @@ var _ mergechecker.MergeChecker = (*mergeChecker)(nil)
 func NewMergeChecker(params Params) mergechecker.MergeChecker {
 	return &mergeChecker{
 		httpClient:   params.HTTPClient,
-		graphQLURL:   params.GraphQLURL,
 		logger:       params.Logger.Named("github_mergechecker"),
 		metricsScope: params.MetricsScope.SubScope("github_mergechecker"),
 	}
@@ -120,7 +115,7 @@ func (c *mergeChecker) fetchPRInfo(ctx context.Context, changeIDs []entitygithub
 		return nil, fmt.Errorf("failed to marshal graphql request: %w", err)
 	}
 
-	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, c.graphQLURL, bytes.NewReader(reqBody))
+	httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, "/graphql", bytes.NewReader(reqBody))
 	if err != nil {
 		return nil, fmt.Errorf("failed to create http request: %w", err)
 	}

extension/mergechecker/github/checker_test.go

@@ -25,20 +25,20 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/uber-go/tally/v4"
+	"github.com/uber/submitqueue/core/httpclient"
 	"github.com/uber/submitqueue/entity"
 	"github.com/uber/submitqueue/extension/mergechecker"
 	"go.uber.org/zap/zaptest"
 )
 
 func newTestMergeChecker(t *testing.T, serverURL string) mergechecker.MergeChecker {
-	logger := zaptest.NewLogger(t).Sugar()
-	scope := tally.NoopScope
-
+	t.Helper()
+	client, err := httpclient.NewClient(serverURL)
+	require.NoError(t, err)
 	return NewMergeChecker(Params{
-		HTTPClient:   &http.Client{},
-		GraphQLURL:   serverURL,
-		Logger:       logger,
-		MetricsScope: scope,
+		HTTPClient:   client,
+		Logger:       zaptest.NewLogger(t).Sugar(),
+		MetricsScope: tally.NoopScope,
 	})
 }