From 0b31ffa587898d14399c9363460f6146c649c5c7 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Sun, 18 Oct 2020 10:01:48 -0400 Subject: [PATCH] use a single log writer Use a single log writer instance for all std library logging. Setup the std log writer in the logging package, and remove boilerplate from test packages. --- backend/local/local_test.go | 13 +------- backend/remote-state/inmem/backend_test.go | 14 ++------ backend/remote/remote_test.go | 12 +------ command/command_test.go | 13 +------- dag/dag_test.go | 12 +------ dag/walk.go | 1 - internal/getproviders/http_mirror_source.go | 6 +--- internal/getproviders/registry_client.go | 7 +--- internal/initwd/module_install_test.go | 12 ++----- internal/logging/logging.go | 36 ++++++++------------- registry/client.go | 6 +--- repl/session_test.go | 14 ++------ states/statemgr/statemgr_test.go | 11 +------ terraform/context.go | 7 ++-- terraform/graph.go | 7 +++- terraform/terraform_test.go | 12 ++----- 16 files changed, 37 insertions(+), 146 deletions(-) diff --git a/backend/local/local_test.go b/backend/local/local_test.go index 852390237..e447921e0 100644 --- a/backend/local/local_test.go +++ b/backend/local/local_test.go @@ -2,24 +2,13 @@ package local import ( "flag" - "io/ioutil" - "log" "os" "testing" - "github.com/hashicorp/terraform/internal/logging" + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - os.Exit(m.Run()) } diff --git a/backend/remote-state/inmem/backend_test.go b/backend/remote-state/inmem/backend_test.go index ecaab0da7..218e706ff 100644 --- a/backend/remote-state/inmem/backend_test.go +++ b/backend/remote-state/inmem/backend_test.go @@ -2,30 +2,20 @@ package inmem import ( "flag" - "io/ioutil" - "log" "os" "testing" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/backend" - "github.com/hashicorp/terraform/internal/logging" statespkg "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/states/remote" + + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - os.Exit(m.Run()) } diff --git a/backend/remote/remote_test.go b/backend/remote/remote_test.go index c164cb815..dbd0a72d6 100644 --- a/backend/remote/remote_test.go +++ b/backend/remote/remote_test.go @@ -2,25 +2,15 @@ package remote import ( "flag" - "io/ioutil" - "log" "os" "testing" - "github.com/hashicorp/terraform/internal/logging" + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - // Make sure TF_FORCE_LOCAL_BACKEND is unset os.Unsetenv("TF_FORCE_LOCAL_BACKEND") diff --git a/command/command_test.go b/command/command_test.go index 8ef664ba9..e28ee6e1f 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -5,11 +5,9 @@ import ( "crypto/md5" "encoding/base64" "encoding/json" - "flag" "fmt" "io" "io/ioutil" - "log" "net/http" "net/http/httptest" "os" @@ -30,7 +28,6 @@ import ( "github.com/hashicorp/terraform/configs/configload" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/internal/copy" - "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/plans/planfile" "github.com/hashicorp/terraform/providers" @@ -44,6 +41,7 @@ import ( backendInit "github.com/hashicorp/terraform/backend/init" backendLocal "github.com/hashicorp/terraform/backend/local" + _ "github.com/hashicorp/terraform/internal/logging" ) // These are the directories for our test data and fixtures. @@ -83,15 +81,6 @@ func init() { func TestMain(m *testing.M) { defer os.RemoveAll(testingDir) - flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - // Make sure backend init is initialized, since our tests tend to assume it. backendInit.Init(nil) diff --git a/dag/dag_test.go b/dag/dag_test.go index 898c035b4..ae2c2387e 100644 --- a/dag/dag_test.go +++ b/dag/dag_test.go @@ -3,8 +3,6 @@ package dag import ( "flag" "fmt" - "io/ioutil" - "log" "os" "reflect" "strconv" @@ -14,19 +12,11 @@ import ( "github.com/hashicorp/terraform/tfdiags" - "github.com/hashicorp/terraform/internal/logging" + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - os.Exit(m.Run()) } diff --git a/dag/walk.go b/dag/walk.go index d7b202d71..f9fdf2dfc 100644 --- a/dag/walk.go +++ b/dag/walk.go @@ -383,7 +383,6 @@ func (w *Walker) walkVertex(v Vertex, info *walkerVertex) { var diags tfdiags.Diagnostics var upstreamFailed bool if depsSuccess { - log.Printf("[TRACE] dag/walk: visiting %q", VertexName(v)) diags = w.Callback(v) } else { log.Printf("[TRACE] dag/walk: upstream of %q errored, so skipping", VertexName(v)) diff --git a/internal/getproviders/http_mirror_source.go b/internal/getproviders/http_mirror_source.go index ca703b627..aaf707ee8 100644 --- a/internal/getproviders/http_mirror_source.go +++ b/internal/getproviders/http_mirror_source.go @@ -68,11 +68,7 @@ func newHTTPMirrorSourceWithHTTPClient(baseURL *url.URL, creds svcauth.Credentia retryableClient.RequestLogHook = requestLogHook retryableClient.ErrorHandler = maxRetryErrorHandler - logOutput, err := logging.LogOutput() - if err != nil { - log.Printf("[WARN] Failed to set up provider HTTP mirror logger, so continuing without client logging: %s", err) - } - retryableClient.Logger = log.New(logOutput, "", log.Flags()) + retryableClient.Logger = log.New(logging.LogOutput(), "", log.Flags()) return &HTTPMirrorSource{ baseURL: baseURL, diff --git a/internal/getproviders/registry_client.go b/internal/getproviders/registry_client.go index 1af060e56..befa74274 100644 --- a/internal/getproviders/registry_client.go +++ b/internal/getproviders/registry_client.go @@ -77,12 +77,7 @@ func newRegistryClient(baseURL *url.URL, creds svcauth.HostCredentials) *registr retryableClient.RequestLogHook = requestLogHook retryableClient.ErrorHandler = maxRetryErrorHandler - logOutput, err := logging.LogOutput() - if err != nil { - log.Printf("[WARN] Failed to set up registry client logger, "+ - "continuing without client logging: %s", err) - } - retryableClient.Logger = log.New(logOutput, "", log.Flags()) + retryableClient.Logger = log.New(logging.LogOutput(), "", log.Flags()) return ®istryClient{ baseURL: baseURL, diff --git a/internal/initwd/module_install_test.go b/internal/initwd/module_install_test.go index 9eba2fb79..2b7f18674 100644 --- a/internal/initwd/module_install_test.go +++ b/internal/initwd/module_install_test.go @@ -4,7 +4,6 @@ import ( "flag" "fmt" "io/ioutil" - "log" "os" "path/filepath" "strings" @@ -15,21 +14,14 @@ import ( "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/configs/configload" "github.com/hashicorp/terraform/internal/copy" - "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/tfdiags" + + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - os.Exit(m.Run()) } diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 0fd650d34..1ba199b92 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -25,16 +25,25 @@ var ValidLevels = []LogLevel{"TRACE", "DEBUG", "INFO", "WARN", "ERROR"} // logger is the global hclog logger var logger hclog.Logger +// logWriter is a global writer for logs, to be used with the std log package +var logWriter io.Writer + func init() { logger = NewHCLogger("") + logWriter = logger.StandardWriter(&hclog.StandardLoggerOptions{InferLevels: true}) + + // setup the default std library logger to use our output + log.SetFlags(0) + log.SetPrefix("") + log.SetOutput(logWriter) } -// LogOutput determines where we should send logs (if anywhere) and the log level. -func LogOutput() (logOutput io.Writer, err error) { - return logger.StandardWriter(&hclog.StandardLoggerOptions{InferLevels: true}), nil +// LogOutput return the default global log io.Writer +func LogOutput() io.Writer { + return logWriter } -// HCLogger returns the default global loggers +// HCLogger returns the default global hclog logger func HCLogger() hclog.Logger { return logger } @@ -63,25 +72,6 @@ func NewHCLogger(name string) hclog.Logger { }) } -// SetOutput checks for a log destination with LogOutput, and calls -// log.SetOutput with the result. If LogOutput returns nil, SetOutput uses -// ioutil.Discard. Any error from LogOutout is fatal. -func SetOutput() { - out, err := LogOutput() - if err != nil { - log.Fatal(err) - } - - if out == nil { - out = ioutil.Discard - } - - // the hclog logger will add the prefix info - log.SetFlags(0) - log.SetPrefix("") - log.SetOutput(out) -} - // CurrentLogLevel returns the current log level string based the environment vars func CurrentLogLevel() string { envLevel := strings.ToUpper(os.Getenv(EnvLog)) diff --git a/registry/client.go b/registry/client.go index b8372b48f..6770d7188 100644 --- a/registry/client.go +++ b/registry/client.go @@ -87,11 +87,7 @@ func NewClient(services *disco.Disco, client *http.Client) *Client { retryableClient.RequestLogHook = requestLogHook retryableClient.ErrorHandler = maxRetryErrorHandler - logOutput, err := logging.LogOutput() - if err != nil { - log.Printf("[WARN] Failed to set up registry client logger, "+ - "continuing without client logging: %s", err) - } + logOutput := logging.LogOutput() retryableClient.Logger = log.New(logOutput, "", log.Flags()) services.Transport = retryableClient.HTTPClient.Transport diff --git a/repl/session_test.go b/repl/session_test.go index 0ff59e828..4e721375c 100644 --- a/repl/session_test.go +++ b/repl/session_test.go @@ -2,8 +2,6 @@ package repl import ( "flag" - "io/ioutil" - "log" "os" "strings" "testing" @@ -13,23 +11,15 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs/configschema" "github.com/hashicorp/terraform/internal/initwd" - "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" "github.com/hashicorp/terraform/terraform" + + _ "github.com/hashicorp/terraform/internal/logging" ) func TestMain(m *testing.M) { flag.Parse() - - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - os.Exit(m.Run()) } diff --git a/states/statemgr/statemgr_test.go b/states/statemgr/statemgr_test.go index c22b6a6ba..41c73d1dc 100644 --- a/states/statemgr/statemgr_test.go +++ b/states/statemgr/statemgr_test.go @@ -4,13 +4,11 @@ import ( "context" "encoding/json" "flag" - "io/ioutil" - "log" "os" "testing" "time" - "github.com/hashicorp/terraform/internal/logging" + _ "github.com/hashicorp/terraform/internal/logging" ) func TestNewLockInfo(t *testing.T) { @@ -91,12 +89,5 @@ func TestLockWithContext(t *testing.T) { func TestMain(m *testing.M) { flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } os.Exit(m.Run()) } diff --git a/terraform/context.go b/terraform/context.go index 44d6ec894..e74a72ae4 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/instances" - "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/lang" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" @@ -19,11 +18,9 @@ import ( "github.com/hashicorp/terraform/states/statefile" "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" -) -func init() { - logging.SetOutput() -} + _ "github.com/hashicorp/terraform/internal/logging" +) // InputMode defines what sort of input will be asked for when Input // is called on Context. diff --git a/terraform/graph.go b/terraform/graph.go index 73996c1f7..c690b356b 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -2,6 +2,7 @@ package terraform import ( "log" + "strings" "github.com/hashicorp/terraform/tfdiags" @@ -77,7 +78,11 @@ func (g *Graph) walk(walker GraphWalker) tfdiags.Diagnostics { subDiags := g.walk(walker) diags = diags.Append(subDiags) if subDiags.HasErrors() { - log.Printf("[TRACE] vertex %q: dynamic subgraph encountered errors", dag.VertexName(v)) + var errs []string + for _, d := range subDiags { + errs = append(errs, d.Description().Summary) + } + log.Printf("[TRACE] vertex %q: dynamic subgraph encountered errors: %s", dag.VertexName(v), strings.Join(errs, ",")) return } log.Printf("[TRACE] vertex %q: dynamic subgraph completed successfully", dag.VertexName(v)) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 31e410000..96fbcdffe 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -4,7 +4,6 @@ import ( "flag" "io" "io/ioutil" - "log" "os" "path/filepath" "strings" @@ -20,12 +19,13 @@ import ( "github.com/hashicorp/terraform/configs/configload" "github.com/hashicorp/terraform/helper/experiment" "github.com/hashicorp/terraform/internal/initwd" - "github.com/hashicorp/terraform/internal/logging" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/provisioners" "github.com/hashicorp/terraform/registry" "github.com/hashicorp/terraform/states" + + _ "github.com/hashicorp/terraform/internal/logging" ) // This is the directory where our test fixtures are. @@ -39,14 +39,6 @@ func TestMain(m *testing.M) { experiment.Flag(flag.CommandLine) flag.Parse() - if testing.Verbose() { - // if we're verbose, use the logging requested by TF_LOG - logging.SetOutput() - } else { - // otherwise silence all logs - log.SetOutput(ioutil.Discard) - } - // Make sure shadow operations fail our real tests contextFailOnShadowError = true