diff --git a/cmd/armada-load-tester/cmd/loadtest.go b/cmd/armada-load-tester/cmd/loadtest.go index 5a2e301490e..f6c09622bcd 100644 --- a/cmd/armada-load-tester/cmd/loadtest.go +++ b/cmd/armada-load-tester/cmd/loadtest.go @@ -6,10 +6,10 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/client" "github.com/armadaproject/armada/pkg/client/domain" "github.com/armadaproject/armada/pkg/client/util" @@ -72,7 +72,7 @@ var loadtestCmd = &cobra.Command{ loadTestSpec := &domain.LoadTestSpecification{} err := util.BindJsonOrYaml(filePath, loadTestSpec) if err != nil { - log.Error(err) + log.Error(err.Error()) os.Exit(1) } diff --git a/cmd/armada-load-tester/cmd/root.go b/cmd/armada-load-tester/cmd/root.go index 5f2bcfb59c3..0803042410d 100644 --- a/cmd/armada-load-tester/cmd/root.go +++ b/cmd/armada-load-tester/cmd/root.go @@ -3,9 +3,9 @@ package cmd import ( "os" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/client" ) @@ -28,7 +28,7 @@ The location of this file can be passed in using --config argument or picked fro // This is called by main.main(). It only needs to happen once to the rootCmd. func Execute() { if err := rootCmd.Execute(); err != nil { - log.Error(err) + log.Error(err.Error()) os.Exit(1) } } @@ -37,7 +37,7 @@ var cfgFile string func initConfig() { if err := client.LoadCommandlineArgsFromConfigFile(cfgFile); err != nil { - log.Error(err) + log.Error(err.Error()) os.Exit(1) } } diff --git a/cmd/binoculars/main.go b/cmd/binoculars/main.go index 86389fbc91b..cd4e0221766 100644 --- a/cmd/binoculars/main.go +++ b/cmd/binoculars/main.go @@ -8,7 +8,6 @@ import ( "syscall" "github.com/grpc-ecosystem/grpc-gateway/runtime" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "google.golang.org/grpc" @@ -19,6 +18,7 @@ import ( "github.com/armadaproject/armada/internal/common/armadacontext" gateway "github.com/armadaproject/armada/internal/common/grpc" "github.com/armadaproject/armada/internal/common/health" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" api "github.com/armadaproject/armada/pkg/api/binoculars" ) @@ -35,7 +35,7 @@ func init() { } func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.BinocularsConfig diff --git a/cmd/eventingester/main.go b/cmd/eventingester/main.go index 93690f33b4d..c6cca9d2791 100644 --- a/cmd/eventingester/main.go +++ b/cmd/eventingester/main.go @@ -1,6 +1,7 @@ package main import ( + "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/eventingester" "github.com/spf13/pflag" @@ -24,7 +25,7 @@ func init() { } func main() { - common.ConfigureLogging() + logging.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.EventIngesterConfiguration diff --git a/cmd/executor/main.go b/cmd/executor/main.go index 04e53b73f41..75567416a1f 100644 --- a/cmd/executor/main.go +++ b/cmd/executor/main.go @@ -7,13 +7,13 @@ import ( "syscall" "github.com/prometheus/client_golang/prometheus" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/health" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/executor" "github.com/armadaproject/armada/internal/executor/configuration" @@ -31,7 +31,7 @@ func init() { } func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.ExecutorConfiguration @@ -61,7 +61,7 @@ func main() { ) defer shutdownMetricServer() - shutdown, wg := executor.StartUp(armadacontext.Background(), log.NewEntry(log.StandardLogger()), config) + shutdown, wg := executor.StartUp(armadacontext.Background(), config) go func() { <-shutdownChannel shutdown() diff --git a/cmd/fakeexecutor/main.go b/cmd/fakeexecutor/main.go index 831d56b650e..61b97a38d7e 100644 --- a/cmd/fakeexecutor/main.go +++ b/cmd/fakeexecutor/main.go @@ -5,12 +5,12 @@ import ( "os/signal" "syscall" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/executor/configuration" "github.com/armadaproject/armada/internal/executor/fake" @@ -29,7 +29,7 @@ func init() { } func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.ExecutorConfiguration @@ -54,7 +54,7 @@ func main() { shutdownMetricServer := common.ServeMetrics(config.Metric.Port) defer shutdownMetricServer() - shutdown, wg := fake.StartUp(config, nodes) + shutdown, wg := fake.StartUp(armadacontext.Background(), config, nodes) go func() { <-shutdownChannel shutdown() diff --git a/cmd/lookoutingesterv2/dbloadtester/main.go b/cmd/lookoutingesterv2/dbloadtester/main.go index 8daf0960f2f..95e4b834673 100644 --- a/cmd/lookoutingesterv2/dbloadtester/main.go +++ b/cmd/lookoutingesterv2/dbloadtester/main.go @@ -4,13 +4,13 @@ import ( "fmt" "time" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "sigs.k8s.io/yaml" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/app" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/lookoutingesterv2/configuration" "github.com/armadaproject/armada/internal/lookoutingesterv2/dbloadtester" ) @@ -45,7 +45,7 @@ const ReportTemplate string = ` ` func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.LookoutIngesterV2Configuration diff --git a/cmd/lookoutingesterv2/main.go b/cmd/lookoutingesterv2/main.go index b60e58f6241..70be8d713f2 100644 --- a/cmd/lookoutingesterv2/main.go +++ b/cmd/lookoutingesterv2/main.go @@ -1,11 +1,11 @@ package main import ( - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/armadaproject/armada/internal/common" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/lookoutingesterv2" "github.com/armadaproject/armada/internal/lookoutingesterv2/benchmark" "github.com/armadaproject/armada/internal/lookoutingesterv2/configuration" @@ -27,7 +27,7 @@ func init() { } func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.LookoutIngesterV2Configuration diff --git a/cmd/lookoutv2/main.go b/cmd/lookoutv2/main.go index 48d48bc97b5..584aac3edf3 100644 --- a/cmd/lookoutv2/main.go +++ b/cmd/lookoutv2/main.go @@ -5,7 +5,6 @@ import ( "os/signal" "syscall" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" "k8s.io/utils/clock" @@ -13,6 +12,7 @@ import ( "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/database" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/lookoutv2" "github.com/armadaproject/armada/internal/lookoutv2/configuration" @@ -117,7 +117,7 @@ func prune(ctx *armadacontext.Context, config configuration.LookoutV2Config) { } func main() { - common.ConfigureLogging() + log.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config configuration.LookoutV2Config @@ -148,7 +148,7 @@ func main() { restapi.UIConfig = config.UIConfig if err := lookoutv2.Serve(config); err != nil { - log.Error(err) + log.Error(err.Error()) os.Exit(1) } } diff --git a/cmd/scheduler/cmd/migrate_database.go b/cmd/scheduler/cmd/migrate_database.go index 9e48f5fba5f..407048e31dc 100644 --- a/cmd/scheduler/cmd/migrate_database.go +++ b/cmd/scheduler/cmd/migrate_database.go @@ -4,11 +4,11 @@ import ( "time" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/database" + log "github.com/armadaproject/armada/internal/common/logging" schedulerdb "github.com/armadaproject/armada/internal/scheduler/database" ) diff --git a/cmd/scheduler/main.go b/cmd/scheduler/main.go index f895b3301cd..7249a2c54d9 100644 --- a/cmd/scheduler/main.go +++ b/cmd/scheduler/main.go @@ -6,10 +6,11 @@ import ( "github.com/armadaproject/armada/cmd/scheduler/cmd" "github.com/armadaproject/armada/internal/common" + "github.com/armadaproject/armada/internal/common/logging" ) func main() { - common.ConfigureLogging() + logging.MustConfigureApplicationLogging() common.BindCommandlineArguments() if err := cmd.RootCmd().Execute(); err != nil { os.Exit(1) diff --git a/cmd/scheduleringester/main.go b/cmd/scheduleringester/main.go index 9aa5207a2ce..527d9d2d627 100644 --- a/cmd/scheduleringester/main.go +++ b/cmd/scheduleringester/main.go @@ -4,6 +4,8 @@ import ( "fmt" "os" + "github.com/armadaproject/armada/internal/common/logging" + "github.com/spf13/pflag" "github.com/spf13/viper" @@ -24,7 +26,7 @@ func init() { } func main() { - common.ConfigureLogging() + logging.MustConfigureApplicationLogging() common.BindCommandlineArguments() var config scheduleringester.Configuration diff --git a/cmd/server/main.go b/cmd/server/main.go index c7318dcecc0..df909f5649d 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -8,7 +8,6 @@ import ( "syscall" "time" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -17,6 +16,7 @@ import ( gateway "github.com/armadaproject/armada/internal/common/grpc" "github.com/armadaproject/armada/internal/common/health" "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/server" "github.com/armadaproject/armada/internal/server/configuration" @@ -35,7 +35,7 @@ func init() { } func main() { - common.ConfigureLogging() + logging.MustConfigureApplicationLogging() common.BindCommandlineArguments() // TODO Load relevant config in one place: don't use viper here and in LoadConfig. @@ -117,6 +117,6 @@ func main() { }() if err := g.Wait(); err != nil { - logging.WithStacktrace(log.NewEntry(log.StandardLogger()), err).Error("Armada server shut down") + logging.WithStacktrace(err).Error("Armada server shut down") } } diff --git a/cmd/simulator/cmd/root.go b/cmd/simulator/cmd/root.go index a1c815633c5..1e758e23afb 100644 --- a/cmd/simulator/cmd/root.go +++ b/cmd/simulator/cmd/root.go @@ -6,13 +6,12 @@ import ( "runtime/pprof" "time" - "github.com/armadaproject/armada/internal/scheduler/simulator/sink" - - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/scheduler/simulator" + "github.com/armadaproject/armada/internal/scheduler/simulator/sink" "github.com/armadaproject/armada/internal/scheduler/testfixtures" ) @@ -139,11 +138,11 @@ func runSimulations(cmd *cobra.Command, args []string) error { log.Infof("Will write profiling information to %s", profilingFile) f, err := os.Create(profilingFile) if err != nil { - log.Fatal(err) + log.Fatal(err.Error()) } err = pprof.StartCPUProfile(f) if err != nil { - log.Fatal(err) + log.Fatal(err.Error()) } defer pprof.StopCPUProfile() } diff --git a/config/logging.yaml b/config/logging.yaml new file mode 100644 index 00000000000..30f04cac185 --- /dev/null +++ b/config/logging.yaml @@ -0,0 +1,13 @@ +console: + level: INFO + format: text +file: + enabled: true + level: INFO + format: json + logfile: app.log + rotation: + maxSizeMb: 5 + maxBackups: 3 + maxAgeDays: 7 + compress: false diff --git a/go.mod b/go.mod index a650830b832..665ec23c7c0 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/golang/protobuf v1.5.4 github.com/google/go-cmp v0.6.0 // indirect github.com/google/uuid v1.6.0 - github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 github.com/grpc-ecosystem/grpc-gateway v1.16.0 github.com/hashicorp/go-memdb v1.3.4 @@ -37,12 +36,10 @@ require ( github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.20.5 github.com/renstrom/shortuuid v3.0.0+incompatible - github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.19.0 github.com/stretchr/testify v1.9.0 - github.com/weaveworks/promrus v1.2.0 golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c golang.org/x/net v0.30.0 golang.org/x/oauth2 v0.23.0 @@ -72,6 +69,8 @@ require ( github.com/go-playground/validator/v10 v10.15.4 github.com/gogo/status v1.1.1 github.com/goreleaser/goreleaser v1.24.0 + github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 + github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.2.0 github.com/jackc/pgx/v5 v5.5.4 github.com/jessevdk/go-flags v1.5.0 github.com/magefile/mage v1.14.0 @@ -83,6 +82,7 @@ require ( github.com/segmentio/fasthash v1.0.3 github.com/xitongsys/parquet-go v1.6.2 go.uber.org/mock v0.5.0 + go.uber.org/zap v1.27.0 golang.org/x/term v0.25.0 golang.org/x/time v0.5.0 google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 @@ -182,6 +182,7 @@ require ( github.com/rivo/uniseg v0.4.2 // indirect github.com/sagikazarmark/locafero v0.6.0 // indirect github.com/sagikazarmark/slog-shim v0.1.0 // indirect + github.com/sirupsen/logrus v1.9.3 // indirect github.com/sourcegraph/conc v0.3.0 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect github.com/spf13/afero v1.11.0 // indirect @@ -204,6 +205,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20241015192408-796eee8c2d53 // indirect google.golang.org/protobuf v1.35.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect + gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/cli-runtime v0.26.15 // indirect diff --git a/go.sum b/go.sum index ac75f1140ad..904d5da9953 100644 --- a/go.sum +++ b/go.sum @@ -73,7 +73,6 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk= github.com/bahlo/generic-list-go v0.2.0/go.mod h1:2KvAjgMlE5NNynlg/5iLrrCCZ2+5xWbdbCW3pNTGyYg= -github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/benbjohnson/immutable v0.4.3 h1:GYHcksoJ9K6HyAUpGxwZURrbTkXA0Dh4otXGqbhdrjA= github.com/benbjohnson/immutable v0.4.3/go.mod h1:qJIKKSmdqz1tVzNtst1DZzvaqOU1onk1rc03IeM3Owk= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= @@ -170,8 +169,6 @@ github.com/go-errors/errors v1.0.1/go.mod h1:f4zRHt4oKfwPJE5k8C9vpYG+aDHdBFUsgrm github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= -github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY= -github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -208,7 +205,6 @@ github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91 github.com/go-playground/validator/v10 v10.15.4 h1:zMXza4EpOdooxPel5xDqXEdXG5r+WggpvnAKMsalBjs= github.com/go-playground/validator/v10 v10.15.4/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= -github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y= github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 h1:ZpnhV/YsD2/4cESfV5+Hoeu/iUR3ruzNvZ+yQfO03a0= @@ -298,8 +294,10 @@ github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl github.com/gorilla/mux v1.7.3/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 h1:pdN6V1QBWetyv/0+wjACpqVH+eVULgEjkurDLq3goeM= github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= -github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI= -github.com/grpc-ecosystem/go-grpc-middleware v1.4.0/go.mod h1:g5qyo/la0ALbONm6Vbp88Yd8NsDy6rZz+RcrMPxvld8= +github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1 h1:qnpSQwGEnkcRpTqNOIR6bJbR0gAorgP9CSALpRcKoAA= +github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.1/go.mod h1:lXGCsh6c22WGtjr+qGHj1otzZpV/1kwTMAqkwZsnWRU= +github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.2.0 h1:kQ0NI7W1B3HwiN5gAYtY+XFItDPbLBwYRxAqbFTyDes= +github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.2.0/go.mod h1:zrT2dxOAjNFPRGjTUe2Xmb4q4YdUwVvQFV6xiCSf+z0= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 h1:Ovs26xHkKqVztRpIrF/92BcuyuQ/YW4NSIpoGtfXNho= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo= @@ -369,7 +367,6 @@ github.com/klauspost/compress v1.13.1/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8 github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47etQ6QUkpK6sk= github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= -github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= @@ -462,7 +459,6 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0 h1:8SG7/vwALn54lVB/0yZ/MMwhFrPYtpEHQb2IpWsCzug= github.com/opencontainers/image-spec v1.1.0/go.mod h1:W4s4sFTMaBeK1BQLXbG4AdM2szdn85PY75RI83NrTrM= -github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc= github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ= github.com/pborman/getopt v0.0.0-20180729010549-6fdd0a2c7117/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= @@ -475,7 +471,6 @@ github.com/pierrec/lz4 v2.0.5+incompatible h1:2xWsjqPFWcplujydGg4WmhC/6fZqK42wMM github.com/pierrec/lz4 v2.0.5+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pierrec/lz4/v4 v4.1.8 h1:ieHkV+i2BRzngO4Wd/3HGowuZStgq6QkPsD1eolNAO4= github.com/pierrec/lz4/v4 v4.1.8/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4= -github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= @@ -521,7 +516,6 @@ github.com/shirou/gopsutil/v3 v3.23.12 h1:z90NtUkp3bMtmICZKpC4+WaknU1eXtp5vtbQ11 github.com/shirou/gopsutil/v3 v3.23.12/go.mod h1:1FrWgea594Jp7qmjHUUPlJDTPgcsb9mGnXDxavtikzM= github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM= github.com/shoenig/go-m1cpu v0.1.6/go.mod h1:1JJMcUBvfNwpq05QDQVAnx3gUHr9IYF7GNg9SUEw2VQ= -github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo= @@ -541,7 +535,6 @@ github.com/spf13/viper v1.19.0 h1:RWq5SEjt8o25SROyN3z2OrDB9l7RPd3lwTWU8EcEdcI= github.com/spf13/viper v1.19.0/go.mod h1:GQUN9bilAbhU/jgc1bKs99f/suXKeUMct8Adx5+Ntkg= github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= @@ -567,8 +560,6 @@ github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFA github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= github.com/tklauser/numcpus v0.6.1/go.mod h1:1XfjsgE2zo8GVw7POkMbHENHzVg3GzmoZ9fESEdAacY= -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/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/xdg-go/pbkdf2 v1.0.0/go.mod h1:jrpuAogTd400dnrH08LKmI/xc1MbPOebTwRqcT5RDeI= @@ -609,16 +600,16 @@ go.opentelemetry.io/otel/trace v1.24.0 h1:CsKnnL4dUAr/0llH9FKuc698G04IrpWV0MQA/Y go.opentelemetry.io/otel/trace v1.24.0/go.mod h1:HPc3Xr/cOApsBI154IU0OI0HJexz+aw5uPdbs3UCjNU= go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 h1:+FNtrFTmVw0YZGpBGX56XDee331t6JAXeK2bcyhLOOc= go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5/go.mod h1:nmDLcffg48OtT/PSW0Hg7FvpRQsQh5OSqIylirxKC7o= -go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= -go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/mock v0.5.0 h1:KAMbZvZPyBPWgD14IrIQ38QCyjwpvVVV6K/bHl1IwQU= go.uber.org/mock v0.5.0/go.mod h1:ge71pBPLYDk7QIi1LupWxdAykm7KIEFchiOqd6z7qMM= -go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= -go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= +go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= +go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= golang.org/x/crypto v0.0.0-20180723164146-c126467f60eb/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -712,7 +703,6 @@ golang.org/x/sys v0.0.0-20190130150945-aca44879d564/go.mod h1:STP8DvDyc/dI5b8T5h golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190507160741-ecd444e8653b/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -734,7 +724,6 @@ golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -776,7 +765,6 @@ golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgw golang.org/x/tools v0.0.0-20190816200558-6889da9d5479/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191012152004-8de300cfc20a/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191113191852-77e3bb0ad9e7/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191115202509-3a792d9c32b2/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= @@ -832,7 +820,6 @@ google.golang.org/genproto v0.0.0-20200122232147-0452cf42e150/go.mod h1:n3cpQtvx google.golang.org/genproto v0.0.0-20200204135345-fa8e72b47b90/go.mod h1:GmwEX6Z4W5gMy59cAlVYjN9JhxgbQH6Gn+gFDQe2lzA= google.golang.org/genproto v0.0.0-20200212174721-66ed5ce911ce/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c= google.golang.org/genproto v0.0.0-20200224152610-e50cd9704f63/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c= -google.golang.org/genproto v0.0.0-20200423170343-7949de9c1215/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c= google.golang.org/genproto v0.0.0-20200513103714-09dca8ec2884/go.mod h1:55QSHmfGQM9UVYDPBsyGGes0y52j32PQ3BqQfXhyH3c= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= google.golang.org/genproto v0.0.0-20201019141844-1ed22bb0c154/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= @@ -851,7 +838,6 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.26.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.27.1/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= -google.golang.org/grpc v1.29.1/go.mod h1:itym6AZVZYACWQqET3MqgPpjcuV5QH3BxFS3IjizoKk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.67.1 h1:zWnc1Vrcno+lHZCOofnIMvycFcc0QRGIzm9dhnDX68E= google.golang.org/grpc v1.67.1/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA= @@ -884,6 +870,8 @@ gopkg.in/jcmturner/goidentity.v3 v3.0.0/go.mod h1:oG2kH0IvSYNIu80dVAyu/yoefjq1mN gopkg.in/jcmturner/gokrb5.v7 v7.3.0/go.mod h1:l8VISx+WGYp+Fp7KRbsiUuXTTOnxIc3Tuvyavf11/WM= gopkg.in/jcmturner/rpc.v1 v1.1.0/go.mod h1:YIdkC4XfD6GXbzje11McwsDuOlZQSb9W4vfLvuNnlv8= gopkg.in/natefinch/lumberjack.v2 v2.0.0/go.mod h1:l0ndWWf7gzL7RNwBG7wST/UCcT4T24xpD6X8LsfU/+k= +gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= +gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= gopkg.in/square/go-jose.v2 v2.4.1/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= gopkg.in/square/go-jose.v2 v2.6.0 h1:NGk74WTnPKBNUhNzQX7PYcTLUjoq7mzKk2OKbvwk2iI= gopkg.in/square/go-jose.v2 v2.6.0/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI= @@ -898,7 +886,6 @@ gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/binoculars/server.go b/internal/binoculars/server.go index bf14abc18f2..d1d604f7c67 100644 --- a/internal/binoculars/server.go +++ b/internal/binoculars/server.go @@ -5,7 +5,6 @@ import ( "sync" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/binoculars/configuration" "github.com/armadaproject/armada/internal/binoculars/server" @@ -13,6 +12,7 @@ import ( "github.com/armadaproject/armada/internal/common/auth" "github.com/armadaproject/armada/internal/common/cluster" grpcCommon "github.com/armadaproject/armada/internal/common/grpc" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/api/binoculars" ) diff --git a/internal/binoculars/service/cordon_test.go b/internal/binoculars/service/cordon_test.go index 43197004d9e..1245b2ee1f1 100644 --- a/internal/binoculars/service/cordon_test.go +++ b/internal/binoculars/service/cordon_test.go @@ -6,7 +6,8 @@ import ( "fmt" "testing" - "github.com/sirupsen/logrus" + "github.com/armadaproject/armada/internal/common/logging" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -81,7 +82,7 @@ func TestCordonNode(t *testing.T) { cordonService, client := setupTest(t, cordonConfig, FakePermissionChecker{ReturnValue: true}) ctx := auth.WithPrincipal(context.Background(), principal) - err := cordonService.CordonNode(armadacontext.New(ctx, logrus.NewEntry(logrus.New())), &binoculars.CordonRequest{ + err := cordonService.CordonNode(armadacontext.New(ctx, logging.StdLogger()), &binoculars.CordonRequest{ NodeName: defaultNode.Name, }) assert.Nil(t, err) diff --git a/internal/binoculars/service/logs.go b/internal/binoculars/service/logs.go index 8bc0f38d1be..34ca6b417ce 100644 --- a/internal/binoculars/service/logs.go +++ b/internal/binoculars/service/logs.go @@ -5,13 +5,13 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/auth" "github.com/armadaproject/armada/internal/common/cluster" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/api/binoculars" ) diff --git a/internal/common/armadacontext/armada_context.go b/internal/common/armadacontext/armada_context.go index 665c19c51f9..f77a413c1d0 100644 --- a/internal/common/armadacontext/armada_context.go +++ b/internal/common/armadacontext/armada_context.go @@ -4,59 +4,119 @@ import ( "context" "time" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" - "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" + + "github.com/armadaproject/armada/internal/common/logging" ) // Context is an extension of Go's context which also includes a logger. This allows us to pass round a contextual logger // while retaining type-safety type Context struct { context.Context - logrus.FieldLogger + logger *logging.Logger +} + +// Debug logs a message at level Debug +func (ctx *Context) Debug(msg string) { + ctx.logger.Debug(msg) +} + +// Info logs a message at level Info +func (ctx *Context) Info(msg string) { + ctx.logger.Info(msg) +} + +// Warn logs a message at level Warn +func (ctx *Context) Warn(msg string) { + ctx.logger.Warn(msg) +} + +// Error logs a message at level Error +func (ctx *Context) Error(msg string) { + ctx.logger.Error(msg) +} + +// Panic logs a message at level Panic +func (ctx *Context) Panic(msg string) { + ctx.logger.Panic(msg) +} + +// Fatal logs a message at level Fatal then the process will exit with status set to 1. +func (ctx *Context) Fatal(msg string) { + ctx.logger.Fatal(msg) +} + +// Debugf logs a message at level Debug. +func (ctx *Context) Debugf(format string, args ...interface{}) { + ctx.logger.Debugf(format, args...) +} + +// Infof logs a message at level Info. +func (ctx *Context) Infof(format string, args ...interface{}) { + ctx.logger.Infof(format, args...) +} + +// Warnf logs a message at level Warn. +func (ctx *Context) Warnf(format string, args ...interface{}) { + ctx.logger.Warnf(format, args...) +} + +// Errorf logs a message at level Error. +func (ctx *Context) Errorf(format string, args ...interface{}) { + ctx.logger.Errorf(format, args...) +} + +// Fatalf logs a message at level Fatal. +func (ctx *Context) Fatalf(format string, args ...interface{}) { + ctx.logger.Fatalf(format, args...) } // Background creates an empty context with a default logger. It is analogous to context.Background() func Background() *Context { return &Context{ - Context: context.Background(), - FieldLogger: logrus.NewEntry(logrus.StandardLogger()), + Context: context.Background(), + logger: logging.StdLogger().WithCallerSkip(1), } } // TODO creates an empty context with a default logger. It is analogous to context.TODO() func TODO() *Context { return &Context{ - Context: context.TODO(), - FieldLogger: logrus.NewEntry(logrus.StandardLogger()), + Context: context.TODO(), + logger: logging.StdLogger().WithCallerSkip(1), } } -// FromGrpcCtx creates a context where the logger is extracted via ctxlogrus's Extract() method. -// Note that this will result in a no-op logger if a logger hasn't already been inserted into the context via ctxlogrus +// FromGrpcCtx Converts a context.Context to an armadacontext.Context func FromGrpcCtx(ctx context.Context) *Context { armadaCtx, ok := ctx.(*Context) if ok { return armadaCtx } - log := ctxlogrus.Extract(ctx) - return New(ctx, log) + logger := logging.StdLogger().WithCallerSkip(1). + WithField("user", ctx.Value("user")). + WithField("requestId", ctx.Value("requestId")) + return New(ctx, logger) } // New returns an armada context that encapsulates both a go context and a logger -func New(ctx context.Context, log *logrus.Entry) *Context { +func New(ctx context.Context, log *logging.Logger) *Context { return &Context{ - Context: ctx, - FieldLogger: log, + Context: ctx, + logger: log, } } +func (ctx *Context) Logger() *logging.Logger { + return ctx.logger.WithCallerSkip(-1) +} + // WithCancel returns a copy of parent with a new Done channel. It is analogous to context.WithCancel() func WithCancel(parent *Context) (*Context, context.CancelFunc) { c, cancel := context.WithCancel(parent.Context) return &Context{ - Context: c, - FieldLogger: parent.FieldLogger, + Context: c, + logger: parent.logger, }, cancel } @@ -65,8 +125,8 @@ func WithCancel(parent *Context) (*Context, context.CancelFunc) { func WithDeadline(parent *Context, d time.Time) (*Context, context.CancelFunc) { c, cancel := context.WithDeadline(parent.Context, d) return &Context{ - Context: c, - FieldLogger: parent.FieldLogger, + Context: c, + logger: parent.logger, }, cancel } @@ -76,18 +136,18 @@ func WithTimeout(parent *Context, timeout time.Duration) (*Context, context.Canc } // WithLogField returns a copy of parent with the supplied key-value added to the logger -func WithLogField(parent *Context, key string, val interface{}) *Context { +func WithLogField(parent *Context, key string, val any) *Context { return &Context{ - Context: parent.Context, - FieldLogger: parent.FieldLogger.WithField(key, val), + Context: parent.Context, + logger: parent.logger.WithField(key, val), } } // WithLogFields returns a copy of parent with the supplied key-values added to the logger -func WithLogFields(parent *Context, fields logrus.Fields) *Context { +func WithLogFields(parent *Context, fields map[string]any) *Context { return &Context{ - Context: parent.Context, - FieldLogger: parent.FieldLogger.WithFields(fields), + Context: parent.Context, + logger: parent.logger.WithFields(fields), } } @@ -95,8 +155,8 @@ func WithLogFields(parent *Context, fields logrus.Fields) *Context { // val. It is analogous to context.WithValue() func WithValue(parent *Context, key, val any) *Context { return &Context{ - Context: context.WithValue(parent, key, val), - FieldLogger: parent.FieldLogger, + Context: context.WithValue(parent, key, val), + logger: parent.logger, } } @@ -105,7 +165,7 @@ func WithValue(parent *Context, key, val any) *Context { func ErrGroup(ctx *Context) (*errgroup.Group, *Context) { group, goctx := errgroup.WithContext(ctx) return group, &Context{ - Context: goctx, - FieldLogger: ctx.FieldLogger, + Context: goctx, + logger: ctx.logger, } } diff --git a/internal/common/armadacontext/armada_context_test.go b/internal/common/armadacontext/armada_context_test.go index 4cda401c1b1..2f9e9f1caad 100644 --- a/internal/common/armadacontext/armada_context_test.go +++ b/internal/common/armadacontext/armada_context_test.go @@ -5,27 +5,24 @@ import ( "testing" "time" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" + + "github.com/armadaproject/armada/internal/common/logging" ) -var defaultLogger = logrus.WithField("foo", "bar") +var defaultLogger = logging.StdLogger().WithField("foo", "bar") func TestNew(t *testing.T) { ctx := New(context.Background(), defaultLogger) - require.Equal(t, defaultLogger, ctx.FieldLogger) + require.Equal(t, defaultLogger, ctx.logger) require.Equal(t, context.Background(), ctx.Context) } -func TestFromGrpcContext(t *testing.T) { - grpcCtx := ctxlogrus.ToContext(context.Background(), defaultLogger) - ctx := FromGrpcCtx(grpcCtx) - require.Equal(t, grpcCtx, ctx.Context) - require.Equal(t, defaultLogger, ctx.FieldLogger) -} - func TestBackground(t *testing.T) { ctx := Background() require.Equal(t, ctx.Context, context.Background()) @@ -37,15 +34,38 @@ func TestTODO(t *testing.T) { } func TestWithLogField(t *testing.T) { - ctx := WithLogField(Background(), "fish", "chips") + logger, observedLogs := testLogger() + ctx := WithLogField(New(context.Background(), logger), "fish", "chips") require.Equal(t, context.Background(), ctx.Context) - require.Equal(t, logrus.Fields{"fish": "chips"}, ctx.FieldLogger.(*logrus.Entry).Data) + + ctx.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "chips", logEntry.ContextMap()["fish"]) } func TestWithLogFields(t *testing.T) { - ctx := WithLogFields(Background(), logrus.Fields{"fish": "chips", "salt": "pepper"}) + logger, observedLogs := testLogger() + ctx := WithLogFields(New(context.Background(), logger), map[string]any{"fish": "chips", "salt": "pepper"}) require.Equal(t, context.Background(), ctx.Context) - require.Equal(t, logrus.Fields{"fish": "chips", "salt": "pepper"}, ctx.FieldLogger.(*logrus.Entry).Data) + + ctx.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "chips", logEntry.ContextMap()["fish"]) + assert.Equal(t, "pepper", logEntry.ContextMap()["salt"]) } func TestWithTimeout(t *testing.T) { @@ -87,3 +107,10 @@ func quiescent(t *testing.T) time.Duration { const arbitraryCleanupMargin = 1 * time.Second return time.Until(deadline) - arbitraryCleanupMargin } + +func testLogger() (*logging.Logger, *observer.ObservedLogs) { + core, observedLogs := observer.New(zapcore.DebugLevel) + baseLogger := zap.New(core) + logger := logging.FromZap(baseLogger) + return logger, observedLogs +} diff --git a/internal/common/armadaerrors/errors_test.go b/internal/common/armadaerrors/errors_test.go index fa70cf43e32..da72a6458c0 100644 --- a/internal/common/armadaerrors/errors_test.go +++ b/internal/common/armadaerrors/errors_test.go @@ -9,7 +9,7 @@ import ( "time" "github.com/apache/pulsar-client-go/pulsar" - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware/v2" "github.com/pkg/errors" "github.com/redis/go-redis/v9" "github.com/stretchr/testify/assert" diff --git a/internal/common/auth/common.go b/internal/common/auth/common.go index f3bd468cffd..604487bdffd 100644 --- a/internal/common/auth/common.go +++ b/internal/common/auth/common.go @@ -4,11 +4,9 @@ import ( "context" "net/http" + "github.com/grpc-ecosystem/go-grpc-middleware/v2/metadata" "golang.org/x/exp/slices" - grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" - "github.com/grpc-ecosystem/go-grpc-middleware/util/metautils" - "github.com/armadaproject/armada/internal/common/util" ) @@ -120,16 +118,13 @@ type AuthService interface { // request context for logging purposes. func CreateGrpcMiddlewareAuthFunction(authService AuthService) func(ctx context.Context) (context.Context, error) { return func(ctx context.Context) (context.Context, error) { - authHeader := metautils.ExtractIncoming(ctx).Get("authorization") + authHeader := metadata.ExtractIncoming(ctx).Get("authorization") principal, err := authService.Authenticate(ctx, authHeader) if err != nil { return nil, err } - // record username for request logging - grpc_ctxtags.Extract(ctx).Set("user", principal.GetName()) - grpc_ctxtags.Extract(ctx).Set("authService", principal.GetAuthMethod()) - + ctx = context.WithValue(ctx, "user", principal.GetName()) return WithPrincipal(ctx, principal), nil } } @@ -150,11 +145,6 @@ func CreateHttpMiddlewareAuthFunction(authService AuthService) func(w http.Respo http.Error(w, "auth error:"+err.Error(), http.StatusInternalServerError) return nil, err } - - // record username for request logging - grpc_ctxtags.Extract(ctx).Set("user", principal.GetName()) - grpc_ctxtags.Extract(ctx).Set("authService", principal.GetAuthMethod()) - return WithPrincipal(ctx, principal), nil } } diff --git a/internal/common/auth/kubernetes_test.go b/internal/common/auth/kubernetes_test.go index 01da9d02b1b..d29ceb4c17b 100644 --- a/internal/common/auth/kubernetes_test.go +++ b/internal/common/auth/kubernetes_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/grpc-ecosystem/go-grpc-middleware/util/metautils" + grpc_metadata "github.com/grpc-ecosystem/go-grpc-middleware/v2/metadata" "github.com/patrickmn/go-cache" "github.com/stretchr/testify/assert" authv1 "k8s.io/api/authentication/v1" @@ -151,7 +151,7 @@ func TestAuthenticateKubernetes(t *testing.T) { // Create authentication context payload := createKubernetesAuthPayload(testToken, testCA) ctx := context.Background() - metadata := metautils.ExtractIncoming(ctx) + metadata := grpc_metadata.ExtractIncoming(ctx) metadata.Set("authorization", payload) ctx = metadata.ToIncoming(ctx) diff --git a/internal/common/certs/cached_certificate.go b/internal/common/certs/cached_certificate.go index 72b7f6ea250..10ea5ea27da 100644 --- a/internal/common/certs/cached_certificate.go +++ b/internal/common/certs/cached_certificate.go @@ -6,9 +6,8 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" - "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" ) type CachedCertificateService struct { diff --git a/internal/common/cluster/kubernetes_client.go b/internal/common/cluster/kubernetes_client.go index 1c280fa55a6..9af32096d6e 100644 --- a/internal/common/cluster/kubernetes_client.go +++ b/internal/common/cluster/kubernetes_client.go @@ -2,13 +2,13 @@ package cluster import ( "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/util/flowcontrol" "github.com/armadaproject/armada/internal/common/armadaerrors" + log "github.com/armadaproject/armada/internal/common/logging" ) type KubernetesClientProvider interface { diff --git a/internal/common/etcdhealth/etcdhealth.go b/internal/common/etcdhealth/etcdhealth.go index 49be27a22fe..1d836c520c1 100644 --- a/internal/common/etcdhealth/etcdhealth.go +++ b/internal/common/etcdhealth/etcdhealth.go @@ -6,11 +6,9 @@ import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/healthmonitor" - "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/metrics" ) @@ -184,10 +182,9 @@ func (srv *EtcdReplicaHealthMonitor) sizeFraction() float64 { return srv.etcdSizeBytes / srv.etcdCapacityBytes } -func (srv *EtcdReplicaHealthMonitor) Run(ctx *armadacontext.Context, log *logrus.Entry) error { - log = log.WithField("service", "EtcdHealthMonitor") - log.Info("starting etcd health monitor") - defer log.Info("stopping etcd health monitor") +func (srv *EtcdReplicaHealthMonitor) Run(ctx *armadacontext.Context) error { + ctx.Info("starting etcd health monitor") + defer ctx.Info("stopping etcd health monitor") ticker := time.NewTicker(srv.scrapeInterval) defer ticker.Stop() for { @@ -196,24 +193,24 @@ func (srv *EtcdReplicaHealthMonitor) Run(ctx *armadacontext.Context, log *logrus return ctx.Err() case <-ticker.C: t := time.Now() - metrics, err := srv.metricsProvider.Collect(ctx, log) + metrics, err := srv.metricsProvider.Collect(ctx) srv.mu.Lock() srv.timeOfMostRecentCollectionAttempt = time.Now() if err != nil { - logging.WithStacktrace(log, err).Errorf("failed to scrape etcd metrics from %s", srv.name) + ctx.Logger().WithStacktrace(err).Errorf("failed to scrape etcd metrics from %s", srv.name) } else { success := true if err := srv.setSizeInUseBytesFromMetrics(metrics); err != nil { success = false - logging.WithStacktrace(log, err).Errorf("failed to scrape etcd metrics from %s", srv.name) + ctx.Logger().WithStacktrace(err).Errorf("failed to scrape etcd metrics from %s", srv.name) } if err := srv.setSizeBytesFromMetrics(metrics); err != nil { success = false - logging.WithStacktrace(log, err).Errorf("failed to scrape etcd metrics from %s", srv.name) + ctx.Logger().WithStacktrace(err).Errorf("failed to scrape etcd metrics from %s", srv.name) } if err := srv.setCapacityBytesFromMetrics(metrics); err != nil { success = false - logging.WithStacktrace(log, err).Errorf("failed to scrape etcd metrics from %s", srv.name) + ctx.Logger().WithStacktrace(err).Errorf("failed to scrape etcd metrics from %s", srv.name) } if success { srv.timeOfMostRecentSuccessfulCollectionAttempt = srv.timeOfMostRecentCollectionAttempt diff --git a/internal/common/etcdhealth/etcdhealth_test.go b/internal/common/etcdhealth/etcdhealth_test.go index 474d4df0e3a..038277e4227 100644 --- a/internal/common/etcdhealth/etcdhealth_test.go +++ b/internal/common/etcdhealth/etcdhealth_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/armadaproject/armada/internal/common/armadacontext" @@ -26,7 +25,7 @@ func TestEtcdReplicaHealthMonitor(t *testing.T) { ctx, cancel := armadacontext.WithCancel(armadacontext.Background()) defer cancel() g, ctx := armadacontext.ErrGroup(ctx) - g.Go(func() error { return hm.Run(ctx, logrus.NewEntry(logrus.New())) }) + g.Go(func() error { return hm.Run(ctx) }) // Should still be unavailable due to missing metrics. hm.BlockUntilNextMetricsCollection(ctx) diff --git a/internal/common/eventutil/eventutil.go b/internal/common/eventutil/eventutil.go index 27fc7ef6f55..fa126b155e5 100644 --- a/internal/common/eventutil/eventutil.go +++ b/internal/common/eventutil/eventutil.go @@ -6,13 +6,13 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "github.com/armadaproject/armada/internal/common/armadaerrors" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/pkg/api" diff --git a/internal/common/grpc/gateway.go b/internal/common/grpc/gateway.go index 8b6e59dc2ad..9ae6aeddd09 100644 --- a/internal/common/grpc/gateway.go +++ b/internal/common/grpc/gateway.go @@ -10,11 +10,12 @@ import ( "github.com/go-openapi/runtime/middleware" "github.com/grpc-ecosystem/grpc-gateway/runtime" - log "github.com/sirupsen/logrus" "golang.org/x/exp/slices" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" + + log "github.com/armadaproject/armada/internal/common/logging" ) // CreateGatewayHandler configures the gRPC API gateway diff --git a/internal/common/grpc/grpc.go b/internal/common/grpc/grpc.go index c4f858f3ba1..9cc844a3152 100644 --- a/internal/common/grpc/grpc.go +++ b/internal/common/grpc/grpc.go @@ -1,6 +1,7 @@ package grpc import ( + "context" "crypto/tls" "fmt" "net" @@ -8,13 +9,12 @@ import ( "sync" "time" - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" - grpc_auth "github.com/grpc-ecosystem/go-grpc-middleware/auth" - grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" - grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" - grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" - grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" - log "github.com/sirupsen/logrus" + grpc_prometheus "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus" + grpc_auth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" + grpc_logging "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging" + grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery" + + "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" @@ -27,6 +27,7 @@ import ( "github.com/armadaproject/armada/internal/common/auth" "github.com/armadaproject/armada/internal/common/certs" "github.com/armadaproject/armada/internal/common/grpc/configuration" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/requestid" ) @@ -37,77 +38,47 @@ func CreateGrpcServer( keepaliveEnforcementPolicy keepalive.EnforcementPolicy, authServices []auth.AuthService, tlsConfig configuration.TlsConfig, - logrusOptions ...grpc_logrus.Option, + loggerOpts ...grpc_logging.Option, ) *grpc.Server { - // Logging, authentication, etc. are implemented via gRPC interceptors - // (i.e., via functions that are called before handling the actual request). - // There are separate interceptors for unary and streaming gRPC calls. - unaryInterceptors := []grpc.UnaryServerInterceptor{} - streamInterceptors := []grpc.StreamServerInterceptor{} - - // Automatically recover from panics - // NOTE This must be the first interceptor, so it can handle panics in any subsequently added interceptor - recovery := grpc_recovery.WithRecoveryHandler(panicRecoveryHandler) - unaryInterceptors = append(unaryInterceptors, grpc_recovery.UnaryServerInterceptor(recovery)) - streamInterceptors = append(streamInterceptors, grpc_recovery.StreamServerInterceptor(recovery)) - - // Logging (using logrus) - // By default, information contained in the request context is logged - // tagsExtractor pulls information out of the request payload (a protobuf) and stores it in - // the context, such that it is logged. - messageDefault := log.NewEntry(log.StandardLogger()) - tagsExtractor := grpc_ctxtags.WithFieldExtractor(grpc_ctxtags.CodeGenRequestFieldExtractor) - unaryInterceptors = append(unaryInterceptors, - grpc_ctxtags.UnaryServerInterceptor(tagsExtractor), - requestid.UnaryServerInterceptor(false), - armadaerrors.UnaryServerInterceptor(2000), - grpc_logrus.UnaryServerInterceptor(messageDefault, logrusOptions...), - ) - streamInterceptors = append(streamInterceptors, - grpc_ctxtags.StreamServerInterceptor(tagsExtractor), - requestid.StreamServerInterceptor(false), - armadaerrors.StreamServerInterceptor(2000), - grpc_logrus.StreamServerInterceptor(messageDefault, logrusOptions...), - ) - - // Authentication - // The provided authServices represents a list of services that can be used to authenticate - // the client (e.g., username/password and OpenId). authFunction is a combination of these. authFunction := auth.CreateGrpcMiddlewareAuthFunction(auth.NewMultiAuthService(authServices)) - unaryInterceptors = append(unaryInterceptors, grpc_auth.UnaryServerInterceptor(authFunction)) - streamInterceptors = append(streamInterceptors, grpc_auth.StreamServerInterceptor(authFunction)) + srvMetrics := setupPromMetrics() - // Prometheus timeseries collection integration - grpc_prometheus.EnableHandlingTimeHistogram() - unaryInterceptors = append(unaryInterceptors, grpc_prometheus.UnaryServerInterceptor) - streamInterceptors = append(streamInterceptors, grpc_prometheus.StreamServerInterceptor) + loggerOpts = append( + loggerOpts, + grpc_logging.WithLogOnEvents(grpc_logging.StartCall, grpc_logging.FinishCall)) - serverOptions := []grpc.ServerOption{ + return grpc.NewServer( grpc.KeepaliveParams(keepaliveParams), grpc.KeepaliveEnforcementPolicy(keepaliveEnforcementPolicy), - grpc.StreamInterceptor(grpc_middleware.ChainStreamServer(streamInterceptors...)), - grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer(unaryInterceptors...)), - } - - if tlsConfig.Enabled { - cachedCertificateService := certs.NewCachedCertificateService(tlsConfig.CertPath, tlsConfig.KeyPath, time.Minute) - go func() { - cachedCertificateService.Run(armadacontext.Background()) - }() - tlsCreds := credentials.NewTLS(&tls.Config{ - GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - cert := cachedCertificateService.GetCertificate() - if cert == nil { - return nil, fmt.Errorf("unexpectedly received nil from certificate cache") - } - return cert, nil - }, - }) - serverOptions = append(serverOptions, grpc.Creds(tlsCreds)) - } + setupTls(tlsConfig), + grpc.ChainUnaryInterceptor( + srvMetrics.UnaryServerInterceptor(), + requestid.UnaryServerInterceptor(false), + grpc_auth.UnaryServerInterceptor(authFunction), + grpc_logging.UnaryServerInterceptor(InterceptorLogger(), loggerOpts...), + armadaerrors.UnaryServerInterceptor(2000), + grpc_recovery.UnaryServerInterceptor(grpc_recovery.WithRecoveryHandler(panicRecoveryHandler)), + ), + grpc.ChainStreamInterceptor( + srvMetrics.StreamServerInterceptor(), + requestid.StreamServerInterceptor(false), + grpc_auth.StreamServerInterceptor(authFunction), + grpc_logging.StreamServerInterceptor(InterceptorLogger(), loggerOpts...), + armadaerrors.StreamServerInterceptor(2000), + grpc_recovery.StreamServerInterceptor(grpc_recovery.WithRecoveryHandler(panicRecoveryHandler)), + ), + ) +} - // Interceptors are registered at server creation - return grpc.NewServer(serverOptions...) +func setupPromMetrics() *grpc_prometheus.ServerMetrics { + srvMetrics := grpc_prometheus.NewServerMetrics( + grpc_prometheus.WithServerHandlingTimeHistogram( + grpc_prometheus.WithHistogramBuckets([]float64{0.001, 0.01, 0.1, 0.3, 0.6, 1, 3, 6, 9, 20, 30, 60, 90, 120}), + ), + ) + reg := prometheus.NewRegistry() + reg.MustRegister(srvMetrics) + return srvMetrics } // TODO We don't need this function. Just do this at the caller. @@ -118,9 +89,9 @@ func Listen(port uint16, grpcServer *grpc.Server, wg *sync.WaitGroup) { } go func() { - defer log.Println("Stopping server.") + defer log.Infof("Stopping server.") - log.Printf("Grpc listening on %d", port) + log.Infof("Grpc listening on %d", port) if err := grpcServer.Serve(lis); err != nil { log.Fatalf("failed to serve: %v", err) } @@ -143,8 +114,55 @@ func CreateShutdownHandler(ctx *armadacontext.Context, gracePeriod time.Duration } } +func setupTls(tlsConfig configuration.TlsConfig) grpc.ServerOption { + if !tlsConfig.Enabled { + return grpc.EmptyServerOption{} + } + + cachedCertificateService := certs.NewCachedCertificateService(tlsConfig.CertPath, tlsConfig.KeyPath, time.Minute) + go func() { + cachedCertificateService.Run(armadacontext.Background()) + }() + tlsCreds := credentials.NewTLS(&tls.Config{ + GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { + cert := cachedCertificateService.GetCertificate() + if cert == nil { + return nil, fmt.Errorf("unexpectedly received nil from certificate cache") + } + return cert, nil + }, + }) + return grpc.Creds(tlsCreds) +} + // This function is called whenever a gRPC handler panics. func panicRecoveryHandler(p interface{}) (err error) { log.Errorf("Request triggered panic with cause %v \n%s", p, string(debug.Stack())) return status.Errorf(codes.Internal, "Internal server error caused by %v", p) } + +func InterceptorLogger() grpc_logging.Logger { + return grpc_logging.LoggerFunc(func(ctx context.Context, lvl grpc_logging.Level, msg string, fields ...any) { + logFields := make(map[string]any, len(fields)/2+2) + logFields["user"] = ctx.Value("user") + logFields["requestId"] = ctx.Value("requestId") + i := grpc_logging.Fields(fields).Iterator() + for i.Next() { + k, v := i.At() + logFields[k] = v + } + l := log.WithFields(logFields) + switch lvl { + case grpc_logging.LevelDebug: + l.Debug(msg) + case grpc_logging.LevelInfo: + l.Info(msg) + case grpc_logging.LevelWarn: + l.Warn(msg) + case grpc_logging.LevelError: + l.Error(msg) + default: + panic(fmt.Sprintf("unknown level %v", lvl)) + } + }) +} diff --git a/internal/common/health/http_handler.go b/internal/common/health/http_handler.go index 5dd2c9b5444..38b1a0f1f82 100644 --- a/internal/common/health/http_handler.go +++ b/internal/common/health/http_handler.go @@ -3,7 +3,7 @@ package health import ( "net/http" - log "github.com/sirupsen/logrus" + log "github.com/armadaproject/armada/internal/common/logging" ) // TODO Doesn't need to exist. Just give a Checker directly. diff --git a/internal/common/healthmonitor/healthmonitor.go b/internal/common/healthmonitor/healthmonitor.go index d5c6b151c1e..2bf7d817b6c 100644 --- a/internal/common/healthmonitor/healthmonitor.go +++ b/internal/common/healthmonitor/healthmonitor.go @@ -2,7 +2,6 @@ package healthmonitor import ( "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" ) @@ -25,5 +24,5 @@ type HealthMonitor interface { // Run initialises and starts the health checker. // Run may be blocking and should be run within a separate goroutine. // Must be called before IsHealthy() or any prometheus.Collector interface methods. - Run(*armadacontext.Context, *logrus.Entry) error + Run(*armadacontext.Context) error } diff --git a/internal/common/healthmonitor/manualhealthmonitor.go b/internal/common/healthmonitor/manualhealthmonitor.go index 7aa2f525068..46797493df0 100644 --- a/internal/common/healthmonitor/manualhealthmonitor.go +++ b/internal/common/healthmonitor/manualhealthmonitor.go @@ -4,7 +4,6 @@ import ( "sync" "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" ) @@ -47,7 +46,7 @@ func (srv *ManualHealthMonitor) IsHealthy() (bool, string, error) { } } -func (srv *ManualHealthMonitor) Run(_ *armadacontext.Context, _ *logrus.Entry) error { +func (srv *ManualHealthMonitor) Run(_ *armadacontext.Context) error { return nil } diff --git a/internal/common/healthmonitor/multihealthmonitor.go b/internal/common/healthmonitor/multihealthmonitor.go index a9f03643d10..cf6197918f9 100644 --- a/internal/common/healthmonitor/multihealthmonitor.go +++ b/internal/common/healthmonitor/multihealthmonitor.go @@ -6,7 +6,6 @@ import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "golang.org/x/exp/maps" "github.com/armadaproject/armada/internal/common/armadacontext" @@ -100,11 +99,11 @@ func (srv *MultiHealthMonitor) IsHealthy() (ok bool, reason string, err error) { } // Run initialises prometheus metrics and starts any child health checkers. -func (srv *MultiHealthMonitor) Run(ctx *armadacontext.Context, log *logrus.Entry) error { +func (srv *MultiHealthMonitor) Run(ctx *armadacontext.Context) error { g, ctx := armadacontext.ErrGroup(ctx) for _, healthMonitor := range srv.healthMonitorsByName { healthMonitor := healthMonitor - g.Go(func() error { return healthMonitor.Run(ctx, log) }) + g.Go(func() error { return healthMonitor.Run(ctx) }) } return g.Wait() } diff --git a/internal/common/ingest/batch.go b/internal/common/ingest/batch.go index 02a35f7490c..f2c4867ce10 100644 --- a/internal/common/ingest/batch.go +++ b/internal/common/ingest/batch.go @@ -4,10 +4,10 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" ) // Batcher batches up events from a channel. Batches are created whenever maxItems have been diff --git a/internal/common/ingest/controlplaneevents/utils.go b/internal/common/ingest/controlplaneevents/utils.go index 13182165ba5..836548d27a4 100644 --- a/internal/common/ingest/controlplaneevents/utils.go +++ b/internal/common/ingest/controlplaneevents/utils.go @@ -2,11 +2,11 @@ package controlplaneevents import ( "github.com/apache/pulsar-client-go/pulsar" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/eventutil" commonmetrics "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/controlplaneevents" ) diff --git a/internal/common/ingest/ingestion_pipeline.go b/internal/common/ingest/ingestion_pipeline.go index 160cf677268..a8f39e07ec6 100644 --- a/internal/common/ingest/ingestion_pipeline.go +++ b/internal/common/ingest/ingestion_pipeline.go @@ -7,12 +7,12 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" commonconfig "github.com/armadaproject/armada/internal/common/config" commonmetrics "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/pulsarutils" "github.com/armadaproject/armada/internal/common/util" ) diff --git a/internal/common/ingest/jobsetevents/utils.go b/internal/common/ingest/jobsetevents/utils.go index 0de21b7d27a..a1b36c9d293 100644 --- a/internal/common/ingest/jobsetevents/utils.go +++ b/internal/common/ingest/jobsetevents/utils.go @@ -2,11 +2,11 @@ package jobsetevents import ( "github.com/apache/pulsar-client-go/pulsar" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/eventutil" commonmetrics "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/pkg/armadaevents" ) diff --git a/internal/common/ingest/retry.go b/internal/common/ingest/retry.go index aa66a7c9166..810b0c346a2 100644 --- a/internal/common/ingest/retry.go +++ b/internal/common/ingest/retry.go @@ -3,7 +3,7 @@ package ingest import ( "time" - log "github.com/sirupsen/logrus" + log "github.com/armadaproject/armada/internal/common/logging" ) // WithRetry executes the supplied action until it either completes successfully or it returns false, indicating that diff --git a/internal/common/logging/application.go b/internal/common/logging/application.go new file mode 100644 index 00000000000..cbc969a224a --- /dev/null +++ b/internal/common/logging/application.go @@ -0,0 +1,145 @@ +package logging + +import ( + "fmt" + "os" + "strings" + + "github.com/pkg/errors" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "gopkg.in/natefinch/lumberjack.v2" + "sigs.k8s.io/yaml" +) + +var ( + defaultLogConfigPath = "config/logging.yaml" + logConfigPathEnvVar = "ARMADA_LOG_CONFIG" +) + +// MustConfigureApplicationLogging sets up logging suitable for an application. Logging configuration is loaded from +// a filepath given by the ARMADA_LOG_CONFIG environmental variable or from config/logging.yaml if this var is unset. +// Note that this function will immediately shut do the application if it fails. +func MustConfigureApplicationLogging() { + err := ConfigureApplicationLogging() + if err != nil { + _, _ = fmt.Fprintln(os.Stderr, "Error initialising logging"+err.Error()) + os.Exit(1) + } +} + +// ConfigureApplicationLogging sets up logging suitable for an application. Logging configuration is loaded from +// a filepath given by the ARMADA_LOG_CONFIG environmental variable or from config/logging.yaml if this var is unset. +func ConfigureApplicationLogging() error { + configPath := getEnv(logConfigPathEnvVar, defaultLogConfigPath) + + logConfig, err := readConfig(configPath) + if err != nil { + return err + } + + encoderConfig := zapcore.EncoderConfig{ + TimeKey: "ts", + LevelKey: "level", + NameKey: "logger", + CallerKey: "caller", + FunctionKey: zapcore.OmitKey, + MessageKey: "msg", + StacktraceKey: "stacktrace", + LineEnding: zapcore.DefaultLineEnding, + EncodeLevel: zapcore.CapitalLevelEncoder, + EncodeTime: zapcore.ISO8601TimeEncoder, + EncodeDuration: zapcore.SecondsDurationEncoder, + EncodeCaller: shortCallerEncoder, + ConsoleSeparator: " ", + } + + var cores []zapcore.Core + + // Console logging + consoleEncoder, err := createEncoder(logConfig.Console.Format, encoderConfig) + if err != nil { + return errors.Wrap(err, "error creating console logger") + } + consoleLevel, err := parseLogLevel(logConfig.Console.Level) + if err != nil { + return errors.Wrap(err, "error creating console logger") + } + cores = append(cores, zapcore.NewCore(consoleEncoder, zapcore.AddSync(os.Stdout), consoleLevel)) + + // File logging + if logConfig.File.Enabled { + fileEncoder, err := createEncoder(logConfig.File.Format, encoderConfig) + if err != nil { + return errors.Wrap(err, "error creating file logger") + } + w := zapcore.AddSync(&lumberjack.Logger{ + Filename: "app.log", + MaxSize: logConfig.File.Rotation.MaxSizeMb, + MaxBackups: logConfig.File.Rotation.MaxBackups, + MaxAge: logConfig.File.Rotation.MaxAgeDays, // days + Compress: logConfig.File.Rotation.Compress, + }) + fileLevel, err := parseLogLevel(logConfig.Console.Level) + if err != nil { + return errors.Wrap(err, "error creating file logger") + } + cores = append(cores, zapcore.NewCore(fileEncoder, w, fileLevel)) + } + + core := zapcore.NewTee(cores...) + + // Create logger + l := zap.New(core, zap.AddCaller()).WithOptions(zap.AddCallerSkip(2)) + ReplaceStdLogger(FromZap(l)) + + return nil +} + +func readConfig(configFilePath string) (Config, error) { + yamlConfig, err := os.ReadFile(configFilePath) + if err != nil { + return Config{}, errors.Wrap(err, "failed to read log config file") + } + + var config Config + err = yaml.Unmarshal(yamlConfig, &config) + if err != nil { + return Config{}, errors.Wrap(err, "failed to unmarshall log config file") + } + err = validate(config) + if err != nil { + return Config{}, errors.Wrap(err, "invalid log configuration") + } + return config, nil +} + +func createEncoder(format string, encoderConfig zapcore.EncoderConfig) (zapcore.Encoder, error) { + switch strings.ToLower(format) { + case "json": + return zapcore.NewJSONEncoder(encoderConfig), nil + case "text": + return zapcore.NewConsoleEncoder(encoderConfig), nil + default: + return nil, errors.Errorf("unknown format: %s", format) + } +} + +// ShortCallerEncoder serializes a caller in to just file:line format. +func shortCallerEncoder(caller zapcore.EntryCaller, enc zapcore.PrimitiveArrayEncoder) { + trimmed := caller.TrimmedPath() + lastSlash := strings.LastIndexByte(trimmed, '/') + if lastSlash != -1 && lastSlash != len(trimmed)-1 { + fileName := trimmed[lastSlash+1:] + enc.AppendString(fileName) + } else { + enc.AppendString(trimmed) + } +} + +func getEnv(key, defaultValue string) string { + if value, exists := os.LookupEnv(key); exists { + return value + } + return defaultValue +} diff --git a/internal/common/logging/config.go b/internal/common/logging/config.go new file mode 100644 index 00000000000..eb69b51a7a8 --- /dev/null +++ b/internal/common/logging/config.go @@ -0,0 +1,116 @@ +package logging + +import ( + "strings" + + "github.com/pkg/errors" + "go.uber.org/zap/zapcore" + "golang.org/x/exp/maps" +) + +var validLogFormats = map[string]bool{ + "text": true, + "json": true, +} + +// Config defines Armada logging configuration. +type Config struct { + // Defines configuration for console logging on stdout + Console struct { + // Log level, e.g. INFO, ERROR etc + Level string `yaml:"level"` + // Logging format, either text or json + Format string `yaml:"format"` + } `yaml:"console"` + // Defines configuration for file logging + File struct { + // Whether file logging is enabled. + Enabled bool `yaml:"enabled"` + // Log level, e.g. INFO, ERROR etc + Level string `yaml:"level"` + // Logging format, either text or json + Format string `yaml:"format"` + // The Location of the logfile on disk + LogFile string `yaml:"logfile"` + // Log Rotation Options + Rotation struct { + // Whether Log Rotation is enabled + Enabled bool `yaml:"enabled"` + // Maximum size in megabytes of the log file before it gets rotated + MaxSizeMb int `yaml:"maxSizeMb"` + // Maximum number of old log files to retain + MaxBackups int `yaml:"maxBackups"` + // Maximum number of days to retain old log files + MaxAgeDays int `yaml:"maxAgeDays"` + // Whether to compress rotated log files + Compress bool `yaml:"compress"` + } `yaml:"rotation"` + } `yaml:"file"` +} + +func validate(c Config) error { + _, err := parseLogLevel(c.Console.Level) + if err != nil { + return err + } + + err = validateLogFormat(c.Console.Format) + if err != nil { + return err + } + + if c.File.Enabled { + _, err := parseLogLevel(c.File.Level) + if err != nil { + return err + } + + err = validateLogFormat(c.File.Format) + if err != nil { + return err + } + + rotation := c.File.Rotation + if rotation.Enabled { + if rotation.MaxSizeMb <= 0 { + return errors.New("rotation.maxSizeMb must be greater than zero") + } + if rotation.MaxBackups <= 0 { + return errors.New("rotation.maxBackups must be greater than zero") + } + if rotation.MaxAgeDays <= 0 { + return errors.New("rotation.maxAgeDays must be greater than zero") + } + } + } + + return nil +} + +func validateLogFormat(f string) error { + _, ok := validLogFormats[f] + if !ok { + err := errors.Errorf("unknown log format: %s. Valid formats are %s", f, maps.Keys(validLogFormats)) + return err + } + return nil +} + +func parseLogLevel(level string) (zapcore.Level, error) { + switch strings.ToLower(level) { + case "debug": + return zapcore.DebugLevel, nil + case "info": + return zapcore.InfoLevel, nil + case "warn", "warning": + return zapcore.WarnLevel, nil + case "error": + return zapcore.ErrorLevel, nil + case "panic": + return zapcore.PanicLevel, nil + case "fatal": + return zapcore.FatalLevel, nil + default: + return zapcore.InfoLevel, errors.Errorf("unknown level: %s", level) + } +} diff --git a/internal/common/logging/formatter.go b/internal/common/logging/formatter.go deleted file mode 100644 index f6db9902424..00000000000 --- a/internal/common/logging/formatter.go +++ /dev/null @@ -1,13 +0,0 @@ -package logging - -import ( - "fmt" - - log "github.com/sirupsen/logrus" -) - -type CommandLineFormatter struct{} - -func (f *CommandLineFormatter) Format(entry *log.Entry) ([]byte, error) { - return []byte(fmt.Sprintf("%s\n", entry.Message)), nil -} diff --git a/internal/common/logging/formatter_test.go b/internal/common/logging/formatter_test.go deleted file mode 100644 index bf79fca89aa..00000000000 --- a/internal/common/logging/formatter_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package logging - -import ( - "testing" - - log "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" -) - -func TestCommandLineFormatter(t *testing.T) { - commandLineFormatter := new(CommandLineFormatter) - - testMessage := "Test" - expectedOutput := testMessage + "\n" - - event := log.Entry{ - Message: testMessage, - } - - output, err := commandLineFormatter.Format(&event) - - assert.Nil(t, err) - assert.Equal(t, expectedOutput, string(output[:])) -} diff --git a/internal/common/logging/global.go b/internal/common/logging/global.go new file mode 100644 index 00000000000..abfdb86ff79 --- /dev/null +++ b/internal/common/logging/global.go @@ -0,0 +1,113 @@ +package logging + +import ( + "os" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// The global Logger. Comes configured with some sensible defaults for e.g. unit tests, but applications should +// generally configure their own logging config via ReplaceStdLogger +var ( + stdLogger = &Logger{underlying: createDefaultLogger()} +) + +// ReplaceStdLogger Replaces the global logger. This should be called once at app startup! +func ReplaceStdLogger(l *Logger) { + stdLogger = l +} + +// StdLogger Returns the default logger +func StdLogger() *Logger { + return stdLogger +} + +// Debug logs a message at level Debug. +func Debug(args ...any) { + stdLogger.Debug(args...) +} + +// Info logs a message at level Info. +func Info(args ...any) { + stdLogger.Info(args...) +} + +// Warn logs a message at level Warn on the standard logger. +func Warn(args ...any) { + stdLogger.Warn(args...) +} + +// Error logs a message at level Error on the standard logger. +func Error(args ...any) { + stdLogger.Error(args...) +} + +// Panic logs a message at level Panic on the standard logger. +func Panic(args ...any) { + stdLogger.Panic(args...) +} + +// Fatal logs a message at level Fatal on the standard logger then the process will exit with status set to 1. +func Fatal(args ...any) { + stdLogger.Fatal(args...) +} + +// Debugf logs a message at level Debug on the standard logger. +func Debugf(format string, args ...any) { + stdLogger.Debugf(format, args...) +} + +// Infof logs a message at level Info on the standard logger. +func Infof(format string, args ...any) { + stdLogger.Infof(format, args...) +} + +// Warnf logs a message at level Warn on the standard logger. +func Warnf(format string, args ...any) { + stdLogger.Warnf(format, args...) +} + +// Errorf logs a message at level Error on the standard logger. +func Errorf(format string, args ...any) { + stdLogger.Errorf(format, args...) +} + +// Fatalf logs a message at level Fatal on the standard logger then the process will exit with status set to 1. +func Fatalf(format string, args ...any) { + stdLogger.Fatalf(format, args...) +} + +// WithField returns a new Logger with the key-value pair added as a new field +func WithField(key string, value any) *Logger { + return stdLogger.WithField(key, value) +} + +// WithFields returns a new Logger with all key-value pairs in the map added as new fields +func WithFields(args map[string]any) *Logger { + return stdLogger.WithFields(args) +} + +// WithError returns a new Logger with the error added as a field +func WithError(err error) *Logger { + return stdLogger.WithError(err) +} + +// WithStacktrace returns a new Logger with the error and (if available) the stacktrace added as fields +func WithStacktrace(err error) *Logger { + return stdLogger.WithStacktrace(err) +} + +// Default logging options +func createDefaultLogger() *zap.SugaredLogger { + pe := zap.NewProductionEncoderConfig() + pe.EncodeTime = zapcore.ISO8601TimeEncoder + pe.ConsoleSeparator = " " + pe.EncodeLevel = zapcore.CapitalLevelEncoder + consoleEncoder := zapcore.NewConsoleEncoder(pe) + core := zapcore.NewCore(consoleEncoder, zapcore.AddSync(os.Stdout), zapcore.DebugLevel) + return zap. + New(core, zap.AddCaller()). + WithOptions(zap.AddCallerSkip(2)). + Sugar() +} diff --git a/internal/common/logging/interceptors.go b/internal/common/logging/interceptors.go deleted file mode 100644 index 33b4ed48889..00000000000 --- a/internal/common/logging/interceptors.go +++ /dev/null @@ -1,50 +0,0 @@ -package logging - -import ( - "context" - "fmt" - - "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" - "github.com/sirupsen/logrus" - "google.golang.org/grpc" - - "github.com/armadaproject/armada/internal/common/requestid" -) - -// UnaryServerInterceptor returns an interceptor that adds the request id as a -// field to the logrus logger embedded in the context. If an error occurs in the handler, -// it also adds a stack trace. -func UnaryServerInterceptor() grpc.UnaryServerInterceptor { - return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { - if id, ok := requestid.FromContext(ctx); ok { - ctxlogrus.AddFields(ctx, logrus.Fields{requestid.MetadataKey: id}) - } - rv, err := handler(ctx, req) - if err != nil { - // %+v prints a stack trace for pkg/errors errors - ctxlogrus.AddFields(ctx, logrus.Fields{"errorVerbose": fmt.Sprintf("%+v", err)}) - } - return rv, err - } -} - -// StreamServerInterceptor returns an interceptor that adds the request id as a -// field to the logrus logger embedded in the context. If an error occurs in the handler, -// it also adds a stack trace. -func StreamServerInterceptor() grpc.StreamServerInterceptor { - return func(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - ctx := stream.Context() - if id, ok := requestid.FromContext(ctx); ok { - ctxlogrus.AddFields(ctx, logrus.Fields{requestid.MetadataKey: id}) - } - // The logrus logger only logs at the end of the call. As streaming calls may last a long time - // We also log here which will produce a log line at the start of the call - ctxlogrus.Extract(ctx).Infof("started streaming call") - err := handler(srv, stream) - if err != nil { - // %+v prints a stack trace for pkg/errors errors - ctxlogrus.AddFields(ctx, logrus.Fields{"errorVerbose": fmt.Sprintf("%+v", err)}) - } - return err - } -} diff --git a/internal/common/logging/interceptors_test.go b/internal/common/logging/interceptors_test.go deleted file mode 100644 index 2d3f76ff159..00000000000 --- a/internal/common/logging/interceptors_test.go +++ /dev/null @@ -1,70 +0,0 @@ -package logging - -import ( - "context" - "testing" - - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" - "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus" - "github.com/renstrom/shortuuid" - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/require" - "google.golang.org/grpc" - "google.golang.org/grpc/metadata" - - "github.com/armadaproject/armada/internal/common/requestid" -) - -func TestUnaryServerInterceptor(t *testing.T) { - ctx := context.Background() - ctx = metadata.NewIncomingContext(ctx, metadata.New(map[string]string{})) - id := shortuuid.New() - ctx, ok := requestid.AddToIncomingContext(ctx, id) - require.True(t, ok, "error adding request id to context") - logger := logrus.New() - entry := logrus.NewEntry(logger) - ctx = ctxlogrus.ToContext(ctx, entry) - handler := func(ctx context.Context, req interface{}) (interface{}, error) { - entry := ctxlogrus.Extract(ctx) - for _, field := range entry.Data { - if s, ok := field.(string); ok && s == id { - return nil, nil - } - } - t.Fatal("request id was not added as a logger field") - return nil, nil - } - - f := UnaryServerInterceptor() - _, err := f(ctx, nil, nil, handler) - require.NoError(t, err) -} - -func TestStreamServerInterceptor(t *testing.T) { - ctx := context.Background() - ctx = metadata.NewIncomingContext(ctx, metadata.New(map[string]string{})) - id := shortuuid.New() - ctx, ok := requestid.AddToIncomingContext(ctx, id) - require.True(t, ok, "error adding request id to context") - - logger := logrus.New() - entry := logrus.NewEntry(logger) - ctx = ctxlogrus.ToContext(ctx, entry) - stream := &grpc_middleware.WrappedServerStream{} - stream.WrappedContext = ctx - handler := func(srv interface{}, stream grpc.ServerStream) error { - ctx := stream.Context() - entry := ctxlogrus.Extract(ctx) - for _, field := range entry.Data { - if s, ok := field.(string); ok && s == id { - return nil - } - } - t.Fatal("request id was not added as a logger field") - return nil - } - - f := StreamServerInterceptor() - err := f(nil, stream, nil, handler) - require.NoError(t, err) -} diff --git a/internal/common/logging/logger.go b/internal/common/logging/logger.go new file mode 100644 index 00000000000..2eb64137e28 --- /dev/null +++ b/internal/common/logging/logger.go @@ -0,0 +1,120 @@ +package logging + +import ( + "github.com/pkg/errors" + "go.uber.org/zap" +) + +// Logger wraps a *zap.SugaredLogger so that the rest of the code doesn't depend directly on Zap +type Logger struct { + underlying *zap.SugaredLogger +} + +// FromZap returns a New Logger backed by the supplied *zap.SugaredLogger +func FromZap(l *zap.Logger) *Logger { + return &Logger{ + underlying: l.Sugar(), + } +} + +// Debug logs a message at level Debug +func (l *Logger) Debug(args ...any) { + l.underlying.Debug(args...) +} + +// Info logs a message at level Info +func (l *Logger) Info(args ...any) { + l.underlying.Info(args...) +} + +// Warn logs a message at level Warn +func (l *Logger) Warn(args ...any) { + l.underlying.Warn(args...) +} + +// Error logs a message at level Error +func (l *Logger) Error(args ...any) { + l.underlying.Error(args...) +} + +// Panic logs a message at level Panic +func (l *Logger) Panic(args ...any) { + l.underlying.Panic(args...) +} + +// Fatal logs a message at level Fatal then the process will exit with status set to 1. +func (l *Logger) Fatal(args ...any) { + l.underlying.Fatal(args...) +} + +// Debugf logs a message at level Debug. +func (l *Logger) Debugf(format string, args ...interface{}) { + l.underlying.Debugf(format, args...) +} + +// Infof logs a message at level Info. +func (l *Logger) Infof(format string, args ...interface{}) { + l.underlying.Infof(format, args...) +} + +// Warnf logs a message at level Warn. +func (l *Logger) Warnf(format string, args ...interface{}) { + l.underlying.Warnf(format, args...) +} + +// Errorf logs a message at level Error. +func (l *Logger) Errorf(format string, args ...interface{}) { + l.underlying.Errorf(format, args...) +} + +// Fatalf logs a message at level Fatal. +func (l *Logger) Fatalf(format string, args ...interface{}) { + l.underlying.Fatalf(format, args...) +} + +// WithError returns a new Logger with the error added as a field +func (l *Logger) WithError(err error) *Logger { + return l.WithField("error", err) +} + +// WithStacktrace returns a new Logger with the error and (if available) the stacktrace added as fields +func (l *Logger) WithStacktrace(err error) *Logger { + logger := l.WithError(err) + if stackErr, ok := err.(stackTracer); ok { + return logger.WithField("stacktrace", stackErr.StackTrace()) + } else { + return logger + } +} + +// WithField returns a new Logger with the key-value pair added as a new field +func (l *Logger) WithField(key string, value any) *Logger { + return &Logger{ + underlying: l.underlying.With(key, value), + } +} + +// WithFields returns a new Logger with all key-value pairs in the map added as new fields +func (l *Logger) WithFields(args map[string]any) *Logger { + fields := make([]any, 0, len(args)) + for key, value := range args { + fields = append(fields, zap.Any(key, value)) + } + return &Logger{ + underlying: l.underlying.With(fields...), + } +} + +// WithCallerSkip returns a new Logger with the number of callers skipped increased by the skip amount. +// This is needed when building wrappers around the Logger so as to prevent us from always reporting the +// wrapper code as the caller. +func (l *Logger) WithCallerSkip(skip int) *Logger { + return &Logger{ + underlying: l.underlying.WithOptions(zap.AddCallerSkip(skip)), + } +} + +// Unexported but considered part of the stable interface of pkg/errors. +type stackTracer interface { + StackTrace() errors.StackTrace +} diff --git a/internal/common/logging/logger_test.go b/internal/common/logging/logger_test.go new file mode 100644 index 00000000000..349aaf623b0 --- /dev/null +++ b/internal/common/logging/logger_test.go @@ -0,0 +1,92 @@ +package logging + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestWithField(t *testing.T) { + logger, observedLogs := testLogger() + + newLogger := logger.WithField("foo", "bar") + newLogger.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "bar", logEntry.ContextMap()["foo"]) +} + +func TestWithFields(t *testing.T) { + logger, observedLogs := testLogger() + + newLogger := logger.WithFields(map[string]any{ + "user": "test_user", + "action": "test_action", + }) + newLogger.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "test_user", logEntry.ContextMap()["user"]) + assert.Equal(t, "test_action", logEntry.ContextMap()["action"]) +} + +func TestWithError(t *testing.T) { + logger, observedLogs := testLogger() + + err := errors.New("test error") + + newLogger := logger.WithError(err) + newLogger.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "test error", logEntry.ContextMap()["error"]) +} + +func TestWithStacktrace(t *testing.T) { + logger, observedLogs := testLogger() + + err := errors.WithStack(errors.New("test error")) + + newLogger := logger.WithStacktrace(err) + newLogger.Info("test message") + + // Check the captured log entries + entries := observedLogs.All() + require.Len(t, entries, 1, "Expected exactly one log entry") + + // Verify the fields + logEntry := entries[0] + assert.Equal(t, "test message", logEntry.Message) + assert.Equal(t, "test error", logEntry.ContextMap()["error"]) + assert.Equal(t, err.(stackTracer).StackTrace(), logEntry.ContextMap()["stacktrace"]) +} + +func testLogger() (*Logger, *observer.ObservedLogs) { + core, observedLogs := observer.New(zapcore.DebugLevel) + baseLogger := zap.New(core).Sugar() + logger := &Logger{underlying: baseLogger} + return logger, observedLogs +} diff --git a/internal/common/logging/null_logger.go b/internal/common/logging/null_logger.go index 25326d3691b..7bafa1e0a39 100644 --- a/internal/common/logging/null_logger.go +++ b/internal/common/logging/null_logger.go @@ -1,14 +1,11 @@ package logging import ( - "io" - - "github.com/sirupsen/logrus" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" ) -var NullLogger = &logrus.Logger{ - Out: io.Discard, - Formatter: new(logrus.TextFormatter), - Hooks: make(logrus.LevelHooks), - Level: logrus.PanicLevel, +// NullLogger is Logger that sends a log lines into the ether +var NullLogger = &Logger{ + underlying: zap.New(zapcore.NewNopCore()).Sugar(), } diff --git a/internal/common/logging/pulsar_adapter.go b/internal/common/logging/pulsar_adapter.go new file mode 100644 index 00000000000..3e3c3e48426 --- /dev/null +++ b/internal/common/logging/pulsar_adapter.go @@ -0,0 +1,73 @@ +package logging + +import ( + pulsarlog "github.com/apache/pulsar-client-go/pulsar/log" +) + +// Wrapper to adapt Logger to the logger interface expected by the pulsar client +type pulsarWrapper struct { + l *Logger +} + +// NewPulsarLogger returns a Logger that can be used by the pulsar client +func NewPulsarLogger() pulsarlog.Logger { + return &pulsarWrapper{ + l: StdLogger(), + } +} + +func (p pulsarWrapper) SubLogger(fs pulsarlog.Fields) pulsarlog.Logger { + return &pulsarWrapper{ + l: p.l.WithFields(fs), + } +} + +func (p pulsarWrapper) WithFields(fs pulsarlog.Fields) pulsarlog.Entry { + return &pulsarWrapper{ + l: p.l.WithFields(fs), + } +} + +func (p pulsarWrapper) WithField(name string, value interface{}) pulsarlog.Entry { + return &pulsarWrapper{ + l: p.l.WithField(name, value), + } +} + +func (p pulsarWrapper) WithError(err error) pulsarlog.Entry { + return &pulsarWrapper{ + l: p.l.WithError(err), + } +} + +func (p pulsarWrapper) Debug(args ...any) { + p.l.Debug(args...) +} + +func (p pulsarWrapper) Info(args ...any) { + p.l.Info(args...) +} + +func (p pulsarWrapper) Warn(args ...any) { + p.l.Warn(args...) +} + +func (p pulsarWrapper) Error(args ...any) { + p.l.Error(args...) +} + +func (p pulsarWrapper) Debugf(format string, args ...any) { + p.l.Debugf(format, args) +} + +func (p pulsarWrapper) Infof(format string, args ...any) { + p.l.Infof(format, args) +} + +func (p pulsarWrapper) Warnf(format string, args ...any) { + p.l.Warnf(format, args) +} + +func (p pulsarWrapper) Errorf(format string, args ...any) { + p.l.Errorf(format, args) +} diff --git a/internal/common/logging/stacktrace.go b/internal/common/logging/stacktrace.go deleted file mode 100644 index 7d546915b31..00000000000 --- a/internal/common/logging/stacktrace.go +++ /dev/null @@ -1,22 +0,0 @@ -package logging - -import ( - "github.com/pkg/errors" - "github.com/sirupsen/logrus" -) - -// Unexported but considered part of the stable interface of pkg/errors. -type stackTracer interface { - StackTrace() errors.StackTrace -} - -// WithStacktrace returns a new logrus.FieldLogger obtained by adding error information and, if available, a stack trace -// as fields to the provided logrus.FieldLogger. -func WithStacktrace(logger logrus.FieldLogger, err error) logrus.FieldLogger { - logger = logger.WithError(err) - if stackErr, ok := err.(stackTracer); ok { - return logger.WithField("stacktrace", stackErr.StackTrace()) - } else { - return logger - } -} diff --git a/internal/common/logging/user_input.go b/internal/common/logging/user_input.go deleted file mode 100644 index 3fb161040b9..00000000000 --- a/internal/common/logging/user_input.go +++ /dev/null @@ -1,8 +0,0 @@ -package logging - -import "strings" - -func SanitizeUserInput(str string) string { - safeStr := strings.Replace(str, "\n", "", -1) - return strings.Replace(safeStr, "\r", "", -1) -} diff --git a/internal/common/metrics/provider.go b/internal/common/metrics/provider.go index 7280f461fa0..28fc08a64ef 100644 --- a/internal/common/metrics/provider.go +++ b/internal/common/metrics/provider.go @@ -8,12 +8,10 @@ import ( "strings" "sync" "time" - - "github.com/sirupsen/logrus" ) type MetricsProvider interface { - Collect(context.Context, *logrus.Entry) (map[string]float64, error) + Collect(context.Context) (map[string]float64, error) } type ManualMetricsProvider struct { @@ -36,7 +34,7 @@ func (srv *ManualMetricsProvider) WithCollectionDelay(d time.Duration) *ManualMe return srv } -func (srv *ManualMetricsProvider) Collect(_ context.Context, _ *logrus.Entry) (map[string]float64, error) { +func (srv *ManualMetricsProvider) Collect(_ context.Context) (map[string]float64, error) { srv.mu.Lock() defer srv.mu.Unlock() if srv.collectionDelay != 0 { @@ -61,7 +59,7 @@ func NewHttpMetricsProvider(url string, client *http.Client) *HttpMetricsProvide } } -func (srv *HttpMetricsProvider) Collect(ctx context.Context, _ *logrus.Entry) (map[string]float64, error) { +func (srv *HttpMetricsProvider) Collect(ctx context.Context) (map[string]float64, error) { req, err := http.NewRequestWithContext(ctx, "GET", srv.url, nil) if err != nil { return nil, err diff --git a/internal/common/profiling/http.go b/internal/common/profiling/http.go index 5557ef190c4..88f60344443 100644 --- a/internal/common/profiling/http.go +++ b/internal/common/profiling/http.go @@ -6,12 +6,11 @@ import ( "net/http" _ "net/http/pprof" - log "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/auth" - "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling/configuration" "github.com/armadaproject/armada/internal/common/serve" ) @@ -54,7 +53,7 @@ func SetupPprof(config *configuration.ProfilingConfig, ctx *armadacontext.Contex serveFunc := func() error { if err := serve.ListenAndServe(ctx, pprofServer); err != nil { - logging.WithStacktrace(ctx, err).Error("pprof server failure") + ctx.Logger().WithStacktrace(err).Error("pprof server failure") } return err } diff --git a/internal/common/pulsarutils/publisher.go b/internal/common/pulsarutils/publisher.go index cb4113f1d4f..90b20839cd9 100644 --- a/internal/common/pulsarutils/publisher.go +++ b/internal/common/pulsarutils/publisher.go @@ -10,7 +10,6 @@ import ( "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/ingest/utils" - "github.com/armadaproject/armada/internal/common/logging" psutils "github.com/armadaproject/armada/internal/common/pulsarutils/utils" ) @@ -78,8 +77,8 @@ func (p *PulsarPublisher[T]) PublishMessages(ctx *armadacontext.Context, events for _, msg := range msgs { p.producer.SendAsync(sendCtx, msg, func(_ pulsar.MessageID, _ *pulsar.ProducerMessage, err error) { if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Error("error sending message to Pulsar") errored = true } diff --git a/internal/common/pulsarutils/pulsarclient.go b/internal/common/pulsarutils/pulsarclient.go index 22d53c852f5..477910ee905 100644 --- a/internal/common/pulsarutils/pulsarclient.go +++ b/internal/common/pulsarutils/pulsarclient.go @@ -8,6 +8,7 @@ import ( "github.com/armadaproject/armada/internal/common/armadaerrors" commonconfig "github.com/armadaproject/armada/internal/common/config" + "github.com/armadaproject/armada/internal/common/logging" ) func NewPulsarClient(config *commonconfig.PulsarConfig) (pulsar.Client, error) { @@ -39,5 +40,6 @@ func NewPulsarClient(config *commonconfig.PulsarConfig) (pulsar.Client, error) { TLSAllowInsecureConnection: config.TLSAllowInsecureConnection, MaxConnectionsPerBroker: config.MaxConnectionsPerBroker, Authentication: authentication, + Logger: logging.NewPulsarLogger(), }) } diff --git a/internal/common/requestid/interceptors.go b/internal/common/requestid/interceptors.go index b5ee03cd058..bd7dd1e7202 100644 --- a/internal/common/requestid/interceptors.go +++ b/internal/common/requestid/interceptors.go @@ -3,7 +3,7 @@ package requestid import ( "context" - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware/v2" "github.com/renstrom/shortuuid" "google.golang.org/grpc" "google.golang.org/grpc/metadata" @@ -43,6 +43,7 @@ func FromContextOrMissing(ctx context.Context) string { // The second return value is true if the operation was successful. func AddToIncomingContext(ctx context.Context, id string) (context.Context, bool) { if md, ok := metadata.FromIncomingContext(ctx); ok { + ctx = context.WithValue(ctx, "requestId", id) md.Set(MetadataKey, id) return metadata.NewIncomingContext(ctx, md), true } diff --git a/internal/common/requestid/interceptors_test.go b/internal/common/requestid/interceptors_test.go index 30cbd63dffa..584bf8798f5 100644 --- a/internal/common/requestid/interceptors_test.go +++ b/internal/common/requestid/interceptors_test.go @@ -4,7 +4,7 @@ import ( "context" "testing" - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" + grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware/v2" "github.com/renstrom/shortuuid" "github.com/stretchr/testify/require" "google.golang.org/grpc" diff --git a/internal/common/resource/resource_test.go b/internal/common/resource/resource_test.go index 80752ca18aa..184d1fa9067 100644 --- a/internal/common/resource/resource_test.go +++ b/internal/common/resource/resource_test.go @@ -142,7 +142,7 @@ func TestTotalResourceRequest_ShouldSumAllContainers(t *testing.T) { func TestTotalResourceRequestShouldReportMaxInitContainerValues(t *testing.T) { highCpuResource := makeContainerResource(1000, 5) highRamResource := makeContainerResource(100, 500) - // With init containers, it should take the max of each individual resource from all init containers + // WithField init containers, it should take the max of each individual resource from all init containers expectedResult := makeContainerResource(1000, 500) pod := makePodWithResource([]*v1.ResourceList{}, []*v1.ResourceList{&highCpuResource, &highRamResource}) diff --git a/internal/common/serve/static.go b/internal/common/serve/static.go index 662965de0b9..355ae05d8db 100644 --- a/internal/common/serve/static.go +++ b/internal/common/serve/static.go @@ -7,7 +7,6 @@ import ( "github.com/pkg/errors" "github.com/armadaproject/armada/internal/common/armadacontext" - "github.com/armadaproject/armada/internal/common/logging" ) // dirWithIndexFallback is a http.FileSystem that serves the index.html file at @@ -41,7 +40,7 @@ func ListenAndServe(ctx *armadacontext.Context, server *http.Server) error { // Shutdown server on ctx done. <-ctx.Done() if err := server.Shutdown(ctx); err != nil { - logging.WithStacktrace(ctx, err).Errorf("failed to shutdown server serving %s", server.Addr) + ctx.Logger().WithStacktrace(err).Errorf("failed to shutdown server serving %s", server.Addr) } }() if err := server.ListenAndServe(); err != nil { diff --git a/internal/common/startup.go b/internal/common/startup.go index 535fb628c5e..6a6207ad566 100644 --- a/internal/common/startup.go +++ b/internal/common/startup.go @@ -5,43 +5,33 @@ import ( "fmt" "net/http" "os" - "path" - "runtime" - "strconv" "strings" "time" "github.com/mitchellh/mapstructure" - "golang.org/x/exp/slices" - - "github.com/armadaproject/armada/internal/common/certs" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - log "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/spf13/viper" - "github.com/weaveworks/promrus" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "golang.org/x/exp/slices" "github.com/armadaproject/armada/internal/common/armadacontext" + "github.com/armadaproject/armada/internal/common/certs" commonconfig "github.com/armadaproject/armada/internal/common/config" - "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" ) const baseConfigFileName = "config" -// RFC3339Millis -const logTimestampFormat = "2006-01-02T15:04:05.999Z07:00" - func BindCommandlineArguments() { err := viper.BindPFlags(pflag.CommandLine) if err != nil { - log.Error() - os.Exit(-1) + log.Fatalf(err.Error()) } } -// TODO Move code relating to config out of common into a new package internal/serverconfig func LoadConfig(config commonconfig.Config, defaultPath string, overrideConfigs []string) *viper.Viper { v := viper.NewWithOptions(viper.KeyDelimiter("::")) v.SetConfigName(baseConfigFileName) @@ -72,8 +62,7 @@ func LoadConfig(config commonconfig.Config, defaultPath string, overrideConfigs var metadata mapstructure.Metadata customHooks := append(slices.Clone(commonconfig.CustomHooks), func(c *mapstructure.DecoderConfig) { c.Metadata = &metadata }) if err := v.Unmarshal(config, customHooks...); err != nil { - log.Error(err) - os.Exit(-1) + log.Fatal(err) } // Log a warning if there are config keys that don't match a config item in the struct the yaml is decoded into. @@ -90,7 +79,7 @@ func LoadConfig(config commonconfig.Config, defaultPath string, overrideConfigs } if err := config.Validate(); err != nil { - log.Error(commonconfig.FormatValidationErrors(err)) + log.Error(commonconfig.FormatValidationErrors(err).Error()) os.Exit(-1) } @@ -101,60 +90,31 @@ func UnmarshalKey(v *viper.Viper, key string, item interface{}) error { return v.UnmarshalKey(key, item, commonconfig.CustomHooks...) } -// TODO Move logging-related code out of common into a new package internal/logging func ConfigureCommandLineLogging() { - commandLineFormatter := new(logging.CommandLineFormatter) - log.SetFormatter(commandLineFormatter) - log.SetOutput(os.Stdout) -} - -func ConfigureLogging() { - log.SetLevel(readEnvironmentLogLevel()) - log.SetFormatter(readEnvironmentLogFormat()) - log.SetReportCaller(true) - log.SetOutput(os.Stdout) -} - -func readEnvironmentLogLevel() log.Level { - level, ok := os.LookupEnv("LOG_LEVEL") - if ok { - logLevel, err := log.ParseLevel(level) - if err == nil { - return logLevel - } - } - return log.InfoLevel -} - -func readEnvironmentLogFormat() log.Formatter { - formatStr, ok := os.LookupEnv("LOG_FORMAT") - if !ok { - formatStr = "colourful" - } - - textFormatter := &log.TextFormatter{ - ForceColors: true, - FullTimestamp: true, - TimestampFormat: logTimestampFormat, - CallerPrettyfier: func(frame *runtime.Frame) (function string, file string) { - fileName := path.Base(frame.File) + ":" + strconv.Itoa(frame.Line) - return "", fileName - }, - } - - switch strings.ToLower(formatStr) { - case "json": - return &log.JSONFormatter{TimestampFormat: logTimestampFormat} - case "colourful": - return textFormatter - case "text": - textFormatter.ForceColors = false - textFormatter.DisableColors = true - return textFormatter - default: - println(os.Stderr, fmt.Sprintf("Unknown log format %s, defaulting to colourful format", formatStr)) - return textFormatter - } + // Define an encoder configuration that only includes the message. + encoderConfig := zapcore.EncoderConfig{ + MessageKey: "message", + LineEnding: "\n", + // Ignore everything other than the message by leaving their keys empty. + LevelKey: "", + TimeKey: "", + NameKey: "", + CallerKey: "", + FunctionKey: "", + StacktraceKey: "", + } + // Use the console encoder with the custom configuration. + encoder := zapcore.NewConsoleEncoder(encoderConfig) + + // Create a core that writes to stdout. + core := zapcore.NewCore( + encoder, + zapcore.AddSync(os.Stdout), + zapcore.InfoLevel, + ) + + l := zap.New(core) + log.ReplaceStdLogger(log.FromZap(l)) } func ServeMetrics(port uint16) (shutdown func()) { @@ -162,9 +122,6 @@ func ServeMetrics(port uint16) (shutdown func()) { } func ServeMetricsFor(port uint16, gatherer prometheus.Gatherer) (shutdown func()) { - hook := promrus.MustNewPrometheusHook() - log.AddHook(hook) - mux := http.NewServeMux() mux.Handle("/metrics", promhttp.HandlerFor(gatherer, promhttp.HandlerOpts{})) return ServeHttp(port, mux) @@ -192,7 +149,7 @@ func serveHttp(port uint16, mux http.Handler, useTls bool, certFile, keyFile str } go func() { - log.Printf("Starting %s server listening on %d", scheme, port) + log.Infof("Starting %s server listening on %d", scheme, port) var err error if useTls { certWatcher := certs.NewCachedCertificateService(certFile, keyFile, time.Minute) @@ -217,7 +174,7 @@ func serveHttp(port uint16, mux http.Handler, useTls bool, certFile, keyFile str return func() { ctx, cancel := armadacontext.WithTimeout(armadacontext.Background(), 5*time.Second) defer cancel() - log.Printf("Stopping %s server listening on %d", scheme, port) + log.Infof("Stopping %s server listening on %d", scheme, port) e := srv.Shutdown(ctx) if e != nil { panic(e) diff --git a/internal/eventingester/convert/conversions.go b/internal/eventingester/convert/conversions.go index 17f69384f40..cbbd629419e 100644 --- a/internal/eventingester/convert/conversions.go +++ b/internal/eventingester/convert/conversions.go @@ -3,7 +3,6 @@ package convert import ( "github.com/gogo/protobuf/proto" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" @@ -11,6 +10,7 @@ import ( "github.com/armadaproject/armada/internal/common/ingest" "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/eventingester/model" "github.com/armadaproject/armada/pkg/armadaevents" ) diff --git a/internal/eventingester/ingester.go b/internal/eventingester/ingester.go index b6b7c207cbc..bf0185665fb 100644 --- a/internal/eventingester/ingester.go +++ b/internal/eventingester/ingester.go @@ -7,7 +7,6 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/pkg/errors" "github.com/redis/go-redis/v9" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/app" @@ -15,6 +14,7 @@ import ( "github.com/armadaproject/armada/internal/common/compress" "github.com/armadaproject/armada/internal/common/ingest" "github.com/armadaproject/armada/internal/common/ingest/jobsetevents" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/eventingester/configuration" "github.com/armadaproject/armada/internal/eventingester/convert" diff --git a/internal/eventingester/store/eventstore.go b/internal/eventingester/store/eventstore.go index 5682acc8f0d..20a5012a48f 100644 --- a/internal/eventingester/store/eventstore.go +++ b/internal/eventingester/store/eventstore.go @@ -6,10 +6,10 @@ import ( "github.com/hashicorp/go-multierror" "github.com/redis/go-redis/v9" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/ingest" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/eventingester/configuration" "github.com/armadaproject/armada/internal/eventingester/model" ) diff --git a/internal/executor/application.go b/internal/executor/application.go index b737eca2a71..a65cdde84f5 100644 --- a/internal/executor/application.go +++ b/internal/executor/application.go @@ -11,7 +11,6 @@ import ( "github.com/go-playground/validator/v10" grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/keepalive" "k8s.io/utils/clock" @@ -39,10 +38,10 @@ import ( "github.com/armadaproject/armada/pkg/executorapi" ) -func StartUp(ctx *armadacontext.Context, log *logrus.Entry, config configuration.ExecutorConfiguration) (func(), *sync.WaitGroup) { +func StartUp(ctx *armadacontext.Context, config configuration.ExecutorConfiguration) (func(), *sync.WaitGroup) { err := validateConfig(config) if err != nil { - log.Errorf("Invalid config: %s", err) + ctx.Errorf("Invalid config: %s", err) os.Exit(-1) } @@ -52,7 +51,7 @@ func StartUp(ctx *armadacontext.Context, log *logrus.Entry, config configuration config.Kubernetes.Burst, ) if err != nil { - log.Errorf("Failed to connect to kubernetes because %s", err) + ctx.Errorf("Failed to connect to kubernetes because %s", err) os.Exit(-1) } @@ -87,17 +86,17 @@ func StartUp(ctx *armadacontext.Context, log *logrus.Entry, config configuration } var etcdClustersHealthMonitoring healthmonitor.HealthMonitor if len(etcdClusterHealthMonitoringByName) > 0 { - log.Info("etcd URLs provided; monitoring etcd health enabled") + ctx.Info("etcd URLs provided; monitoring etcd health enabled") etcdClustersHealthMonitoring = healthmonitor.NewMultiHealthMonitor( "overall_etcd", etcdClusterHealthMonitoringByName, ).WithMetricsPrefix( metrics.ArmadaExecutorMetricsPrefix, ) - g.Go(func() error { return etcdClustersHealthMonitoring.Run(ctx, log) }) + g.Go(func() error { return etcdClustersHealthMonitoring.Run(ctx) }) prometheus.MustRegister(etcdClustersHealthMonitoring) } else { - log.Info("no etcd URLs provided; etcd health isn't monitored") + ctx.Info("no etcd URLs provided; etcd health isn't monitored") } clusterContext := executor_context.NewClusterContext( @@ -113,11 +112,11 @@ func StartUp(ctx *armadacontext.Context, log *logrus.Entry, config configuration taskManager := task.NewBackgroundTaskManager(metrics.ArmadaExecutorMetricsPrefix) taskManager.Register(clusterContext.ProcessPodsToDelete, config.Task.PodDeletionInterval, "pod_deletion") - return StartUpWithContext(log, config, clusterContext, etcdClustersHealthMonitoring, taskManager, wg) + return StartUpWithContext(ctx, config, clusterContext, etcdClustersHealthMonitoring, taskManager, wg) } func StartUpWithContext( - log *logrus.Entry, + ctx *armadacontext.Context, config configuration.ExecutorConfiguration, clusterContext executor_context.ClusterContext, clusterHealthMonitor healthmonitor.HealthMonitor, @@ -133,21 +132,18 @@ func StartUpWithContext( ) if config.Kubernetes.PendingPodChecks == nil { - log.Error("Config error: Missing pending pod checks") - os.Exit(-1) + ctx.Fatalf("Config error: Missing pending pod checks") } pendingPodChecker, err := podchecks.NewPodChecks(*config.Kubernetes.PendingPodChecks) if err != nil { - log.Errorf("Config error in pending pod checks: %s", err) - os.Exit(-1) + ctx.Fatalf("Config error in pending pod checks: %s", err) } - stopExecutorApiComponents := setupExecutorApiComponents(log, config, clusterContext, clusterHealthMonitor, taskManager, pendingPodChecker, nodeInfoService, podUtilisationService) + stopExecutorApiComponents := setupExecutorApiComponents(ctx, config, clusterContext, clusterHealthMonitor, taskManager, pendingPodChecker, nodeInfoService, podUtilisationService) resourceCleanupService, err := service.NewResourceCleanupService(clusterContext, config.Kubernetes) if err != nil { - log.Errorf("Error creating resource cleanup service: %s", err) - os.Exit(-1) + ctx.Fatalf("Error creating resource cleanup service: %s", err) } taskManager.Register(resourceCleanupService.CleanupResources, config.Task.ResourceCleanupInterval, "resource_cleanup") @@ -159,15 +155,15 @@ func StartUpWithContext( clusterContext.Stop() stopExecutorApiComponents() if taskManager.StopAll(10 * time.Second) { - log.Warnf("Graceful shutdown timed out") + ctx.Warnf("Graceful shutdown timed out") } - log.Infof("Shutdown complete") + ctx.Infof("Shutdown complete") wg.Done() }, wg } func setupExecutorApiComponents( - log *logrus.Entry, + ctx *armadacontext.Context, config configuration.ExecutorConfiguration, clusterContext executor_context.ClusterContext, clusterHealthMonitor healthmonitor.HealthMonitor, @@ -178,8 +174,7 @@ func setupExecutorApiComponents( ) func() { conn, err := createConnectionToApi(config.ExecutorApiConnection, config.Client.MaxMessageSizeBytes, config.GRPC) if err != nil { - log.Errorf("Failed to connect to Executor API because: %s", err) - os.Exit(-1) + ctx.Fatalf("Failed to connect to Executor API because: %s", err) } executorApiClient := executorapi.NewExecutorApiClient(conn) @@ -203,8 +198,7 @@ func setupExecutorApiComponents( clock.RealClock{}, 200) if err != nil { - log.Errorf("Failed to create job event reporter: %s", err) - os.Exit(-1) + ctx.Fatalf("Failed to create job event reporter: %s", err) } submitter := job.NewSubmitter( @@ -243,8 +237,7 @@ func setupExecutorApiComponents( config.Kubernetes.StuckTerminatingPodExpiry, ) if err != nil { - log.Errorf("Failed to create pod issue service: %s", err) - os.Exit(-1) + ctx.Fatalf("Failed to create pod issue service: %s", err) } taskManager.Register(podIssueService.HandlePodIssues, config.Task.PodIssueHandlingInterval, "pod_issue_handling") @@ -255,8 +248,7 @@ func setupExecutorApiComponents( taskManager.Register(eventReporter.ReportMissingJobEvents, config.Task.MissingJobEventReconciliationInterval, "event_reconciliation") _, err = pod_metrics.ExposeClusterContextMetrics(clusterContext, clusterUtilisationService, podUtilisationService, nodeInfoService) if err != nil { - log.Errorf("Failed to setup cluster context metrics: %s", err) - os.Exit(-1) + ctx.Fatalf("Failed to setup cluster context metrics: %s", err) } runStateMetricsCollector := runstate.NewJobRunStateStoreMetricsCollector(jobRunState) prometheus.MustRegister(runStateMetricsCollector) @@ -268,8 +260,7 @@ func setupExecutorApiComponents( eventReporter, config.Task.UtilisationEventReportingInterval) if err != nil { - log.Errorf("Failed to pod utilisation reporter: %s", err) - os.Exit(-1) + ctx.Fatalf("Failed to pod utilisation reporter: %s", err) } taskManager.Register( podUtilisationReporter.ReportUtilisationEvents, diff --git a/internal/executor/context/cluster_context.go b/internal/executor/context/cluster_context.go index 53773658e45..25c4cc8176a 100644 --- a/internal/executor/context/cluster_context.go +++ b/internal/executor/context/cluster_context.go @@ -6,7 +6,6 @@ import ( "time" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" networking "k8s.io/api/networking/v1" @@ -27,6 +26,7 @@ import ( "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/cluster" + log "github.com/armadaproject/armada/internal/common/logging" util2 "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/internal/executor/configuration" "github.com/armadaproject/armada/internal/executor/domain" diff --git a/internal/executor/fake/application.go b/internal/executor/fake/application.go index 8f26b43405f..ed34153c8e0 100644 --- a/internal/executor/fake/application.go +++ b/internal/executor/fake/application.go @@ -3,8 +3,7 @@ package fake import ( "sync" - "github.com/sirupsen/logrus" - + "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/task" "github.com/armadaproject/armada/internal/executor" "github.com/armadaproject/armada/internal/executor/configuration" @@ -12,11 +11,11 @@ import ( "github.com/armadaproject/armada/internal/executor/metrics" ) -func StartUp(config configuration.ExecutorConfiguration, nodes []*context.NodeSpec) (func(), *sync.WaitGroup) { +func StartUp(ctx *armadacontext.Context, config configuration.ExecutorConfiguration, nodes []*context.NodeSpec) (func(), *sync.WaitGroup) { wg := &sync.WaitGroup{} wg.Add(1) return executor.StartUpWithContext( - logrus.NewEntry(logrus.StandardLogger()), + ctx, config, context.NewFakeClusterContext(config.Application, config.Kubernetes.NodeIdLabel, nodes), nil, diff --git a/internal/executor/fake/context/context.go b/internal/executor/fake/context/context.go index e4ec04a0c9e..612cd33ecd8 100644 --- a/internal/executor/fake/context/context.go +++ b/internal/executor/fake/context/context.go @@ -12,7 +12,6 @@ import ( "github.com/google/uuid" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" @@ -24,6 +23,7 @@ import ( "k8s.io/kubelet/pkg/apis/stats/v1alpha1" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" "github.com/armadaproject/armada/internal/executor/configuration" cluster_context "github.com/armadaproject/armada/internal/executor/context" diff --git a/internal/executor/job/job_run_state_store.go b/internal/executor/job/job_run_state_store.go index dfe648fd7eb..646edc1f229 100644 --- a/internal/executor/job/job_run_state_store.go +++ b/internal/executor/job/job_run_state_store.go @@ -4,10 +4,10 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/util" ) diff --git a/internal/executor/job/processors/preempt_runs.go b/internal/executor/job/processors/preempt_runs.go index 9d98a73cfde..959c86f9d42 100644 --- a/internal/executor/job/processors/preempt_runs.go +++ b/internal/executor/job/processors/preempt_runs.go @@ -4,10 +4,10 @@ import ( "fmt" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" executorContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" "github.com/armadaproject/armada/internal/executor/job" diff --git a/internal/executor/job/processors/remove_runs.go b/internal/executor/job/processors/remove_runs.go index 37942110605..d81f2958e5d 100644 --- a/internal/executor/job/processors/remove_runs.go +++ b/internal/executor/job/processors/remove_runs.go @@ -3,10 +3,10 @@ package processors import ( "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" executorContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" "github.com/armadaproject/armada/internal/executor/job" diff --git a/internal/executor/job/submit.go b/internal/executor/job/submit.go index 9943a0f3b68..03265ba3395 100644 --- a/internal/executor/job/submit.go +++ b/internal/executor/job/submit.go @@ -6,12 +6,12 @@ import ( "sync" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/armadaproject/armada/internal/common/armadaerrors" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/internal/executor/configuration" "github.com/armadaproject/armada/internal/executor/context" diff --git a/internal/executor/metrics/pod_metrics/cluster_context.go b/internal/executor/metrics/pod_metrics/cluster_context.go index e86881411b1..028c5f1f86a 100644 --- a/internal/executor/metrics/pod_metrics/cluster_context.go +++ b/internal/executor/metrics/pod_metrics/cluster_context.go @@ -3,11 +3,11 @@ package pod_metrics import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/client-go/tools/cache" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" diff --git a/internal/executor/podchecks/container_state_checks.go b/internal/executor/podchecks/container_state_checks.go index 51826afd88b..aaaee5faa48 100644 --- a/internal/executor/podchecks/container_state_checks.go +++ b/internal/executor/podchecks/container_state_checks.go @@ -6,11 +6,11 @@ import ( "strings" "time" + v1 "k8s.io/api/core/v1" + + log "github.com/armadaproject/armada/internal/common/logging" config "github.com/armadaproject/armada/internal/executor/configuration/podchecks" "github.com/armadaproject/armada/internal/executor/util" - - log "github.com/sirupsen/logrus" - v1 "k8s.io/api/core/v1" ) type containerStateChecker interface { diff --git a/internal/executor/podchecks/event_checks.go b/internal/executor/podchecks/event_checks.go index ddfe5469c4d..ee0a6a24631 100644 --- a/internal/executor/podchecks/event_checks.go +++ b/internal/executor/podchecks/event_checks.go @@ -6,9 +6,9 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + log "github.com/armadaproject/armada/internal/common/logging" config "github.com/armadaproject/armada/internal/executor/configuration/podchecks" ) diff --git a/internal/executor/podchecks/pod_checks.go b/internal/executor/podchecks/pod_checks.go index 496de36fd89..73be56223b1 100644 --- a/internal/executor/podchecks/pod_checks.go +++ b/internal/executor/podchecks/pod_checks.go @@ -5,9 +5,9 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/slices" config "github.com/armadaproject/armada/internal/executor/configuration/podchecks" "github.com/armadaproject/armada/internal/executor/util" diff --git a/internal/executor/reporter/job_event_reporter.go b/internal/executor/reporter/job_event_reporter.go index 002b2d8d833..3df11d42696 100644 --- a/internal/executor/reporter/job_event_reporter.go +++ b/internal/executor/reporter/job_event_reporter.go @@ -4,11 +4,11 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/utils/clock" + log "github.com/armadaproject/armada/internal/common/logging" clusterContext "github.com/armadaproject/armada/internal/executor/context" domain2 "github.com/armadaproject/armada/internal/executor/domain" "github.com/armadaproject/armada/internal/executor/job" diff --git a/internal/executor/service/cluster_allocation.go b/internal/executor/service/cluster_allocation.go index 405a7308128..703d41a0526 100644 --- a/internal/executor/service/cluster_allocation.go +++ b/internal/executor/service/cluster_allocation.go @@ -3,12 +3,11 @@ package service import ( "fmt" - "github.com/sirupsen/logrus" - log "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/api/errors" "github.com/armadaproject/armada/internal/common/healthmonitor" "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" util2 "github.com/armadaproject/armada/internal/common/util" executorContext "github.com/armadaproject/armada/internal/executor/context" @@ -49,7 +48,7 @@ func (allocationService *ClusterAllocationService) AllocateSpareClusterCapacity( // If a health monitor is provided, avoid leasing jobs when the cluster is unhealthy. if allocationService.clusterHealthMonitor != nil { if ok, reason, err := allocationService.clusterHealthMonitor.IsHealthy(); err != nil { - logging.WithStacktrace(logrus.NewEntry(logrus.StandardLogger()), err).Error("failed to check cluster health") + logging.WithStacktrace(err).Error("failed to check cluster health") return } else if !ok { log.Warnf("cluster is not healthy; will not request more jobs: %s", reason) diff --git a/internal/executor/service/job_requester.go b/internal/executor/service/job_requester.go index 9f06c0c0cc6..f6dd2c38b2d 100644 --- a/internal/executor/service/job_requester.go +++ b/internal/executor/service/job_requester.go @@ -4,10 +4,10 @@ import ( "fmt" "time" - log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/internal/executor/configuration" executorContext "github.com/armadaproject/armada/internal/executor/context" diff --git a/internal/executor/service/lease_requester.go b/internal/executor/service/lease_requester.go index c95f31b4725..39913fd0e79 100644 --- a/internal/executor/service/lease_requester.go +++ b/internal/executor/service/lease_requester.go @@ -4,13 +4,13 @@ import ( "fmt" "io" - grpcretry "github.com/grpc-ecosystem/go-grpc-middleware/retry" + grpcretry "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/retry" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/encoding/gzip" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" clusterContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/pkg/executorapi" diff --git a/internal/executor/service/pod_issue_handler.go b/internal/executor/service/pod_issue_handler.go index c94a5b175e0..72e9ea57531 100644 --- a/internal/executor/service/pod_issue_handler.go +++ b/internal/executor/service/pod_issue_handler.go @@ -6,13 +6,13 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" "k8s.io/kubectl/pkg/describe" "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/executor/configuration" executorContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/job" diff --git a/internal/executor/service/resource_cleanup.go b/internal/executor/service/resource_cleanup.go index 2657d142750..41196e20721 100644 --- a/internal/executor/service/resource_cleanup.go +++ b/internal/executor/service/resource_cleanup.go @@ -4,10 +4,10 @@ import ( "sort" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/executor/configuration" clusterContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/util" diff --git a/internal/executor/util/pod_util.go b/internal/executor/util/pod_util.go index 4848f8044cd..72543c14875 100644 --- a/internal/executor/util/pod_util.go +++ b/internal/executor/util/pod_util.go @@ -6,11 +6,11 @@ import ( "strconv" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/internal/executor/domain" "github.com/armadaproject/armada/internal/server/configuration" diff --git a/internal/executor/utilisation/cluster_utilisation.go b/internal/executor/utilisation/cluster_utilisation.go index a26726a8ea5..e2549731796 100644 --- a/internal/executor/utilisation/cluster_utilisation.go +++ b/internal/executor/utilisation/cluster_utilisation.go @@ -4,9 +4,9 @@ import ( "fmt" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" armadaslices "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/internal/executor/context" diff --git a/internal/executor/utilisation/job_utilisation_reporter.go b/internal/executor/utilisation/job_utilisation_reporter.go index d40ce0919ae..b4387db22bf 100644 --- a/internal/executor/utilisation/job_utilisation_reporter.go +++ b/internal/executor/utilisation/job_utilisation_reporter.go @@ -4,10 +4,10 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/cache" + log "github.com/armadaproject/armada/internal/common/logging" clusterContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" "github.com/armadaproject/armada/internal/executor/reporter" diff --git a/internal/executor/utilisation/pod_utilisation.go b/internal/executor/utilisation/pod_utilisation.go index ae8d43092c5..ec075b198e8 100644 --- a/internal/executor/utilisation/pod_utilisation.go +++ b/internal/executor/utilisation/pod_utilisation.go @@ -5,9 +5,9 @@ import ( "sync" "time" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" + log "github.com/armadaproject/armada/internal/common/logging" armadaresource "github.com/armadaproject/armada/internal/common/resource" commonUtil "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/internal/executor/configuration" diff --git a/internal/executor/utilisation/pod_utilisation_custom_metrics.go b/internal/executor/utilisation/pod_utilisation_custom_metrics.go index dffaf5c25c5..bf6102dc1f8 100644 --- a/internal/executor/utilisation/pod_utilisation_custom_metrics.go +++ b/internal/executor/utilisation/pod_utilisation_custom_metrics.go @@ -5,11 +5,11 @@ import ( "net/http" "github.com/prometheus/common/model" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/utils/clock" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/executor/configuration" clusterContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" diff --git a/internal/executor/utilisation/pod_utilisation_kubelet_metrics.go b/internal/executor/utilisation/pod_utilisation_kubelet_metrics.go index d7b32d01c7e..4a7aef1ac11 100644 --- a/internal/executor/utilisation/pod_utilisation_kubelet_metrics.go +++ b/internal/executor/utilisation/pod_utilisation_kubelet_metrics.go @@ -4,13 +4,12 @@ import ( "sync" "time" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/kubelet/pkg/apis/stats/v1alpha1" - log "github.com/sirupsen/logrus" - v1 "k8s.io/api/core/v1" - "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" clusterContext "github.com/armadaproject/armada/internal/executor/context" "github.com/armadaproject/armada/internal/executor/domain" ) diff --git a/internal/executor/utilisation/prometheus_scraping.go b/internal/executor/utilisation/prometheus_scraping.go index 2e71dcf4cac..be98f0964f3 100644 --- a/internal/executor/utilisation/prometheus_scraping.go +++ b/internal/executor/utilisation/prometheus_scraping.go @@ -6,13 +6,12 @@ import ( "net/http" "sync" + "github.com/prometheus/common/expfmt" + "github.com/prometheus/common/model" discovery "k8s.io/api/discovery/v1" + log "github.com/armadaproject/armada/internal/common/logging" commonUtil "github.com/armadaproject/armada/internal/common/util" - - "github.com/prometheus/common/expfmt" - "github.com/prometheus/common/model" - log "github.com/sirupsen/logrus" ) type httpGetter interface { diff --git a/internal/lookoutingesterv2/dbloadtester/queue.go b/internal/lookoutingesterv2/dbloadtester/queue.go index 6ad9775488f..30c60cbe3b5 100644 --- a/internal/lookoutingesterv2/dbloadtester/queue.go +++ b/internal/lookoutingesterv2/dbloadtester/queue.go @@ -8,11 +8,11 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/google/uuid" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "github.com/armadaproject/armada/internal/common/database/lookout" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/pkg/armadaevents" @@ -79,7 +79,7 @@ func (q *QueueEventGenerator) Generate(eventsCh chan<- *utils.EventsWithIds[*arm } events, err := q.generateEventsAtTime(i) if err != nil { - log.Panicf("failed to generate events %s", err) + log.Fatalf("failed to generate events %s", err) } if len(events) == 0 { continue diff --git a/internal/lookoutingesterv2/dbloadtester/simulator.go b/internal/lookoutingesterv2/dbloadtester/simulator.go index c15e31825c9..b298bf74827 100644 --- a/internal/lookoutingesterv2/dbloadtester/simulator.go +++ b/internal/lookoutingesterv2/dbloadtester/simulator.go @@ -8,7 +8,6 @@ import ( "time" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "github.com/armadaproject/armada/internal/common/armadacontext" @@ -16,6 +15,7 @@ import ( "github.com/armadaproject/armada/internal/common/database" "github.com/armadaproject/armada/internal/common/ingest" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/lookoutingesterv2/configuration" "github.com/armadaproject/armada/internal/lookoutingesterv2/instructions" "github.com/armadaproject/armada/internal/lookoutingesterv2/lookoutdb" diff --git a/internal/lookoutingesterv2/ingester.go b/internal/lookoutingesterv2/ingester.go index 38eceaf7665..feadfce9939 100644 --- a/internal/lookoutingesterv2/ingester.go +++ b/internal/lookoutingesterv2/ingester.go @@ -5,7 +5,6 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/app" @@ -14,6 +13,7 @@ import ( "github.com/armadaproject/armada/internal/common/database" "github.com/armadaproject/armada/internal/common/ingest" "github.com/armadaproject/armada/internal/common/ingest/jobsetevents" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/lookoutingesterv2/configuration" "github.com/armadaproject/armada/internal/lookoutingesterv2/instructions" diff --git a/internal/lookoutingesterv2/instructions/instructions.go b/internal/lookoutingesterv2/instructions/instructions.go index 6be893b85ea..04e85cc13e6 100644 --- a/internal/lookoutingesterv2/instructions/instructions.go +++ b/internal/lookoutingesterv2/instructions/instructions.go @@ -5,7 +5,6 @@ import ( "time" "github.com/gogo/protobuf/proto" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/utils/pointer" @@ -15,6 +14,7 @@ import ( "github.com/armadaproject/armada/internal/common/eventutil" "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/internal/lookoutingesterv2/model" @@ -81,7 +81,7 @@ func (c *InstructionConverter) convertSequence( var err error if event.Created == nil { c.metrics.RecordPulsarMessageError(metrics.PulsarMessageErrorProcessing) - log.WithError(err).Warnf("Missing timestamp for event at index %d.", idx) + log.Warnf("Missing timestamp for event at index %d.", idx) continue } ts := protoutil.ToStdTime(event.Created) diff --git a/internal/lookoutingesterv2/lookoutdb/insertion.go b/internal/lookoutingesterv2/lookoutdb/insertion.go index 14af9937ffa..6b68c40d27c 100644 --- a/internal/lookoutingesterv2/lookoutdb/insertion.go +++ b/internal/lookoutingesterv2/lookoutdb/insertion.go @@ -8,12 +8,12 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/armadaerrors" "github.com/armadaproject/armada/internal/common/database/lookout" commonmetrics "github.com/armadaproject/armada/internal/common/ingest/metrics" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/lookoutingesterv2/metrics" "github.com/armadaproject/armada/internal/lookoutingesterv2/model" ) diff --git a/internal/lookoutv2/application.go b/internal/lookoutv2/application.go index bf7c03a9ecd..ec6dbf6b10e 100644 --- a/internal/lookoutv2/application.go +++ b/internal/lookoutv2/application.go @@ -8,12 +8,12 @@ import ( "github.com/go-openapi/runtime/middleware" "github.com/jessevdk/go-flags" "github.com/prometheus/client_golang/prometheus" - "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" "github.com/armadaproject/armada/internal/common/database" + "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/internal/lookoutv2/configuration" "github.com/armadaproject/armada/internal/lookoutv2/conversions" @@ -48,7 +48,7 @@ func Serve(configuration configuration.LookoutV2Config) error { // create new service API api := operations.NewLookoutAPI(swaggerSpec) - logger := logrus.NewEntry(logrus.StandardLogger()) + logger := logging.StdLogger() api.Logger = logger.Debugf diff --git a/internal/lookoutv2/generate/main.go b/internal/lookoutv2/generate/main.go index a916b94fd2d..28a607d89df 100644 --- a/internal/lookoutv2/generate/main.go +++ b/internal/lookoutv2/generate/main.go @@ -6,7 +6,7 @@ import ( "os/exec" "path/filepath" - log "github.com/sirupsen/logrus" + log "github.com/armadaproject/armada/internal/common/logging" ) const ( diff --git a/internal/lookoutv2/pruner/pruner.go b/internal/lookoutv2/pruner/pruner.go index 397bb3b1538..425185a3e33 100644 --- a/internal/lookoutv2/pruner/pruner.go +++ b/internal/lookoutv2/pruner/pruner.go @@ -6,10 +6,10 @@ import ( "github.com/hashicorp/go-multierror" "github.com/jackc/pgx/v5" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" ) func PruneDb( diff --git a/internal/lookoutv2/repository/getjoberror.go b/internal/lookoutv2/repository/getjoberror.go index ebcb14a59fa..d418ed89990 100644 --- a/internal/lookoutv2/repository/getjoberror.go +++ b/internal/lookoutv2/repository/getjoberror.go @@ -4,10 +4,10 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" + log "github.com/armadaproject/armada/internal/common/logging" ) type GetJobErrorRepository interface { diff --git a/internal/lookoutv2/repository/getjobrundebugmessage.go b/internal/lookoutv2/repository/getjobrundebugmessage.go index 5b4841dc7c7..ace8f6d0ca7 100644 --- a/internal/lookoutv2/repository/getjobrundebugmessage.go +++ b/internal/lookoutv2/repository/getjobrundebugmessage.go @@ -4,10 +4,10 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" + log "github.com/armadaproject/armada/internal/common/logging" ) type GetJobRunDebugMessageRepository interface { diff --git a/internal/lookoutv2/repository/getjobrunerror.go b/internal/lookoutv2/repository/getjobrunerror.go index b878c9291fb..b97b0ab8206 100644 --- a/internal/lookoutv2/repository/getjobrunerror.go +++ b/internal/lookoutv2/repository/getjobrunerror.go @@ -4,10 +4,10 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" + log "github.com/armadaproject/armada/internal/common/logging" ) type GetJobRunErrorRepository interface { diff --git a/internal/lookoutv2/repository/getjobspec.go b/internal/lookoutv2/repository/getjobspec.go index 8e51fe89871..ea1216e3a30 100644 --- a/internal/lookoutv2/repository/getjobspec.go +++ b/internal/lookoutv2/repository/getjobspec.go @@ -5,10 +5,10 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/api" ) diff --git a/internal/lookoutv2/repository/querybuilder.go b/internal/lookoutv2/repository/querybuilder.go index 01255998086..18c93c3ed2f 100644 --- a/internal/lookoutv2/repository/querybuilder.go +++ b/internal/lookoutv2/repository/querybuilder.go @@ -5,10 +5,10 @@ import ( "strings" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "golang.org/x/exp/slices" "github.com/armadaproject/armada/internal/common/database/lookout" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/lookoutv2/model" ) @@ -411,7 +411,7 @@ func operatorForMatch(match string) (string, error) { return "<=", nil default: err := errors.Errorf("unsupported match type: %s", match) - logrus.Error(err) + log.Error(err.Error()) return "", err } } diff --git a/internal/lookoutv2/repository/util.go b/internal/lookoutv2/repository/util.go index 1072f91a77b..6c136532626 100644 --- a/internal/lookoutv2/repository/util.go +++ b/internal/lookoutv2/repository/util.go @@ -8,7 +8,6 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/gogo/protobuf/types" "github.com/google/uuid" - log "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/utils/clock" @@ -18,6 +17,7 @@ import ( "github.com/armadaproject/armada/internal/common/database/lookout" "github.com/armadaproject/armada/internal/common/eventutil" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/common/pulsarutils" "github.com/armadaproject/armada/internal/common/util" diff --git a/internal/scheduler/api.go b/internal/scheduler/api.go index daf8efe8fd8..608faa82a85 100644 --- a/internal/scheduler/api.go +++ b/internal/scheduler/api.go @@ -7,7 +7,6 @@ import ( "github.com/gogo/protobuf/proto" "github.com/gogo/protobuf/types" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" v1 "k8s.io/api/core/v1" @@ -17,7 +16,7 @@ import ( "github.com/armadaproject/armada/internal/common/armadaerrors" "github.com/armadaproject/armada/internal/common/auth" "github.com/armadaproject/armada/internal/common/compress" - "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/maps" "github.com/armadaproject/armada/internal/common/pulsarutils" priorityTypes "github.com/armadaproject/armada/internal/common/types" @@ -371,7 +370,7 @@ func (srv *ExecutorApi) executorFromLeaseRequest(ctx *armadacontext.Context, req now := srv.clock.Now().UTC() for _, nodeInfo := range req.Nodes { if node, err := executorapi.NewNodeFromNodeInfo(nodeInfo, req.ExecutorId, srv.allowedPriorities, now); err != nil { - logging.WithStacktrace(ctx, err).Warnf( + ctx.Logger().WithStacktrace(err).Warnf( "skipping node %s from executor %s", nodeInfo.GetName(), req.GetExecutorId(), ) } else { diff --git a/internal/scheduler/metrics.go b/internal/scheduler/metrics.go index 8cc2f3f49d1..a64f040efb5 100644 --- a/internal/scheduler/metrics.go +++ b/internal/scheduler/metrics.go @@ -10,7 +10,6 @@ import ( "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" - "github.com/armadaproject/armada/internal/common/logging" armadamaps "github.com/armadaproject/armada/internal/common/maps" commonmetrics "github.com/armadaproject/armada/internal/common/metrics" "github.com/armadaproject/armada/internal/common/resource" @@ -103,8 +102,8 @@ func (c *MetricsCollector) Run(ctx *armadacontext.Context) error { case <-ticker.C(): err := c.refresh(ctx) if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Warnf("error refreshing metrics state") } } diff --git a/internal/scheduler/nodedb/nodeiteration.go b/internal/scheduler/nodedb/nodeiteration.go index 3e171007139..fda04565657 100644 --- a/internal/scheduler/nodedb/nodeiteration.go +++ b/internal/scheduler/nodedb/nodeiteration.go @@ -6,9 +6,9 @@ import ( "github.com/hashicorp/go-memdb" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "golang.org/x/exp/slices" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/scheduler/internaltypes" ) @@ -219,7 +219,7 @@ type NodeTypeIterator struct { priority int32 // Used to index into node.keys to assert that keys are always increasing. // This to detect if the iterator gets stuck. - // TODO(albin): With better testing we should be able to remove this. + // TODO(albin): WithField better testing we should be able to remove this. keyIndex int // Name of the memdb index used for node iteration. // Should correspond to the priority set for this iterator. diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index fd796e1bd82..356817ddaef 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -13,7 +13,6 @@ import ( "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" - "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/scheduler/database" "github.com/armadaproject/armada/internal/scheduler/jobdb" @@ -153,7 +152,7 @@ func (s *Scheduler) Run(ctx *armadacontext.Context) error { syncContext, cancel := armadacontext.WithTimeout(ctx, 5*time.Minute) err := s.ensureDbUpToDate(syncContext, 1*time.Second) if err != nil { - logging.WithStacktrace(ctx, err).Error("could not become leader") + ctx.Logger().WithStacktrace(err).Error("could not become leader") leaderToken = leader.InvalidLeaderToken() } else { fullUpdate = true @@ -175,7 +174,7 @@ func (s *Scheduler) Run(ctx *armadacontext.Context) error { result, err := s.cycle(ctx, fullUpdate, leaderToken, shouldSchedule) if err != nil { - logging.WithStacktrace(ctx, err).Error("scheduling cycle failure") + ctx.Logger().WithStacktrace(err).Error("scheduling cycle failure") leaderToken = leader.InvalidLeaderToken() } @@ -981,7 +980,9 @@ func (s *Scheduler) initialise(ctx *armadacontext.Context) error { return nil default: if _, _, err := s.syncState(ctx, true); err != nil { - logging.WithStacktrace(ctx, err).Error("failed to initialise; trying again in 1 second") + ctx.Logger(). + WithStacktrace(err). + Error("failed to initialise; trying again in 1 second") time.Sleep(1 * time.Second) } else { ctx.Info("initialisation succeeded") @@ -1008,7 +1009,9 @@ func (s *Scheduler) ensureDbUpToDate(ctx *armadacontext.Context, pollInterval ti default: numSent, err = s.publisher.PublishMarkers(ctx, groupId) if err != nil { - logging.WithStacktrace(ctx, err).Error("Error sending marker messages to pulsar") + ctx.Logger(). + WithStacktrace(err). + Error("Error sending marker messages to pulsar") s.clock.Sleep(pollInterval) } else { messagesSent = true @@ -1024,8 +1027,8 @@ func (s *Scheduler) ensureDbUpToDate(ctx *armadacontext.Context, pollInterval ti default: numReceived, err := s.jobRepository.CountReceivedPartitions(ctx, groupId) if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Error("Error querying the database or marker messages") } if numSent == numReceived { diff --git a/internal/scheduler/schedulerapp.go b/internal/scheduler/schedulerapp.go index 7482c38ffa7..c45097a59df 100644 --- a/internal/scheduler/schedulerapp.go +++ b/internal/scheduler/schedulerapp.go @@ -10,10 +10,9 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/google/uuid" - grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" + grpc_logging "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -26,7 +25,7 @@ import ( dbcommon "github.com/armadaproject/armada/internal/common/database" grpcCommon "github.com/armadaproject/armada/internal/common/grpc" "github.com/armadaproject/armada/internal/common/health" - "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/internal/common/pulsarutils" "github.com/armadaproject/armada/internal/common/pulsarutils/jobsetevents" @@ -118,8 +117,8 @@ func Run(config schedulerconfig.Configuration) error { defer func() { err := conn.Close() if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Warnf("Armada api client didn't close down cleanly") } }() @@ -183,7 +182,7 @@ func Run(config schedulerconfig.Configuration) error { if err != nil { return errors.WithMessage(err, "error creating auth services") } - grpcServer := grpcCommon.CreateGrpcServer(config.Grpc.KeepaliveParams, config.Grpc.KeepaliveEnforcementPolicy, authServices, config.Grpc.Tls, createLogrusLoggingOption()) + grpcServer := grpcCommon.CreateGrpcServer(config.Grpc.KeepaliveParams, config.Grpc.KeepaliveEnforcementPolicy, authServices, config.Grpc.Tls, createGrpcLoggingOption()) defer grpcServer.GracefulStop() lis, err := net.Listen("tcp", fmt.Sprintf(":%d", config.Grpc.Port)) if err != nil { @@ -363,19 +362,19 @@ func loadClusterConfig(ctx *armadacontext.Context) (*rest.Config, error) { return config, err } -// This changes the default logrus grpc logging to log OK messages at trace level +// This changes the default grpc logging to log OK messages at trace level // The reason for doing this are: // - Reduced logging // - We only care about failures, so lets only log failures // - We normally use these logs to work out who is calling us, however the Executor API is not public // and is only called by other Armada components -func createLogrusLoggingOption() grpc_logrus.Option { - return grpc_logrus.WithLevels(func(code codes.Code) log.Level { +func createGrpcLoggingOption() grpc_logging.Option { + return grpc_logging.WithLevels(func(code codes.Code) grpc_logging.Level { switch code { case codes.OK: - return log.TraceLevel + return grpc_logging.LevelDebug default: - return grpc_logrus.DefaultCodeToLevel(code) + return grpc_logging.DefaultServerCodeToLevel(code) } }) } diff --git a/internal/scheduler/scheduling/context/job.go b/internal/scheduler/scheduling/context/job.go index e9ad86ed71a..81ea403c5ce 100644 --- a/internal/scheduler/scheduling/context/job.go +++ b/internal/scheduler/scheduling/context/job.go @@ -8,11 +8,11 @@ import ( "time" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "golang.org/x/exp/maps" v1 "k8s.io/api/core/v1" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" armadamaps "github.com/armadaproject/armada/internal/common/maps" armadaslices "github.com/armadaproject/armada/internal/common/slices" schedulerconfig "github.com/armadaproject/armada/internal/scheduler/configuration" @@ -196,7 +196,7 @@ func JobSchedulingContextsFromJobs[J *jobdb.Job](jobs []J) []*JobSchedulingConte func JobSchedulingContextFromJob(job *jobdb.Job) *JobSchedulingContext { gangInfo, err := GangInfoFromLegacySchedulerJob(job) if err != nil { - logrus.Errorf("failed to extract gang info from job %s: %s", job.Id(), err) + log.Errorf("failed to extract gang info from job %s: %s", job.Id(), err) } return &JobSchedulingContext{ Created: time.Now(), diff --git a/internal/scheduler/scheduling/market_driven_preempting_queue_scheduler_test.go b/internal/scheduler/scheduling/market_driven_preempting_queue_scheduler_test.go index e2363a19b37..24ece4726f3 100644 --- a/internal/scheduler/scheduling/market_driven_preempting_queue_scheduler_test.go +++ b/internal/scheduler/scheduling/market_driven_preempting_queue_scheduler_test.go @@ -351,7 +351,7 @@ func TestMarketDrivenPreemptingQueueScheduler(t *testing.T) { cordonedNodes := map[int]bool{} ctx := armadacontext.Background() for i, round := range tc.Rounds { - ctx.FieldLogger = ctx.WithField("round", i) + ctx = armadacontext.WithLogField(ctx, "round", i) ctx.Infof("starting scheduling round %d", i) jobsByNodeId := map[string][]*jobdb.Job{} diff --git a/internal/scheduler/scheduling/preempting_queue_scheduler.go b/internal/scheduler/scheduling/preempting_queue_scheduler.go index 47207a5b8e0..c5609fc7c3f 100644 --- a/internal/scheduler/scheduling/preempting_queue_scheduler.go +++ b/internal/scheduler/scheduling/preempting_queue_scheduler.go @@ -99,7 +99,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche scheduledJobsById := make(map[string]*schedulercontext.JobSchedulingContext) // Evict preemptible jobs. - ctx.WithField("stage", "scheduling-algo").Infof("Evicting preemptible jobs") + ctx.Logger().WithField("stage", "scheduling-algo").Infof("Evicting preemptible jobs") evictorResult, inMemoryJobRepo, err := sch.evict( armadacontext.WithLogField(ctx, "stage", "evict for resource balancing"), NewNodeEvictor( @@ -144,14 +144,14 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche if err != nil { return nil, err } - ctx.WithField("stage", "scheduling-algo").Info("Finished evicting preemptible jobs") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Finished evicting preemptible jobs") for _, jctx := range evictorResult.EvictedJctxsByJobId { preemptedJobsById[jctx.JobId] = jctx } maps.Copy(sch.nodeIdByJobId, evictorResult.NodeIdByJobId) // Re-schedule evicted jobs/schedule new jobs. - ctx.WithField("stage", "scheduling-algo").Info("Performing initial scheduling of jobs onto nodes") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Performing initial scheduling of jobs onto nodes") schedulerResult, err := sch.schedule( armadacontext.WithLogField(ctx, "stage", "re-schedule after balancing eviction"), inMemoryJobRepo, @@ -162,7 +162,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche if err != nil { return nil, err } - ctx.WithField("stage", "scheduling-algo").Info("Finished initial scheduling of jobs onto nodes") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Finished initial scheduling of jobs onto nodes") for _, jctx := range schedulerResult.ScheduledJobs { if _, ok := preemptedJobsById[jctx.JobId]; ok { delete(preemptedJobsById, jctx.JobId) @@ -173,7 +173,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche maps.Copy(sch.nodeIdByJobId, schedulerResult.NodeIdByJobId) // Evict jobs on oversubscribed nodes. - ctx.WithField("stage", "scheduling-algo").Info("Evicting jobs from oversubscribed nodes") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Evicting jobs from oversubscribed nodes") reevictResult, inMemoryJobRepo, err := sch.evict( armadacontext.WithLogField(ctx, "stage", "evict oversubscribed"), NewOversubscribedEvictor( @@ -185,7 +185,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche if err != nil { return nil, err } - ctx.WithField("stage", "scheduling-algo").Info("Finished evicting jobs from oversubscribed nodes") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Finished evicting jobs from oversubscribed nodes") scheduledAndEvictedJobsById := armadamaps.FilterKeys( scheduledJobsById, func(jobId string) bool { @@ -205,7 +205,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche // Re-schedule evicted jobs/schedule new jobs. // Only necessary if a non-zero number of jobs were evicted. if len(reevictResult.EvictedJctxsByJobId) > 0 { - ctx.WithField("stage", "scheduling-algo").Info("Performing second scheduling ") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Performing second scheduling ") rescheduleSchedulerResult, rescheduleErr := sch.schedule( armadacontext.WithLogField(ctx, "stage", "schedule after oversubscribed eviction"), inMemoryJobRepo, @@ -217,7 +217,7 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche if rescheduleErr != nil { return nil, rescheduleErr } - ctx.WithField("stage", "scheduling-algo").Info("Finished second scheduling pass") + ctx.Logger().WithField("stage", "scheduling-algo").Info("Finished second scheduling pass") for _, jctx := range rescheduleSchedulerResult.ScheduledJobs { if _, ok := preemptedJobsById[jctx.JobId]; ok { delete(preemptedJobsById, jctx.JobId) @@ -231,14 +231,14 @@ func (sch *PreemptingQueueScheduler) Schedule(ctx *armadacontext.Context) (*Sche preemptedJobs := maps.Values(preemptedJobsById) scheduledJobs := maps.Values(scheduledJobsById) - ctx.WithField("stage", "scheduling-algo").Infof("Unbinding %d preempted and %d evicted jobs", len(preemptedJobs), len(maps.Values(scheduledAndEvictedJobsById))) + ctx.Logger().WithField("stage", "scheduling-algo").Infof("Unbinding %d preempted and %d evicted jobs", len(preemptedJobs), len(maps.Values(scheduledAndEvictedJobsById))) if err := sch.unbindJobs(append( slices.Clone(preemptedJobs), maps.Values(scheduledAndEvictedJobsById)...), ); err != nil { return nil, err } - ctx.WithField("stage", "scheduling-algo").Infof("Finished unbinding preempted and evicted jobs") + ctx.Logger().WithField("stage", "scheduling-algo").Infof("Finished unbinding preempted and evicted jobs") PopulatePreemptionDescriptions(preemptedJobs, scheduledJobs) schedulercontext.PrintJobSummary(ctx, "Preempting running jobs;", preemptedJobs) diff --git a/internal/scheduler/scheduling/preempting_queue_scheduler_test.go b/internal/scheduler/scheduling/preempting_queue_scheduler_test.go index 88d70e37257..52c27aca4b0 100644 --- a/internal/scheduler/scheduling/preempting_queue_scheduler_test.go +++ b/internal/scheduler/scheduling/preempting_queue_scheduler_test.go @@ -1,6 +1,7 @@ package scheduling import ( + "context" "fmt" "math/rand" "testing" @@ -25,7 +26,7 @@ import ( "github.com/armadaproject/armada/internal/scheduler/jobdb" "github.com/armadaproject/armada/internal/scheduler/nodedb" schedulerconstraints "github.com/armadaproject/armada/internal/scheduler/scheduling/constraints" - "github.com/armadaproject/armada/internal/scheduler/scheduling/context" + schedulingcontext "github.com/armadaproject/armada/internal/scheduler/scheduling/context" "github.com/armadaproject/armada/internal/scheduler/scheduling/fairness" "github.com/armadaproject/armada/internal/scheduler/testfixtures" "github.com/armadaproject/armada/pkg/api" @@ -1939,7 +1940,7 @@ func TestPreemptingQueueScheduler(t *testing.T) { cordonedNodes := map[int]bool{} ctx := armadacontext.Background() for i, round := range tc.Rounds { - ctx.FieldLogger = ctx.WithField("round", i) + ctx = armadacontext.WithLogField(ctx, "round", i) ctx.Infof("starting scheduling round %d", i) jobsByNodeId := map[string][]*jobdb.Job{} @@ -2019,7 +2020,7 @@ func TestPreemptingQueueScheduler(t *testing.T) { tc.SchedulingConfig, ) require.NoError(t, err) - sctx := context.NewSchedulingContext( + sctx := schedulingcontext.NewSchedulingContext( testfixtures.TestPool, fairnessCostProvider, limiter, @@ -2205,7 +2206,7 @@ func TestPreemptingQueueScheduler(t *testing.T) { // which jobs are preempted). slices.SortFunc( result.ScheduledJobs, - func(a, b *context.JobSchedulingContext) int { + func(a, b *schedulingcontext.JobSchedulingContext) int { if a.Job.SubmitTime().Before(b.Job.SubmitTime()) { return -1 } else if b.Job.SubmitTime().Before(a.Job.SubmitTime()) { @@ -2238,7 +2239,7 @@ func TestPreemptingQueueScheduler(t *testing.T) { } } -func jobIdsByQueueFromJobContexts(jctxs []*context.JobSchedulingContext) map[string][]string { +func jobIdsByQueueFromJobContexts(jctxs []*schedulingcontext.JobSchedulingContext) map[string][]string { rv := make(map[string][]string) for _, jctx := range jctxs { job := jctx.Job @@ -2332,9 +2333,7 @@ func BenchmarkPreemptingQueueScheduler(b *testing.B) { } for name, tc := range tests { b.Run(name, func(b *testing.B) { - ctx := armadacontext.Background() - ctx.FieldLogger = logging.NullLogger - + ctx := armadacontext.New(context.Background(), logging.NullLogger) jobsByQueue := make(map[string][]*jobdb.Job) priorityFactorByQueue := make(map[string]float64) for i := 0; i < tc.NumQueues; i++ { @@ -2380,7 +2379,7 @@ func BenchmarkPreemptingQueueScheduler(b *testing.B) { tc.SchedulingConfig, ) require.NoError(b, err) - sctx := context.NewSchedulingContext( + sctx := schedulingcontext.NewSchedulingContext( testfixtures.TestPool, fairnessCostProvider, limiter, @@ -2426,7 +2425,7 @@ func BenchmarkPreemptingQueueScheduler(b *testing.B) { err = jobDbTxn.BatchDelete( armadaslices.Map( result.ScheduledJobs, - func(jctx *context.JobSchedulingContext) string { + func(jctx *schedulingcontext.JobSchedulingContext) string { return jctx.JobId }, ), @@ -2451,7 +2450,7 @@ func BenchmarkPreemptingQueueScheduler(b *testing.B) { b.ResetTimer() for n := 0; n < b.N; n++ { - sctx := context.NewSchedulingContext( + sctx := schedulingcontext.NewSchedulingContext( "pool", fairnessCostProvider, limiter, diff --git a/internal/scheduler/scheduling/scheduling_algo.go b/internal/scheduler/scheduling/scheduling_algo.go index 7c70c902881..ec1a5b4e651 100644 --- a/internal/scheduler/scheduling/scheduling_algo.go +++ b/internal/scheduler/scheduling/scheduling_algo.go @@ -6,13 +6,13 @@ import ( "time" "github.com/pkg/errors" - "github.com/sirupsen/logrus" "golang.org/x/exp/maps" "golang.org/x/exp/slices" "golang.org/x/time/rate" "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" + log "github.com/armadaproject/armada/internal/common/logging" armadamaps "github.com/armadaproject/armada/internal/common/maps" armadaslices "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/internal/scheduler/configuration" @@ -603,7 +603,7 @@ func (l *FairSchedulingAlgo) populateNodeDb(nodeDb *nodedb.NodeDb, homeJobs []*j } nodeId := job.LatestRun().NodeId() if _, ok := nodesById[nodeId]; !ok { - logrus.Errorf( + log.Errorf( "job %s assigned to node %s on executor %s, but no such node found", job.Id(), nodeId, job.LatestRun().Executor(), ) @@ -618,7 +618,7 @@ func (l *FairSchedulingAlgo) populateNodeDb(nodeDb *nodedb.NodeDb, homeJobs []*j nodeId := job.LatestRun().NodeId() node, ok := nodesById[nodeId] if !ok { - logrus.Errorf( + log.Errorf( "job %s assigned to node %s on executor %s, but no such node found", job.Id(), nodeId, job.LatestRun().Executor(), ) diff --git a/internal/scheduler/simulator/simulator.go b/internal/scheduler/simulator/simulator.go index a2059fa317b..12d3754f808 100644 --- a/internal/scheduler/simulator/simulator.go +++ b/internal/scheduler/simulator/simulator.go @@ -525,7 +525,7 @@ func (s *Simulator) pushScheduleEvent(time time.Time) { func (s *Simulator) handleSimulatorEvent(ctx *armadacontext.Context, event Event) error { s.time = event.time - ctx = armadacontext.New(ctx.Context, ctx.FieldLogger.WithField("simulated time", event.time)) + ctx = armadacontext.WithLogField(ctx, "simulated time", event.time) switch e := event.eventSequenceOrScheduleEvent.(type) { case *armadaevents.EventSequence: if err := s.handleEventSequence(ctx, e); err != nil { @@ -609,10 +609,7 @@ func (s *Simulator) handleScheduleEvent(ctx *armadacontext.Context) error { schedulerCtx := ctx if s.SuppressSchedulerLogs { - schedulerCtx = &armadacontext.Context{ - Context: ctx.Context, - FieldLogger: logging.NullLogger, - } + schedulerCtx = armadacontext.New(ctx.Context, logging.NullLogger) } result, err := sch.Schedule(schedulerCtx) if err != nil { diff --git a/internal/scheduler/submitcheck.go b/internal/scheduler/submitcheck.go index 49bee2d2786..def6dd7b317 100644 --- a/internal/scheduler/submitcheck.go +++ b/internal/scheduler/submitcheck.go @@ -11,7 +11,6 @@ import ( "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" - "github.com/armadaproject/armada/internal/common/logging" armadaslices "github.com/armadaproject/armada/internal/common/slices" "github.com/armadaproject/armada/internal/scheduler/configuration" "github.com/armadaproject/armada/internal/scheduler/database" @@ -90,16 +89,16 @@ func (srv *SubmitChecker) Run(ctx *armadacontext.Context) error { func (srv *SubmitChecker) updateExecutors(ctx *armadacontext.Context) { queues, err := srv.queueCache.GetAll(ctx) if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Error("Error fetching queues") return } executors, err := srv.executorRepository.GetExecutors(ctx) if err != nil { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Error("Error fetching executors") return } @@ -140,8 +139,8 @@ func (srv *SubmitChecker) updateExecutors(ctx *armadacontext.Context) { nodeDb: nodeDb, } } else { - logging. - WithStacktrace(ctx, err). + ctx.Logger(). + WithStacktrace(err). Warnf("Error constructing nodedb for executor: %s", ex.Id) } } diff --git a/internal/scheduleringester/ingester.go b/internal/scheduleringester/ingester.go index 92e13647b18..fe0578f6028 100644 --- a/internal/scheduleringester/ingester.go +++ b/internal/scheduleringester/ingester.go @@ -6,7 +6,6 @@ import ( "github.com/apache/pulsar-client-go/pulsar" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common" "github.com/armadaproject/armada/internal/common/app" @@ -16,6 +15,7 @@ import ( controlplaneevents_ingest_utils "github.com/armadaproject/armada/internal/common/ingest/controlplaneevents" "github.com/armadaproject/armada/internal/common/ingest/jobsetevents" "github.com/armadaproject/armada/internal/common/ingest/metrics" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/profiling" "github.com/armadaproject/armada/pkg/armadaevents" "github.com/armadaproject/armada/pkg/controlplaneevents" diff --git a/internal/scheduleringester/instructions.go b/internal/scheduleringester/instructions.go index 2c971a95df8..d64a2f0b83e 100644 --- a/internal/scheduleringester/instructions.go +++ b/internal/scheduleringester/instructions.go @@ -6,7 +6,6 @@ import ( "github.com/gogo/protobuf/proto" "github.com/google/uuid" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "golang.org/x/exp/maps" "golang.org/x/exp/slices" @@ -14,6 +13,7 @@ import ( "github.com/armadaproject/armada/internal/common/compress" "github.com/armadaproject/armada/internal/common/ingest/metrics" "github.com/armadaproject/armada/internal/common/ingest/utils" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/scheduler/adapters" schedulerdb "github.com/armadaproject/armada/internal/scheduler/database" diff --git a/internal/server/event/conversion/conversions.go b/internal/server/event/conversion/conversions.go index 227f9084a93..7db8eaf72d4 100644 --- a/internal/server/event/conversion/conversions.go +++ b/internal/server/event/conversion/conversions.go @@ -3,9 +3,8 @@ package conversion import ( "time" - log "github.com/sirupsen/logrus" - "github.com/armadaproject/armada/internal/common/eventutil" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/pkg/api" "github.com/armadaproject/armada/pkg/armadaevents" diff --git a/internal/server/event/event.go b/internal/server/event/event.go index f2cb1b9d843..9c59c7bebfd 100644 --- a/internal/server/event/event.go +++ b/internal/server/event/event.go @@ -6,13 +6,13 @@ import ( "github.com/gogo/protobuf/types" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/armadaerrors" "github.com/armadaproject/armada/internal/common/auth" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/server/event/sequence" "github.com/armadaproject/armada/internal/server/permissions" armadaqueue "github.com/armadaproject/armada/internal/server/queue" diff --git a/internal/server/event/event_repository.go b/internal/server/event/event_repository.go index 124abdd021c..4292f7553b5 100644 --- a/internal/server/event/event_repository.go +++ b/internal/server/event/event_repository.go @@ -10,10 +10,10 @@ import ( pool "github.com/jolestar/go-commons-pool" "github.com/pkg/errors" "github.com/redis/go-redis/v9" - log "github.com/sirupsen/logrus" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/compress" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/server/event/conversion" "github.com/armadaproject/armada/internal/server/event/sequence" "github.com/armadaproject/armada/pkg/api" diff --git a/internal/server/server.go b/internal/server/server.go index badcc5689f8..e35e7974244 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -12,7 +12,6 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/redis/go-redis/extra/redisprometheus/v9" "github.com/redis/go-redis/v9" - log "github.com/sirupsen/logrus" "google.golang.org/grpc" "github.com/armadaproject/armada/internal/common/armadacontext" @@ -21,6 +20,7 @@ import ( "github.com/armadaproject/armada/internal/common/database" grpcCommon "github.com/armadaproject/armada/internal/common/grpc" "github.com/armadaproject/armada/internal/common/health" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/pulsarutils" controlplaneeventspulsarutils "github.com/armadaproject/armada/internal/common/pulsarutils/controlplaneevents" "github.com/armadaproject/armada/internal/common/pulsarutils/jobsetevents" diff --git a/internal/server/submit/submit.go b/internal/server/submit/submit.go index 63ea39d1fa6..0541e3f86a6 100644 --- a/internal/server/submit/submit.go +++ b/internal/server/submit/submit.go @@ -6,13 +6,13 @@ import ( "github.com/gogo/protobuf/types" "github.com/gogo/status" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "k8s.io/utils/clock" "github.com/armadaproject/armada/internal/common/armadacontext" "github.com/armadaproject/armada/internal/common/auth" "github.com/armadaproject/armada/internal/common/auth/permission" + log "github.com/armadaproject/armada/internal/common/logging" protoutil "github.com/armadaproject/armada/internal/common/proto" "github.com/armadaproject/armada/internal/common/pulsarutils" "github.com/armadaproject/armada/internal/common/slices" diff --git a/internal/testsuite/eventwatcher/eventwatcher.go b/internal/testsuite/eventwatcher/eventwatcher.go index 51d619d4050..c39f1d100c0 100644 --- a/internal/testsuite/eventwatcher/eventwatcher.go +++ b/internal/testsuite/eventwatcher/eventwatcher.go @@ -12,7 +12,7 @@ import ( "strings" "time" - "github.com/grpc-ecosystem/go-grpc-middleware/util/backoffutils" + "github.com/grpc-ecosystem/go-grpc-middleware/v2/util/backoffutils" "github.com/pkg/errors" "golang.org/x/exp/maps" "golang.org/x/sync/errgroup" diff --git a/pkg/client/auth/kubernetes/authentication.go b/pkg/client/auth/kubernetes/authentication.go index 85f14fe3c19..a3bb4a47f7a 100644 --- a/pkg/client/auth/kubernetes/authentication.go +++ b/pkg/client/auth/kubernetes/authentication.go @@ -4,13 +4,13 @@ import ( "context" "os" - log "github.com/sirupsen/logrus" "golang.org/x/oauth2" authv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/client/auth/oidc" ) diff --git a/pkg/client/auth/oidc/device.go b/pkg/client/auth/oidc/device.go index e6f9ae63474..3abee6473da 100644 --- a/pkg/client/auth/oidc/device.go +++ b/pkg/client/auth/oidc/device.go @@ -13,8 +13,6 @@ import ( openId "github.com/coreos/go-oidc" "golang.org/x/oauth2" - - "github.com/armadaproject/armada/internal/common/logging" ) type DeviceDetails struct { @@ -152,6 +150,11 @@ func makeErrorForHTTPResponse(resp *http.Response) error { if err != nil { return err } - safeURL := logging.SanitizeUserInput(resp.Request.URL.String()) + safeURL := sanitize(resp.Request.URL.String()) return fmt.Errorf("%s %s returned HTTP %s; \n\n %#q", resp.Request.Method, safeURL, resp.Status, bodyBytes) } + +func sanitize(str string) string { + safeStr := strings.ReplaceAll(str, "\n", "") + return strings.ReplaceAll(safeStr, "\r", "") +} diff --git a/pkg/client/auth/oidc/kubernetes.go b/pkg/client/auth/oidc/kubernetes.go index 3d540e6598d..4e141f4bf4c 100644 --- a/pkg/client/auth/oidc/kubernetes.go +++ b/pkg/client/auth/oidc/kubernetes.go @@ -12,8 +12,9 @@ import ( "strings" "time" - log "github.com/sirupsen/logrus" "golang.org/x/oauth2" + + log "github.com/armadaproject/armada/internal/common/logging" ) type KubernetesDetails struct { diff --git a/pkg/client/auth/oidc/pkce.go b/pkg/client/auth/oidc/pkce.go index f50b7bf2491..7d02a47a366 100644 --- a/pkg/client/auth/oidc/pkce.go +++ b/pkg/client/auth/oidc/pkce.go @@ -12,12 +12,10 @@ import ( "strconv" "strings" - "github.com/sirupsen/logrus" - - "github.com/armadaproject/armada/internal/common/logging" - openId "github.com/coreos/go-oidc" "golang.org/x/oauth2" + + log "github.com/armadaproject/armada/internal/common/logging" ) type PKCEDetails struct { @@ -29,7 +27,6 @@ type PKCEDetails struct { func AuthenticatePkce(config PKCEDetails) (*TokenCredentials, error) { ctx := context.Background() - log := logrus.StandardLogger().WithField("auth", "AuthenticatePkce") result := make(chan *oauth2.Token) errorResult := make(chan error) @@ -97,14 +94,14 @@ func AuthenticatePkce(config PKCEDetails) (*TokenCredentials, error) { go func() { if err := server.Serve(listener); err != nil { - logging.WithStacktrace(log, err).Error("unable to serve") + log.WithStacktrace(err).Error("unable to serve") } }() cmd, err := openBrowser("http://" + localUrl) defer func() { if err := cmd.Process.Kill(); err != nil { - logging.WithStacktrace(log, err).Error("unable to kill process") + log.WithStacktrace(err).Error("unable to kill process") } }() diff --git a/pkg/client/connection.go b/pkg/client/connection.go index 64e3b5dfed5..a9b086b93f6 100644 --- a/pkg/client/connection.go +++ b/pkg/client/connection.go @@ -6,7 +6,7 @@ import ( "strings" "time" - grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/retry" + grpc_retry "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/retry" "github.com/pkg/errors" "google.golang.org/grpc" "google.golang.org/grpc/credentials" diff --git a/pkg/client/load-test.go b/pkg/client/load-test.go index 448e2000b0d..ac947603289 100644 --- a/pkg/client/load-test.go +++ b/pkg/client/load-test.go @@ -6,13 +6,11 @@ import ( "sync" "time" - "github.com/sirupsen/logrus" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/armadaproject/armada/internal/common" - "github.com/armadaproject/armada/internal/common/logging" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/pkg/api" "github.com/armadaproject/armada/pkg/client/domain" ) @@ -231,7 +229,6 @@ func (apiLoadTester ArmadaLoadTester) monitorJobsUntilCompletion( jobIds chan string, eventChannel chan api.Event, ) []string { - log := logrus.StandardLogger().WithField("Armada", "LoadTester") var submittedIds []string = nil go func() { ids := []string{} @@ -258,7 +255,7 @@ func (apiLoadTester ArmadaLoadTester) monitorJobsUntilCompletion( return nil }) if err != nil { - logging.WithStacktrace(log, err).Error("unable to monitor jobs") + log.WithStacktrace(err).Error("unable to monitor jobs") } return submittedIds } @@ -281,8 +278,6 @@ func createJobSubmitRequestItems(jobDescs []*domain.JobSubmissionDescription) [] } func (apiLoadTester ArmadaLoadTester) cancelRemainingJobs(queue string, jobSetId string) { - log := logrus.StandardLogger().WithField("Armada", "LoadTester") - err := WithSubmitClient(apiLoadTester.apiConnectionDetails, func(client api.SubmitClient) error { timeout, _ := common.ContextWithDefaultTimeout() cancelRequest := &api.JobCancelRequest{ @@ -293,7 +288,7 @@ func (apiLoadTester ArmadaLoadTester) cancelRemainingJobs(queue string, jobSetId return err }) if err != nil { - logging.WithStacktrace(log, err).Error("unable to cancel jobs") + log.WithStacktrace(err).Error("unable to cancel jobs") } } diff --git a/pkg/client/queue/get_all.go b/pkg/client/queue/get_all.go index 2357115792d..785732f02e0 100644 --- a/pkg/client/queue/get_all.go +++ b/pkg/client/queue/get_all.go @@ -6,12 +6,12 @@ import ( "io" "time" - "github.com/armadaproject/armada/pkg/api" - "github.com/armadaproject/armada/pkg/client" - - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + + log "github.com/armadaproject/armada/internal/common/logging" + "github.com/armadaproject/armada/pkg/api" + "github.com/armadaproject/armada/pkg/client" ) type GetAllAPI func() ([]*api.Queue, error) @@ -35,7 +35,7 @@ func GetAll(getConnectionDetails client.ConnectionDetails) GetAllAPI { allQueues := make([]*api.Queue, 0) queueStream, err := client.GetQueues(ctx, &api.StreamingQueueGetRequest{}) if err != nil { - log.Error(err) + log.Error(err.Error()) return nil, err } @@ -57,7 +57,7 @@ func GetAll(getConnectionDetails client.ConnectionDetails) GetAllAPI { return nil, e } if !isTransportClosingError(e) { - log.Error(e) + log.Error(e.Error()) } break } diff --git a/pkg/client/watch.go b/pkg/client/watch.go index 2b9a96ae498..7b395cf5938 100644 --- a/pkg/client/watch.go +++ b/pkg/client/watch.go @@ -5,10 +5,10 @@ import ( "io" "time" - log "github.com/sirupsen/logrus" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + log "github.com/armadaproject/armada/internal/common/logging" "github.com/armadaproject/armada/internal/common/util" "github.com/armadaproject/armada/pkg/api" "github.com/armadaproject/armada/pkg/client/domain" @@ -71,7 +71,7 @@ func WatchJobSetWithJobIdsFilter( ) if e != nil { - log.Error(e) + log.Error(e.Error()) time.Sleep(5 * time.Second) continue } @@ -94,7 +94,7 @@ func WatchJobSetWithJobIdsFilter( return state } if !isTransportClosingError(e) { - log.Error(e) + log.Error(e.Error()) } time.Sleep(5 * time.Second) break @@ -104,7 +104,7 @@ func WatchJobSetWithJobIdsFilter( event, e := api.UnwrapEvent(msg.Message) if e != nil { // This can mean that the event type reported from server is unknown to the client - log.Error(e) + log.Error(e.Error()) continue }