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..5c0558aeb3a 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" ) diff --git a/cmd/executor/main.go b/cmd/executor/main.go index 04e53b73f41..a91c0304f49 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" @@ -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..69d9ce03c33 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" @@ -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..87f91fa1f73 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" ) diff --git a/cmd/lookoutingesterv2/main.go b/cmd/lookoutingesterv2/main.go index b60e58f6241..a0e48a6374f 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" diff --git a/cmd/lookoutv2/main.go b/cmd/lookoutv2/main.go index 48d48bc97b5..da8bb63e57c 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" 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/server/main.go b/cmd/server/main.go index 96e51f293e1..3815049164a 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" @@ -119,6 +119,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..1ddd0e6ba34 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" ) diff --git a/go.mod b/go.mod index c2623ca95ce..f1b9d7a05de 100644 --- a/go.mod +++ b/go.mod @@ -36,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 @@ -81,6 +79,7 @@ require ( github.com/prometheus/common v0.60.0 github.com/redis/go-redis/extra/redisprometheus/v9 v9.0.5 github.com/redis/go-redis/v9 v9.7.0 + github.com/rs/zerolog v1.33.0 github.com/segmentio/fasthash v1.0.3 github.com/xitongsys/parquet-go v1.6.2 go.uber.org/mock v0.5.0 @@ -161,7 +160,8 @@ require ( github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/magiconair/properties v1.8.7 // indirect github.com/mailru/easyjson v0.7.7 // indirect - github.com/mattn/go-isatty v0.0.18 // indirect + github.com/mattn/go-colorable v0.1.13 // indirect + github.com/mattn/go-isatty v0.0.19 // indirect github.com/mattn/go-runewidth v0.0.15 // indirect github.com/microcosm-cc/bluemonday v1.0.25 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect @@ -183,6 +183,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 diff --git a/go.sum b/go.sum index 1491cc31df8..789178833e1 100644 --- a/go.sum +++ b/go.sum @@ -114,6 +114,7 @@ github.com/containerd/log v0.1.0 h1:TCJt7ioM2cr/tfR8GPbGf9/VRAX8D2B4PjzCpfX540I= github.com/containerd/log v0.1.0/go.mod h1:VRRf09a7mHDIRezVKTRCrOq78v577GXq3bSa3EhrzVo= github.com/coreos/go-oidc v2.2.1+incompatible h1:mh48q/BqXqgjVHpy2ZY7WnWAbenxRjsz9N1i1YxjHAk= github.com/coreos/go-oidc v2.2.1+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc= +github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/cpuguy83/dockercfg v0.3.1 h1:/FpZ+JaygUR/lZP2NlFI2DVfrOEMAIKP5wWEJdoYe9E= github.com/cpuguy83/dockercfg v0.3.1/go.mod h1:sugsbF4//dDlL/i+S+rtpIWp+5h0BHJHfjj5/jFyUJc= github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= @@ -209,6 +210,7 @@ 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= github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= +github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/gogo/googleapis v0.0.0-20180223154316-0cd9801be74a h1:dR8+Q0uO5S2ZBcs2IH6VBKYwSxPo2vYCYq0ot0mu7xA= github.com/gogo/googleapis v0.0.0-20180223154316-0cd9801be74a/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= @@ -393,8 +395,11 @@ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0 github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/matryer/is v1.4.0 h1:sosSmIWwkYITGrxZ25ULNDeKiMNzFSr4V/eqBQP0PeE= github.com/matryer/is v1.4.0/go.mod h1:8I/i5uYgLzgsgEloJE1U6xx5HkBQpAZvepWuujKwMRU= -github.com/mattn/go-isatty v0.0.18 h1:DOKFKCQ7FNG2L1rbrmstDN4QVRdS89Nkh85u68Uwp98= -github.com/mattn/go-isatty v0.0.18/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= +github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= +github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= +github.com/mattn/go-isatty v0.0.19 h1:JITubQf0MOLdlGRuRq+jtsDlekdYPia9ZFsB8h/APPA= +github.com/mattn/go-isatty v0.0.19/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U= @@ -503,6 +508,9 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= +github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= +github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sagikazarmark/locafero v0.6.0 h1:ON7AQg37yzcRPU69mt7gwhFEBwxI6P9T4Qu3N51bwOk= github.com/sagikazarmark/locafero v0.6.0/go.mod h1:77OmuIc6VTraTXKXIs/uvUxKGUXjE1GbemJYHqdNjX0= @@ -560,8 +568,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= @@ -725,7 +731,9 @@ golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBc 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= +golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= 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 30fb2b466c9..b96f7cf197f 100644 --- a/internal/common/armadacontext/armada_context.go +++ b/internal/common/armadacontext/armada_context.go @@ -4,60 +4,119 @@ import ( "context" "time" - "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(), } } // 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(), } } -// 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 } - logger := logrus.NewEntry(logrus.StandardLogger()). + logger := logging.StdLogger(). 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(logging.StdSkipFrames - 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 } @@ -66,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 } @@ -77,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), } } @@ -96,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, } } @@ -106,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 ef7c84aa705..60c94fdc441 100644 --- a/internal/common/armadacontext/armada_context_test.go +++ b/internal/common/armadacontext/armada_context_test.go @@ -1,19 +1,31 @@ package armadacontext import ( + "bytes" "context" + "encoding/json" "testing" "time" - "github.com/sirupsen/logrus" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/armadaproject/armada/internal/common/logging" ) -var defaultLogger = logrus.WithField("foo", "bar") +type testLogEntry struct { + Level string `json:"level"` + Message string `json:"message"` + CustomField1 string `json:"customField1,omitempty"` + CustomField2 string `json:"customField2,omitempty"` +} + +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) } @@ -27,16 +39,128 @@ func TestTODO(t *testing.T) { require.Equal(t, ctx.Context, context.TODO()) } +func TestLogAtLevel(t *testing.T) { + tests := map[string]struct { + logFunction func(ctx *Context) + expectedLevel string + expectedMsg string + }{ + "Debug": { + logFunction: func(ctx *Context) { + ctx.Debug("test message") + }, + expectedMsg: "test message", + expectedLevel: "debug", + }, + "Debugf": { + logFunction: func(ctx *Context) { + ctx.Debugf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "debug", + }, + "Info": { + logFunction: func(ctx *Context) { + ctx.Info("test message") + }, + expectedMsg: "test message", + expectedLevel: "info", + }, + "Infof": { + logFunction: func(ctx *Context) { + ctx.Infof("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "info", + }, + "Warn": { + logFunction: func(ctx *Context) { + ctx.Warn("test message") + }, + expectedMsg: "test message", + expectedLevel: "warn", + }, + "Warnf": { + logFunction: func(ctx *Context) { + ctx.Warnf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "warn", + }, + "Error": { + logFunction: func(ctx *Context) { + ctx.Errorf("test message") + }, + expectedMsg: "test message", + expectedLevel: "error", + }, + "Errorf": { + logFunction: func(ctx *Context) { + ctx.Errorf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "error", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + logger, buf := testLogger() + ctx := New(context.Background(), logger) + tc.logFunction(ctx) + assertLogLineExpected( + t, + &testLogEntry{ + Level: tc.expectedLevel, + Message: tc.expectedMsg, + }, + buf, + ) + }) + } +} + func TestWithLogField(t *testing.T) { - ctx := WithLogField(Background(), "fish", "chips") + logger, buf := testLogger() + ctx := WithLogField(New(context.Background(), logger), "customField1", "foo") + + ctx.Info("test message") + require.Equal(t, context.Background(), ctx.Context) - require.Equal(t, logrus.Fields{"fish": "chips"}, ctx.FieldLogger.(*logrus.Entry).Data) + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + CustomField1: "foo", + }, + buf, + ) } func TestWithLogFields(t *testing.T) { - ctx := WithLogFields(Background(), logrus.Fields{"fish": "chips", "salt": "pepper"}) + logger, buf := testLogger() + + ctx := WithLogFields( + New(context.Background(), logger), + map[string]interface{}{ + "customField1": "bar", + "customField2": "baz", + }, + ) + + ctx.Info("test message") + require.Equal(t, context.Background(), ctx.Context) - require.Equal(t, logrus.Fields{"fish": "chips", "salt": "pepper"}, ctx.FieldLogger.(*logrus.Entry).Data) + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + CustomField1: "bar", + CustomField2: "baz", + }, + buf, + ) } func TestWithTimeout(t *testing.T) { @@ -78,3 +202,22 @@ func quiescent(t *testing.T) time.Duration { const arbitraryCleanupMargin = 1 * time.Second return time.Until(deadline) - arbitraryCleanupMargin } + +// testLogger sets up a Zerolog logger that writes to a buffer for testing +func testLogger() (*logging.Logger, *bytes.Buffer) { + var buf bytes.Buffer + baseLogger := zerolog.New(&buf).Level(zerolog.DebugLevel).With().Timestamp().Logger() + logger := logging.FromZerolog(baseLogger) + return logger, &buf +} + +func assertLogLineExpected(t *testing.T, expected *testLogEntry, logOutput *bytes.Buffer) { + var entry testLogEntry + err := json.Unmarshal(logOutput.Bytes(), &entry) + require.NoError(t, err, "Failed to unmarshal log entry") + + assert.Equal(t, expected.Message, entry.Message) + assert.Equal(t, expected.Level, entry.Level) + assert.Equal(t, expected.CustomField1, entry.CustomField1) + assert.Equal(t, expected.CustomField2, entry.CustomField2) +} 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 1139e04a427..9cc844a3152 100644 --- a/internal/common/grpc/grpc.go +++ b/internal/common/grpc/grpc.go @@ -15,7 +15,6 @@ import ( grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery" "github.com/prometheus/client_golang/prometheus" - log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" @@ -28,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" ) @@ -89,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) } 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/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..507e1d49478 --- /dev/null +++ b/internal/common/logging/global.go @@ -0,0 +1,116 @@ +package logging + +import ( + "os" + + "github.com/rs/zerolog" +) + +var ( + StdSkipFrames = 4 + // 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 + stdLogger = createDefaultLogger() +) + +// ReplaceStdLogger Replaces the global logger. This should be called once at app startup! +func ReplaceStdLogger(l *Logger) { + stdLogger = l.WithCallerSkip(StdSkipFrames) +} + +// StdLogger Returns the default logger +func StdLogger() *Logger { + return stdLogger +} + +// Debug logs a message at level Debug on the default logger. +func Debug(args ...any) { + stdLogger.Debug(args...) +} + +// Info logs a message at level Info on the default logger. +func Info(args ...any) { + stdLogger.Info(args...) +} + +// Warn logs a message at level Warn on the default logger. +func Warn(args ...any) { + stdLogger.Warn(args...) +} + +// Error logs a message at level Error on the default logger. +func Error(args ...any) { + stdLogger.Error(args...) +} + +// Panic logs a message at level Panic on the default logger. +func Panic(args ...any) { + stdLogger.Panic(args...) +} + +// Fatal logs a message at level Fatal on the default 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 default logger. +func Debugf(format string, args ...any) { + stdLogger.Debugf(format, args...) +} + +// Infof logs a message at level Info on the default logger. +func Infof(format string, args ...any) { + stdLogger.Infof(format, args...) +} + +// Warnf logs a message at level Warn on the default logger. +func Warnf(format string, args ...any) { + stdLogger.Warnf(format, args...) +} + +// Errorf logs a message at level Error on the default logger. +func Errorf(format string, args ...any) { + stdLogger.Errorf(format, args...) +} + +// Fatalf logs a message at level Fatal on the default 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) +} + +// createDefaultLogger returns a new Logger configured with default settings using zerolog. +func createDefaultLogger() *Logger { + consoleWriter := zerolog.ConsoleWriter{ + Out: os.Stdout, + TimeFormat: "2006-01-02T15:04:05.000Z07:00", + NoColor: true, + } + zerologLogger := zerolog.New(consoleWriter). + Level(zerolog.InfoLevel). + With(). + CallerWithSkipFrameCount(StdSkipFrames). + Timestamp(). + Logger() + + return FromZerolog(zerologLogger) +} diff --git a/internal/common/logging/levelwriter.go b/internal/common/logging/levelwriter.go new file mode 100644 index 00000000000..edf63a298ba --- /dev/null +++ b/internal/common/logging/levelwriter.go @@ -0,0 +1,26 @@ +package logging + +import ( + "io" + + "github.com/rs/zerolog" +) + +type FilteredLevelWriter struct { + writer io.Writer + level zerolog.Level +} + +// Write writes to the underlying Writer. +func (w *FilteredLevelWriter) Write(p []byte) (int, error) { + return w.writer.Write(p) +} + +// WriteLevel calls WriteLevel of the underlying Writer only if the level is equal +// or above the Level. +func (w *FilteredLevelWriter) WriteLevel(level zerolog.Level, p []byte) (int, error) { + if level >= w.level { + return w.writer.Write(p) + } + return len(p), nil +} diff --git a/internal/common/logging/logger.go b/internal/common/logging/logger.go new file mode 100644 index 00000000000..b6e5605f6ea --- /dev/null +++ b/internal/common/logging/logger.go @@ -0,0 +1,136 @@ +package logging + +import ( + "fmt" + + "github.com/pkg/errors" + "github.com/rs/zerolog" +) + +// Logger wraps a zerolog.Logger so that the rest of the code doesn't depend directly on zerolog +type Logger struct { + underlying zerolog.Logger +} + +// NullLogger is Logger that discards all log lines +var NullLogger = &Logger{ + underlying: zerolog.Nop(), +} + +// FromZerolog returns a new Logger backed by the supplied zerolog.Logger +func FromZerolog(l zerolog.Logger) *Logger { + return &Logger{ + underlying: l, + } +} + +// Debug logs a message at level Debug +func (l *Logger) Debug(args ...any) { + l.underlying.Debug().Msg(fmt.Sprint(args...)) +} + +// Info logs a message at level Info +func (l *Logger) Info(args ...any) { + l.underlying.Info().Msg(fmt.Sprint(args...)) +} + +// Warn logs a message at level Warn +func (l *Logger) Warn(args ...any) { + l.underlying.Warn().Msg(fmt.Sprint(args...)) +} + +// Error logs a message at level Error +func (l *Logger) Error(args ...any) { + l.underlying.Error().Msg(fmt.Sprint(args...)) +} + +// Panic logs a message at level Panic and panics +func (l *Logger) Panic(args ...any) { + l.underlying.Panic().Msg(fmt.Sprint(args...)) +} + +// Fatal logs a message at level Fatal and exits the application +func (l *Logger) Fatal(args ...any) { + l.underlying.Fatal().Msg(fmt.Sprint(args...)) +} + +// Debugf logs a formatted message at level Debug +func (l *Logger) Debugf(format string, args ...interface{}) { + l.underlying.Debug().Msgf(format, args...) +} + +// Infof logs a formatted message at level Info +func (l *Logger) Infof(format string, args ...interface{}) { + l.underlying.Info().Msgf(format, args...) +} + +// Warnf logs a formatted message at level Warn +func (l *Logger) Warnf(format string, args ...interface{}) { + l.underlying.Warn().Msgf(format, args...) +} + +// Errorf logs a formatted message at level Error +func (l *Logger) Errorf(format string, args ...interface{}) { + l.underlying.Error().Msgf(format, args...) +} + +// Fatalf logs a formatted message at level Fatal and exits the application +func (l *Logger) Fatalf(format string, args ...interface{}) { + l.underlying.Fatal().Msgf(format, args...) +} + +// WithError returns a new Logger with the error added as a field +func (l *Logger) WithError(err error) *Logger { + return &Logger{ + underlying: l.underlying. + With(). + AnErr("error", err). + CallerWithSkipFrameCount(StdSkipFrames - 1). + Logger(), + } +} + +// 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", fmt.Sprintf("%v", stackErr.StackTrace())) + } + 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(). + Interface(key, value). + CallerWithSkipFrameCount(StdSkipFrames - 1). + Logger(), + } +} + +// 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 { + event := l.underlying.With() + for key, value := range args { + event = event.Interface(key, value) + } + return &Logger{ + underlying: event.CallerWithSkipFrameCount(StdSkipFrames - 1).Logger(), + } +} + +// 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.With().CallerWithSkipFrameCount(skip).Logger(), + } +} + +// 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..1f8d9b76d5d --- /dev/null +++ b/internal/common/logging/logger_test.go @@ -0,0 +1,196 @@ +package logging + +import ( + "bytes" + "encoding/json" + "fmt" + "testing" + + "github.com/pkg/errors" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type testLogEntry struct { + Level string `json:"level"` + Message string `json:"message"` + CustomField1 string `json:"customField1,omitempty"` + CustomField2 string `json:"customField2,omitempty"` + Error string `json:"error,omitempty"` + Stacktrace string `json:"stacktrace,omitempty"` +} + +func TestWithField(t *testing.T) { + logger, buf := testLogger() + logger = logger.WithField("customField1", "foo") + + logger.Info("test message") + + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + CustomField1: "foo", + }, + buf, + ) +} + +func TestWithFields(t *testing.T) { + logger, buf := testLogger() + logger = logger.WithFields( + map[string]any{"customField1": "bar", "customField2": "baz"}, + ) + + logger.Info("test message") + + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + CustomField1: "bar", + CustomField2: "baz", + }, + buf, + ) +} + +func TestWithError(t *testing.T) { + logger, buf := testLogger() + err := errors.New("test error") + logger = logger.WithError(err) + + logger.Info("test message") + + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + Error: "test error", + }, + buf, + ) +} + +func TestWithStacktrace(t *testing.T) { + logger, buf := testLogger() + err := errors.New("test error") + logger = logger.WithStacktrace(err) + + logger.Info("test message") + + assertLogLineExpected( + t, + &testLogEntry{ + Level: "info", + Message: "test message", + Error: "test error", + Stacktrace: fmt.Sprintf("%v", err.(stackTracer).StackTrace()), + }, + buf, + ) +} + +func TestLogAtLevel(t *testing.T) { + tests := map[string]struct { + logFunction func(logger *Logger) + expectedLevel string + expectedMsg string + }{ + "Debug": { + logFunction: func(l *Logger) { + l.Debug("test message") + }, + expectedMsg: "test message", + expectedLevel: "debug", + }, + "Debugf": { + logFunction: func(l *Logger) { + l.Debugf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "debug", + }, + "Info": { + logFunction: func(l *Logger) { + l.Info("test message") + }, + expectedMsg: "test message", + expectedLevel: "info", + }, + "Infof": { + logFunction: func(l *Logger) { + l.Infof("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "info", + }, + "Warn": { + logFunction: func(l *Logger) { + l.Warn("test message") + }, + expectedMsg: "test message", + expectedLevel: "warn", + }, + "Warnf": { + logFunction: func(l *Logger) { + l.Warnf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "warn", + }, + "Error": { + logFunction: func(l *Logger) { + l.Errorf("test message") + }, + expectedMsg: "test message", + expectedLevel: "error", + }, + "Errorf": { + logFunction: func(l *Logger) { + l.Errorf("test message %d", 1) + }, + expectedMsg: "test message 1", + expectedLevel: "error", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + logger, buf := testLogger() + tc.logFunction(logger) + assertLogLineExpected( + t, + &testLogEntry{ + Level: tc.expectedLevel, + Message: tc.expectedMsg, + }, + buf, + ) + }) + } +} + +// testLogger sets up a Zerolog logger that writes to a buffer for testing +func testLogger() (*Logger, *bytes.Buffer) { + var buf bytes.Buffer + baseLogger := zerolog.New(&buf).Level(zerolog.DebugLevel).With().Timestamp().Logger() + logger := FromZerolog(baseLogger) + return logger, &buf +} + +func assertLogLineExpected(t *testing.T, expected *testLogEntry, logOutput *bytes.Buffer) { + var entry testLogEntry + err := json.Unmarshal(logOutput.Bytes(), &entry) + require.NoError(t, err, "Failed to unmarshal log entry") + + assert.Equal(t, expected.Message, entry.Message) + assert.Equal(t, expected.Level, entry.Level) + assert.Equal(t, expected.CustomField1, entry.CustomField1) + assert.Equal(t, expected.CustomField2, entry.CustomField2) + assert.Equal(t, expected.Error, entry.Error) + assert.Equal(t, expected.Stacktrace, entry.Stacktrace) +} diff --git a/internal/common/logging/null_logger.go b/internal/common/logging/null_logger.go deleted file mode 100644 index 25326d3691b..00000000000 --- a/internal/common/logging/null_logger.go +++ /dev/null @@ -1,14 +0,0 @@ -package logging - -import ( - "io" - - "github.com/sirupsen/logrus" -) - -var NullLogger = &logrus.Logger{ - Out: io.Discard, - Formatter: new(logrus.TextFormatter), - Hooks: make(logrus.LevelHooks), - Level: logrus.PanicLevel, -} diff --git a/internal/common/logging/prometheus.go b/internal/common/logging/prometheus.go new file mode 100644 index 00000000000..7e5fa3d1814 --- /dev/null +++ b/internal/common/logging/prometheus.go @@ -0,0 +1,41 @@ +package logging + +import ( + "github.com/prometheus/client_golang/prometheus" + "github.com/rs/zerolog" +) + +// PrometheusHook implements zerolog.Hook +type PrometheusHook struct { + counters map[zerolog.Level]prometheus.Counter +} + +// NewPrometheusHook creates and registers Prometheus counters for each log level. +func NewPrometheusHook() *PrometheusHook { + counters := make(map[zerolog.Level]prometheus.Counter) + + for _, level := range []zerolog.Level{ + zerolog.DebugLevel, + zerolog.InfoLevel, + zerolog.WarnLevel, + zerolog.ErrorLevel, + } { + counter := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "log_messages", + Help: "Total number of log lines logged by level", + ConstLabels: prometheus.Labels{ + "level": level.String(), + }, + }) + // Register the counter with Prometheus. + prometheus.MustRegister(counter) + counters[level] = counter + } + return &PrometheusHook{counters: counters} +} + +func (h *PrometheusHook) Run(_ *zerolog.Event, level zerolog.Level, _ string) { + if counter, ok := h.counters[level]; ok { + counter.Inc() + } +} diff --git a/internal/common/logging/prometheus_test.go b/internal/common/logging/prometheus_test.go new file mode 100644 index 00000000000..bc09b31f861 --- /dev/null +++ b/internal/common/logging/prometheus_test.go @@ -0,0 +1,41 @@ +package logging + +import ( + "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" +) + +// TestPrometheusHook_IncrementsCounter verifies that calling Run on monitored levels +// properly increments the corresponding counter. +func TestPrometheusHook_IncrementsCounter(t *testing.T) { + hook := NewPrometheusHook() + + // Clean up: Unregister the counters to avoid polluting the global registry in further tests. + t.Cleanup(func() { + for _, counter := range hook.counters { + prometheus.Unregister(counter) + } + }) + + // Define the log levels we are monitoring. + levels := []zerolog.Level{ + zerolog.DebugLevel, + zerolog.InfoLevel, + zerolog.WarnLevel, + zerolog.ErrorLevel, + } + + for _, level := range levels { + n := 3 + for i := 0; i < n; i++ { + hook.Run(nil, level, "dummy message") + } + // Verify the counter value. + value := testutil.ToFloat64(hook.counters[level]) + assert.Equal(t, float64(n), value, "expected counter for level %s to be %d, got %f", level, n, value) + } +} diff --git a/internal/common/logging/pulsar_adapter.go b/internal/common/logging/pulsar_adapter.go new file mode 100644 index 00000000000..d4e13d04b59 --- /dev/null +++ b/internal/common/logging/pulsar_adapter.go @@ -0,0 +1,79 @@ +package logging + +import ( + pulsarlog "github.com/apache/pulsar-client-go/pulsar/log" +) + +var pulsarAdapterSkipFrames = StdSkipFrames + +// 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 newPulsarLogger(StdLogger()) +} + +func (p pulsarWrapper) SubLogger(fs pulsarlog.Fields) pulsarlog.Logger { + return newPulsarLogger( + p.l.WithFields(fs), + ) +} + +func (p pulsarWrapper) WithFields(fs pulsarlog.Fields) pulsarlog.Entry { + return newPulsarLogger( + p.l.WithFields(fs), + ) +} + +func (p pulsarWrapper) WithField(name string, value interface{}) pulsarlog.Entry { + return newPulsarLogger( + p.l.WithField(name, value), + ) +} + +func (p pulsarWrapper) WithError(err error) pulsarlog.Entry { + return newPulsarLogger( + 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...) +} + +func newPulsarLogger(l *Logger) pulsarlog.Logger { + return &pulsarWrapper{ + l: l.WithCallerSkip(pulsarAdapterSkipFrames), + } +} 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/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..87b05f068de 100644 --- a/internal/common/startup.go +++ b/internal/common/startup.go @@ -3,45 +3,39 @@ package common import ( "crypto/tls" "fmt" + "io" "net/http" "os" - "path" - "runtime" - "strconv" + "path/filepath" "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/rs/zerolog" "github.com/spf13/pflag" "github.com/spf13/viper" - "github.com/weaveworks/promrus" + "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" +const logTimestampFormat = "2006-01-02T15:04:05.000Z07: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 +66,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 +83,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) } @@ -103,58 +96,76 @@ func UnmarshalKey(v *viper.Viper, key string, item interface{}) error { // 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) + zerolog.TimestampFieldName = "" + zerolog.LevelFieldName = "" + + // Create a ConsoleWriter that writes to stdout. + // Since we’ve cleared out the field names above, only the message will be printed. + consoleWriter := zerolog.ConsoleWriter{ + Out: os.Stdout, + TimeFormat: "", // No timestamp + } + + l := zerolog.New(consoleWriter).Level(zerolog.InfoLevel).With().Logger() + log.ReplaceStdLogger(log.FromZerolog(l)) } func ConfigureLogging() { - log.SetLevel(readEnvironmentLogLevel()) - log.SetFormatter(readEnvironmentLogFormat()) - log.SetReportCaller(true) - log.SetOutput(os.Stdout) + // needs to be higher or greater precision than the writer format. + zerolog.TimeFieldFormat = logTimestampFormat + + level := readEnvironmentLogLevel() + format := readEnvironmentLogFormat() + + var outStream io.Writer = os.Stdout + + if format != "json" { + outStream = zerolog.ConsoleWriter{ + Out: os.Stdout, + TimeFormat: logTimestampFormat, + FormatLevel: func(i interface{}) string { + return strings.ToUpper(fmt.Sprintf("%s", i)) + }, + FormatCaller: func(i interface{}) string { + return filepath.Base(fmt.Sprintf("%s", i)) + }, + NoColor: format == "text", + } + } + + zerologLogger := zerolog.New(outStream). + Level(level). + Hook(log.NewPrometheusHook()). + With(). + CallerWithSkipFrameCount(log.StdSkipFrames). + Timestamp(). + Logger() + + log.ReplaceStdLogger(log.FromZerolog(zerologLogger)) } -func readEnvironmentLogLevel() log.Level { +func readEnvironmentLogLevel() zerolog.Level { level, ok := os.LookupEnv("LOG_LEVEL") if ok { - logLevel, err := log.ParseLevel(level) + logLevel, err := zerolog.ParseLevel(level) if err == nil { return logLevel } } - return log.InfoLevel + return zerolog.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 +func readEnvironmentLogFormat() string { + format, ok := os.LookupEnv("LOG_FORMAT") + if ok { + format = strings.ToLower(format) + if format == "text" || format == "colourful" || format == "json" { + return format + } else { + _, _ = fmt.Fprintf(os.Stderr, "Invalid log format %s\n", format) + } } + return "colourful" } func ServeMetrics(port uint16) (shutdown func()) { @@ -162,9 +173,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 +200,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 +225,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 c9451962b89..39913fd0e79 100644 --- a/internal/executor/service/lease_requester.go +++ b/internal/executor/service/lease_requester.go @@ -6,11 +6,11 @@ import ( 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..d8260270fa2 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" ) 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 70c6067117b..6c21f07cf39 100644 --- a/internal/scheduler/schedulerapp.go +++ b/internal/scheduler/schedulerapp.go @@ -13,7 +13,6 @@ import ( 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") } }() @@ -363,7 +362,7 @@ 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 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 e56e01f3d90..90be011edc5 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" @@ -130,7 +130,7 @@ func (l *FairSchedulingAlgo) Schedule( start := time.Now() schedulerResult, sctx, err := l.SchedulePool(ctx, fsctx, pool.Name, pool.MarketDriven) - ctx.Infof("Scheduled on executor pool %s in %v with error %v", pool, time.Now().Sub(start), err) + ctx.Infof("Scheduled on executor pool %s in %v with error %v", pool.Name, time.Now().Sub(start), err) if errors.Is(err, context.DeadlineExceeded) { // We've reached the scheduling time limit; @@ -611,7 +611,7 @@ func (l *FairSchedulingAlgo) populateNodeDb(nodeDb *nodedb.NodeDb, currentPoolJo } 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(), ) diff --git a/internal/scheduler/simulator/simulator.go b/internal/scheduler/simulator/simulator.go index 0305c4cab57..88755abf438 100644 --- a/internal/scheduler/simulator/simulator.go +++ b/internal/scheduler/simulator/simulator.go @@ -523,7 +523,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 { @@ -607,10 +607,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 d96fd740c2e..40afd8ea91f 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 } @@ -141,8 +140,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 514176e37fb..5a61af58acc 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/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/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 }