Skip to content

Commit

Permalink
Refactor logging (#733)
Browse files Browse the repository at this point in the history
* remove go-kit/log dependency
* remove go-stack dependency
* use 'any' instead of 'interface{}'
* add noop logger to tests that need it
  • Loading branch information
jranson authored Aug 16, 2024
1 parent 3c708dc commit c98f5fd
Show file tree
Hide file tree
Showing 113 changed files with 829 additions and 3,775 deletions.
3 changes: 0 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ require (
github.com/alicebob/miniredis v2.5.0+incompatible
github.com/andybalholm/brotli v1.1.0
github.com/dgraph-io/badger v1.6.2
github.com/go-kit/log v0.2.1
github.com/go-redis/redis v6.15.9+incompatible
github.com/go-stack/stack v1.8.1
github.com/golang/snappy v0.0.4
github.com/influxdata/influxdb v1.11.6
github.com/influxdata/influxql v1.2.0
Expand Down Expand Up @@ -41,7 +39,6 @@ require (
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/dgraph-io/ristretto v0.1.1 // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/golang/glog v1.2.2 // indirect
Expand Down
6 changes: 0 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,9 @@ github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU=
github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logfmt/logfmt v0.6.0 h1:wGYYu3uicYdqXVgoYbvnkrPVXkuLM1p1ifugDMEdRi4=
github.com/go-logfmt/logfmt v0.6.0/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
Expand All @@ -114,8 +110,6 @@ github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre
github.com/go-redis/redis v6.15.9+incompatible h1:K0pv1D7EQUjfyoMql+r/jZqCLizCGKFlFgcHWWmHQjg=
github.com/go-redis/redis v6.15.9+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA=
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
github.com/go-stack/stack v1.8.1 h1:ntEHSVwIt7PNXNpgPmVfMrNhLtgjlmnZha2kOpuRiDw=
github.com/go-stack/stack v1.8.1/go.mod h1:dcoOX6HbPZSZptuspn9bctJ+N/CnF5gGygcUP3XYfe4=
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
Expand Down
5 changes: 4 additions & 1 deletion pkg/backends/alb/response_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,22 @@ import (

"github.com/trickstercache/trickster/v2/pkg/backends/alb/pool"
"github.com/trickstercache/trickster/v2/pkg/backends/healthcheck"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/proxy/request"
"github.com/trickstercache/trickster/v2/pkg/proxy/response/merge"
tu "github.com/trickstercache/trickster/v2/pkg/testutil"
)

var testLogger = logging.NoopLogger()

func testMergeFunc(w http.ResponseWriter, r *http.Request, rgs merge.ResponseGates) {

}

func TestHandleResponseMerge(t *testing.T) {

r, _ := http.NewRequest("GET", "http://trickstercache.org/", nil)
rsc := request.NewResources(nil, nil, nil, nil, nil, nil, nil)
rsc := request.NewResources(nil, nil, nil, nil, nil, nil, testLogger)
rsc.ResponseMergeFunc = testMergeFunc
rsc.IsMergeMember = true
r = request.SetResources(r, rsc)
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
cr "github.com/trickstercache/trickster/v2/pkg/cache/registration"
"github.com/trickstercache/trickster/v2/pkg/config"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

func TestConfiguration(t *testing.T) {
Expand All @@ -44,7 +44,7 @@ func TestCache(t *testing.T) {
t.Fatalf("Could not load configuration: %s", err.Error())
}

caches := cr.LoadCachesFromConfig(conf, tl.ConsoleLogger("error"))
caches := cr.LoadCachesFromConfig(conf, logging.ConsoleLogger("error"))
defer cr.CloseCaches(caches)
cache, ok := caches["default"]
if !ok {
Expand Down
3 changes: 2 additions & 1 deletion pkg/backends/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ import (

"github.com/trickstercache/trickster/v2/pkg/backends/healthcheck"
bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

// Backends represents a map of Backends keyed by Name
type Backends map[string]Backend

// StartHealthChecks iterates the backends to fully configure health checkers
// and start up any intervaled health checks
func (b Backends) StartHealthChecks(logger interface{}) (healthcheck.HealthChecker, error) {
func (b Backends) StartHealthChecks(logger logging.Logger) (healthcheck.HealthChecker, error) {
hc := healthcheck.New()
for k, c := range b {
bo := c.Configuration()
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/clickhouse/clickhouse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
cr "github.com/trickstercache/trickster/v2/pkg/cache/registration"
"github.com/trickstercache/trickster/v2/pkg/config"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

var testModeler = model.NewModeler()
Expand Down Expand Up @@ -60,7 +60,7 @@ func TestNewClient(t *testing.T) {
t.Fatalf("Could not load configuration: %s", err.Error())
}

caches := cr.LoadCachesFromConfig(conf, tl.ConsoleLogger("error"))
caches := cr.LoadCachesFromConfig(conf, logging.ConsoleLogger("error"))
defer cr.CloseCaches(caches)
cache, ok := caches["default"]
if !ok {
Expand Down
5 changes: 3 additions & 2 deletions pkg/backends/healthcheck/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@ import (
"time"

ho "github.com/trickstercache/trickster/v2/pkg/backends/healthcheck/options"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

// HealthChecker defines the Health Checker interface
type HealthChecker interface {
Register(string, string, *ho.Options, *http.Client, interface{}) (*Status, error)
Register(string, string, *ho.Options, *http.Client, logging.Logger) (*Status, error)
Unregister(string)
Status(string) *Status
Statuses() StatusLookup
Expand Down Expand Up @@ -66,7 +67,7 @@ func (hc *healthChecker) Shutdown() {
}

func (hc *healthChecker) Register(name, description string, o *ho.Options,
client *http.Client, logger interface{}) (*Status, error) {
client *http.Client, logger logging.Logger) (*Status, error) {
if o == nil {
return nil, ho.ErrNoOptionsProvided
}
Expand Down
19 changes: 11 additions & 8 deletions pkg/backends/healthcheck/healthcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ import (
"testing"

ho "github.com/trickstercache/trickster/v2/pkg/backends/healthcheck/options"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

var testLogger = logging.NoopLogger()

func TestNew(t *testing.T) {

hc := New()
Expand Down Expand Up @@ -58,23 +61,23 @@ func TestRegister(t *testing.T) {
hc := New().(*healthChecker)
o := ho.New()
o.IntervalMS = 500
_, err := hc.Register("test", "test", o, http.DefaultClient, nil)
_, err := hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
target := hc.targets["test"]
target.Start()
target.Stop()
_, err = hc.Register("test", "test", o, http.DefaultClient, nil)
_, err = hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
o.Body = "test-body"
_, err = hc.Register("test", "test", o, http.DefaultClient, nil)
_, err = hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
_, err = hc.Register("test", "test", nil, http.DefaultClient, nil)
_, err = hc.Register("test", "test", nil, http.DefaultClient, testLogger)
if err != ho.ErrNoOptionsProvided {
t.Errorf("expected %v got %v", ho.ErrNoOptionsProvided, err)
}
Expand All @@ -84,7 +87,7 @@ func TestUnregister(t *testing.T) {
hc := New().(*healthChecker)
o := ho.New()
o.IntervalMS = 500
_, err := hc.Register("test", "test", o, http.DefaultClient, nil)
_, err := hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
Expand All @@ -99,7 +102,7 @@ func TestStatus(t *testing.T) {
hc := New().(*healthChecker)
o := ho.New()
o.IntervalMS = 500
_, err := hc.Register("test", "test", o, http.DefaultClient, nil)
_, err := hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
Expand All @@ -125,7 +128,7 @@ func TestStatuses(t *testing.T) {
hc := New().(*healthChecker)
o := ho.New()
o.IntervalMS = 500
_, err := hc.Register("test", "test", o, http.DefaultClient, nil)
_, err := hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
Expand All @@ -144,7 +147,7 @@ func TestHealthCheckerProbe(t *testing.T) {
ts := newTestServer(200, "OK", map[string]string{})
r, _ := http.NewRequest("GET", ts.URL+"/", nil)

_, err := hc.Register("test", "test", o, http.DefaultClient, nil)
_, err := hc.Register("test", "test", o, http.DefaultClient, testLogger)
if err != nil {
t.Error(err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/backends/healthcheck/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type target struct {
eb string
eh http.Header
ec []int
logger interface{}
logger logging.Logger
isInLoop bool
}

Expand All @@ -66,7 +66,7 @@ type DemandProbe func(w http.ResponseWriter)
func newTarget(ctx context.Context,
name, description string, o *ho.Options,
client *http.Client,
logger interface{}) (*target, error) {
logger logging.Logger) (*target, error) {

if o == nil {
return nil, ho.ErrNoOptionsProvided
Expand Down Expand Up @@ -225,15 +225,15 @@ func (t *target) probe() {
t.status.failingSince = time.Now()
t.status.Set(-1)
t.ks = -1
logging.Info(t.logger, "hc status changed",
t.logger.Info("hc status changed",
logging.Pairs{"targetName": t.name, "status": "failed",
"detail": t.status.detail, "threshold": t.failureThreshold})
} else if passed && t.ks != 1 && (successCnt == t.recoveryThreshold || t.ks == 0) {
t.status.failingSince = time.Time{}
t.status.Set(1)
t.ks = 1
t.status.detail = "" // this is only populated with failure details, so it is cleared upon recovery
logging.Info(t.logger, "hc status changed",
t.logger.Info("hc status changed",
logging.Pairs{"targetName": t.name, "status": "available",
"threshold": t.recoveryThreshold})
}
Expand Down
1 change: 1 addition & 0 deletions pkg/backends/healthcheck/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ func TestProbe(t *testing.T) {
baseRequest: r,
httpClient: ts.Client(),
ec: []int{200},
logger: testLogger,
}
target.probe()
if target.successConsecutiveCnt.Load() != 1 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/influxdb/influxdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
cr "github.com/trickstercache/trickster/v2/pkg/cache/registration"
"github.com/trickstercache/trickster/v2/pkg/config"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
)

var testModeler = model.NewModeler()
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestNewClient(t *testing.T) {
t.Fatalf("Could not load configuration: %s", err.Error())
}

caches := cr.LoadCachesFromConfig(conf, tl.ConsoleLogger("error"))
caches := cr.LoadCachesFromConfig(conf, logging.ConsoleLogger("error"))
defer cr.CloseCaches(caches)
cache, ok := caches["default"]
if !ok {
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/influxdb/model/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func writeEpochTime(w io.Writer, epoch epoch.Epoch, m int64) {
w.Write([]byte(strconv.FormatInt(int64(epoch)/m, 10)))
}

func writeValue(w io.Writer, v interface{}, nilVal string) {
func writeValue(w io.Writer, v any, nilVal string) {
if v == nil {
w.Write([]byte(nilVal))
}
Expand All @@ -91,7 +91,7 @@ func writeValue(w io.Writer, v interface{}, nilVal string) {
}
}

func writeCSVValue(w io.Writer, v interface{}, nilVal string) {
func writeCSVValue(w io.Writer, v any, nilVal string) {
if v == nil {
w.Write([]byte(nilVal))
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/influxdb/model/marshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestWriteEpochTime(t *testing.T) {
func TestWriteValue(t *testing.T) {

tests := []struct {
val interface{}
val any
nilVal string
expectedErr error
expectedVal string
Expand Down Expand Up @@ -175,7 +175,7 @@ func TestWriteValue(t *testing.T) {
func TestWriteCSVValue(t *testing.T) {

tests := []struct {
val interface{}
val any
nilVal string
expectedErr error
expectedVal string
Expand Down
7 changes: 4 additions & 3 deletions pkg/backends/irondb/handler_caql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import (
"time"

bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/proxy/request"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
tu "github.com/trickstercache/trickster/v2/pkg/testutil"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
)

func TestCAQLHandler(t *testing.T) {
Expand Down Expand Up @@ -88,7 +88,8 @@ func TestCaqlHandlerSetExtent(t *testing.T) {
t.Error(err)
}

r = request.SetResources(r, request.NewResources(cfg, nil, nil, nil, client, nil, tl.ConsoleLogger("error")))
r = request.SetResources(r, request.NewResources(cfg, nil, nil, nil, client,
nil, logging.ConsoleLogger("error")))

now := time.Now()
then := now.Add(-5 * time.Hour)
Expand Down
7 changes: 4 additions & 3 deletions pkg/backends/irondb/handler_fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import (
"time"

bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/proxy/request"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
tu "github.com/trickstercache/trickster/v2/pkg/testutil"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
)

func TestFetchHandler(t *testing.T) {
Expand Down Expand Up @@ -112,7 +112,8 @@ func TestFetchHandlerSetExtent(t *testing.T) {
t.Error(err)
}

r = request.SetResources(r, request.NewResources(o, nil, nil, nil, client, nil, tl.ConsoleLogger("error")))
r = request.SetResources(r, request.NewResources(o, nil, nil, nil, client,
nil, logging.ConsoleLogger("error")))

now := time.Now()
then := now.Add(-5 * time.Hour)
Expand Down
8 changes: 4 additions & 4 deletions pkg/backends/irondb/handler_histogram_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (
"time"

bo "github.com/trickstercache/trickster/v2/pkg/backends/options"
tl "github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/observability/logging"
"github.com/trickstercache/trickster/v2/pkg/proxy/errors"
"github.com/trickstercache/trickster/v2/pkg/proxy/request"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
tu "github.com/trickstercache/trickster/v2/pkg/testutil"
"github.com/trickstercache/trickster/v2/pkg/timeseries"
)

func TestHistogramHandler(t *testing.T) {
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestHistogramHandlerSetExtent(t *testing.T) {
}

r = request.SetResources(r,
request.NewResources(o, nil, nil, nil, client, nil, tl.ConsoleLogger("error")))
request.NewResources(o, nil, nil, nil, client, nil, logging.ConsoleLogger("error")))

now := time.Now()
then := now.Add(-5 * time.Hour)
Expand Down Expand Up @@ -241,7 +241,7 @@ func TestHistogramHandlerFastForwardRequestError(t *testing.T) {
t.Error(err)
}

rsc := request.NewResources(o, nil, nil, nil, client, nil, tl.ConsoleLogger("error"))
rsc := request.NewResources(o, nil, nil, nil, client, nil, logging.ConsoleLogger("error"))
r = request.SetResources(r, rsc)

r.URL.Path = "/histogram/x/900/300/00112233-4455-6677-8899-aabbccddeeff/metric"
Expand Down
Loading

0 comments on commit c98f5fd

Please sign in to comment.