Skip to content

Commit

Permalink
Remove Legacy Logger (#249)
Browse files Browse the repository at this point in the history
* move-to-structuredlogger

* pass context through request
  • Loading branch information
samrabelachew authored May 13, 2022
1 parent a8f4853 commit 481c3cd
Show file tree
Hide file tree
Showing 143 changed files with 1,075 additions and 791 deletions.
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd
github.com/antlr/antlr4 v0.0.0-20201206235148-c87e55b61113 h1:+Je12tQpLUUQEfMUrLkTPXe1wh8VXCPjFsdwY29co30=
github.com/antlr/antlr4 v0.0.0-20201206235148-c87e55b61113/go.mod h1:T7PbCXFs94rrTttyxjbyT5+/1V8T2TYDejxUfHJjw1Y=
github.com/apparentlymart/go-dump v0.0.0-20180507223929-23540a00eaa3/go.mod h1:oL81AME2rN47vu18xqj1S1jPIPuN7afo62yKTNn3XMM=
github.com/apparentlymart/go-textseg v1.0.0 h1:rRmlIsPEEhUTIKQb7T++Nz/A5Q6C9IuX2wFoYVvnCs0=
github.com/apparentlymart/go-textseg v1.0.0/go.mod h1:z96Txxhf3xSFMPmb5X/1W05FF/Nj9VFpLOpjS5yuumk=
github.com/apparentlymart/go-textseg/v12 v12.0.0 h1:bNEQyAGak9tojivJNkoqWErVCQbjdL7GzRt3F8NvfJ0=
github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec=
Expand Down
2 changes: 0 additions & 2 deletions server/controllers/events/events_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func NewRequestResolvers(
}

func NewVCSEventsController(
legacyLogger logging.SimpleLogging,
scope tally.Scope,
githubWebhookSecret []byte,
allowDraftPRs bool,
Expand Down Expand Up @@ -155,7 +154,6 @@ func NewVCSEventsController(

return &VCSEventsController{
RequestRouter: router,
Logger: legacyLogger,
Scope: scope,
Parser: eventParser,
CommentParser: commentParser,
Expand Down
13 changes: 6 additions & 7 deletions server/controllers/events/events_controller_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig

logger := logging.NewNoopLogger(t)
ctxLogger := logging.NewNoopCtxLogger(t)
featureAllocator, _ := feature.NewStringSourcedAllocator(logger)
featureAllocator, _ := feature.NewStringSourcedAllocator(ctxLogger)
terraformClient, err := terraform.NewE2ETestClient(logger, binDir, cacheDir, "", "", "", "default-tf-version", "https://releases.hashicorp.com", downloader, false, projectCmdOutputHandler, featureAllocator)
Ok(t, err)

Expand Down Expand Up @@ -715,7 +715,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
WorkingDir: workingDir,
PreWorkflowHookRunner: runtime.DefaultPreWorkflowHookRunner{},
}
statsScope, _, err := metrics.NewLoggingScope(logger, "atlantis")
statsScope, _, err := metrics.NewLoggingScope(ctxLogger, "atlantis")
Ok(t, err)

projectContextBuilder := wrappers.
Expand Down Expand Up @@ -743,7 +743,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
&events.DefaultPendingPlanFinder{},
false,
"**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl",
logger,
ctxLogger,
events.InfiniteProjectsPerPR,
)

Expand All @@ -753,7 +753,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
conftestVersion, err := version.NewVersion(ConftestVersion)
Ok(t, err)

conftextExec := policy.NewConfTestExecutorWorkflow(logger, binDir, downloader)
conftextExec := policy.NewConfTestExecutorWorkflow(ctxLogger, binDir, downloader)

// swapping out version cache to something that always returns local contest
// binary
Expand Down Expand Up @@ -837,7 +837,7 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig

deleteLockCommand := &events.DefaultDeleteLockCommand{
Locker: lockingClient,
Logger: logger,
Logger: ctxLogger,
WorkingDir: workingDir,
WorkingDirLocker: locker,
DB: boltdb,
Expand Down Expand Up @@ -921,7 +921,6 @@ func setupE2E(t *testing.T, repoFixtureDir string, userConfig *server.UserConfig
PullStatusFetcher: boltdb,
StaleCommandChecker: staleCommandChecker,
Logger: ctxLogger,
LegacyLogger: logger,
}

repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*")
Expand Down Expand Up @@ -1275,7 +1274,7 @@ func (m *testLockURLGenerator) GenerateLockURL(lockID string) string {

type testWebhookSender struct{}

func (w *testWebhookSender) Send(log logging.SimpleLogging, result webhooks.ApplyResult) error {
func (w *testWebhookSender) Send(log logging.Logger, result webhooks.ApplyResult) error {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions server/controllers/events/events_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func TestPost_BBServerPullClosed(t *testing.T) {
allowlist, err := events.NewRepoAllowlistChecker("*")
Ok(t, err)
logger := logging.NewNoopLogger(t)
scope, _, _ := metrics.NewLoggingScope(logger, "null")
scope, _, _ := metrics.NewLoggingScope(logging.NewNoopCtxLogger(t), "null")
ec := &events_controllers.VCSEventsController{
Parser: &events.EventParser{
BitbucketUser: "bb-user",
Expand Down Expand Up @@ -303,7 +303,7 @@ func setup(t *testing.T) (events_controllers.VCSEventsController, *mocks.MockGit
repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*")
Ok(t, err)
logger := logging.NewNoopLogger(t)
scope, _, _ := metrics.NewLoggingScope(logger, "null")
scope, _, _ := metrics.NewLoggingScope(logging.NewNoopCtxLogger(t), "null")
e := events_controllers.VCSEventsController{

Logger: logger,
Expand Down
22 changes: 11 additions & 11 deletions server/controllers/github_app_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// GithubAppController handles the creation and setup of a new GitHub app
type GithubAppController struct {
AtlantisURL *url.URL
Logger logging.SimpleLogging
Logger logging.Logger
GithubSetupComplete bool
GithubHostname string
GithubOrg string
Expand Down Expand Up @@ -45,13 +45,13 @@ type githubAppRequest struct {
func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Request) {

if g.GithubSetupComplete {
g.respond(w, logging.Error, http.StatusBadRequest, "Atlantis already has GitHub credentials")
g.respond(w, http.StatusBadRequest, "Atlantis already has GitHub credentials")
return
}

code := r.URL.Query().Get("code")
if code == "" {
g.respond(w, logging.Debug, http.StatusOK, "Ignoring callback, missing code query parameter")
g.respond(w, http.StatusOK, "Ignoring callback, missing code query parameter")
}

creds := &vcs.GithubAnonymousCredentials{}
Expand All @@ -61,13 +61,13 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques
client, err := vcs.NewGithubClient(g.GithubHostname, creds, g.Logger, mergeabilityChecker)

if err != nil {
g.respond(w, logging.Error, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
g.respond(w, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
return
}

app, err := client.ExchangeCode(code)
if err != nil {
g.respond(w, logging.Error, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
g.respond(w, http.StatusInternalServerError, "Failed to exchange code for github app: %s", err)
return
}

Expand All @@ -80,15 +80,15 @@ func (g *GithubAppController) ExchangeCode(w http.ResponseWriter, r *http.Reques
URL: app.URL,
})
if err != nil {
g.Logger.Errorf(err.Error())
g.Logger.Error(err.Error())
}
}

// New redirects the user to create a new GitHub app
func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {

if g.GithubSetupComplete {
g.respond(w, logging.Error, http.StatusBadRequest, "Atlantis already has GitHub credentials")
g.respond(w, http.StatusBadRequest, "Atlantis already has GitHub credentials")
return
}

Expand Down Expand Up @@ -136,7 +136,7 @@ func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {

jsonManifest, err := json.MarshalIndent(manifest, "", " ")
if err != nil {
g.respond(w, logging.Error, http.StatusBadRequest, "Failed to serialize manifest: %s", err)
g.respond(w, http.StatusBadRequest, "Failed to serialize manifest: %s", err)
return
}

Expand All @@ -145,13 +145,13 @@ func (g *GithubAppController) New(w http.ResponseWriter, r *http.Request) {
Manifest: string(jsonManifest),
})
if err != nil {
g.Logger.Errorf(err.Error())
g.Logger.Error(err.Error())
}
}

func (g *GithubAppController) respond(w http.ResponseWriter, lvl logging.LogLevel, code int, format string, args ...interface{}) {
func (g *GithubAppController) respond(w http.ResponseWriter, code int, format string, args ...interface{}) {
response := fmt.Sprintf(format, args...)
g.Logger.Log(lvl, response)
g.Logger.Error(response)
w.WriteHeader(code)
fmt.Fprintln(w, response)
}
12 changes: 6 additions & 6 deletions server/controllers/jobs_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (g JobIDKeyGenerator) Generate(r *http.Request) (string, error) {
type JobsController struct {
AtlantisVersion string
AtlantisURL *url.URL
Logger logging.SimpleLogging
Logger logging.Logger
ProjectJobsTemplate templates.TemplateWriter
ProjectJobsErrorTemplate templates.TemplateWriter
Db *db.BoltDB
Expand All @@ -41,7 +41,7 @@ func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request)
jobID, err := j.KeyGenerator.Generate(r)

if err != nil {
j.respond(w, logging.Error, http.StatusBadRequest, err.Error())
j.respond(w, http.StatusBadRequest, err.Error())
return err
}

Expand All @@ -52,7 +52,7 @@ func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request)
}

if err = j.ProjectJobsTemplate.Execute(w, viewData); err != nil {
j.Logger.Errorf(err.Error())
j.Logger.Error(err.Error())
return err
}

Expand All @@ -71,7 +71,7 @@ func (j *JobsController) getProjectJobsWS(w http.ResponseWriter, r *http.Request
err := j.WsMux.Handle(w, r)

if err != nil {
j.respond(w, logging.Error, http.StatusBadRequest, err.Error())
j.respond(w, http.StatusBadRequest, err.Error())
return err
}

Expand All @@ -90,9 +90,9 @@ func (j *JobsController) GetProjectJobsWS(w http.ResponseWriter, r *http.Request
}
}

func (j *JobsController) respond(w http.ResponseWriter, lvl logging.LogLevel, responseCode int, format string, args ...interface{}) {
func (j *JobsController) respond(w http.ResponseWriter, responseCode int, format string, args ...interface{}) {
response := fmt.Sprintf(format, args...)
j.Logger.Log(lvl, response)
j.Logger.Error(response)
w.WriteHeader(responseCode)
fmt.Fprintln(w, response)
}
25 changes: 18 additions & 7 deletions server/controllers/locks_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type LocksController struct {
AtlantisVersion string
AtlantisURL *url.URL
Locker locking.Locker
Logger logging.SimpleLogging
Logger logging.Logger
ApplyLocker locking.ApplyLocker
VCSClient vcs.Client
LockDetailTemplate templates.TemplateWriter
Expand Down Expand Up @@ -93,7 +93,7 @@ func (l *LocksController) GetLock(w http.ResponseWriter, r *http.Request) {

err = l.LockDetailTemplate.Execute(w, viewData)
if err != nil {
l.Logger.Errorf(err.Error())
l.Logger.Error(err.Error())
}
}

Expand Down Expand Up @@ -129,23 +129,23 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) {
if lock.Pull.BaseRepo != (models.Repo{}) {
unlock, err := l.WorkingDirLocker.TryLock(lock.Pull.BaseRepo.FullName, lock.Pull.Num, lock.Workspace)
if err != nil {
l.Logger.Errorf("unable to obtain working dir lock when trying to delete old plans: %s", err)
l.Logger.Error(fmt.Sprintf("unable to obtain working dir lock when trying to delete old plans: %s", err))
} else {
defer unlock()
// nolint: vetshadow
if err := l.WorkingDir.DeleteForWorkspace(lock.Pull.BaseRepo, lock.Pull, lock.Workspace); err != nil {
l.Logger.Errorf("unable to delete workspace: %s", err)
l.Logger.Error(fmt.Sprintf("unable to delete workspace: %s", err))
}
}
if err := l.DB.UpdateProjectStatus(lock.Pull, lock.Workspace, lock.Project.Path, models.DiscardedPlanStatus); err != nil {
l.Logger.Errorf("unable to update project status: %s", err)
l.Logger.Error(fmt.Sprintf("unable to update project status: %s", err))
}

// Once the lock has been deleted, comment back on the pull request.
comment := fmt.Sprintf("**Warning**: The plan for dir: `%s` workspace: `%s` was **discarded** via the Atlantis UI.\n\n"+
"To `apply` this plan you must run `plan` again.", lock.Project.Path, lock.Workspace)
if err = l.VCSClient.CreateComment(lock.Pull.BaseRepo, lock.Pull.Num, comment, ""); err != nil {
l.Logger.Warnf("failed commenting on pull request: %s", err)
l.Logger.Warn(fmt.Sprintf("failed commenting on pull request: %s", err))
}
}
l.respond(w, logging.Info, http.StatusOK, "Deleted lock id %q", id)
Expand All @@ -155,7 +155,18 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) {
// level to log at, code is the HTTP response code.
func (l *LocksController) respond(w http.ResponseWriter, lvl logging.LogLevel, responseCode int, format string, args ...interface{}) {
response := fmt.Sprintf(format, args...)
l.Logger.Log(lvl, response)
switch lvl {
case logging.Error:
l.Logger.Error(response)
case logging.Info:
l.Logger.Info(response)
case logging.Warn:
l.Logger.Warn(response)
case logging.Debug:
l.Logger.Debug(response)
default:
l.Logger.Error(response)
}
w.WriteHeader(responseCode)
fmt.Fprintln(w, response)
}
Loading

0 comments on commit 481c3cd

Please sign in to comment.