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

Replace logging with structured logging #21

Merged
merged 3 commits into from
Jun 13, 2024
Merged
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
22 changes: 11 additions & 11 deletions internal/github/checkPendingCI.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package github
import (
"context"
"fmt"
"log"
"strings"
"time"

"github.com/google/go-github/v60/github" // Ensure your go-github library version matches

"github.com/chia-network/github-bot/internal/config"
log "github.com/chia-network/github-bot/internal/logger"
)

// CheckForPendingCI returns a list of PR URLs that are ready for CI to run but haven't started yet.
Expand All @@ -18,10 +18,10 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
var pendingPRs []string

for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
log.Logger.Info("Checking repository", "repository", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
log.Printf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
log.Logger.Error("Invalid repository name - must contain owner and repository", "repository", fullRepo.Name)
continue
}
owner, repo := parts[0], parts[1]
Expand All @@ -38,29 +38,29 @@ func CheckForPendingCI(ctx context.Context, githubClient *github.Client, cfg *co
// Dynamic cutoff time based on the last commit to the PR
lastCommitTime, err := getLastCommitTime(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error retrieving last commit time for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
log.Logger.Error("Error retrieving last commit time", "PR", pr.GetNumber(), "repository", fullRepo.Name, "error", err)
continue
}
cutoffTime := lastCommitTime.Add(2 * time.Hour) // 2 hours after the last commit

if time.Now().Before(cutoffTime) {
log.Printf("Skipping PR #%d from %s/%s repo as it's still within the 2-hour window from the last commit.", pr.GetNumber(), owner, repo)
log.Logger.Info("Skipping PR as it's still within the 2-hour window from the last commit", "PR", pr.GetNumber(), "repository", fullRepo.Name)
continue
}

hasCIRuns, err := checkCIStatus(prctx, githubClient, owner, repo, pr.GetNumber())
if err != nil {
log.Printf("Error checking CI status for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
log.Logger.Error("Error checking CI status", "PR", pr.GetNumber(), "repository", fullRepo.Name, "error", err)
continue
}

teamMemberActivity, err := checkTeamMemberActivity(prctx, githubClient, owner, repo, pr.GetNumber(), teamMembers, lastCommitTime)
if err != nil {
log.Printf("Error checking team member activity for PR #%d in %s/%s: %v", pr.GetNumber(), owner, repo, err)
log.Logger.Error("Error checking team member activity", "PR", pr.GetNumber(), "repository", fullRepo.Name, "error", err)
continue // or handle the error as needed
}
if !hasCIRuns && !teamMemberActivity {
log.Printf("PR #%d in %s/%s by %s is ready for CI since %v but no CI actions have started yet, or it requires re-approval.", pr.GetNumber(), owner, repo, pr.User.GetLogin(), pr.CreatedAt)
log.Logger.Info("PR is ready for CI but no CI actions have started yet, or it requires re-approval", "PR", pr.GetNumber(), "repository", fullRepo.Name, "user", pr.User.GetLogin(), "created_at", pr.CreatedAt)
pendingPRs = append(pendingPRs, pr.GetHTMLURL())
}
}
Expand All @@ -85,7 +85,7 @@ func getLastCommitTime(ctx context.Context, client *github.Client, owner, repo s
if commitTime == nil {
return time.Time{}, fmt.Errorf("commit time is nil for PR #%d of repo %s", prNumber, repo)
}
log.Printf("The last commit time is %s for PR #%d of repo %s", commitTime.Format(time.RFC3339), prNumber, repo)
log.Logger.Info("The last commit time", "time", commitTime.Format(time.RFC3339), "PR", prNumber, "repository", repo)

return *commitTime, nil // Safely dereference *time.Time to get time.Time
}
Expand Down Expand Up @@ -119,9 +119,9 @@ func checkTeamMemberActivity(ctx context.Context, client *github.Client, owner,
}

for _, comment := range comments {
log.Printf("Checking comment by %s at %s for PR #%d of repo %s", comment.User.GetLogin(), comment.CreatedAt.Format(time.RFC3339), prNumber, repo)
log.Logger.Info("Checking comment by team member", "user", comment.User.GetLogin(), "created_at", comment.CreatedAt.Format(time.RFC3339), "PR", prNumber, "repository", repo)
pmaslana marked this conversation as resolved.
Show resolved Hide resolved
if _, ok := teamMembers[comment.User.GetLogin()]; ok && comment.CreatedAt.After(lastCommitTime) {
log.Printf("Found team member comment after last commit time: %s for PR #%d of repo %s", comment.CreatedAt.Format(time.RFC3339), prNumber, repo)
log.Logger.Info("Found team member comment after last commit time", "time", comment.CreatedAt.Format(time.RFC3339), "PR", prNumber, "repository", repo)
// Check if the comment is after the last commit
return true, nil // Active and relevant participation
}
Expand Down
12 changes: 7 additions & 5 deletions internal/github/checkStalePRs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package github
import (
"context"
"fmt"
"log"
"strings"
"time"

"github.com/google/go-github/v60/github"

"github.com/chia-network/github-bot/internal/config"
log "github.com/chia-network/github-bot/internal/logger"
)

// CheckStalePRs will return a list of PR URLs that have not been updated in the last 7 days by internal team members.
Expand All @@ -22,7 +22,7 @@ func CheckStalePRs(ctx context.Context, githubClient *github.Client, cfg *config
}

for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
log.Logger.Info("Checking repository", "repository", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
return nil, fmt.Errorf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
Expand All @@ -35,10 +35,11 @@ func CheckStalePRs(ctx context.Context, githubClient *github.Client, cfg *config
}

for _, pr := range communityPRs {
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
repoName := pr.GetBase().GetRepo().GetFullName() // Get the full name of the repository
log.Logger.Info("Checking PR", "PR", pr.GetHTMLURL())
stale, err := isStale(ctx, githubClient, pr, teamMembers, cutoffDate) // Handle both returned values
if err != nil {
log.Printf("Error checking if PR in repo %s is stale: %v", repoName, err)
log.Logger.Error("Error checking if PR is stale", "repository", repoName, "error", err)
continue // Skip this PR or handle the error appropriately
}
if stale {
Expand All @@ -58,7 +59,8 @@ func isStale(ctx context.Context, githubClient *github.Client, pr *github.PullRe
defer staleCancel()
events, resp, err := githubClient.Issues.ListIssueTimeline(staleCtx, pr.Base.Repo.Owner.GetLogin(), pr.Base.Repo.GetName(), pr.GetNumber(), listOptions)
if err != nil {
return false, fmt.Errorf("failed to get timeline for PR #%d of repo %s: %w", pr.GetNumber(), pr.Base.Repo.GetName(), err)
log.Logger.Error("Failed to get timeline for PR", "PR", pr.GetNumber(), "repository", pr.Base.Repo.GetName(), "error", err)
return false, err
}
for _, event := range events {
if event.Event == nil || event.Actor == nil || event.Actor.Login == nil {
Expand Down
10 changes: 5 additions & 5 deletions internal/label/pullrequests.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package label
import (
"context"
"fmt"
"log"
"strings"

"github.com/google/go-github/v60/github"

"github.com/chia-network/github-bot/internal/config"
github2 "github.com/chia-network/github-bot/internal/github"
log "github.com/chia-network/github-bot/internal/logger"
)

// PullRequests applies internal or community labels to pull requests
Expand All @@ -20,10 +20,10 @@ func PullRequests(githubClient *github.Client, cfg *config.Config) error {
}

for _, fullRepo := range cfg.CheckRepos {
log.Println("Checking repository:", fullRepo.Name)
log.Logger.Info("Checking repository", "repository", fullRepo.Name)
parts := strings.Split(fullRepo.Name, "/")
if len(parts) != 2 {
log.Printf("invalid repository name - must contain owner and repository: %s", fullRepo.Name)
log.Logger.Error("Invalid repository name - must contain owner and repository", "repository", fullRepo.Name)
continue
}
owner, repo := parts[0], parts[1]
Expand All @@ -43,11 +43,11 @@ func PullRequests(githubClient *github.Client, cfg *config.Config) error {
label = cfg.LabelExternal
}
if label != "" {
log.Printf("Pull Request %d by %s will be labeled %s\n", *pullRequest.Number, user, label)
log.Logger.Info("Labeling pull request", "PR", *pullRequest.Number, "user", user, "label", label)
hasLabel := false
for _, existingLabel := range pullRequest.Labels {
if *existingLabel.Name == label {
log.Println("Already labeled, skipping...")
log.Logger.Info("Already labeled, skipping", "PR", *pullRequest.Number, "label", label)
hasLabel = true
break
}
Expand Down
13 changes: 13 additions & 0 deletions internal/logger/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package logger

import (
"log/slog"
"os"
)

// Logger is a custom logger from the stdlib slog package
var Logger *slog.Logger

func init() {
Logger = slog.New(slog.NewTextHandler(os.Stdout, nil))
}