From 55dbd36f055da38edb45e66b179b66a311773d26 Mon Sep 17 00:00:00 2001 From: Salim Afiune Maya Date: Thu, 21 Nov 2024 09:08:27 -0800 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20new=20`logger/zerologad?= =?UTF-8?q?apter`=20for=20retryable=20requests=20(#4903)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We use the `retryablehttp` package to retry http requests and we always want the logs to go to `debug` level so, we need an adapter. We have implemented this adapter many times, this change removes all the duplicated code and creates a util package that all providers can use. Example: ```go retryClient := retryablehttp.NewClient() retryClient.Logger = zerologadapter.New(log.Logger) ``` * ↪️ move package to `logger/` --------- Signed-off-by: Salim Afiune Maya --- logger/zerologadapter/adapter.go | 57 +++++++++++++ .../zerologadapter/adapter_internal_test.go | 61 ++++++++++++++ logger/zerologadapter/adapter_test.go | 84 +++++++++++++++++++ providers/aws/connection/connection.go | 33 +------- providers/github/connection/connection.go | 32 +------ providers/providers.go | 36 +------- providers/slack/connection/connection.go | 33 +------- 7 files changed, 210 insertions(+), 126 deletions(-) create mode 100644 logger/zerologadapter/adapter.go create mode 100644 logger/zerologadapter/adapter_internal_test.go create mode 100644 logger/zerologadapter/adapter_test.go diff --git a/logger/zerologadapter/adapter.go b/logger/zerologadapter/adapter.go new file mode 100644 index 0000000000..2c0ec49823 --- /dev/null +++ b/logger/zerologadapter/adapter.go @@ -0,0 +1,57 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package zerologadapter + +import "github.com/rs/zerolog" + +// New returns a new adapter for the zerolog logger to the LeveledLogger interface. This struct is +// mainly used in conjunction with a retryable http client to convert all retry logs to debug logs. +// +// NOTE that all messages will go to debug level. +// +// e.g. +// ```go +// retryClient := retryablehttp.NewClient() +// retryClient.Logger = zerologadapter.New(log.Logger) +// ``` +func New(logger zerolog.Logger) *Adapter { + return &Adapter{logger} +} + +type Adapter struct { + logger zerolog.Logger +} + +func (z *Adapter) Msg(msg string, keysAndValues ...interface{}) { + z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) +} + +func (z *Adapter) Error(msg string, keysAndValues ...interface{}) { + z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) +} + +func (z *Adapter) Info(msg string, keysAndValues ...interface{}) { + z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) +} + +func (z *Adapter) Debug(msg string, keysAndValues ...interface{}) { + z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) +} + +func (z *Adapter) Warn(msg string, keysAndValues ...interface{}) { + z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) +} + +func convertToFields(keysAndValues ...interface{}) map[string]interface{} { + fields := make(map[string]interface{}) + for i := 0; i < len(keysAndValues); i += 2 { + if i+1 < len(keysAndValues) { + keyString, ok := keysAndValues[i].(string) + if ok { // safety first, eventhough we always expect a string + fields[keyString] = keysAndValues[i+1] + } + } + } + return fields +} diff --git a/logger/zerologadapter/adapter_internal_test.go b/logger/zerologadapter/adapter_internal_test.go new file mode 100644 index 0000000000..7821717023 --- /dev/null +++ b/logger/zerologadapter/adapter_internal_test.go @@ -0,0 +1,61 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package zerologadapter + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConvertToFields(t *testing.T) { + t.Run("Valid key-value pairs", func(t *testing.T) { + input := []interface{}{"key1", "value1", "key2", 42, "key3", true} + expected := map[string]interface{}{ + "key1": "value1", + "key2": 42, + "key3": true, + } + + result := convertToFields(input...) + assert.Equal(t, expected, result) + }) + + t.Run("Odd number of elements", func(t *testing.T) { + input := []interface{}{"key1", "value1", "key2"} + expected := map[string]interface{}{ + "key1": "value1", + } + + result := convertToFields(input...) + assert.Equal(t, expected, result) + }) + + t.Run("Non-string keys are ignored", func(t *testing.T) { + input := []interface{}{123, "value1", "key2", 42, 3.14, "value3", "key3", true} + expected := map[string]interface{}{ + "key2": 42, + "key3": true, + } + + result := convertToFields(input...) + assert.Equal(t, expected, result) + }) + + t.Run("Empty input", func(t *testing.T) { + input := []interface{}{} + expected := map[string]interface{}{} + + result := convertToFields(input...) + assert.Equal(t, expected, result) + }) + + t.Run("Nil input", func(t *testing.T) { + var input []interface{} + expected := map[string]interface{}{} + + result := convertToFields(input...) + assert.Equal(t, expected, result) + }) +} diff --git a/logger/zerologadapter/adapter_test.go b/logger/zerologadapter/adapter_test.go new file mode 100644 index 0000000000..e550a3c24c --- /dev/null +++ b/logger/zerologadapter/adapter_test.go @@ -0,0 +1,84 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package zerologadapter_test + +import ( + "bytes" + "testing" + + subject "go.mondoo.com/cnquery/v11/logger/zerologadapter" + + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +func TestNewAdapter(t *testing.T) { + var logOutput bytes.Buffer + logger := zerolog.New(&logOutput).Level(zerolog.DebugLevel) + adapter := subject.New(logger) + + t.Run("Msg method logs correctly", func(t *testing.T) { + logOutput.Reset() + adapter.Msg("Test message", "key1", "value1", "key2", 42) + + expectedLog := `{"level":"debug","key1":"value1","key2":42,"message":"Test message"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Error method logs correctly", func(t *testing.T) { + logOutput.Reset() + adapter.Error("Error occurred", "error_code", 500) + + expectedLog := `{"level":"debug","error_code":500,"message":"Error occurred"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Info method logs correctly", func(t *testing.T) { + logOutput.Reset() + adapter.Info("Info message", "key", "value") + + expectedLog := `{"level":"debug","key":"value","message":"Info message"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Debug method logs correctly", func(t *testing.T) { + logOutput.Reset() + adapter.Debug("Debugging issue", "context", "test") + + expectedLog := `{"level":"debug","context":"test","message":"Debugging issue"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Warn method logs correctly", func(t *testing.T) { + logOutput.Reset() + adapter.Warn("Warning issued", "warning_level", "high") + + expectedLog := `{"level":"debug","warning_level":"high","message":"Warning issued"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Handles non-string keys gracefully", func(t *testing.T) { + logOutput.Reset() + adapter.Info("Non-string key test", 123, "value", "key2", 42) + + expectedLog := `{"level":"debug","key2":42,"message":"Non-string key test"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Handles odd number of key-value pairs gracefully", func(t *testing.T) { + logOutput.Reset() + adapter.Debug("Odd number test", "key1", "value1", "key2") + + expectedLog := `{"level":"debug","key1":"value1","message":"Odd number test"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) + + t.Run("Empty key-value pairs", func(t *testing.T) { + logOutput.Reset() + adapter.Warn("Empty key-value test") + + expectedLog := `{"level":"debug","message":"Empty key-value test"}` + assert.JSONEq(t, expectedLog, logOutput.String()) + }) +} diff --git a/providers/aws/connection/connection.go b/providers/aws/connection/connection.go index d38149d9cb..583cbbd8d7 100644 --- a/providers/aws/connection/connection.go +++ b/providers/aws/connection/connection.go @@ -17,9 +17,9 @@ import ( "github.com/aws/aws-sdk-go-v2/service/ec2" "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/hashicorp/go-retryablehttp" - "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/afero" + "go.mondoo.com/cnquery/v11/logger/zerologadapter" "go.mondoo.com/cnquery/v11/providers-sdk/v1/inventory" "go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin" "go.mondoo.com/cnquery/v11/providers-sdk/v1/vault" @@ -102,7 +102,7 @@ func NewAwsConnection(id uint32, asset *inventory.Asset, conf *inventory.Config) // custom retry client retryClient := retryablehttp.NewClient() retryClient.RetryMax = 5 - retryClient.Logger = &zeroLogAdapter{} + retryClient.Logger = zerologadapter.New(log.Logger) c.awsConfigOptions = append(c.awsConfigOptions, config.WithHTTPClient(retryClient.StandardClient())) cfg, err := config.LoadDefaultConfig(context.Background(), c.awsConfigOptions...) @@ -398,32 +398,3 @@ func (h *AwsConnection) Regions() ([]string, error) { h.clientcache.Store("_regions", &CacheEntry{Data: regions}) return regions, nil } - -// zeroLogAdapter is the adapter for retryablehttp is outputting debug messages -type zeroLogAdapter struct{} - -func (l *zeroLogAdapter) Msg(msg string, keysAndValues ...interface{}) { - var e *zerolog.Event - // retry messages should only go to debug - e = log.Debug() - for i := 0; i < len(keysAndValues); i += 2 { - e = e.Interface(keysAndValues[i].(string), keysAndValues[i+1]) - } - e.Msg(msg) -} - -func (l *zeroLogAdapter) Error(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Info(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Debug(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Warn(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} diff --git a/providers/github/connection/connection.go b/providers/github/connection/connection.go index 4285808d74..5a71e6af1b 100644 --- a/providers/github/connection/connection.go +++ b/providers/github/connection/connection.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/go-retryablehttp" "github.com/rs/zerolog" "github.com/rs/zerolog/log" + "go.mondoo.com/cnquery/v11/logger/zerologadapter" "go.mondoo.com/cnquery/v11/providers-sdk/v1/inventory" "go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin" "go.mondoo.com/cnquery/v11/providers-sdk/v1/vault" @@ -181,7 +182,7 @@ func newGithubTokenClient(conf *inventory.Config) (*github.Client, error) { func newGithubRetryableClient(httpClient *http.Client) *http.Client { retryClient := retryablehttp.NewClient() retryClient.RetryMax = 5 - retryClient.Logger = &zeroLogAdapter{} + retryClient.Logger = zerologadapter.New(log.Logger) if httpClient == nil { httpClient = http.DefaultClient @@ -240,32 +241,3 @@ func newGithubRetryableClient(httpClient *http.Client) *http.Client { return retryClient.StandardClient() } - -// zeroLogAdapter is the adapter for retryablehttp is outputting debug messages -type zeroLogAdapter struct{} - -func (l *zeroLogAdapter) Msg(msg string, keysAndValues ...interface{}) { - var e *zerolog.Event - // retry messages should only go to debug - e = log.Debug() - for i := 0; i < len(keysAndValues); i += 2 { - e = e.Interface(keysAndValues[i].(string), keysAndValues[i+1]) - } - e.Msg(msg) -} - -func (l *zeroLogAdapter) Error(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Info(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Debug(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Warn(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} diff --git a/providers/providers.go b/providers/providers.go index 11af036166..1575c32aa0 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -20,11 +20,11 @@ import ( "github.com/cockroachdb/errors" "github.com/hashicorp/go-retryablehttp" - "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/spf13/afero" "github.com/ulikunitz/xz" "go.mondoo.com/cnquery/v11/cli/config" + "go.mondoo.com/cnquery/v11/logger/zerologadapter" "go.mondoo.com/cnquery/v11/providers-sdk/v1/inventory" "go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin" "go.mondoo.com/cnquery/v11/providers-sdk/v1/resources" @@ -191,7 +191,7 @@ func httpClientWithRetry() (*http.Client, error) { retryClient := retryablehttp.NewClient() retryClient.RetryMax = 3 - retryClient.Logger = &ZerologAdapter{logger: log.Logger} + retryClient.Logger = zerologadapter.New(log.Logger) retryClient.HTTPClient = &http.Client{ Transport: &http.Transport{ Proxy: proxyFn, @@ -931,38 +931,6 @@ func MustLoadSchemaFromFile(name string, path string) *resources.Schema { return MustLoadSchema(name, raw) } -// ZerologAdapter adapts the zerolog logger to the LeveledLogger interface. -// Converts all retry logs to debug logs -type ZerologAdapter struct { - logger zerolog.Logger -} - -func (z *ZerologAdapter) Error(msg string, keysAndValues ...interface{}) { - z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) -} - -func (z *ZerologAdapter) Info(msg string, keysAndValues ...interface{}) { - z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) -} - -func (z *ZerologAdapter) Debug(msg string, keysAndValues ...interface{}) { - z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) -} - -func (z *ZerologAdapter) Warn(msg string, keysAndValues ...interface{}) { - z.logger.Debug().Fields(convertToFields(keysAndValues...)).Msg(msg) -} - -func convertToFields(keysAndValues ...interface{}) map[string]interface{} { - fields := make(map[string]interface{}) - for i := 0; i < len(keysAndValues); i += 2 { - if i+1 < len(keysAndValues) { - fields[keysAndValues[i].(string)] = keysAndValues[i+1] - } - } - return fields -} - func LoadAssetUrlSchema() (*inventory.AssetUrlSchema, error) { providers, err := ListAll() if err != nil { diff --git a/providers/slack/connection/connection.go b/providers/slack/connection/connection.go index 7cdf02080e..565562061d 100644 --- a/providers/slack/connection/connection.go +++ b/providers/slack/connection/connection.go @@ -9,9 +9,9 @@ import ( "os" "github.com/hashicorp/go-retryablehttp" - "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/slack-go/slack" + "go.mondoo.com/cnquery/v11/logger/zerologadapter" "go.mondoo.com/cnquery/v11/providers-sdk/v1/inventory" "go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin" "go.mondoo.com/cnquery/v11/providers-sdk/v1/vault" @@ -60,7 +60,7 @@ func NewSlackConnection(id uint32, asset *inventory.Asset, conf *inventory.Confi // retryablehttp is able to handle the Retry-After header, so we do not have to do it ourselves retryClient := retryablehttp.NewClient() retryClient.RetryMax = 5 - retryClient.Logger = &zeroLogAdapter{} + retryClient.Logger = zerologadapter.New(log.Logger) client := slack.New(token, slack.OptionHTTPClient(retryClient.StandardClient())) teamID := conf.Options["team-id"] @@ -102,32 +102,3 @@ func (s *SlackConnection) Client() *slack.Client { func (p *SlackConnection) TeamInfo() *slack.TeamInfo { return p.teamInfo } - -// zeroLogAdapter is the adapter for retryablehttp is outputting debug messages -type zeroLogAdapter struct{} - -func (l *zeroLogAdapter) Msg(msg string, keysAndValues ...interface{}) { - var e *zerolog.Event - // retry messages should only go to debug - e = log.Debug() - for i := 0; i < len(keysAndValues); i += 2 { - e = e.Interface(keysAndValues[i].(string), keysAndValues[i+1]) - } - e.Msg(msg) -} - -func (l *zeroLogAdapter) Error(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Info(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Debug(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -} - -func (l *zeroLogAdapter) Warn(msg string, keysAndValues ...interface{}) { - l.Msg(msg, keysAndValues...) -}