diff --git a/cmd/server.go b/cmd/server.go index 059142d22f..17fa61b93f 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -76,6 +76,7 @@ const ( HidePrevPlanComments = "hide-prev-plan-comments" LogLevelFlag = "log-level" ParallelPoolSize = "parallel-pool-size" + StatsNamespace = "stats-namespace" AllowDraftPRs = "allow-draft-prs" PortFlag = "port" RepoConfigFlag = "repo-config" @@ -112,6 +113,7 @@ const ( DefaultGitlabHostname = "gitlab.com" DefaultLogLevel = "info" DefaultParallelPoolSize = 15 + DefaultStatsNamespace = "atlantis" DefaultPort = 4141 DefaultTFDownloadURL = "https://releases.hashicorp.com" DefaultTFEHostname = "app.terraform.io" @@ -229,6 +231,10 @@ var stringFlags = map[string]stringFlag{ description: "Log level. Either debug, info, warn, or error.", defaultValue: DefaultLogLevel, }, + StatsNamespace: { + description: "Namespace for aggregating stats.", + defaultValue: DefaultStatsNamespace, + }, RepoConfigFlag: { description: "Path to a repo config file, used to customize how Atlantis runs on each repo. See runatlantis.io/docs for more details.", }, @@ -603,6 +609,9 @@ func (s *ServerCmd) setDefaults(c *server.UserConfig) { if c.ParallelPoolSize == 0 { c.ParallelPoolSize = DefaultParallelPoolSize } + if c.StatsNamespace == "" { + c.StatsNamespace = DefaultStatsNamespace + } if c.Port == 0 { c.Port = DefaultPort } diff --git a/cmd/server_test.go b/cmd/server_test.go index 7d7e4916bb..39bca0cacc 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -84,6 +84,7 @@ var testFlags = map[string]interface{}{ GitlabUserFlag: "gitlab-user", GitlabWebhookSecretFlag: "gitlab-secret", LogLevelFlag: "debug", + StatsNamespace: "atlantis", AllowDraftPRs: true, PortFlag: 8181, ParallelPoolSize: 100, diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index eabfda2c38..06cc0fbad0 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -16,6 +16,7 @@ import ( "github.com/google/go-github/v31/github" "github.com/hashicorp/go-getter" "github.com/hashicorp/go-version" + stats "github.com/lyft/gostats" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server" events_controllers "github.com/runatlantis/atlantis/server/controllers/events" @@ -724,6 +725,8 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl WorkingDir: workingDir, PreWorkflowHookRunner: mockPreWorkflowHookRunner, } + statsScope := stats.NewStore(stats.NewNullSink(), false) + projectCommandBuilder := events.NewProjectCommandBuilder( userConfig.EnablePolicyChecksFlag, parser, @@ -737,6 +740,8 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + statsScope, + logger, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) @@ -871,6 +876,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl GithubPullGetter: e2eGithubGetter, GitlabMergeRequestGetter: e2eGitlabGetter, Logger: logger, + StatsScope: statsScope, AllowForkPRs: allowForkPRs, AllowForkPRsFlag: "allow-fork-prs", CommentCommandRunnerByCmd: commentCommandRunnerByCmd, diff --git a/server/events/command_context.go b/server/events/command_context.go index a4d6ebb13a..6d700760df 100644 --- a/server/events/command_context.go +++ b/server/events/command_context.go @@ -13,6 +13,7 @@ package events import ( + stats "github.com/lyft/gostats" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" ) @@ -38,8 +39,9 @@ type CommandContext struct { HeadRepo models.Repo Pull models.PullRequest // User is the user that triggered this command. - User models.User - Log logging.SimpleLogging + User models.User + Log logging.SimpleLogging + Scope stats.Scope // PullMergeable is true if Pull is able to be merged. This is available in // the CommandContext because we want to collect this information before we // set our own build statuses which can affect mergeability if users have diff --git a/server/events/command_runner.go b/server/events/command_runner.go index 24c717a01d..4abff85010 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -18,8 +18,10 @@ import ( "strconv" "github.com/google/go-github/v31/github" + stats "github.com/lyft/gostats" "github.com/mcdafydd/go-azuredevops/azuredevops" "github.com/pkg/errors" + "github.com/runatlantis/atlantis/server/events/metrics" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/logging" @@ -95,6 +97,7 @@ type DefaultCommandRunner struct { DisableAutoplan bool EventParser EventParsing Logger logging.SimpleLogging + StatsScope stats.Scope // AllowForkPRs controls whether we operate on pull requests from forks. AllowForkPRs bool // ParallelPoolSize controls the size of the wait group used to run @@ -134,9 +137,14 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo log.Err("Unable to fetch pull status, this is likely a bug.", err) } + scope := c.StatsScope.Scope("autoplan") + timer := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer timer.Complete() + ctx := &CommandContext{ User: user, Log: log, + Scope: scope, Pull: pull, HeadRepo: headRepo, PullStatus: status, @@ -177,6 +185,14 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead log := c.buildLogger(baseRepo.FullName, pullNum) defer c.logPanics(baseRepo, pullNum, log) + scope := c.StatsScope.Scope("comment") + + if cmd != nil { + scope = scope.Scope(cmd.Name.String()) + } + timer := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer timer.Complete() + headRepo, pull, err := c.ensureValidRepoMetadata(baseRepo, maybeHeadRepo, maybePull, user, pullNum, log) if err != nil { return @@ -195,6 +211,7 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead PullStatus: status, HeadRepo: headRepo, Trigger: Comment, + Scope: scope, } if !c.validateCtxAndComment(ctx) { diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 1254e96d03..c8d3a552ce 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -19,6 +19,7 @@ import ( "strings" "testing" + stats "github.com/lyft/gostats" "github.com/runatlantis/atlantis/server/events/db" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" @@ -173,6 +174,8 @@ func setup(t *testing.T) *vcsmocks.MockClient { When(preWorkflowHooksCommandRunner.RunPreHooks(matchers.AnyPtrToEventsCommandContext())).ThenReturn(nil) + scope := stats.NewDefaultStore() + ch = events.DefaultCommandRunner{ VCSClient: vcsClient, CommentCommandRunnerByCmd: commentCommandRunnerByCmd, @@ -181,6 +184,7 @@ func setup(t *testing.T) *vcsmocks.MockClient { GitlabMergeRequestGetter: gitlabGetter, AzureDevopsPullGetter: azuredevopsGetter, Logger: logger, + StatsScope: scope, AllowForkPRs: false, AllowForkPRsFlag: "allow-fork-prs-flag", Drainer: drainer, diff --git a/server/events/instrumented_project_command_builder.go b/server/events/instrumented_project_command_builder.go new file mode 100644 index 0000000000..5b7d680d92 --- /dev/null +++ b/server/events/instrumented_project_command_builder.go @@ -0,0 +1,76 @@ +package events + +import ( + "github.com/runatlantis/atlantis/server/events/metrics" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/logging" +) + +type InstrumentedProjectCommandBuilder struct { + ProjectCommandBuilder + Logger logging.SimpleLogging +} + +func (b *InstrumentedProjectCommandBuilder) BuildApplyCommands(ctx *CommandContext, comment *CommentCommand) ([]models.ProjectCommandContext, error) { + scope := ctx.Scope.Scope("builder") + + timer := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer timer.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + projectCmds, err := b.ProjectCommandBuilder.BuildApplyCommands(ctx, comment) + + if err != nil { + executionError.Inc() + b.Logger.Err("Error building apply commands: %s", err) + } else { + executionSuccess.Inc() + } + + return projectCmds, err + +} +func (b *InstrumentedProjectCommandBuilder) BuildAutoplanCommands(ctx *CommandContext) ([]models.ProjectCommandContext, error) { + scope := ctx.Scope.Scope("builder") + + timer := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer timer.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + projectCmds, err := b.ProjectCommandBuilder.BuildAutoplanCommands(ctx) + + if err != nil { + executionError.Inc() + b.Logger.Err("Error building auto plan commands: %s", err) + } else { + executionSuccess.Inc() + } + + return projectCmds, err + +} +func (b *InstrumentedProjectCommandBuilder) BuildPlanCommands(ctx *CommandContext, comment *CommentCommand) ([]models.ProjectCommandContext, error) { + scope := ctx.Scope.Scope("builder") + + timer := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer timer.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + projectCmds, err := b.ProjectCommandBuilder.BuildPlanCommands(ctx, comment) + + if err != nil { + executionError.Inc() + b.Logger.Err("Error building plan commands: %s", err) + } else { + executionSuccess.Inc() + } + + return projectCmds, err + +} diff --git a/server/events/instrumented_project_command_runner.go b/server/events/instrumented_project_command_runner.go new file mode 100644 index 0000000000..aac3ddfeb8 --- /dev/null +++ b/server/events/instrumented_project_command_runner.go @@ -0,0 +1,56 @@ +package events + +import ( + "github.com/runatlantis/atlantis/server/events/metrics" + "github.com/runatlantis/atlantis/server/events/models" +) + +type InstrumentedProjectCommandRunner struct { + ProjectCommandRunner +} + +func (p *InstrumentedProjectCommandRunner) Plan(ctx models.ProjectCommandContext) models.ProjectResult { + return RunAndEmitStats("plan", ctx, p.ProjectCommandRunner.Plan) +} + +func (p *InstrumentedProjectCommandRunner) PolicyCheck(ctx models.ProjectCommandContext) models.ProjectResult { + return RunAndEmitStats("policy check", ctx, p.ProjectCommandRunner.PolicyCheck) +} + +func (p *InstrumentedProjectCommandRunner) Apply(ctx models.ProjectCommandContext) models.ProjectResult { + return RunAndEmitStats("apply", ctx, p.ProjectCommandRunner.Apply) +} + +func RunAndEmitStats(commandName string, ctx models.ProjectCommandContext, execute func(ctx models.ProjectCommandContext) models.ProjectResult) models.ProjectResult { + + // ensures we are differentiating between project level command and overall command + ctx.SetScope("project") + + scope := ctx.Scope + logger := ctx.Log + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + executionFailure := scope.NewCounter(metrics.ExecutionFailureMetric) + + result := execute(ctx) + + if result.Error != nil { + executionError.Inc() + logger.Err("Error running %s operation: %s", commandName, result.Error.Error()) + return result + } + + if result.Failure == "" { + executionFailure.Inc() + logger.Err("Failure running %s operation: %s", commandName, result.Failure) + return result + } + + executionSuccess.Inc() + return result + +} diff --git a/server/events/metrics/common.go b/server/events/metrics/common.go new file mode 100644 index 0000000000..e32d59a4ce --- /dev/null +++ b/server/events/metrics/common.go @@ -0,0 +1,8 @@ +package metrics + +const ( + ExecutionTimeMetric = "execution_time" + ExecutionSuccessMetric = "execution_success" + ExecutionErrorMetric = "execution_error" + ExecutionFailureMetric = "execution_failure" +) diff --git a/server/events/models/models.go b/server/events/models/models.go index 04cf1b8a5a..3ad1912923 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -25,6 +25,7 @@ import ( "time" "github.com/hashicorp/go-version" + stats "github.com/lyft/gostats" "github.com/runatlantis/atlantis/server/logging" "github.com/pkg/errors" @@ -364,6 +365,8 @@ type ProjectCommandContext struct { HeadRepo Repo // Log is a logger that's been set up for this context. Log logging.SimpleLogging + // Scope is the scope for reporting stats setup for this context + Scope stats.Scope // PullMergeable is true if the pull request for this project is able to be merged. PullMergeable bool // CurrentProjectPlanStatus is the status of the current project prior to this command. @@ -402,6 +405,12 @@ type ProjectCommandContext struct { DeleteSourceBranchOnMerge bool } +// SetScope sets the scope of the stats object field. Note: we deliberately set this on the value +// instead of a pointer since we want scopes to mirror our function stack +func (p ProjectCommandContext) SetScope(scope string) { + p.Scope = p.Scope.Scope(scope) +} + // GetShowResultFileName returns the filename (not the path) to store the tf show result func (p ProjectCommandContext) GetShowResultFileName() string { if p.ProjectName == "" { diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index fc7c9b6f16..732ce61103 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -4,7 +4,9 @@ import ( "fmt" "os" + stats "github.com/lyft/gostats" "github.com/runatlantis/atlantis/server/events/yaml/valid" + "github.com/runatlantis/atlantis/server/logging" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/models" @@ -42,7 +44,9 @@ func NewProjectCommandBuilder( skipCloneNoChanges bool, EnableRegExpCmd bool, AutoplanFileList string, -) *DefaultProjectCommandBuilder { + scope stats.Scope, + logger logging.SimpleLogging, +) ProjectCommandBuilder { projectCommandBuilder := &DefaultProjectCommandBuilder{ ParserValidator: parserValidator, ProjectFinder: projectFinder, @@ -57,10 +61,14 @@ func NewProjectCommandBuilder( ProjectCommandContextBuilder: NewProjectCommandContextBulder( policyChecksSupported, commentBuilder, + scope, ), } - return projectCommandBuilder + return &InstrumentedProjectCommandBuilder{ + ProjectCommandBuilder: projectCommandBuilder, + Logger: logger, + } } type ProjectPlanCommandBuilder interface { diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 9d8891907e..8442032422 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -586,20 +586,21 @@ projects: Ok(t, ioutil.WriteFile(filepath.Join(tmp, "atlantis.yaml"), []byte(c.repoCfg), 0600)) } - builder := NewProjectCommandBuilder( - false, - parser, - &DefaultProjectFinder{}, - vcsClient, - workingDir, - NewDefaultWorkingDirLocker(), - globalCfg, - &DefaultPendingPlanFinder{}, - &CommentParser{}, - false, - false, - "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", - ) + builder := &DefaultProjectCommandBuilder{ + ParserValidator: &yaml.ParserValidator{}, + ProjectFinder: &DefaultProjectFinder{}, + VCSClient: vcsClient, + WorkingDir: workingDir, + WorkingDirLocker: NewDefaultWorkingDirLocker(), + GlobalCfg: globalCfg, + PendingPlanFinder: &DefaultPendingPlanFinder{}, + SkipCloneNoChanges: false, + ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ + CommentBuilder: &CommentParser{}, + }, + AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + EnableRegExpCmd: false, + } // We run a test for each type of command. for _, cmd := range []models.CommandName{models.PlanCommand, models.ApplyCommand} { @@ -773,20 +774,21 @@ projects: Ok(t, ioutil.WriteFile(filepath.Join(tmp, "atlantis.yaml"), []byte(c.repoCfg), 0600)) } - builder := NewProjectCommandBuilder( - false, - parser, - &DefaultProjectFinder{}, - vcsClient, - workingDir, - NewDefaultWorkingDirLocker(), - globalCfg, - &DefaultPendingPlanFinder{}, - &CommentParser{}, - false, - true, - "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", - ) + builder := &DefaultProjectCommandBuilder{ + ParserValidator: &yaml.ParserValidator{}, + ProjectFinder: &DefaultProjectFinder{}, + VCSClient: vcsClient, + WorkingDir: workingDir, + WorkingDirLocker: NewDefaultWorkingDirLocker(), + GlobalCfg: globalCfg, + PendingPlanFinder: &DefaultPendingPlanFinder{}, + SkipCloneNoChanges: true, + ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ + CommentBuilder: &CommentParser{}, + }, + AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + EnableRegExpCmd: true, + } // We run a test for each type of command, again specific projects for _, cmd := range []models.CommandName{models.PlanCommand, models.ApplyCommand} { @@ -986,20 +988,23 @@ workflows: Ok(t, ioutil.WriteFile(filepath.Join(tmp, "atlantis.yaml"), []byte(c.repoCfg), 0600)) } - builder := NewProjectCommandBuilder( - true, - parser, - &DefaultProjectFinder{}, - vcsClient, - workingDir, - NewDefaultWorkingDirLocker(), - globalCfg, - &DefaultPendingPlanFinder{}, - &CommentParser{}, - false, - false, - "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", - ) + builder := &DefaultProjectCommandBuilder{ + ParserValidator: &yaml.ParserValidator{}, + ProjectFinder: &DefaultProjectFinder{}, + VCSClient: vcsClient, + WorkingDir: workingDir, + WorkingDirLocker: NewDefaultWorkingDirLocker(), + GlobalCfg: globalCfg, + PendingPlanFinder: &DefaultPendingPlanFinder{}, + SkipCloneNoChanges: true, + ProjectCommandContextBuilder: &PolicyCheckProjectCommandContextBuilder{ + ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ + CommentBuilder: &CommentParser{}, + }, + CommentBuilder: &CommentParser{}, + }, + AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + } cmd := models.PolicyCheckCommand t.Run(cmd.String(), func(t *testing.T) { diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index c22c8df267..d7fb3751cf 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + stats "github.com/lyft/gostats" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/matchers" @@ -118,6 +119,7 @@ projects: } logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewLoggingSink(), false) for _, c := range cases { t.Run(c.Description, func(t *testing.T) { @@ -156,11 +158,14 @@ projects: false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ PullMergeable: true, Log: logger, + Scope: scope, }) Ok(t, err) Equals(t, len(c.exp), len(ctxs)) @@ -377,6 +382,7 @@ projects: } logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewNullSink(), false) for _, c := range cases { // NOTE: we're testing both plan and apply here. @@ -418,16 +424,19 @@ projects: false, true, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) var actCtxs []models.ProjectCommandContext var err error if cmdName == models.PlanCommand { actCtxs, err = builder.BuildPlanCommands(&events.CommandContext{ - Log: logger, + Log: logger, + Scope: scope, }, &c.Cmd) } else { - actCtxs, err = builder.BuildApplyCommands(&events.CommandContext{Log: logger}, &c.Cmd) + actCtxs, err = builder.BuildApplyCommands(&events.CommandContext{Log: logger, Scope: scope}, &c.Cmd) } if c.ExpErr != "" { @@ -533,6 +542,7 @@ projects: } logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewNullSink(), false) for name, c := range cases { t.Run(name, func(t *testing.T) { RegisterMockTestingT(t) @@ -569,11 +579,14 @@ projects: false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) ctxs, err := builder.BuildPlanCommands( &events.CommandContext{ - Log: logger, + Log: logger, + Scope: scope, }, &events.CommentCommand{ RepoRelDir: "", @@ -642,6 +655,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { ApprovedReq: false, UnDivergedReq: false, } + scope := stats.NewStore(stats.NewNullSink(), false) builder := events.NewProjectCommandBuilder( false, @@ -656,11 +670,14 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) ctxs, err := builder.BuildApplyCommands( &events.CommandContext{ - Log: logger, + Log: logger, + Scope: scope, }, &events.CommentCommand{ RepoRelDir: "", @@ -722,6 +739,8 @@ projects: ApprovedReq: false, UnDivergedReq: false, } + scope := stats.NewStore(stats.NewNullSink(), false) + logger := logging.NewNoopLogger(t) builder := events.NewProjectCommandBuilder( false, @@ -736,13 +755,16 @@ projects: false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) ctx := &events.CommandContext{ HeadRepo: models.Repo{}, Pull: models.PullRequest{}, User: models.User{}, - Log: logging.NewNoopLogger(t), + Log: logger, + Scope: scope, } _, err = builder.BuildPlanCommands(ctx, &events.CommentCommand{ RepoRelDir: ".", @@ -776,6 +798,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { } logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewNullSink(), false) for _, c := range cases { t.Run(strings.Join(c.ExtraArgs, " "), func(t *testing.T) { @@ -811,12 +834,15 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) var actCtxs []models.ProjectCommandContext var err error actCtxs, err = builder.BuildPlanCommands(&events.CommandContext{ - Log: logger, + Log: logger, + Scope: scope, }, &events.CommentCommand{ RepoRelDir: ".", Flags: c.ExtraArgs, @@ -947,6 +973,7 @@ projects: } logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewNullSink(), false) for name, testCase := range testCases { t.Run(name, func(t *testing.T) { @@ -990,11 +1017,14 @@ projects: false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) actCtxs, err := builder.BuildPlanCommands( &events.CommandContext{ - Log: logger, + Log: logger, + Scope: scope, }, &events.CommentCommand{ RepoRelDir: "", @@ -1039,6 +1069,7 @@ projects: ApprovedReq: false, UnDivergedReq: false, } + scope := stats.NewStore(stats.NewNullSink(), false) builder := events.NewProjectCommandBuilder( false, @@ -1053,6 +1084,8 @@ projects: true, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) var actCtxs []models.ProjectCommandContext @@ -1063,6 +1096,7 @@ projects: User: models.User{}, Log: logger, PullMergeable: true, + Scope: scope, }) Ok(t, err) Equals(t, 0, len(actCtxs)) @@ -1077,6 +1111,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman defer cleanup() logger := logging.NewNoopLogger(t) + scope := stats.NewStore(stats.NewNullSink(), false) workingDir := mocks.NewMockWorkingDir() When(workingDir.Clone(matchers.AnyPtrToLoggingSimpleLogger(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), AnyString())).ThenReturn(tmpDir, false, nil) @@ -1105,11 +1140,14 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman false, false, "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", + scope, + logger, ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ PullMergeable: true, Log: logger, + Scope: scope, }) Ok(t, err) diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 8b352a8a0a..ffd8c86839 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -6,11 +6,12 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-config-inspect/tfconfig" + stats "github.com/lyft/gostats" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" ) -func NewProjectCommandContextBulder(policyCheckEnabled bool, commentBuilder CommentBuilder) ProjectCommandContextBuilder { +func NewProjectCommandContextBulder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope stats.Scope) ProjectCommandContextBuilder { projectCommandContextBuilder := &DefaultProjectCommandContextBuilder{ CommentBuilder: commentBuilder, } @@ -22,7 +23,10 @@ func NewProjectCommandContextBulder(policyCheckEnabled bool, commentBuilder Comm } } - return projectCommandContextBuilder + return &CommandScopedStatsProjectCommandContextBuilder{ + ProjectCommandContextBuilder: projectCommandContextBuilder, + ProjectCounter: scope.NewCounter("projects"), + } } type ProjectCommandContextBuilder interface { @@ -37,6 +41,43 @@ type ProjectCommandContextBuilder interface { ) []models.ProjectCommandContext } +// CommandScopedStatsProjectCommandContextBuilder ensures that project command context contains a scoped stats +// object relevant to the command it applies to. +type CommandScopedStatsProjectCommandContextBuilder struct { + ProjectCommandContextBuilder + // Conciously making this global since it gets flushed periodically anyways + ProjectCounter stats.Counter +} + +// BuildProjectContext builds the context and injects the appropriate command level scope after the fact. +func (cb *CommandScopedStatsProjectCommandContextBuilder) BuildProjectContext( + ctx *CommandContext, + cmdName models.CommandName, + prjCfg valid.MergedProjectCfg, + commentFlags []string, + repoDir string, + automerge, deleteSourceBranchOnMerge, parallelApply, parallelPlan, verbose bool, +) (projectCmds []models.ProjectCommandContext) { + cb.ProjectCounter.Inc() + + cmds := cb.ProjectCommandContextBuilder.BuildProjectContext( + ctx, cmdName, prjCfg, commentFlags, repoDir, automerge, deleteSourceBranchOnMerge, parallelApply, parallelPlan, verbose, + ) + + projectCmds = []models.ProjectCommandContext{} + + for _, cmd := range cmds { + + // specifically use the command name in the context instead of the arg + // since we can return multiple commands worth of contexts for a given command name arg + // to effectively pipeline them. + cmd.SetScope(cmd.CommandName.String()) + projectCmds = append(projectCmds, cmd) + } + + return +} + type DefaultProjectCommandContextBuilder struct { CommentBuilder CommentBuilder } @@ -79,6 +120,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( parallelApply, parallelPlan, verbose, + ctx.Scope, )) return @@ -136,6 +178,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( parallelApply, parallelPlan, verbose, + ctx.Scope, )) } @@ -157,6 +200,7 @@ func newProjectCommandContext(ctx *CommandContext, parallelApplyEnabled bool, parallelPlanEnabled bool, verbose bool, + scope stats.Scope, ) models.ProjectCommandContext { var projectPlanStatus models.ProjectPlanStatus @@ -190,6 +234,7 @@ func newProjectCommandContext(ctx *CommandContext, Steps: steps, HeadRepo: ctx.HeadRepo, Log: ctx.Log, + Scope: scope, PullMergeable: ctx.PullMergeable, ProjectPlanStatus: projectPlanStatus, Pull: ctx.Pull, diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index e5751cad9f..11a9ed6342 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -326,8 +326,7 @@ func (g *GithubClient) getSubmitQueueMergeability(repo models.Repo, pull models. return ownersCheckApplied, nil } -// GetPullRequest returns the pull request. -func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRequest, error) { +func (g *GithubClient) GetPullRequestFromName(repoName string, repoOwner string, num int) (*github.PullRequest, error) { var err error var pull *github.PullRequest @@ -337,7 +336,7 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe numRetries := 3 retryDelay := 1 * time.Second for i := 0; i < numRetries; i++ { - pull, _, err = g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num) + pull, _, err = g.client.PullRequests.Get(g.ctx, repoOwner, repoName, num) if err == nil { return pull, nil } @@ -350,6 +349,11 @@ func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRe return pull, err } +// GetPullRequest returns the pull request. +func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRequest, error) { + return g.GetPullRequestFromName(repo.Name, repo.Owner, num) +} + func (g *GithubClient) getRepoStatuses(repo models.Repo, pull models.PullRequest) ([]*github.RepoStatus, error) { // Get Combined statuses diff --git a/server/events/vcs/instrumented_client.go b/server/events/vcs/instrumented_client.go new file mode 100644 index 0000000000..f13a832059 --- /dev/null +++ b/server/events/vcs/instrumented_client.go @@ -0,0 +1,242 @@ +package vcs + +import ( + "fmt" + "strconv" + + "github.com/google/go-github/v31/github" + stats "github.com/lyft/gostats" + "github.com/runatlantis/atlantis/server/events/metrics" + "github.com/runatlantis/atlantis/server/events/models" + "github.com/runatlantis/atlantis/server/logging" +) + +// NewInstrumentedGithubClient creates a client proxy responsible for gathering stats and logging +func NewInstrumentedGithubClient(client *GithubClient, statsScope stats.Scope, logger logging.SimpleLogging) IGithubClient { + scope := statsScope.Scope("github") + + instrumentedGHClient := &InstrumentedClient{ + Client: client, + StatsScope: scope, + Logger: logger, + } + + return &InstrumentedGithubClient{ + InstrumentedClient: instrumentedGHClient, + PullRequestGetter: client, + StatsScope: scope, + Logger: logger, + } +} + +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_github_pull_request_getter.go GithubPullRequestGetter + +type GithubPullRequestGetter interface { + GetPullRequest(repo models.Repo, pullNum int) (*github.PullRequest, error) + GetPullRequestFromName(repoName string, repoOwner string, pullNum int) (*github.PullRequest, error) +} + +// IGithubClient exists to bridge the gap between GithubPullRequestGetter and Client interface to allow +// for a single instrumented client +type IGithubClient interface { + Client + GithubPullRequestGetter +} + +// InstrumentedGithubClient should delegate to the underlying InstrumentedClient for vcs provider-agnostic +// methods and implement soley any github specific interfaces. +type InstrumentedGithubClient struct { + *InstrumentedClient + PullRequestGetter GithubPullRequestGetter + StatsScope stats.Scope + Logger logging.SimpleLogging +} + +func (c *InstrumentedGithubClient) GetPullRequest(repo models.Repo, pullNum int) (*github.PullRequest, error) { + return c.GetPullRequestFromName(repo.Name, repo.Owner, pullNum) + +} + +func (c *InstrumentedGithubClient) GetPullRequestFromName(repoName string, repoOwner string, pullNum int) (*github.PullRequest, error) { + scope := c.StatsScope.Scope("get_pull_request") + logger := c.Logger.WithHistory([]interface{}{ + "repository", fmt.Sprintf("%s/%s", repoOwner, repoName), + "pull-num", strconv.Itoa(pullNum), + }...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + pull, err := c.PullRequestGetter.GetPullRequestFromName(repoName, repoOwner, pullNum) + + if err != nil { + executionError.Inc() + logger.Err("Unable to get pull number for repo, error: %s", err.Error()) + } else { + executionSuccess.Inc() + } + + return pull, err +} + +type InstrumentedClient struct { + Client + StatsScope stats.Scope + Logger logging.SimpleLogging +} + +func (c *InstrumentedClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) { + scope := c.StatsScope.Scope("get_modified_files") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pull.Num)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + files, err := c.Client.GetModifiedFiles(repo, pull) + + if err != nil { + executionError.Inc() + logger.Err("Unable to get modified files, error: %s", err.Error()) + } else { + executionSuccess.Inc() + } + + return files, err + +} +func (c *InstrumentedClient) CreateComment(repo models.Repo, pullNum int, comment string, command string) error { + scope := c.StatsScope.Scope("create_comment") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pullNum)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + if err := c.Client.CreateComment(repo, pullNum, comment, command); err != nil { + executionError.Inc() + logger.Err("Unable to create comment for command %s, error: %s", command, err.Error()) + return err + } + + executionSuccess.Inc() + return nil +} +func (c *InstrumentedClient) HidePrevCommandComments(repo models.Repo, pullNum int, command string) error { + scope := c.StatsScope.Scope("hide_prev_plan_comments") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pullNum)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + if err := c.Client.HidePrevCommandComments(repo, pullNum, command); err != nil { + executionError.Inc() + logger.Err("Unable to hide previous %s comments, error: %s", command, err.Error()) + return err + } + + executionSuccess.Inc() + return nil + +} +func (c *InstrumentedClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) { + scope := c.StatsScope.Scope("pull_is_approved") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pull.Num)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + approved, err := c.Client.PullIsApproved(repo, pull) + + if err != nil { + executionError.Inc() + logger.Err("Unable to check pull approval status, error: %s", err.Error()) + } else { + executionSuccess.Inc() + } + + return approved, err + +} +func (c *InstrumentedClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + scope := c.StatsScope.Scope("pull_is_mergeable") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pull.Num)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + mergeable, err := c.Client.PullIsMergeable(repo, pull) + + if err != nil { + executionError.Inc() + logger.Err("Unable to check pull mergeable status, error: %s", err.Error()) + } else { + executionSuccess.Inc() + } + + return mergeable, err +} + +func (c *InstrumentedClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error { + scope := c.StatsScope.Scope("update_status") + logger := c.Logger.WithHistory(fmtLogSrc(repo, pull.Num)...) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + if err := c.Client.UpdateStatus(repo, pull, state, src, description, url); err != nil { + executionError.Inc() + logger.Err("Unable to update status at url: %s, error: %s", url, err.Error()) + return err + } + + executionSuccess.Inc() + return nil + +} +func (c *InstrumentedClient) MergePull(pull models.PullRequest, pullOptions models.PullRequestOptions) error { + scope := c.StatsScope.Scope("merge_pull") + logger := c.Logger.WithHistory("pull-num", pull.Num) + + executionTime := scope.NewTimer(metrics.ExecutionTimeMetric).AllocateSpan() + defer executionTime.Complete() + + executionSuccess := scope.NewCounter(metrics.ExecutionSuccessMetric) + executionError := scope.NewCounter(metrics.ExecutionErrorMetric) + + if err := c.Client.MergePull(pull, pullOptions); err != nil { + executionError.Inc() + logger.Err("Unable to merge pull, error: %s", err.Error()) + } + + executionSuccess.Inc() + return nil + +} + +// taken from other parts of the code, would be great to have this in a shared spot +func fmtLogSrc(repo models.Repo, pullNum int) []interface{} { + return []interface{}{ + "repository", repo.FullName, + "pull-num", strconv.Itoa(pullNum), + } +} diff --git a/server/server.go b/server/server.go index e36adbbd8c..2bee0b5a0d 100644 --- a/server/server.go +++ b/server/server.go @@ -136,11 +136,11 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { return nil, err } - statsScope := stats.NewDefaultStore().Scope("atlantis") + statsScope := stats.NewDefaultStore().Scope(userConfig.StatsNamespace) statsScope.Store().AddStatGenerator(stats.NewRuntimeStats(statsScope.Scope("go"))) var supportedVCSHosts []models.VCSHostType - var githubClient *vcs.GithubClient + var githubClient vcs.IGithubClient var githubAppEnabled bool var githubCredentials vcs.GithubCredentials var gitlabClient *vcs.GitlabClient @@ -172,10 +172,12 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } var err error - githubClient, err = vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) + rawGithubClient, err := vcs.NewGithubClient(userConfig.GithubHostname, githubCredentials, logger) if err != nil { return nil, err } + + githubClient = vcs.NewInstrumentedGithubClient(rawGithubClient, statsScope, logger) } if userConfig.GitlabUser != "" { supportedVCSHosts = append(supportedVCSHosts, models.Gitlab) @@ -437,6 +439,8 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { userConfig.SkipCloneNoChanges, userConfig.EnableRegExpCmd, userConfig.AutoplanFileList, + statsScope, + logger, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) @@ -499,11 +503,15 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { GlobalAutomerge: userConfig.Automerge, } + instrumentedProjectCmdRunner := &events.InstrumentedProjectCommandRunner{ + ProjectCommandRunner: projectCommandRunner, + } + policyCheckCommandRunner := events.NewPolicyCheckCommandRunner( dbUpdater, pullUpdater, commitStatusUpdater, - projectCommandRunner, + instrumentedProjectCmdRunner, userConfig.ParallelPoolSize, userConfig.SilenceVCSStatusNoProjects, ) @@ -516,7 +524,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { workingDir, commitStatusUpdater, projectCommandBuilder, - projectCommandRunner, + instrumentedProjectCmdRunner, dbUpdater, pullUpdater, policyCheckCommandRunner, @@ -532,7 +540,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { applyLockingClient, commitStatusUpdater, projectCommandBuilder, - projectCommandRunner, + instrumentedProjectCmdRunner, autoMerger, pullUpdater, dbUpdater, @@ -545,7 +553,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { approvePoliciesCommandRunner := events.NewApprovePoliciesCommandRunner( commitStatusUpdater, projectCommandBuilder, - projectCommandRunner, + instrumentedProjectCmdRunner, pullUpdater, dbUpdater, userConfig.SilenceNoProjects, @@ -573,6 +581,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { CommentCommandRunnerByCmd: commentCommandRunnerByCmd, EventParser: eventParser, Logger: logger, + StatsScope: statsScope.Scope("cmd"), AllowForkPRs: userConfig.AllowForkPRs, AllowForkPRsFlag: config.AllowForkPRsFlag, SilenceForkPRErrors: userConfig.SilenceForkPRErrors, diff --git a/server/user_config.go b/server/user_config.go index 5354203fe7..c6cdca24b7 100644 --- a/server/user_config.go +++ b/server/user_config.go @@ -45,6 +45,7 @@ type UserConfig struct { HidePrevPlanComments bool `mapstructure:"hide-prev-plan-comments"` LogLevel string `mapstructure:"log-level"` ParallelPoolSize int `mapstructure:"parallel-pool-size"` + StatsNamespace string `mapstructure:"stats-namespace"` PlanDrafts bool `mapstructure:"allow-draft-prs"` Port int `mapstructure:"port"` RepoConfig string `mapstructure:"repo-config"`