Use context to pass logger
- Dominant language
- Go
- Stars
- 58
- Forks
- 55
- Avg merge
- 9h 9m
- Merged PRs (30d)
- 424
Description
**Motivation**
Currently, we are passing around a `*logp.Logger` object around in many of our functions. This means that whenever we need a logger in the current function, we need to refactor code to have a logging object in the current context. This can sometimes prove annoying and can lead to big diffs, effectively discouraging developers from proper logging practices.
One alternative that is common in go, is to save the logger object inside the context: https://andrebritopassos.medium.com/logging-with-context-in-go-30366fa29d08. This would not be great for other types of arguments like function-specific arguments relevant to specific functions. However, loggers are closely related to the context of the current call so they do belong together.
I am opening this ticket to start a discussion of the pros and cons of doing that and if it would be beneficial for our codebase.
**Definition of done**
What needs to be completed at the end of this task
- [ ] Convenience functions are written to allow to safely save & restore log objects inside the current context
- [ ] log objects are not passed around in functions that already take a `context.Context` argument
**POC**
Proof of concept diff on how it would look like to remove `log` from `benchmark.Benchmark.Initialize()`:
Click for diff
```diff
diff --git a/flavors/benchmark/aws.go b/flavors/benchmark/aws.go
index 7af9c75e..3eb36532 100644
--- a/flavors/benchmark/aws.go
+++ b/flavors/benchmark/aws.go
@@ -23,7 +23,6 @@ import (
"fmt"
"github.com/elastic/beats/v7/x-pack/libbeat/common/aws"
- "github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/dataprovider"
@@ -32,13 +31,14 @@ import (
"github.com/elastic/cloudbeat/resources/fetching/factory"
"github.com/elastic/cloudbeat/resources/fetching/registry"
"github.com/elastic/cloudbeat/resources/providers/awslib"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
)
type AWS struct {
IdentityProvider awslib.IdentityProviderGetter
}
-func (a *AWS) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
+func (a *AWS) Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
if err := a.checkDependencies(); err != nil {
return nil, nil, err
}
@@ -54,6 +54,7 @@ func (a *AWS) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Conf
return nil, nil, fmt.Errorf("failed to get AWS identity: %w", err)
}
+ log := clog.GetLogger(ctx)
return registry.NewRegistry(
log,
factory.NewCisAwsFactory(log, awsConfig, ch, awsIdentity),
diff --git a/flavors/benchmark/aws_org.go b/flavors/benchmark/aws_org.go
index e3a528c4..5fcd777d 100644
--- a/flavors/benchmark/aws_org.go
+++ b/flavors/benchmark/aws_org.go
@@ -26,7 +26,6 @@ import (
"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
"github.com/aws/aws-sdk-go-v2/service/sts"
"github.com/elastic/beats/v7/x-pack/libbeat/common/aws"
- "github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/dataprovider"
@@ -35,6 +34,7 @@ import (
"github.com/elastic/cloudbeat/resources/fetching/factory"
"github.com/elastic/cloudbeat/resources/fetching/registry"
"github.com/elastic/cloudbeat/resources/providers/awslib"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
)
type AWSOrg struct {
@@ -42,7 +42,7 @@ type AWSOrg struct {
AccountProvider awslib.AccountProviderAPI
}
-func (a *AWSOrg) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
+func (a *AWSOrg) Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
if err := a.checkDependencies(); err != nil {
return nil, nil, err
}
@@ -63,6 +63,7 @@ func (a *AWSOrg) Initialize(ctx context.Context, log *logp.Logger, cfg *config.C
return nil, nil, fmt.Errorf("failed to get AWS accounts: %w", err)
}
+ log := clog.GetLogger(ctx)
return registry.NewRegistry(
log,
factory.NewCisAwsOrganizationFactory(ctx, log, ch, accounts),
diff --git a/flavors/benchmark/aws_org_test.go b/flavors/benchmark/aws_org_test.go
index ddd3eec7..ae775e3c 100644
--- a/flavors/benchmark/aws_org_test.go
+++ b/flavors/benchmark/aws_org_test.go
@@ -18,7 +18,6 @@
package benchmark
import (
- "context"
"errors"
"testing"
@@ -31,6 +30,7 @@ import (
"github.com/elastic/cloudbeat/dataprovider/providers/cloud"
"github.com/elastic/cloudbeat/resources/fetching"
"github.com/elastic/cloudbeat/resources/providers/awslib"
+ "github.com/elastic/cloudbeat/resources/utils/testhelper"
)
func TestAWSOrg_Initialize(t *testing.T) {
@@ -146,7 +146,7 @@ func Test_getAwsAccounts(t *testing.T) {
IdentityProvider: nil,
AccountProvider: tt.accountProvider,
}
- got, err := A.getAwsAccounts(context.Background(), aws.Config{}, &tt.rootIdentity)
+ got, err := A.getAwsAccounts(testhelper.NewContext(t), aws.Config{}, &tt.rootIdentity)
if tt.wantErr != "" {
assert.ErrorContains(t, err, tt.wantErr)
return
diff --git a/flavors/benchmark/benchmark.go b/flavors/benchmark/benchmark.go
index 9e4dbcdc..a50a13f1 100644
--- a/flavors/benchmark/benchmark.go
+++ b/flavors/benchmark/benchmark.go
@@ -21,8 +21,6 @@ import (
"context"
"fmt"
- "github.com/elastic/elastic-agent-libs/logp"
-
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/dataprovider"
"github.com/elastic/cloudbeat/dataprovider/providers/k8s"
@@ -36,7 +34,7 @@ import (
type Benchmark interface {
Run(ctx context.Context) error
- Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error)
+ Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error)
Stop()
checkDependencies() error
diff --git a/flavors/benchmark/benchmark_test.go b/flavors/benchmark/benchmark_test.go
index 7ebe04c4..84fac763 100644
--- a/flavors/benchmark/benchmark_test.go
+++ b/flavors/benchmark/benchmark_test.go
@@ -104,7 +104,8 @@ func TestNewBenchmark(t *testing.T) {
func testInitialize(t *testing.T, benchmark Benchmark, cfg *config.Config, wantErr string, want []string) {
t.Helper()
- reg, dp, err := benchmark.Initialize(context.Background(), testhelper.NewLogger(t), cfg, make(chan fetching.ResourceInfo))
+ ctx := testhelper.NewContext(t)
+ reg, dp, err := benchmark.Initialize(ctx, cfg, make(chan fetching.ResourceInfo))
if wantErr != "" {
assert.ErrorContains(t, err, wantErr)
return
@@ -113,7 +114,7 @@ func testInitialize(t *testing.T, benchmark Benchmark, cfg *config.Config, wantE
require.NoError(t, err)
assert.Len(t, reg.Keys(), len(want))
- require.NoError(t, benchmark.Run(context.Background()))
+ require.NoError(t, benchmark.Run(ctx))
defer benchmark.Stop()
for _, fetcher := range want {
diff --git a/flavors/benchmark/eks.go b/flavors/benchmark/eks.go
index 23cec3f3..6c1ba96b 100644
--- a/flavors/benchmark/eks.go
+++ b/flavors/benchmark/eks.go
@@ -24,7 +24,6 @@ import (
awssdk "github.com/aws/aws-sdk-go-v2/aws"
"github.com/elastic/beats/v7/x-pack/libbeat/common/aws"
"github.com/elastic/elastic-agent-autodiscover/kubernetes"
- "github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/dataprovider"
@@ -35,6 +34,7 @@ import (
"github.com/elastic/cloudbeat/resources/fetching/factory"
"github.com/elastic/cloudbeat/resources/fetching/registry"
"github.com/elastic/cloudbeat/resources/providers/awslib"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
"github.com/elastic/cloudbeat/uniqueness"
)
@@ -48,11 +48,12 @@ type EKS struct {
leaderElector uniqueness.Manager
}
-func (k *EKS) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
+func (k *EKS) Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
if err := k.checkDependencies(); err != nil {
return nil, nil, err
}
+ log := clog.GetLogger(ctx)
kubeClient, err := k.ClientProvider.GetClient(log, cfg.KubeConfig, kubernetes.KubeClientOptions{})
if err != nil {
return nil, nil, fmt.Errorf("failed to create kubernetes client: %w", err)
diff --git a/flavors/benchmark/gcp.go b/flavors/benchmark/gcp.go
index 0b744504..2d712a1d 100644
--- a/flavors/benchmark/gcp.go
+++ b/flavors/benchmark/gcp.go
@@ -22,8 +22,6 @@ import (
"errors"
"fmt"
- "github.com/elastic/elastic-agent-libs/logp"
-
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/dataprovider"
"github.com/elastic/cloudbeat/dataprovider/providers/cloud"
@@ -33,6 +31,7 @@ import (
"github.com/elastic/cloudbeat/resources/providers/gcplib/auth"
"github.com/elastic/cloudbeat/resources/providers/gcplib/identity"
"github.com/elastic/cloudbeat/resources/providers/gcplib/inventory"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
)
type GCP struct {
@@ -43,11 +42,12 @@ type GCP struct {
func (g *GCP) Run(context.Context) error { return nil }
-func (g *GCP) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
+func (g *GCP) Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
if err := g.checkDependencies(); err != nil {
return nil, nil, err
}
+ log := clog.GetLogger(ctx)
gcpConfig, err := g.CfgProvider.GetGcpClientConfig(ctx, cfg.CloudConfig.Gcp, log)
if err != nil {
return nil, nil, fmt.Errorf("failed to initialize gcp config: %w", err)
diff --git a/flavors/benchmark/k8s.go b/flavors/benchmark/k8s.go
index dd3f20a1..cd719cdb 100644
--- a/flavors/benchmark/k8s.go
+++ b/flavors/benchmark/k8s.go
@@ -34,6 +34,7 @@ import (
"github.com/elastic/cloudbeat/resources/fetching"
"github.com/elastic/cloudbeat/resources/fetching/factory"
"github.com/elastic/cloudbeat/resources/fetching/registry"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
"github.com/elastic/cloudbeat/uniqueness"
"github.com/elastic/cloudbeat/version"
)
@@ -44,11 +45,12 @@ type K8S struct {
leaderElector uniqueness.Manager
}
-func (k *K8S) Initialize(ctx context.Context, log *logp.Logger, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
+func (k *K8S) Initialize(ctx context.Context, cfg *config.Config, ch chan fetching.ResourceInfo) (registry.Registry, dataprovider.CommonDataProvider, error) {
if err := k.checkDependencies(); err != nil {
return nil, nil, err
}
+ log := clog.GetLogger(ctx)
kubeClient, err := k.ClientProvider.GetClient(log, cfg.KubeConfig, kubernetes.KubeClientOptions{})
if err != nil {
return nil, nil, fmt.Errorf("failed to create kubernetes client :%w", err)
diff --git a/flavors/posture.go b/flavors/posture.go
index 40144b74..13db0074 100644
--- a/flavors/posture.go
+++ b/flavors/posture.go
@@ -24,7 +24,6 @@ import (
"github.com/elastic/beats/v7/libbeat/beat"
agentconfig "github.com/elastic/elastic-agent-libs/config"
- "github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/cloudbeat/config"
"github.com/elastic/cloudbeat/evaluator"
@@ -33,6 +32,7 @@ import (
_ "github.com/elastic/cloudbeat/processor" // Add cloudbeat default processors.
"github.com/elastic/cloudbeat/resources/fetching"
"github.com/elastic/cloudbeat/resources/fetching/manager"
+ "github.com/elastic/cloudbeat/resources/utils/clog"
"github.com/elastic/cloudbeat/transformer"
)
@@ -56,9 +56,9 @@ func NewPosture(b *beat.Beat, agentConfig *agentconfig.C) (beat.Beater, error) {
// NewPosture creates an instance of posture.
func newPostureFromCfg(b *beat.Beat, cfg *config.Config) (*posture, error) {
- log := logp.NewLogger("posture")
- log.Info("Config initiated with cycle period of ", cfg.Period)
ctx, cancel := context.WithCancel(context.Background())
+ ctx, log := clog.NewLogger(ctx, "posture")
+ log.Info("Config initiated with cycle period of ", cfg.Period)
bench, err := benchmark.NewBenchmark(cfg)
if err != nil {
@@ -69,7 +69,7 @@ func newPostureFromCfg(b *beat.Beat, cfg *config.Config) (*posture, error) {
resourceCh := make(chan fetching.ResourceInfo, resourceChBuffer)
log.Infof("Initializing benchmark %T", b)
- fetchersRegistry, cdp, err := bench.Initialize(ctx, log, cfg, resourceCh)
+ fetchersRegistry, cdp, err := bench.Initialize(ctx, cfg, resourceCh)
if err != nil {
cancel()
return nil, err
diff --git a/resources/utils/clog/clog.go b/resources/utils/clog/clog.go
new file mode 100644
index 00000000..fdc12c06
--- /dev/null
+++ b/resources/utils/clog/clog.go
@@ -0,0 +1,47 @@
+// Licensed to Elasticsearch B.V. under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Elasticsearch B.V. licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package clog
+
+import (
+ "context"
+
+ "github.com/elastic/elastic-agent-libs/logp"
+)
+
+const loggerKey = "cloudbeat_logger"
+
+func NewLogger(ctx context.Context, name string) (context.Context, *logp.Logger) {
+ log := logp.NewLogger(name)
+ return context.WithValue(ctx, loggerKey, log), log
+}
+
+func GetLogger(ctx context.Context) *logp.Logger {
+ loggerValue := ctx.Value(loggerKey)
+ if loggerValue == nil {
+ log := logp.NewLogger("cloudbeat")
+ log.Warn("Context did not have logger key")
+ return log
+ }
+ log, ok := loggerValue.(*logp.Logger)
+ if !ok {
+ log = logp.NewLogger("cloudbeat")
+ log.Errorf("Unexpected logger type %T", loggerValue)
+ return log
+ }
+ return log
+}
diff --git a/resources/utils/testhelper/helper.go b/resources/utils/testhelper/helper.go
index dc17b817..af46b730 100644
--- a/resources/utils/testhelper/helper.go
+++ b/resources/utils/testhelper/helper.go
@@ -18,12 +18,15 @@
package testhelper
import (
+ "context"
"sync"
"testing"
"time"
"github.com/elastic/elastic-agent-libs/logp"
"github.com/stretchr/testify/require"
+
+ "github.com/elastic/cloudbeat/resources/utils/clog"
)
// CollectResources fetches items from a channel and returns them in a slice.
@@ -86,14 +89,24 @@ func CreateMockClients[T any](client T, regions []string) map[string]T {
return m
}
-var once sync.Once
+func NewContext(t *testing.T) context.Context {
+ t.Helper()
+ ensureLoggingTestSetup(t)
+ ctx, _ := clog.NewLogger(context.Background(), t.Name())
+ return ctx
+}
func NewLogger(t *testing.T) *logp.Logger {
t.Helper()
+ ensureLoggingTestSetup(t)
+ return logp.NewLogger(t.Name())
+}
+var once sync.Once
+
+func ensureLoggingTestSetup(t *testing.T) {
+ t.Helper()
once.Do(func() {
require.NoError(t, logp.TestingSetup())
})
-
- return logp.NewLogger(t.Name())
}
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.