Skip to content

Commit

Permalink
chore: switch to slog instead of standard log pkg (#8)
Browse files Browse the repository at this point in the history
* chore: switch to slog instead of standard log pkg

* chore: bump go v1.21

* chore: use json handler

* chore: use a single logger instance

* feat: use ctx logger

* feat: add new context logger to check config

* feat: instrument

* chore: add logging to config validation

* chore: inline test cases
  • Loading branch information
lvlcn-t committed Nov 17, 2023
1 parent 8ccf05c commit 5c9edb9
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 33 deletions.
16 changes: 10 additions & 6 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package cmd

import (
"context"
"log"

"github.com/caas-team/sparrow/internal/logger"
"github.com/caas-team/sparrow/pkg/config"
"github.com/caas-team/sparrow/pkg/sparrow"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -51,6 +51,9 @@ func NewCmdRun() *cobra.Command {
// run is the entry point to start the sparrow
func run(fm *config.RunFlagsNameMapping) func(cmd *cobra.Command, args []string) {
return func(cmd *cobra.Command, args []string) {
log := logger.NewLogger()
ctx := logger.IntoContext(context.Background(), log)

cfg := config.NewConfig()

cfg.SetLoaderType(viper.GetString(fm.LoaderType))
Expand All @@ -61,15 +64,16 @@ func run(fm *config.RunFlagsNameMapping) func(cmd *cobra.Command, args []string)
cfg.SetLoaderHttpRetryCount(viper.GetInt(fm.LoaderHttpRetryCount))
cfg.SetLoaderHttpRetryDelay(viper.GetInt(fm.LoaderHttpRetryDelay))

if err := cfg.Validate(fm); err != nil {
log.Panic(err)
if err := cfg.Validate(ctx, fm); err != nil {
log.Error("Error while validating the config", "error", err)
panic(err)
}

sparrow := sparrow.New(cfg)

log.Println("running sparrow")
if err := sparrow.Run(context.Background()); err != nil {
log.Panic(err)
log.Info("Running sparrow")
if err := sparrow.Run(ctx); err != nil {
panic(err)
}
}
}
7 changes: 5 additions & 2 deletions internal/helper/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package helper

import (
"context"
"log"
"fmt"
"math"
"time"

"github.com/caas-team/sparrow/internal/logger"
)

type RetryConfig struct {
Expand All @@ -18,14 +20,15 @@ type Effector func(context.Context) error
// Retry will retry the run the effector function in an exponential backoff
func Retry(effector Effector, rc RetryConfig) func(ctx context.Context) error {
return func(ctx context.Context) error {
log := logger.FromContext(ctx)
for r := 1; ; r++ {
err := effector(ctx)
if err == nil || r > rc.Count {
return err
}

delay := getExpBackoff(rc.Delay, r)
log.Printf("Effector call failed, retrying in %v", delay)
log.WarnContext(ctx, fmt.Sprintf("Effector call failed, retrying in %v", delay))

timer := time.NewTimer(delay)
defer timer.Stop()
Expand Down
50 changes: 50 additions & 0 deletions internal/logger/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package logger

import (
"context"
"log/slog"
"os"
)

type logger struct{}

// NewLogger creates a new slog.Logger instance.
// If handlers are provided, the first handler in the slice is used; otherwise,
// a default JSON handler writing to os.Stderr is used. This function allows for
// custom configuration of logging handlers.
func NewLogger(h ...slog.Handler) *slog.Logger {
var handler slog.Handler
if len(h) > 0 {
handler = h[0]
} else {
handler = slog.NewJSONHandler(os.Stderr, nil)
}
return slog.New(handler)
}

// NewContextWithLogger creates a new context based on the provided parent context.
// It embeds a logger into this new context, which is a child of the logger from the parent context.
// The child logger inherits settings from the parent and is grouped under the provided childName.
// It also returns a cancel function to cancel the new context.
func NewContextWithLogger(parent context.Context, childName string) (context.Context, context.CancelFunc) {
ctx, cancel := context.WithCancel(parent)
return IntoContext(ctx, FromContext(parent).WithGroup(childName)), cancel
}

// IntoContext embeds the provided slog.Logger into the given context and returns the modified context.
// This function is used for passing loggers through context, allowing for context-aware logging.
func IntoContext(ctx context.Context, log *slog.Logger) context.Context {
return context.WithValue(ctx, logger{}, log)
}

// FromContext extracts the slog.Logger from the provided context.
// If the context does not have a logger, it returns a new logger with the default configuration.
// This function is useful for retrieving loggers from context in different parts of an application.
func FromContext(ctx context.Context) *slog.Logger {
if ctx != nil {
if logger, ok := ctx.Value(logger{}).(*slog.Logger); ok {
return logger
}
}
return NewLogger()
}
3 changes: 3 additions & 0 deletions pkg/checks/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"net/http"

"github.com/caas-team/sparrow/internal/logger"
"github.com/getkin/kin-openapi/openapi3"
)

Expand All @@ -27,6 +28,8 @@ func GetRoundtripCheck() Check {
}

func (rt *RoundTrip) Run(ctx context.Context) (Result, error) {
ctx, cancel := logger.NewContextWithLogger(ctx, "roundTrip")
defer cancel()
return Result{}, nil
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/config/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,21 @@ import (
"context"
"fmt"
"io"
"log/slog"
"net/http"
"time"

"github.com/caas-team/sparrow/internal/helper"
"github.com/caas-team/sparrow/internal/logger"
"gopkg.in/yaml.v3"
)

type HttpLoader struct {
log *slog.Logger
cfg *Config
cCfgChecks chan<- map[string]any
}

func NewHttpLoader(cfg *Config, cCfgChecks chan<- map[string]any) *HttpLoader {
return &HttpLoader{
// TODO: set logger from cfg
log: slog.Default().WithGroup("httpLoader"),
cfg: cfg,
cCfgChecks: cCfgChecks,
}
Expand All @@ -33,8 +30,11 @@ func NewHttpLoader(cfg *Config, cCfgChecks chan<- map[string]any) *HttpLoader {
// loader interval configuration. A failed request will be retried defined
// by the retry configuration
func (gl *HttpLoader) Run(ctx context.Context) {
var runtimeCfg *RuntimeConfig
ctx, cancel := logger.NewContextWithLogger(ctx, "httpLoader")
defer cancel()
log := logger.FromContext(ctx)

var runtimeCfg *RuntimeConfig
for {
getConfigRetry := helper.Retry(func(ctx context.Context) error {
var err error
Expand All @@ -44,11 +44,11 @@ func (gl *HttpLoader) Run(ctx context.Context) {
}, gl.cfg.Loader.http.retryCfg)

if err := getConfigRetry(ctx); err != nil {
gl.log.Error("Could not get remote runtime configuration", "error", err)
log.Error("Could not get remote runtime configuration", "error", err)
return
}

gl.log.Info("Successfully got remote runtime configuration")
log.Info("Successfully got remote runtime configuration")
gl.cCfgChecks <- runtimeCfg.Checks

timer := time.NewTimer(gl.cfg.Loader.Interval)
Expand All @@ -64,7 +64,7 @@ func (gl *HttpLoader) Run(ctx context.Context) {

// GetRuntimeConfig gets the remote runtime configuration
func (gl *HttpLoader) GetRuntimeConfig(ctx context.Context) (*RuntimeConfig, error) {
log := gl.log.With("url", gl.cfg.Loader.http.url)
log := logger.FromContext(ctx).With("url", gl.cfg.Loader.http.url)

client := http.DefaultClient
client.Timeout = gl.cfg.Loader.http.timeout
Expand Down
7 changes: 4 additions & 3 deletions pkg/config/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"
"time"

"github.com/caas-team/sparrow/internal/logger"
"github.com/jarcoal/httpmock"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -117,12 +118,12 @@ func TestHttpLoader_GetRuntimeConfig(t *testing.T) {
},
)

ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
handler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})
ctx := logger.IntoContext(context.Background(), logger.NewLogger(handler).WithGroup("httpLoader-test"))
ctx, cancel := context.WithTimeout(ctx, time.Second*10)
defer cancel()

handler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})
gl := &HttpLoader{
log: slog.New(handler),
cfg: tt.cfg,
cCfgChecks: make(chan<- map[string]any),
}
Expand Down
15 changes: 9 additions & 6 deletions pkg/config/validate.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
package config

import (
"context"
"fmt"
"log/slog"
"net/url"

"github.com/caas-team/sparrow/internal/logger"
)

// Validates the config
func (c *Config) Validate(fm *RunFlagsNameMapping) error {
// TODO: get logger from context
log := slog.Default().WithGroup("validation")
ok := true
func (c *Config) Validate(ctx context.Context, fm *RunFlagsNameMapping) error {
ctx, cancel := logger.NewContextWithLogger(ctx, "configValidation")
defer cancel()
log := logger.FromContext(ctx)

ok := true
if _, err := url.ParseRequestURI(c.Loader.http.url); err != nil {
ok = false
log.Error("The loader http url is not a valid url",
log.ErrorContext(ctx, "The loader http url is not a valid url",
fm.LoaderHttpUrl, c.Loader.http.url)
}

Expand Down
7 changes: 6 additions & 1 deletion pkg/config/validate_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package config

import (
"context"
"testing"
"time"

"github.com/caas-team/sparrow/internal/helper"
"github.com/caas-team/sparrow/internal/logger"
)

func TestConfig_Validate(t *testing.T) {
ctx, cancel := logger.NewContextWithLogger(context.Background(), "validation-test")
defer cancel()

type fields struct {
Loader LoaderConfig
}
Expand Down Expand Up @@ -91,7 +96,7 @@ func TestConfig_Validate(t *testing.T) {
Checks: nil,
Loader: tt.fields.Loader,
}
if err := c.Validate(&RunFlagsNameMapping{}); (err != nil) != tt.wantErr {
if err := c.Validate(ctx, &RunFlagsNameMapping{}); (err != nil) != tt.wantErr {
t.Errorf("Config.Validate() error = %v, wantErr %v", err, tt.wantErr)
}
})
Expand Down
13 changes: 8 additions & 5 deletions pkg/sparrow/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ package sparrow
import (
"context"
"fmt"
"log"

"github.com/caas-team/sparrow/pkg/db"

"github.com/caas-team/sparrow/internal/logger"
"github.com/caas-team/sparrow/pkg/checks"
"github.com/caas-team/sparrow/pkg/config"
"github.com/getkin/kin-openapi/openapi3"
Expand Down Expand Up @@ -41,6 +41,8 @@ func New(cfg *config.Config) *Sparrow {

// Run starts the sparrow
func (s *Sparrow) Run(ctx context.Context) error {
ctx, cancel := logger.NewContextWithLogger(ctx, "sparrow")
defer cancel()
// TODO Setup before checks run
// setup http server

Expand Down Expand Up @@ -68,18 +70,19 @@ func (s *Sparrow) Run(ctx context.Context) error {
// Register new Checks, unregister removed Checks & reset Configs of Checks
func (s *Sparrow) ReconcileChecks(ctx context.Context) {
for name, checkCfg := range s.cfg.Checks {
log := logger.FromContext(ctx).With("name", name)
if existingCheck, ok := s.checks[name]; ok {
// Check already registered, reset config
err := existingCheck.SetConfig(ctx, checkCfg)
if err != nil {
log.Printf("Failed to reset config for check, check will run with last applies config - %s: %s", name, err.Error())
log.ErrorContext(ctx, "Failed to reset config for check, check will run with last applies config", "error", err)
}
continue
}
// Check is a new Check and needs to be registered
getRegisteredCheck := checks.RegisteredChecks[name]
if getRegisteredCheck == nil {
log.Printf("Check %s is not registered", name)
log.WarnContext(ctx, "Check is not registered")
continue
}
check := getRegisteredCheck()
Expand All @@ -91,12 +94,12 @@ func (s *Sparrow) ReconcileChecks(ctx context.Context) {

err := check.SetConfig(ctx, checkCfg)
if err != nil {
log.Printf("Failed to set config for check %s: %s", name, err.Error())
log.ErrorContext(ctx, "Failed to set config for check", "name", name, "error", err)
}
go fanInResults(checkChan, s.cResult, name)
err = check.Startup(ctx, checkChan)
if err != nil {
log.Printf("Failed to startup check %s: %s", name, err.Error())
log.ErrorContext(ctx, "Failed to startup check", "name", name, "error", err)
close(checkChan)
// TODO discuss whether this should return an error instead?
continue
Expand Down
7 changes: 5 additions & 2 deletions pkg/sparrow/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/caas-team/sparrow/pkg/db"

"github.com/caas-team/sparrow/internal/logger"
"github.com/caas-team/sparrow/pkg/checks"
"github.com/caas-team/sparrow/pkg/config"
"github.com/getkin/kin-openapi/openapi3"
Expand All @@ -28,7 +29,6 @@ func TestSparrow_getOpenapi(t *testing.T) {
wantErr bool
}
tests := []test{

{name: "no checks registered", fields: fields{checks: map[string]checks.Check{}, config: config.NewConfig()}, want: oapiBoilerplate, wantErr: false},
{name: "check registered", fields: fields{checks: map[string]checks.Check{"rtt": checks.GetRoundtripCheck()}, config: config.NewConfig()}, want: oapiBoilerplate, wantErr: false},
}
Expand Down Expand Up @@ -70,6 +70,9 @@ func TestSparrow_getOpenapi(t *testing.T) {
}

func TestSparrow_ReconcileChecks(t *testing.T) {
ctx, cancel := logger.NewContextWithLogger(context.Background(), "sparrow-test")
defer cancel()

mockCheck := checks.CheckMock{
RunFunc: func(ctx context.Context) (checks.Result, error) {
return checks.Result{}, nil
Expand Down Expand Up @@ -181,7 +184,7 @@ func TestSparrow_ReconcileChecks(t *testing.T) {
// Send new config to channel
s.cfg.Checks = tt.newChecksConfig

s.ReconcileChecks(context.Background())
s.ReconcileChecks(ctx)

for newChecksConfigName := range tt.newChecksConfig {
check := checks.RegisteredChecks[newChecksConfigName]()
Expand Down

0 comments on commit 5c9edb9

Please sign in to comment.