From a0583b7a18a08a7100802f230365adc24e389c54 Mon Sep 17 00:00:00 2001 From: Adriel Perkins Date: Tue, 25 Apr 2023 11:35:45 -0400 Subject: [PATCH] chore(gh): fix formatting & update comment --- receiver/githubreceiver/factory_test.go | 57 +++++++------ receiver/githubreceiver/scraper.go | 107 ++++++++++++------------ 2 files changed, 81 insertions(+), 83 deletions(-) diff --git a/receiver/githubreceiver/factory_test.go b/receiver/githubreceiver/factory_test.go index c8b6b5f4..505dcd77 100644 --- a/receiver/githubreceiver/factory_test.go +++ b/receiver/githubreceiver/factory_test.go @@ -5,9 +5,9 @@ import ( "github.com/liatrio/otel-liatrio-contrib/receiver/githubreceiver/internal/metadata" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/config/confighttp" + "go.opentelemetry.io/collector/receiver/scraperhelper" "testing" "time" - "go.opentelemetry.io/collector/receiver/scraperhelper" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/consumer/consumertest" @@ -43,35 +43,34 @@ func TestNewFactory(t *testing.T) { assert.Equal(t, expectefCfg, factory.CreateDefaultConfig()) }, }, - { - desc: "create new factory and metric receiver without returing an error", - tf: func(t *testing.T) { - factory := NewFactory() - cfg := factory.CreateDefaultConfig() - _, err := factory.CreateMetricsReceiver( - context.Background(), - receivertest.NewNopCreateSettings(), - cfg, - consumertest.NewNop(), - ) - assert.NoError(t, err) - }, - }, - { - desc: "create new factory and metric receiver returning errors and invalid config", - tf: func(t *testing.T) { - factory := NewFactory() - _, err := factory.CreateMetricsReceiver( - context.Background(), - receivertest.NewNopCreateSettings(), - nil, - consumertest.NewNop(), - ) - assert.ErrorIs(t, err, ghConfigNotValid) - + { + desc: "create new factory and metric receiver without returing an error", + tf: func(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig() + _, err := factory.CreateMetricsReceiver( + context.Background(), + receivertest.NewNopCreateSettings(), + cfg, + consumertest.NewNop(), + ) + assert.NoError(t, err) + }, + }, + { + desc: "create new factory and metric receiver returning errors and invalid config", + tf: func(t *testing.T) { + factory := NewFactory() + _, err := factory.CreateMetricsReceiver( + context.Background(), + receivertest.NewNopCreateSettings(), + nil, + consumertest.NewNop(), + ) + assert.ErrorIs(t, err, ghConfigNotValid) - }, - }, + }, + }, } for _, tt := range tc { diff --git a/receiver/githubreceiver/scraper.go b/receiver/githubreceiver/scraper.go index 7703f76d..e3721a98 100644 --- a/receiver/githubreceiver/scraper.go +++ b/receiver/githubreceiver/scraper.go @@ -49,9 +49,10 @@ func newScraper(cfg *Config, settings receiver.CreateSettings) *ghScraper { } } +// TODO: This is pretty bad because it's making an API call per commit // TODO: This needs to be unit tested & might not be the best way to do this // TODO: Commenting this out for now given the slowness and rate limiting of the GitHub API -// Checks if the commit paassed is in the parent branch to determine if the +// Checks if the commit paassed is in the parent branch to determine if the // branch at given point of commit is divergent. //func diverges(ctx context.Context, client *github.Client, org string, repo string, parentBranch string, commit string) (bool, error) { // comparison, resp, err := client.Repositories.CompareCommits(ctx, org, repo, parentBranch, commit, nil) @@ -75,9 +76,8 @@ func (ghs *ghScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { now := pcommon.NewTimestampFromTime(time.Now()) ghs.logger.Sugar().Debugf("current time: %v", now) - - currentDate := time.Now().Day() - ghs.logger.Sugar().Debugf("current date: %v", currentDate) + currentDate := time.Now().Day() + ghs.logger.Sugar().Debugf("current date: %v", currentDate) ghs.logger.Sugar().Debug("creating a new github client") client := github.NewClient(ghs.client) @@ -89,56 +89,55 @@ func (ghs *ghScraper) scrape(ctx context.Context) (pmetric.Metrics, error) { ghs.mb.RecordGhRepoCountDataPoint(now, int64(len(repos)), ghs.cfg.GitHubOrg) - for _, repo := range repos { - // branch list - branches, _, err := client.Repositories.ListBranches(ctx, ghs.cfg.GitHubOrg, *repo.Name, nil) - if err != nil { - ghs.logger.Sugar().Errorf("Error getting branches", zap.Error(err)) - } - ghs.mb.RecordGhRepoBranchesCountDataPoint(now, int64(len(branches)), *repo.Name, ghs.cfg.GitHubOrg) - - defaultBranch := *repo.DefaultBranch - - // For each branch, get some additional metrics and associate them with the branch - for _, branch := range branches { - if *branch.Name != defaultBranch { - cmts, _, err := client.Repositories.ListCommits(ctx, ghs.cfg.GitHubOrg, *repo.Name, &github.CommitsListOptions{SHA: *branch.Name}) - if err != nil { - ghs.logger.Sugar().Errorf("Error getting commits", zap.Error(err)) - } - - numCommits := int64(len(cmts)) - ghs.mb.RecordGhRepoBranchCommitsCountDataPoint(now, numCommits, *repo.Name, ghs.cfg.GitHubOrg, *branch.Name) - - // TODO: this should be a goroutine in order to maximize efficiency - // TODO: there's a could potential for leveraging the graphql API for github - // Commenting this out for now given the above, leverages the diverged func - // instead of the REST API - //for _, cmt := range cmts { - // sha := cmt.SHA - // diverged, err := diverges(ctx, client, ghs.cfg.GitHubOrg, *repo.Name, defaultBranch, *sha) - // if err != nil { - // ghs.logger.Sugar().Errorf("error when checking if the commit has diverged from the default branch: %v", zap.Error(err)) - // } - - // if len(cmt.Parents) == 1 && !diverged { - // createdDate := cmt.Commit.Author.Date.Time - // ageDelta := now.AsTime().Sub(createdDate) - // ghs.mb.RecordGhRepoBranchTotalAgeDataPoint(now, int64(ageDelta.Hours()), *repo.Name, ghs.cfg.GitHubOrg, *branch.Name) - // } - //} - } - } - - - // contributor list - contribs, _, err := client.Repositories.ListContributors(ctx, ghs.cfg.GitHubOrg, *repo.Name, nil) - if err != nil { - ghs.logger.Sugar().Errorf("Error getting contributors", zap.Error(err)) - } - ghs.mb.RecordGhRepoContributorsCountDataPoint(now, int64(len(contribs)), *repo.Name, ghs.cfg.GitHubOrg) - //ghs.mb.RecordGhRepoBranchesCountDataPoint(now, int64(repo.Bran)) - } + for _, repo := range repos { + // branch list + branches, _, err := client.Repositories.ListBranches(ctx, ghs.cfg.GitHubOrg, *repo.Name, nil) + if err != nil { + ghs.logger.Sugar().Errorf("Error getting branches", zap.Error(err)) + } + ghs.mb.RecordGhRepoBranchesCountDataPoint(now, int64(len(branches)), *repo.Name, ghs.cfg.GitHubOrg) + + defaultBranch := *repo.DefaultBranch + + // For each branch, get some additional metrics and associate them with the branch + for _, branch := range branches { + if *branch.Name != defaultBranch { + cmts, _, err := client.Repositories.ListCommits(ctx, ghs.cfg.GitHubOrg, *repo.Name, &github.CommitsListOptions{SHA: *branch.Name}) + if err != nil { + ghs.logger.Sugar().Errorf("Error getting commits", zap.Error(err)) + } + + numCommits := int64(len(cmts)) + ghs.mb.RecordGhRepoBranchCommitsCountDataPoint(now, numCommits, *repo.Name, ghs.cfg.GitHubOrg, *branch.Name) + + // TODO: this should be a goroutine in order to maximize efficiency + // TODO: there's a could potential for leveraging the graphql API for github + // Commenting this out for now given the above, leverages the diverged func + // instead of the REST API + //for _, cmt := range cmts { + // sha := cmt.SHA + // diverged, err := diverges(ctx, client, ghs.cfg.GitHubOrg, *repo.Name, defaultBranch, *sha) + // if err != nil { + // ghs.logger.Sugar().Errorf("error when checking if the commit has diverged from the default branch: %v", zap.Error(err)) + // } + + // if len(cmt.Parents) == 1 && !diverged { + // createdDate := cmt.Commit.Author.Date.Time + // ageDelta := now.AsTime().Sub(createdDate) + // ghs.mb.RecordGhRepoBranchTotalAgeDataPoint(now, int64(ageDelta.Hours()), *repo.Name, ghs.cfg.GitHubOrg, *branch.Name) + // } + //} + } + } + + // contributor list + contribs, _, err := client.Repositories.ListContributors(ctx, ghs.cfg.GitHubOrg, *repo.Name, nil) + if err != nil { + ghs.logger.Sugar().Errorf("Error getting contributors", zap.Error(err)) + } + ghs.mb.RecordGhRepoContributorsCountDataPoint(now, int64(len(contribs)), *repo.Name, ghs.cfg.GitHubOrg) + //ghs.mb.RecordGhRepoBranchesCountDataPoint(now, int64(repo.Bran)) + } ghs.logger.Sugar().Debugf("repos: %v", repos) ghs.logger.Sugar().Debugf("metrics: %v", ghs.cfg.Metrics.GhRepoCount)