From 42f9d177d2cb49a4c185fcacd770190171d91d34 Mon Sep 17 00:00:00 2001 From: Carson Long <12767276+ctlong@users.noreply.github.com> Date: Thu, 9 May 2024 21:49:35 -0700 Subject: [PATCH 1/2] chore(gateway): Remove unused metricsLoggr variable --- src/cmd/gateway/main.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cmd/gateway/main.go b/src/cmd/gateway/main.go index 8854530da..84357825b 100644 --- a/src/cmd/gateway/main.go +++ b/src/cmd/gateway/main.go @@ -22,7 +22,6 @@ import ( ) func main() { - var metricsLoggr *log.Logger var gatewayLoggr *log.Logger cfg, err := LoadConfig() @@ -31,12 +30,10 @@ func main() { } if cfg.UseRFC339 { - metricsLoggr = log.New(new(plumbing.LogWriter), "[METRICS] ", 0) gatewayLoggr = log.New(new(plumbing.LogWriter), "[GATEWAY] ", 0) log.SetOutput(new(plumbing.LogWriter)) log.SetFlags(0) } else { - metricsLoggr = log.New(os.Stderr, "[METRICS] ", log.LstdFlags) gatewayLoggr = log.New(os.Stderr, "[GATEWAY] ", log.LstdFlags) log.SetFlags(log.LstdFlags | log.Lmicroseconds) } From 6d0bdb0f15796eafb2a6137a11eed952f51c56fb Mon Sep 17 00:00:00 2001 From: Carson Long <12767276+ctlong@users.noreply.github.com> Date: Thu, 9 May 2024 22:03:10 -0700 Subject: [PATCH 2/2] feat(gateway): Use slog instead of log Uses the new structured logger in lieu of the standard logger. cmd/gateway: - Inits structured logger to a text handler that outputs to stderr. - Panics instead of logging fatally if config cannot be loaded. - If config cannot be written out to stderr, logs an error instead of logging fatally. - Passes the handler to lower packages in the guise of standard logger. internal/gateway: - Uses default slog for logging, will inherit the slog from cmd/gateway. - Tests set slog output to GinkgoWriter so that logs are only output on test failures. - Removes unnecessary WithGatewayLogger option. - Panics instead of logging fatally in several cases. --- src/cmd/gateway/config.go | 5 ++-- src/cmd/gateway/main.go | 29 +++++++----------- src/internal/gateway/gateway.go | 35 ++++++++-------------- src/internal/gateway/gateway_suite_test.go | 4 +++ 4 files changed, 29 insertions(+), 44 deletions(-) diff --git a/src/cmd/gateway/config.go b/src/cmd/gateway/config.go index bbb748af2..27e967473 100644 --- a/src/cmd/gateway/config.go +++ b/src/cmd/gateway/config.go @@ -1,6 +1,8 @@ package main import ( + "log/slog" + envstruct "code.cloudfoundry.org/go-envstruct" "code.cloudfoundry.org/log-cache/internal/config" "code.cloudfoundry.org/log-cache/internal/tls" @@ -18,7 +20,6 @@ type Config struct { TLS tls.TLS MetricsServer config.MetricsServer - UseRFC339 bool `env:"USE_RFC339"` } // LoadConfig creates Config object from environment variables @@ -39,7 +40,7 @@ func LoadConfig() (*Config, error) { err := envstruct.WriteReport(&c) if err != nil { - return nil, err + slog.Error("Failed to write report", "error", err) } return &c, nil diff --git a/src/cmd/gateway/main.go b/src/cmd/gateway/main.go index 84357825b..3772f1c75 100644 --- a/src/cmd/gateway/main.go +++ b/src/cmd/gateway/main.go @@ -2,7 +2,7 @@ package main import ( "fmt" - "log" + "log/slog" "os" "time" @@ -15,32 +15,24 @@ import ( _ "net/http/pprof" . "code.cloudfoundry.org/log-cache/internal/gateway" - "code.cloudfoundry.org/log-cache/internal/plumbing" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/credentials/insecure" ) +func init() { + slog.SetDefault(slog.New(slog.NewTextHandler(os.Stderr, nil))) +} + func main() { - var gatewayLoggr *log.Logger + slog.Info("Starting Log Cache Gateway...") + defer slog.Info("Log Cache Gateway stopped.") cfg, err := LoadConfig() if err != nil { - log.Fatalf("invalid configuration: %s", err) - } - - if cfg.UseRFC339 { - gatewayLoggr = log.New(new(plumbing.LogWriter), "[GATEWAY] ", 0) - log.SetOutput(new(plumbing.LogWriter)) - log.SetFlags(0) - } else { - gatewayLoggr = log.New(os.Stderr, "[GATEWAY] ", log.LstdFlags) - log.SetFlags(log.LstdFlags | log.Lmicroseconds) + panic(err) } - log.Print("Starting Log Cache Gateway...") - defer log.Print("Closing Log Cache Gateway.") - metricServerOption := metrics.WithTLSServer( int(cfg.MetricsServer.Port), cfg.MetricsServer.CertFile, @@ -51,7 +43,7 @@ func main() { metricServerOption = metrics.WithPublicServer(int(cfg.MetricsServer.Port)) } m := metrics.NewRegistry( - metricsLoggr, + slog.NewLogLogger(slog.Default().Handler(), slog.LevelInfo), metricServerOption, ) if cfg.MetricsServer.DebugMetrics { @@ -61,11 +53,10 @@ func main() { Handler: http.DefaultServeMux, ReadHeaderTimeout: 2 * time.Second, } - go func() { log.Println("PPROF SERVER STOPPED " + pprofServer.ListenAndServe().Error()) }() + go func() { slog.Info("pprof server stopped", "error", pprofServer.ListenAndServe().Error()) }() } gatewayOptions := []GatewayOption{ - WithGatewayLogger(gatewayLoggr), WithGatewayVersion(cfg.Version), WithGatewayBlock(), } diff --git a/src/internal/gateway/gateway.go b/src/internal/gateway/gateway.go index c4cc8afb5..c3570167c 100644 --- a/src/internal/gateway/gateway.go +++ b/src/internal/gateway/gateway.go @@ -3,7 +3,7 @@ package gateway import ( "fmt" "io" - "log" + "log/slog" "net" "net/http" "time" @@ -21,8 +21,6 @@ import ( // Gateway provides a RESTful API into LogCache's gRPC API. type Gateway struct { - log *log.Logger - logCacheAddr string logCacheVersion string uptimeFn func() int64 @@ -40,7 +38,6 @@ type Gateway struct { // invoked before using the Gateway. func NewGateway(logCacheAddr, gatewayAddr string, opts ...GatewayOption) *Gateway { g := &Gateway{ - log: log.New(io.Discard, "", 0), logCacheAddr: logCacheAddr, gatewayAddr: gatewayAddr, uptimeFn: uptimeInSeconds, @@ -56,14 +53,6 @@ func NewGateway(logCacheAddr, gatewayAddr string, opts ...GatewayOption) *Gatewa // GatewayOption configures a Gateway. type GatewayOption func(*Gateway) -// WithGatewayLogger returns a GatewayOption that configures the logger for -// the Gateway. It defaults to no logging. -func WithGatewayLogger(l *log.Logger) GatewayOption { - return func(g *Gateway) { - g.log = l - } -} - // WithGatewayBlock returns a GatewayOption that determines if Start launches // a go-routine or not. It defaults to launching a go-routine. If this is set, // start will block on serving the HTTP endpoint. @@ -107,12 +96,12 @@ func WithGatewayTLSServer(certPath, keyPath string) GatewayOption { // Start starts the gateway to start receiving and forwarding requests. It // does not block unless WithGatewayBlock was set. func (g *Gateway) Start() { + slog.Info("Starting server", "address", g.gatewayAddr) lis, err := net.Listen("tcp", g.gatewayAddr) if err != nil { - g.log.Fatalf("failed to listen on addr %s: %s", g.gatewayAddr, err) + panic(err) } g.lis = lis - g.log.Printf("listening on %s...", lis.Addr().String()) if g.blockOnStart { g.listenAndServe() @@ -138,7 +127,7 @@ func (g *Gateway) listenAndServe() { conn, err := grpc.NewClient(g.logCacheAddr, g.logCacheDialOpts...) if err != nil { - g.log.Fatalf("failed to dial Log Cache: %s", err) + panic(err) } err = logcache_v1.RegisterEgressHandlerClient( @@ -147,7 +136,7 @@ func (g *Gateway) listenAndServe() { logcache_v1.NewEgressClient(conn), ) if err != nil { - g.log.Fatalf("failed to register LogCache handler: %s", err) + panic(err) } err = logcache_v1.RegisterPromQLQuerierHandlerClient( @@ -156,7 +145,7 @@ func (g *Gateway) listenAndServe() { logcache_v1.NewPromQLQuerierClient(conn), ) if err != nil { - g.log.Fatalf("failed to register PromQLQuerier handler: %s", err) + panic(err) } topLevelMux := http.NewServeMux() @@ -169,11 +158,11 @@ func (g *Gateway) listenAndServe() { } if g.certPath != "" || g.keyPath != "" { if err := server.ServeTLS(g.lis, g.certPath, g.keyPath); err != nil { - g.log.Fatalf("failed to serve HTTPS endpoint: %s", err) + panic(err) } } else { if err := server.Serve(g.lis); err != nil { - g.log.Fatalf("failed to serve HTTP endpoint: %s", err) + panic(err) } } } @@ -181,7 +170,7 @@ func (g *Gateway) listenAndServe() { func (g *Gateway) handleInfoEndpoint(w http.ResponseWriter, r *http.Request) { _, err := w.Write([]byte(fmt.Sprintf(`{"version":"%s","vm_uptime":"%d"}`+"\n", g.logCacheVersion, g.uptimeFn()))) if err != nil { - g.log.Println("Cannot send result for the info endpoint") + slog.Error("Failed to send result for the info endpoint", "error", err) } } @@ -222,16 +211,16 @@ func (g *Gateway) httpErrorHandler( buf, merr := marshaler.Marshal(body) if merr != nil { - g.log.Printf("Failed to marshal error message %q: %v", body, merr) + slog.Error("Failed to marshal error message", "error", merr) w.WriteHeader(http.StatusInternalServerError) if _, err := io.WriteString(w, fallback); err != nil { - g.log.Printf("Failed to write response: %v", err) + slog.Error("Failed to write response", "error", err) } return } w.WriteHeader(runtime.HTTPStatusFromCode(status.Code(err))) if _, err := w.Write(buf); err != nil { - g.log.Printf("Failed to write response: %v", err) + slog.Error("Failed to write response", "error", err) } } diff --git a/src/internal/gateway/gateway_suite_test.go b/src/internal/gateway/gateway_suite_test.go index 358c38f8d..5bb109e63 100644 --- a/src/internal/gateway/gateway_suite_test.go +++ b/src/internal/gateway/gateway_suite_test.go @@ -1,6 +1,8 @@ package gateway_test import ( + "log/slog" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -8,6 +10,8 @@ import ( ) func TestGateway(t *testing.T) { + slog.SetDefault(slog.New(slog.NewTextHandler(GinkgoWriter, nil))) + RegisterFailHandler(Fail) RunSpecs(t, "Gateway Suite") }