logging: Fix console logger instantiation

This commit fixes the console loggers so that messages are emitted
regardless of the debug log level. The problem was that in 3fcc875 we
updated the plugins to use a console logger obtained from the plugin
manager as opposed to a global logger instantiated in the plugins
package--the console logger obtained from the plugin manager was
instantiated in the runtime package by calling
logging.NewStandardLogger. Unfortunately, logging.NewStandardLogger
does not create a new logger--it returns the global logrus
logger.

This commit fixes the issue by deprecating logging.NewStandardLogger
and introducing two new functions in the logging package:

* logging.Get() - this replaces the old logging.NewStandardLogger
  function--this function should be called to obtain the debug logger
  used throughout OPA.

* logging.New() - this actually returns a new logger that can be
  configured independently from the debug logger used throughout
  OPA.

The runtime and sdk packages have been updated to call logging.New()
to obtain console loggers and the rest of the codebase has been
updated to call logging.Get() in place of logging.NewStandardLogger().

Fixes #3654

Signed-off-by: Torin Sandall <[email protected]>

logging/logging.go

@@ -38,8 +38,20 @@ type StandardLogger struct {
 	fields map[string]interface{}
 }
 
-// NewStandardLogger instantiates new default OPA logger
+// New returns a new standard logger.
+func New() *StandardLogger {
+	return &StandardLogger{
+		logger: logrus.New(),
+	}
+}
+
+// NewStandardLogger is deprecated. Use Get instead.
 func NewStandardLogger() *StandardLogger {
+	return Get()
+}
+
+// Get returns the standard logger used throughout OPA.
+func Get() *StandardLogger {
 	return &StandardLogger{
 		logger: logrus.StandardLogger(),
 	}

plugins/bundle/plugin.go

@@ -603,7 +603,7 @@ func loadBundleFromDisk(path, name string, src *Source) (*bundle.Bundle, error)
 
 func (p *Plugin) log(name string) logging.Logger {
 	if p.logger == nil {
-		p.logger = logging.NewStandardLogger()
+		p.logger = logging.Get()
 	}
 	return p.logger.WithFields(map[string]interface{}{"name": name, "plugin": Name})
 }

plugins/plugins.go

@@ -289,11 +289,11 @@ func New(raw []byte, id string, store storage.Store, opts ...func(*Manager)) (*M
 	}
 
 	if m.logger == nil {
-		m.logger = logging.NewStandardLogger()
+		m.logger = logging.Get()
 	}
 
 	if m.consoleLogger == nil {
-		m.consoleLogger = logging.NewStandardLogger()
+		m.consoleLogger = logging.New()
 	}
 
 	serviceOpts := cfg.ServiceOptions{

plugins/plugins_test.go

@@ -301,7 +301,7 @@ func TestPluginManagerAuthPlugin(t *testing.T) {
 
 func TestPluginManagerLogger(t *testing.T) {
 
-	logger := logging.NewStandardLogger().WithFields(map[string]interface{}{"context": "myloggincontext"})
+	logger := logging.Get().WithFields(map[string]interface{}{"context": "myloggincontext"})
 
 	m, err := New([]byte(`{}`), "test", inmem.New(), Logger(logger))
 	if err != nil {

plugins/rest/aws_test.go

@@ -109,7 +109,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: "this is not a URL", // malformed
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	_, err := cs.credentials()
 	assertErr("unsupported protocol scheme \"\"", err, t)
@@ -118,7 +118,7 @@ func TestMetadataCredentialService(t *testing.T) {
 	os.Unsetenv(ecsRelativePathEnvVar)
 	cs = awsMetadataCredentialService{
 		RegionName: "us-east-1",
-		logger:     logging.NewStandardLogger(),
+		logger:     logging.Get(),
 	}
 	_, err = cs.credentials()
 	assertErr("metadata endpoint cannot be determined from settings and environment", err, t)
@@ -129,7 +129,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	_, err = cs.credentials()
 	assertErr("metadata HTTP request returned unexpected status: 404 Not Found", err, t)
@@ -140,7 +140,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	_, err = cs.credentials()
 	assertErr("failed to parse credential response from metadata service: invalid character 'T' looking for beginning of value", err, t)
@@ -151,7 +151,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/missing_token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	} // will 404
 	_, err = cs.credentials()
 	assertErr("metadata token HTTP request returned unexpected status: 404 Not Found", err, t)
@@ -162,7 +162,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/bad_token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	} // not good
 	_, err = cs.credentials()
 	assertErr("metadata HTTP request returned unexpected status: 401 Unauthorized", err, t)
@@ -179,7 +179,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	_, err = cs.credentials()
 	assertErr("metadata service query did not succeed: Failure", err, t)
@@ -196,7 +196,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	var creds awsCredentials
 	creds, err = cs.credentials()
@@ -230,7 +230,7 @@ func TestMetadataCredentialService(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -275,7 +275,7 @@ func TestV4Signing(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	req, _ := http.NewRequest("GET", "https://mybucket.s3.amazonaws.com/bundle.tar.gz", strings.NewReader(""))
 	err := signV4(req, "s3", cs, time.Unix(1556129697, 0))
@@ -288,7 +288,7 @@ func TestV4Signing(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -325,7 +325,7 @@ func TestV4SigningForApiGateway(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -365,7 +365,7 @@ func TestV4SigningOmitsIgnoredHeaders(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -408,7 +408,7 @@ func TestV4SigningCustomPort(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -445,7 +445,7 @@ func TestV4SigningDoesNotMutateBody(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -477,7 +477,7 @@ func TestV4SigningWithMultiValueHeaders(t *testing.T) {
 		RegionName:      "us-east-1",
 		credServicePath: ts.server.URL + "/latest/meta-data/iam/security-credentials/",
 		tokenPath:       ts.server.URL + "/latest/api/token",
-		logger:          logging.NewStandardLogger(),
+		logger:          logging.Get(),
 	}
 	ts.payload = metadataPayload{
 		AccessKeyID:     "MYAWSACCESSKEYGOESHERE",
@@ -569,7 +569,7 @@ func TestWebIdentityCredentialService(t *testing.T) {
 	defer ts.stop()
 	cs := awsWebIdentityCredentialService{
 		stsURL: ts.server.URL,
-		logger: logging.NewStandardLogger(),
+		logger: logging.Get(),
 	}
 
 	goodTokenFile, err := ioutil.TempFile(os.TempDir(), "opa-aws-test-")

plugins/rest/rest.go

@@ -165,7 +165,7 @@ func New(config []byte, keys map[string]*keys.Config, opts ...func(*Client)) (Cl
 	}
 
 	if client.logger == nil {
-		client.logger = logging.NewStandardLogger()
+		client.logger = logging.Get()
 	}
 	client.config.logger = client.logger
 

runtime/runtime.go

@@ -263,9 +263,9 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) {
 	var consoleLogger logging.Logger
 
 	if params.ConsoleLogger == nil {
-		stdLogger := logging.NewStandardLogger()
-		stdLogger.SetFormatter(getFormatter(params.Logging.Format))
-		consoleLogger = stdLogger
+		l := logging.New()
+		l.SetFormatter(getFormatter(params.Logging.Format))
+		consoleLogger = l
 	} else {
 		consoleLogger = params.ConsoleLogger
 	}

sdk/options.go

@@ -51,7 +51,7 @@ func (o *Options) init() error {
 	}
 
 	if o.ConsoleLogger == nil {
-		l := logging.NewStandardLogger()
+		l := logging.New()
 		l.SetFormatter(&logrus.JSONFormatter{})
 		o.ConsoleLogger = l
 	}