From 201691a30edd1ce1b273d31c341d748920594e56 Mon Sep 17 00:00:00 2001 From: Christophe Collot Date: Sun, 12 Mar 2023 14:52:09 +0100 Subject: [PATCH 1/3] feat(logger): add structured logging wrapper for zap --- cmd/frontman/main.go | 26 ++++++++++--- frontman.yaml | 42 ++++++++++----------- gateway.go | 41 +++++++++++--------- gateway_test.go | 53 +++++++++++++------------- go.mod | 3 ++ go.sum | 7 ++++ log/logger.go | 68 +++++++++++++++++++++++++++++++++ log/logger_test.go | 21 +++++++++++ log/zap.go | 90 ++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 279 insertions(+), 72 deletions(-) create mode 100644 log/logger.go create mode 100644 log/logger_test.go create mode 100644 log/zap.go diff --git a/cmd/frontman/main.go b/cmd/frontman/main.go index f4305d9..493aa0d 100644 --- a/cmd/frontman/main.go +++ b/cmd/frontman/main.go @@ -2,17 +2,21 @@ package main import ( "flag" - "log" + "fmt" + "os" "github.com/Frontman-Labs/frontman" "github.com/Frontman-Labs/frontman/config" + "github.com/Frontman-Labs/frontman/log" ) func main() { // Define command-line flags var configFile string + var logLevel string flag.StringVar(&configFile, "config", "", "path to configuration file") + flag.StringVar(&logLevel, "log-level", "", "set log level to debug") // Parse command-line flags flag.Parse() @@ -25,15 +29,27 @@ func main() { config, err := config.LoadConfig(configPath) if err != nil { - log.Fatalf("failed to load configuration: %v", err) + fmt.Printf("failed to load configuration: %v", err) + os.Exit(1) } + if config.LoggingConfig.Level != "" && logLevel == "" { + logLevel = config.LoggingConfig.Level + } else { + logLevel = "info" + } + logger, err := log.NewDefaultLogger(log.ParseLevel(logLevel)) + if err != nil { + fmt.Println("failed to initialize logger") + os.Exit(1) + } + // Create a new Gateway instance - gateway, err := frontman.NewGateway(config) + gateway, err := frontman.NewGateway(config, logger) if err != nil { - log.Fatalf("failed to create gateway: %v", err) + logger.Fatalf("failed to create gateway: %v", err) } // Start the server - log.Fatal(gateway.Start()) + logger.Fatal(gateway.Start()) } diff --git a/frontman.yaml b/frontman.yaml index 781de78..2b35117 100644 --- a/frontman.yaml +++ b/frontman.yaml @@ -1,29 +1,25 @@ global: - service_type: SERVICE_TYPE - services_file: SERVICES_FILE - redis_uri: REDIS_URI - redis_namespace: REDIS_NAMESPACE - mongo_uri: MONGO_URI - mongo_db_name: MONGO_DB_NAME - mongo_collection_name: MONGO_COLLECTION_NAME - - + service_type: "yaml" + services_file: "services.yaml" + redis_uri: "redis://localhost:6379" + redis_namespace: "my_service" + mongo_uri: "mongodb://localhost:27017" + mongo_db_name: "my_db" + mongo_collection_name: "my_collection" api: - addr: API_ADDR + addr: ":8080" ssl: - enabled: API_SSL_ENABLED - cert: API_SSL_CERT - key: API_SSL_KEY - + enabled: false + cert: "/path/to/cert.pem" + key: "/path/to/key.pem" gateway: - addr: GATEWAY_ADDR + addr: ":8081" ssl: - enabled: GATEWAY_SSL_ENABLED - cert: GATEWAY_SSL_CERT - key: GATEWAY_SSL_KEY - + enabled: false logging: - level: LOG_LEVEL - - - + level: "debug" +# plugins: +# enabled: true +# order: +# - "/path/to/plugin1.so" +# - "/path/to/plugin2.so" diff --git a/gateway.go b/gateway.go index 06bc136..c8c6f21 100644 --- a/gateway.go +++ b/gateway.go @@ -3,11 +3,12 @@ package frontman import ( "context" "crypto/tls" - "log" + "fmt" "net/http" "strings" "github.com/Frontman-Labs/frontman/config" + "github.com/Frontman-Labs/frontman/log" "github.com/Frontman-Labs/frontman/plugins" "github.com/Frontman-Labs/frontman/service" "github.com/gorilla/mux" @@ -19,6 +20,7 @@ type Gateway struct { service *mux.Router backendServices service.ServiceRegistry conf *config.Config + log log.Logger } func NewServicesRouter(backendServices service.ServiceRegistry) *mux.Router { @@ -34,7 +36,7 @@ func NewServicesRouter(backendServices service.ServiceRegistry) *mux.Router { } // NewGateway creates a new Gateway instance with a Redis client connection factory -func NewGateway(conf *config.Config) (*Gateway, error) { +func NewGateway(conf *config.Config, log log.Logger) (*Gateway, error) { // Retrieve the Redis client connection from the factory ctx := context.Background() @@ -78,6 +80,7 @@ func NewGateway(conf *config.Config) (*Gateway, error) { service: servicesRouter, backendServices: backendServices, conf: conf, + log : log, }, nil } @@ -101,13 +104,13 @@ func (gw *Gateway) Start() error { return err } apiServer := createServer(apiAddr, apiHandler, &cert) - log.Printf("Started Frontman API with SSL on %s\n", apiAddr) - go startServer(apiServer) + gw.log.Infof("Started Frontman API with SSL on %s", apiAddr) + go func(){if err := startServer(apiServer); err!=nil{ gw.log.Fatal(err)}}() } else { apiHandler = gw.service api := createServer(apiAddr, apiHandler, nil) - log.Printf("Started Frontman API on %s\n", apiAddr) - go startServer(api) + gw.log.Infof("Started Frontman API on %s", apiAddr) + go func(){if err := startServer(api); err!=nil{ gw.log.Fatal(err)}}() } if gw.conf.GatewayConfig.SSL.Enabled { @@ -120,18 +123,22 @@ func (gw *Gateway) Start() error { // Redirect HTTP traffic to HTTPS httpAddr := "0.0.0.0:80" httpRedirect := createRedirectServer(httpAddr, gatewayAddr) - log.Printf("Started HTTP redirect server on %s\n", httpAddr) - go startServer(httpRedirect) + gw.log.Infof("Started HTTP redirect server on %s", httpAddr) + go func(){if err := startServer(httpRedirect); err!=nil{ gw.log.Fatal(err)}}() gatewayServer := createServer(gatewayAddr, gatewayHandler, &cert) - log.Printf("Started Frontman Gateway with SSL on %s\n", gatewayAddr) - startServer(gatewayServer) + gw.log.Infof("Started Frontman Gateway with SSL on %s", gatewayAddr) + if err := startServer(gatewayServer); err!=nil { + return err + } } else { gatewayHandler = gw.router gateway := createServer(gatewayAddr, gatewayHandler, nil) - log.Printf("Started Frontman Gateway on %s", gatewayAddr) - startServer(gateway) + gw.log.Infof("Started Frontman Gateway on %s", gatewayAddr) + if err := startServer(gateway); err!=nil { + return err + } } return nil @@ -152,8 +159,7 @@ func createRedirectServer(addr string, redirectAddr string) *http.Server { func loadCert(certFile, keyFile string) (tls.Certificate, error) { cert, err := tls.LoadX509KeyPair(certFile, keyFile) if err != nil { - log.Fatalf("Failed to load certificate: %v", err) - return tls.Certificate{}, err + return tls.Certificate{}, fmt.Errorf("Failed to load certificate: %w", err) } return cert, nil } @@ -171,14 +177,15 @@ func createServer(addr string, handler http.Handler, cert *tls.Certificate) *htt return server } -func startServer(server *http.Server) { +func startServer(server *http.Server) error { if server.TLSConfig != nil { if err := server.ListenAndServeTLS("", ""); err != nil { - log.Fatalf("Failed to start server with TLS: %v", err) + return fmt.Errorf("Failed to start server with TLS: %w", err) } } else { if err := server.ListenAndServe(); err != nil { - log.Fatalf("Failed to start server without TLS: %v", err) + return fmt.Errorf("Failed to start server without TLS: %w", err) } } + return nil } diff --git a/gateway_test.go b/gateway_test.go index 7ed0ef0..8df765b 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -6,31 +6,30 @@ import ( "testing" ) - func TestCreateRedirectServer(t *testing.T) { - redirectAddr := "0.0.0.0:8000" - redirectServer := createRedirectServer("0.0.0.0:80", redirectAddr) - - // Create a test request to the redirect server - req, err := http.NewRequest("GET", "http://example.com/foo", nil) - if err != nil { - t.Fatal(err) - } - - // Create a test response recorder - rr := httptest.NewRecorder() - - // Call the redirect server's handler function - redirectServer.Handler.ServeHTTP(rr, req) - - // Check that the response has a 301 status code - if status := rr.Code; status != http.StatusMovedPermanently { - t.Errorf("Unexpected status code: got %v, expected %v", status, http.StatusMovedPermanently) - } - - // Check that the response includes a "Location" header with the expected value - expectedURL := "https://example.com/foo" - if location := rr.Header().Get("Location"); location != expectedURL { - t.Errorf("Unexpected Location header value: got %v, expected %v", location, expectedURL) - } -} \ No newline at end of file + redirectAddr := "0.0.0.0:8000" + redirectServer := createRedirectServer("0.0.0.0:80", redirectAddr) + + // Create a test request to the redirect server + req, err := http.NewRequest("GET", "http://example.com/foo", nil) + if err != nil { + t.Fatal(err) + } + + // Create a test response recorder + rr := httptest.NewRecorder() + + // Call the redirect server's handler function + redirectServer.Handler.ServeHTTP(rr, req) + + // Check that the response has a 301 status code + if status := rr.Code; status != http.StatusMovedPermanently { + t.Errorf("Unexpected status code: got %v, expected %v", status, http.StatusMovedPermanently) + } + + // Check that the response includes a "Location" header with the expected value + expectedURL := "https://example.com/foo" + if location := rr.Header().Get("Location"); location != expectedURL { + t.Errorf("Unexpected Location header value: got %v, expected %v", location, expectedURL) + } +} diff --git a/go.mod b/go.mod index ac8a675..1ada88d 100644 --- a/go.mod +++ b/go.mod @@ -30,6 +30,9 @@ require ( github.com/xdg-go/scram v1.1.1 // indirect github.com/xdg-go/stringprep v1.0.3 // indirect github.com/youmark/pkcs8 v0.0.0-20181117223130-1be2e3e5546d // indirect + go.uber.org/atomic v1.7.0 // indirect + go.uber.org/multierr v1.6.0 // indirect + go.uber.org/zap v1.24.0 // indirect golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d // indirect golang.org/x/net v0.7.0 // indirect golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect diff --git a/go.sum b/go.sum index 69ae25d..b38e558 100644 --- a/go.sum +++ b/go.sum @@ -55,6 +55,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= @@ -72,6 +73,12 @@ github.com/youmark/pkcs8 v0.0.0-20181117223130-1be2e3e5546d h1:splanxYIlg+5LfHAM github.com/youmark/pkcs8 v0.0.0-20181117223130-1be2e3e5546d/go.mod h1:rHwXgn7JulP+udvsHwJoVG1YGAP6VLg4y9I5dyZdqmA= go.mongodb.org/mongo-driver v1.11.2 h1:+1v2rDQUWNcGW7/7E0Jvdz51V38XXxJfhzbV17aNHCw= go.mongodb.org/mongo-driver v1.11.2/go.mod h1:s7p5vEtfbeR1gYi6pnj3c3/urpbLv2T5Sfd6Rp2HBB8= +go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw= +go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= +go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4= +go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= +go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60= +go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20220427172511-eb4f295cb31f/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220622213112-05595931fe9d h1:sK3txAijHtOK88l68nt020reeT1ZdKLIYetKl95FzVY= diff --git a/log/logger.go b/log/logger.go new file mode 100644 index 0000000..533cdcf --- /dev/null +++ b/log/logger.go @@ -0,0 +1,68 @@ +package log + +import "fmt" + +type logLevel string + +const ( + InfoLevel logLevel = "info" + DebugLevel logLevel = "debug" + WarnLevel logLevel = "warn" + ErrorLevel logLevel = "error" +) + +type Logger interface { + Debug(args ...interface{}) + Debugf(format string, args ...interface{}) + Info(args ...interface{}) + Infof(format string, args ...interface{}) + Error(args ...interface{}) + Errorf(format string, args ...interface{}) + Warn(args ...interface{}) + Warnf(format string, args ...interface{}) + Fatal(args ...interface{}) + Fatalf(format string, args ...interface{}) + WithFields(level logLevel, msg string, fields ...Field) +} + +// Field used for structured logging +type Field struct { + key string + value string +} + +func String(key string, value string) Field { + return Field{ + key: key, + value: value, + } +} + +func Error(value string) Field { + return Field{ + key: "err", + value: value, + } +} + +func ParseLevel(str string) logLevel { + var lvl logLevel + lvl.unmarshalString(str) + return lvl +} + +func (l *logLevel) unmarshalString(str string) { + switch str { + case "debug", "DEBUG": + *l = DebugLevel + case "info", "INFO", "": // make the zero value useful + *l = InfoLevel + case "warn", "WARN": + *l = WarnLevel + case "error", "ERROR": + *l = ErrorLevel + default: + fmt.Println("unknown log level ", str, " proceeding with log levle Info") + *l = InfoLevel + } +} diff --git a/log/logger_test.go b/log/logger_test.go new file mode 100644 index 0000000..6054421 --- /dev/null +++ b/log/logger_test.go @@ -0,0 +1,21 @@ +package log + +import ( + "testing" +) + +func TestLogger(t *testing.T) { + logger, err := NewDefaultLogger("debug") + if err != nil { + t.Error(err) + } + // For illustrative purposes + logger.Info("basic ", "logging") + logger.Infof("info formatted log: %s to %d", "I can count", 123) + logger.Debug("basic ", "logging") + logger.Debugf("debug formatted log: %s to %d", "I can count", 123) + logger.Error("basic ", "logging") + logger.Errorf("error formatted log: %s to %d", "I could not count", 123) + logger.WithFields(ErrorLevel, "unexpected traffic received", Error("terrible error message")) + logger.WithFields(InfoLevel, "ingress traffic received", String("url", "https://github.com/Frontman-Labs/frontman"), String("host", "162.1.3.2"), String("port", "32133")) +} diff --git a/log/zap.go b/log/zap.go new file mode 100644 index 0000000..c17b981 --- /dev/null +++ b/log/zap.go @@ -0,0 +1,90 @@ +package log + +import ( + "os" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +type ZapLogger struct { + zap *zap.Logger + sugarZap *zap.SugaredLogger +} + +func (l ZapLogger) Debugf(format string, args ...interface{}) { + l.sugarZap.Debugf(format, args...) +} + +func (l ZapLogger) Debug(args ...interface{}) { + l.sugarZap.Debug(args...) +} + +func (l ZapLogger) Fatalf(format string, args ...interface{}) { + l.sugarZap.Fatalf(format, args...) +} + +func (l ZapLogger) Fatal(args ...interface{}) { + l.sugarZap.Fatal(args...) +} + +func (l ZapLogger) Infof(format string, args ...interface{}) { + l.sugarZap.Infof(format, args...) +} + +func (l ZapLogger) Info(args ...interface{}) { + l.sugarZap.Info(args...) +} + +func (l ZapLogger) Warnf(format string, args ...interface{}) { + l.sugarZap.Warnf(format, args...) +} + +func (l ZapLogger) Warn(args ...interface{}) { + l.sugarZap.Warn(args...) +} +func (l ZapLogger) Errorf(format string, args ...interface{}) { + l.sugarZap.Errorf(format, args...) +} + +func (l ZapLogger) Error(args ...interface{}) { + l.sugarZap.Error(args...) +} + +func fieldsToZap(fields ...Field) (zfields []zapcore.Field) { + for _, field := range fields { + zfields = append(zfields, zap.String(field.key, field.value)) + } + return zfields +} + +func (l ZapLogger) WithFields(level logLevel, msg string, fields ...Field) { + lvl, err := zapcore.ParseLevel(string(level)) + if err != nil { + l.Fatalf("Unknown log level: %s", level) + os.Exit(1) + } + l.zap.Log(lvl, msg, fieldsToZap(fields...)...) +} + +func NewZapLogger(level logLevel) (Logger, error) { + cfg := zap.NewProductionConfig() + lvl, err := zapcore.ParseLevel(string(level)) + if err != nil { + return nil, err + } + cfg.Level = zap.NewAtomicLevelAt(lvl) + zap, err := cfg.Build(zap.AddCallerSkip(1)) + if err != nil { + return nil, err + } + logger := &ZapLogger{ + zap: zap, + sugarZap: zap.Sugar(), + } + return logger, nil +} + +func NewDefaultLogger(level logLevel) (Logger, error) { + return NewZapLogger(level) +} From 6a9d5e713a928d932c01f5f006e6f94b2f109974 Mon Sep 17 00:00:00 2001 From: Christophe Collot Date: Sun, 12 Mar 2023 14:56:37 +0100 Subject: [PATCH 2/3] revert frontman.yaml changes --- frontman.yaml | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/frontman.yaml b/frontman.yaml index 2b35117..56f7b14 100644 --- a/frontman.yaml +++ b/frontman.yaml @@ -1,25 +1,28 @@ global: - service_type: "yaml" - services_file: "services.yaml" - redis_uri: "redis://localhost:6379" - redis_namespace: "my_service" - mongo_uri: "mongodb://localhost:27017" - mongo_db_name: "my_db" - mongo_collection_name: "my_collection" + service_type: SERVICE_TYPE + services_file: SERVICES_FILE + redis_uri: REDIS_URI + redis_namespace: REDIS_NAMESPACE + mongo_uri: MONGO_URI + mongo_db_name: MONGO_DB_NAME + mongo_collection_name: MONGO_COLLECTION_NAME + + api: - addr: ":8080" + addr: API_ADDR ssl: - enabled: false - cert: "/path/to/cert.pem" - key: "/path/to/key.pem" + enabled: API_SSL_ENABLED + cert: API_SSL_CERT + key: API_SSL_KEY + gateway: - addr: ":8081" + addr: GATEWAY_ADDR ssl: - enabled: false + enabled: GATEWAY_SSL_ENABLED + cert: GATEWAY_SSL_CERT + key: GATEWAY_SSL_KEY + logging: - level: "debug" -# plugins: -# enabled: true -# order: -# - "/path/to/plugin1.so" -# - "/path/to/plugin2.so" + level: LOG_LEVEL + + From 2ebd64f7298d9d03db2cab0acbec1ca70083f5f7 Mon Sep 17 00:00:00 2001 From: Christophe Collot Date: Sun, 12 Mar 2023 14:58:29 +0100 Subject: [PATCH 3/3] revert formatting changes --- frontman.yaml | 1 + gateway_test.go | 53 +++++++++++++++++++++++++------------------------ 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/frontman.yaml b/frontman.yaml index 56f7b14..781de78 100644 --- a/frontman.yaml +++ b/frontman.yaml @@ -26,3 +26,4 @@ logging: level: LOG_LEVEL + diff --git a/gateway_test.go b/gateway_test.go index 8df765b..7ed0ef0 100644 --- a/gateway_test.go +++ b/gateway_test.go @@ -6,30 +6,31 @@ import ( "testing" ) + func TestCreateRedirectServer(t *testing.T) { - redirectAddr := "0.0.0.0:8000" - redirectServer := createRedirectServer("0.0.0.0:80", redirectAddr) - - // Create a test request to the redirect server - req, err := http.NewRequest("GET", "http://example.com/foo", nil) - if err != nil { - t.Fatal(err) - } - - // Create a test response recorder - rr := httptest.NewRecorder() - - // Call the redirect server's handler function - redirectServer.Handler.ServeHTTP(rr, req) - - // Check that the response has a 301 status code - if status := rr.Code; status != http.StatusMovedPermanently { - t.Errorf("Unexpected status code: got %v, expected %v", status, http.StatusMovedPermanently) - } - - // Check that the response includes a "Location" header with the expected value - expectedURL := "https://example.com/foo" - if location := rr.Header().Get("Location"); location != expectedURL { - t.Errorf("Unexpected Location header value: got %v, expected %v", location, expectedURL) - } -} + redirectAddr := "0.0.0.0:8000" + redirectServer := createRedirectServer("0.0.0.0:80", redirectAddr) + + // Create a test request to the redirect server + req, err := http.NewRequest("GET", "http://example.com/foo", nil) + if err != nil { + t.Fatal(err) + } + + // Create a test response recorder + rr := httptest.NewRecorder() + + // Call the redirect server's handler function + redirectServer.Handler.ServeHTTP(rr, req) + + // Check that the response has a 301 status code + if status := rr.Code; status != http.StatusMovedPermanently { + t.Errorf("Unexpected status code: got %v, expected %v", status, http.StatusMovedPermanently) + } + + // Check that the response includes a "Location" header with the expected value + expectedURL := "https://example.com/foo" + if location := rr.Header().Get("Location"); location != expectedURL { + t.Errorf("Unexpected Location header value: got %v, expected %v", location, expectedURL) + } +} \ No newline at end of file