Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade dskit and improve log.InitLogger function #2799

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/tempo/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func main() {
level.Error(log.Logger).Log("msg", "invalid log level")
os.Exit(1)
}
log.InitLogger(&config.Server)
config.Server.Log = log.InitLogger(config.Server.LogFormat, config.Server.LogLevel)

// Verifying the config's validity and log warnings now that the logger is initialized
isValid := configIsValid(config)
Expand Down
7 changes: 3 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/Azure/go-autorest/autorest/azure/auth v0.5.11
github.com/alecthomas/kong v0.8.0
github.com/alicebob/miniredis/v2 v2.21.0
github.com/aws/aws-sdk-go v1.44.220
github.com/aws/aws-sdk-go v1.44.321
github.com/cespare/xxhash v1.1.0
github.com/cespare/xxhash/v2 v2.2.0
github.com/cristalhq/hedgedhttp v0.7.0
Expand All @@ -30,7 +30,7 @@ require (
github.com/google/go-cmp v0.5.9
github.com/google/uuid v1.3.0
github.com/gorilla/mux v1.8.0
github.com/grafana/dskit v0.0.0-20230808152345-e772133474bd
github.com/grafana/dskit v0.0.0-20230817134647-90d7ee0bed7c
github.com/grafana/e2e v0.1.1-0.20221018202458-cffd2bb71c7b
github.com/grpc-ecosystem/grpc-opentracing v0.0.0-20180507213350-8e809c8a8645
github.com/hashicorp/go-hclog v1.4.0
Expand Down Expand Up @@ -60,7 +60,7 @@ require (
github.com/prometheus/prometheus v1.8.2-0.20221021121301-51a44e6657c3
github.com/prometheus/statsd_exporter v0.22.7 // indirect
github.com/segmentio/fasthash v0.0.0-20180216231524-a72b379d632e
github.com/sirupsen/logrus v1.9.0
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/sony/gobreaker v0.4.1
github.com/spf13/viper v1.14.0
github.com/stretchr/testify v1.8.4
Expand Down Expand Up @@ -264,7 +264,6 @@ require (
github.com/tklauser/go-sysconf v0.3.11 // indirect
github.com/tklauser/numcpus v0.6.0 // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
github.com/weaveworks/promrus v1.2.0 // indirect
github.com/willf/bitset v1.1.11 // indirect
github.com/xdg-go/pbkdf2 v1.0.0 // indirect
github.com/xdg-go/scram v1.1.2 // indirect
Expand Down
10 changes: 4 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,8 @@ github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
github.com/aws/aws-sdk-go v1.38.35/go.mod h1:hcU610XS61/+aQV88ixoOzUoG7v3b31pl2zKMmprdro=
github.com/aws/aws-sdk-go v1.43.11/go.mod h1:y4AeaBuwd2Lk+GepC1E9v0qOiTws0MIWAX4oIKwKHZo=
github.com/aws/aws-sdk-go v1.44.220 h1:yAj99qAt0Htjle9Up3DglgHfOP77lmFPrElA4jKnrBo=
github.com/aws/aws-sdk-go v1.44.220/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
github.com/aws/aws-sdk-go v1.44.321 h1:iXwFLxWjZPjYqjPq0EcCs46xX7oDLEELte1+BzgpKk8=
github.com/aws/aws-sdk-go v1.44.321/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
github.com/aws/aws-sdk-go-v2 v1.9.2/go.mod h1:cK/D0BBs0b/oWPIcX/Z/obahJK1TT7IPVjy53i/mX/4=
github.com/aws/aws-sdk-go-v2 v1.16.0 h1:cBAYjiiexRAg9v2z9vb6IdxAa7ef4KCtjW7w7e3GxGo=
github.com/aws/aws-sdk-go-v2/config v1.8.3/go.mod h1:4AEiLtAb8kLs7vgw2ZV3p2VZ1+hBavOc84hqxVNpCyw=
Expand Down Expand Up @@ -826,8 +826,8 @@ github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB7
github.com/gorilla/securecookie v1.1.1/go.mod h1:ra0sb63/xPlUeL+yeDciTfxMRAA+MP+HVt/4epWDjd4=
github.com/gorilla/sessions v1.2.1/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/grafana/dskit v0.0.0-20230808152345-e772133474bd h1:nsjY1yHyUVAkDLDwhd+3f91ox45Wv+xRxrEYxDeF6Y8=
github.com/grafana/dskit v0.0.0-20230808152345-e772133474bd/go.mod h1:hbcgTCAMxGfZmjbieS4VZpnJjzvcWlI8V7/uTJ9nTMM=
github.com/grafana/dskit v0.0.0-20230817134647-90d7ee0bed7c h1:UFt2isLqVSMfT2ajK6ew1Vg+3iePcNngqxnMpv3Rj5w=
github.com/grafana/dskit v0.0.0-20230817134647-90d7ee0bed7c/go.mod h1:3u7fr4hmOhuUL9Yc1QP/oa3za73kxvqJnRJH4BA5fOM=
github.com/grafana/e2e v0.1.1-0.20221018202458-cffd2bb71c7b h1:Ha+kSIoTutf4ytlVw/SaEclDUloYx0+FXDKJWKhNbE4=
github.com/grafana/e2e v0.1.1-0.20221018202458-cffd2bb71c7b/go.mod h1:3UsooRp7yW5/NJQBlXcTsAHOoykEhNUYXkQ3r6ehEEY=
github.com/grafana/gomemcache v0.0.0-20230316202710-a081dae0aba9 h1:WB3bGH2f1UN6jkd6uAEWfHB8OD7dKJ0v2Oo6SNfhpfQ=
Expand Down Expand Up @@ -1321,8 +1321,6 @@ github.com/uber/jaeger-client-go v2.30.0+incompatible/go.mod h1:WVhlPFC8FDjOFMMW
github.com/uber/jaeger-lib v2.4.1+incompatible h1:td4jdvLcExb4cBISKIpHuGoVXh+dVKhn2Um6rjCsSsg=
github.com/uber/jaeger-lib v2.4.1+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U=
github.com/vultr/govultr/v2 v2.17.2 h1:gej/rwr91Puc/tgh+j33p/BLR16UrIPnSr+AIwYWZQs=
github.com/weaveworks/promrus v1.2.0 h1:jOLf6pe6/vss4qGHjXmGz4oDJQA+AOCqEL3FvvZGz7M=
github.com/weaveworks/promrus v1.2.0/go.mod h1:SaE82+OJ91yqjrE1rsvBWVzNZKcHYFtMUyS1+Ogs/KA=
github.com/willf/bitset v1.1.11 h1:N7Z7E9UvjW+sGsEl7k/SJrvY2reP1A07MrGuCjIOjRE=
github.com/willf/bitset v1.1.11/go.mod h1:83CECat5yLh5zVOf4P1ErAgKA5UDvKtgyUABdr3+MjI=
github.com/willf/bloom v2.0.3+incompatible h1:QDacWdqcAUI1MPOwIQZRy9kOR7yxfyEmxX8Wdm2/JPA=
Expand Down
33 changes: 16 additions & 17 deletions modules/distributor/forwarder/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
zaplogfmt "github.com/jsternberg/zap-logfmt"
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
"github.com/open-telemetry/opentelemetry-collector-contrib/processor/filterprocessor"
"github.com/sirupsen/logrus"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/extension"
Expand Down Expand Up @@ -191,23 +190,23 @@ func (c consumerToForwarderAdapter) Capabilities() consumer.Capabilities {
return consumer.Capabilities{MutatesData: false}
}

func newLogger(level dslog.Level) *zap.Logger {
zapLevel := zapcore.InfoLevel

switch level.Logrus {
case logrus.PanicLevel:
zapLevel = zapcore.PanicLevel
case logrus.FatalLevel:
zapLevel = zapcore.FatalLevel
case logrus.ErrorLevel:
zapLevel = zapcore.ErrorLevel
case logrus.WarnLevel:
zapLevel = zapcore.WarnLevel
case logrus.InfoLevel:
zapLevel = zapcore.InfoLevel
case logrus.TraceLevel, logrus.DebugLevel:
zapLevel = zapcore.DebugLevel
func ZapLevel(level dslog.Level) zapcore.Level {
switch level.String() {
case "error":
return zapcore.ErrorLevel
case "warn":
return zapcore.WarnLevel
case "info":
return zapcore.InfoLevel
case "debug":
return zapcore.DebugLevel
default:
return zapcore.InfoLevel
}
}

func newLogger(level dslog.Level) *zap.Logger {
zapLevel := ZapLevel(level)

config := zap.NewProductionEncoderConfig()
config.EncodeTime = func(ts time.Time, encoder zapcore.PrimitiveArrayEncoder) {
Expand Down
20 changes: 2 additions & 18 deletions modules/distributor/receiver/shim.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/go-kit/log/level"
dslog "github.com/grafana/dskit/log"
"github.com/grafana/dskit/services"
"github.com/grafana/tempo/modules/distributor/forwarder"
zaplogfmt "github.com/jsternberg/zap-logfmt"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/jaegerreceiver"
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/kafkareceiver"
Expand All @@ -17,7 +18,6 @@ import (
"github.com/opentracing/opentracing-go"
prom_client "github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/sirupsen/logrus"
"go.opencensus.io/stats/view"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/confmap"
Expand Down Expand Up @@ -313,23 +313,7 @@ func (r *receiversShim) GetExporters() map[component.DataType]map[component.ID]c

// observability shims
func newLogger(level dslog.Level) *zap.Logger {
zapLevel := zapcore.InfoLevel

switch level.Logrus {
case logrus.PanicLevel:
zapLevel = zapcore.PanicLevel
case logrus.FatalLevel:
zapLevel = zapcore.FatalLevel
case logrus.ErrorLevel:
zapLevel = zapcore.ErrorLevel
case logrus.WarnLevel:
zapLevel = zapcore.WarnLevel
case logrus.InfoLevel:
zapLevel = zapcore.InfoLevel
case logrus.DebugLevel:
case logrus.TraceLevel:
zapLevel = zapcore.DebugLevel
}
zapLevel := forwarder.ZapLevel(level)

config := zap.NewProductionEncoderConfig()
config.EncodeTime = func(ts time.Time, encoder zapcore.PrimitiveArrayEncoder) {
Expand Down
47 changes: 10 additions & 37 deletions pkg/util/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +6,24 @@ import (
kitlog "github.com/go-kit/log"
"github.com/go-kit/log/level"
dslog "github.com/grafana/dskit/log"
"github.com/grafana/dskit/server"
)

// Logger is a shared go-kit logger.
// TODO: Change all components to take a non-global logger via their constructors.
// Prefer accepting a non-global logger as an argument.
var Logger = kitlog.NewNopLogger()

// InitLogger initialises the global gokit logger and overrides the
// default logger for the server.
func InitLogger(cfg *server.Config) {
logger := kitlog.NewLogfmtLogger(kitlog.NewSyncWriter(os.Stderr))
if cfg.LogFormat.String() == "json" {
logger = kitlog.NewJSONLogger(kitlog.NewSyncWriter(os.Stderr))
}
// InitLogger initialises the global gokit logger and returns that logger.
func InitLogger(logFormat string, logLevel dslog.Level) kitlog.Logger {
writer := kitlog.NewSyncWriter(os.Stderr)
logger := dslog.NewGoKitWithWriter(logFormat, writer)

// add support for level based logging
logger = level.NewFilter(logger, LevelFilter(cfg.LogLevel.String()))
// use UTC timestamps and skip 5 stack frames.
logger = kitlog.With(logger, "ts", kitlog.DefaultTimestampUTC, "caller", kitlog.Caller(5))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean to change the call stack depth here? Do we now report a different line is doing the logging?

Copy link
Author

@duricanikolic duricanikolic Aug 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @zalegrala,
The reason for that is the implementation of dslog.NewGoKitWithWriter(), that is now used to create the actual logger, and that replaces the “manual” creation that was used before.
I did a manual test to check the outcome and it prints the same source and line it used to print before my change.
I will prepare a Unit test that ensures the right thing is printed once I am back from PTO. I will also replace the usage of RateLimitedLogger with the one from dskit.


// use UTC timestamps
logger = kitlog.With(logger, "ts", kitlog.DefaultTimestampUTC)
// Must put the level filter last for efficiency.
logger = level.NewFilter(logger, logLevel.Option)

// when use util_log.Logger, skip 3 stack frames.
Logger = kitlog.With(logger, "caller", kitlog.Caller(3))

// cfg.Log wraps log function, skip 4 stack frames to get caller information.
// this works in go 1.12, but doesn't work in versions earlier.
// it will always shows the wrapper function generated by compiler
// marked <autogenerated> in old versions.
cfg.Log = dslog.GoKit(kitlog.With(logger, "caller", kitlog.Caller(4)))
}

// TODO: remove once weaveworks/common updates to go-kit/log
// -> we can then revert to using Level.Gokit
func LevelFilter(l string) level.Option {
switch l {
case "debug":
return level.AllowDebug()
case "info":
return level.AllowInfo()
case "warn":
return level.AllowWarn()
case "error":
return level.AllowError()
default:
return level.AllowAll()
}
Logger = logger
return logger
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 46 additions & 18 deletions vendor/github.com/aws/aws-sdk-go/aws/config.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading