vcs/github_client: add logging around github API calls (#1042)

server/events/vcs/github_client.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs/common"
+	"github.com/runatlantis/atlantis/server/logging"
 
 	"github.com/Laisky/graphql"
 	"github.com/google/go-github/v28/github"
@@ -38,10 +39,11 @@ type GithubClient struct {
 	client         *github.Client
 	v4MutateClient *graphql.Client
 	ctx            context.Context
+	logger         *logging.SimpleLogger
 }
 
 // NewGithubClient returns a valid GitHub client.
-func NewGithubClient(hostname string, user string, pass string) (*GithubClient, error) {
+func NewGithubClient(hostname string, user string, pass string, logger *logging.SimpleLogger) (*GithubClient, error) {
 	tp := github.BasicAuthTransport{
 		Username: strings.TrimSpace(user),
 		Password: strings.TrimSpace(pass),
@@ -84,6 +86,7 @@ func NewGithubClient(hostname string, user string, pass string) (*GithubClient,
 		client:         client,
 		v4MutateClient: v4MutateClient,
 		ctx:            context.Background(),
+		logger:         logger,
 	}, nil
 }
 
@@ -99,6 +102,7 @@ func (g *GithubClient) GetModifiedFiles(repo models.Repo, pull models.PullReques
 		if nextPage != 0 {
 			opts.Page = nextPage
 		}
+		g.logger.Debug("GET /repos/%v/%v/pulls/%d/files", repo.Owner, repo.Name, pull.Num)
 		pageFiles, resp, err := g.client.PullRequests.ListFiles(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
 		if err != nil {
 			return files, err
@@ -131,6 +135,7 @@ func (g *GithubClient) CreateComment(repo models.Repo, pullNum int, comment stri
 
 	comments := common.SplitComment(comment, maxCommentLength, sepEnd, sepStart)
 	for _, c := range comments {
+		g.logger.Debug("POST /repos/%v/%v/issues/%d/comments", repo.Owner, repo.Name, pullNum)
 		_, _, err := g.client.Issues.CreateComment(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueComment{Body: &c})
 		if err != nil {
 			return err
@@ -143,6 +148,7 @@ func (g *GithubClient) HidePrevPlanComments(repo models.Repo, pullNum int) error
 	var allComments []*github.IssueComment
 	nextPage := 0
 	for {
+		g.logger.Debug("GET /repos/%v/%v/issues/%d/comments", repo.Owner, repo.Name, pullNum)
 		comments, resp, err := g.client.Issues.ListComments(g.ctx, repo.Owner, repo.Name, pullNum, &github.IssueListCommentsOptions{
 			Sort:        "created",
 			Direction:   "asc",
@@ -210,6 +216,7 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest)
 		if nextPage != 0 {
 			opts.Page = nextPage
 		}
+		g.logger.Debug("GET /repos/%v/%v/pulls/%d/reviews", repo.Owner, repo.Name, pull.Num)
 		pageReviews, resp, err := g.client.PullRequests.ListReviews(g.ctx, repo.Owner, repo.Name, pull.Num, &opts)
 		if err != nil {
 			return false, errors.Wrap(err, "getting reviews")
@@ -282,6 +289,7 @@ func (g *GithubClient) UpdateStatus(repo models.Repo, pull models.PullRequest, s
 func (g *GithubClient) MergePull(pull models.PullRequest) error {
 	// Users can set their repo to disallow certain types of merging.
 	// We detect which types aren't allowed and use the type that is.
+	g.logger.Debug("GET /repos/%v/%v", pull.BaseRepo.Owner, pull.BaseRepo.Name)
 	repo, _, err := g.client.Repositories.Get(g.ctx, pull.BaseRepo.Owner, pull.BaseRepo.Name)
 	if err != nil {
 		return errors.Wrap(err, "fetching repo info")
@@ -304,6 +312,7 @@ func (g *GithubClient) MergePull(pull models.PullRequest) error {
 	options := &github.PullRequestOptions{
 		MergeMethod: method,
 	}
+	g.logger.Debug("PUT /repos/%v/%v/pulls/%d/merge", repo.Owner, repo.Name, pull.Num)
 	mergeResult, _, err := g.client.PullRequests.Merge(
 		g.ctx,
 		pull.BaseRepo.Owner,

server/events/vcs/github_client_internal_test.go

@@ -21,14 +21,14 @@ import (
 
 // If the hostname is github.com, should use normal BaseURL.
 func TestNewGithubClient_GithubCom(t *testing.T) {
-	client, err := NewGithubClient("github.com", "user", "pass")
+	client, err := NewGithubClient("github.com", "user", "pass", nil)
 	Ok(t, err)
 	Equals(t, "https://api.github.com/", client.client.BaseURL.String())
 }
 
 // If the hostname is a non-github hostname should use the right BaseURL.
 func TestNewGithubClient_NonGithub(t *testing.T) {
-	client, err := NewGithubClient("example.com", "user", "pass")
+	client, err := NewGithubClient("example.com", "user", "pass", nil)
 	Ok(t, err)
 	Equals(t, "https://example.com/api/v3/", client.client.BaseURL.String())
 }

server/events/vcs/github_client_test.go

@@ -59,7 +59,7 @@ func TestGithubClient_GetModifiedFiles(t *testing.T) {
 
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
-	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -114,7 +114,7 @@ func TestGithubClient_GetModifiedFilesMovedFile(t *testing.T) {
 
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
-	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -208,7 +208,7 @@ func TestGithubClient_PaginatesComments(t *testing.T) {
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
 
-	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -294,7 +294,7 @@ func TestGithubClient_HideOldComments(t *testing.T) {
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
 
-	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -358,7 +358,7 @@ func TestGithubClient_UpdateStatus(t *testing.T) {
 
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 			Ok(t, err)
 			defer disableSSLVerification()()
 
@@ -444,7 +444,7 @@ func TestGithubClient_PullIsApproved(t *testing.T) {
 
 	testServerURL, err := url.Parse(testServer.URL)
 	Ok(t, err)
-	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+	client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 	Ok(t, err)
 	defer disableSSLVerification()()
 
@@ -535,7 +535,7 @@ func TestGithubClient_PullIsMergeable(t *testing.T) {
 				}))
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 			Ok(t, err)
 			defer disableSSLVerification()()
 
@@ -617,7 +617,7 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) {
 
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 			Ok(t, err)
 			defer disableSSLVerification()()
 
@@ -739,7 +739,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 
 			testServerURL, err := url.Parse(testServer.URL)
 			Ok(t, err)
-			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass")
+			client, err := vcs.NewGithubClient(testServerURL.Host, "user", "pass", nil)
 			Ok(t, err)
 			defer disableSSLVerification()()
 
@@ -764,7 +764,7 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) {
 }
 
 func TestGithubClient_MarkdownPullLink(t *testing.T) {
-	client, err := vcs.NewGithubClient("hostname", "user", "pass")
+	client, err := vcs.NewGithubClient("hostname", "user", "pass", nil)
 	Ok(t, err)
 	pull := models.PullRequest{Num: 1}
 	s, _ := client.MarkdownPullLink(pull)

server/server.go

@@ -122,7 +122,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) {
 	if userConfig.GithubUser != "" {
 		supportedVCSHosts = append(supportedVCSHosts, models.Github)
 		var err error
-		githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, userConfig.GithubUser, userConfig.GithubToken)
+		githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, userConfig.GithubUser, userConfig.GithubToken, logger)
 		if err != nil {
 			return nil, err
 		}