Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: allow configuring relay-proxy request log verbosity #1946

Merged
merged 24 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions cmd/editor/main.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package main

import (
"net/http"

"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
custommiddleware "github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/api/middleware"
Expand All @@ -9,18 +11,18 @@
"github.com/thomaspoignant/go-feature-flag/internal/flag"
"github.com/thomaspoignant/go-feature-flag/internal/utils"
"github.com/thomaspoignant/go-feature-flag/model"
"net/http"
)

// This service is an API used to evaluate a flag with an evaluation context
// This API is made for the Flag Editor to be able to evaluate a flag remotely and see if the configuration
// of the flag is working as expected.

func main() {
zapLog := log.InitLogger()
defer func() { _ = zapLog.Sync() }()
logger := log.InitLogger()
defer func() { _ = logger.ZapLogger.Sync() }()

Check warning on line 22 in cmd/editor/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/editor/main.go#L21-L22

Added lines #L21 - L22 were not covered by tests

e := echo.New()
e.Use(custommiddleware.ZapLogger(zapLog, nil))
e.Use(custommiddleware.ZapLogger(logger.ZapLogger, nil))

Check warning on line 25 in cmd/editor/main.go

View check run for this annotation

Codecov / codecov/patch

cmd/editor/main.go#L25

Added line #L25 was not covered by tests
e.Use(middleware.CORSWithConfig(middleware.CORSConfig{
AllowOrigins: []string{
"http://gofeatureflag.org",
Expand Down
13 changes: 7 additions & 6 deletions cmd/relayproxy/api/middleware/zap.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package middleware

import (
"fmt"
"strings"
"time"

"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/config"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"strings"
"time"
)

// DefaultSkipper is what we use as a default.
Expand All @@ -30,10 +31,10 @@ func DebugSkipper(_ echo.Context) bool {
}

// ZapLogger is a middleware and zap to provide an "access log" like logging for each request.
func ZapLogger(log *zap.Logger, config *config.Config) echo.MiddlewareFunc {
func ZapLogger(log *zap.Logger, cfg *config.Config) echo.MiddlewareFunc {
fals marked this conversation as resolved.
Show resolved Hide resolved
// select the right skipper
skipper := DefaultSkipper
if config != nil && config.Debug {
if cfg != nil && cfg.IsDebugEnabled() {
skipper = DebugSkipper
}

Expand Down Expand Up @@ -66,9 +67,9 @@ func ZapLogger(log *zap.Logger, config *config.Config) echo.MiddlewareFunc {
case n >= 400:
log.With(zap.Error(v.Error)).Warn("Client error", fields...)
case n >= 300:
log.Info("Redirection", fields...)
log.Debug("Redirection", fields...)
default:
log.Info("Success", fields...)
log.Debug("Success", fields...)
}
return nil
},
Expand Down
9 changes: 5 additions & 4 deletions cmd/relayproxy/api/middleware/zap_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package middleware_test

import (
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/api/middleware"
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/config"
"net/http"
"net/http/httptest"
"testing"

"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/api/middleware"
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/config"

"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
"go.uber.org/zap"
Expand Down Expand Up @@ -91,7 +92,7 @@ func TestZapLogger500(t *testing.T) {
return c.String(http.StatusInternalServerError, "")
}

obs, logs := observer.New(zap.DebugLevel)
obs, logs := observer.New(zap.ErrorLevel)
logger := zap.New(obs)
err := middleware.ZapLogger(logger, &config.Config{})(h)(c)
assert.Nil(t, err)
Expand Down Expand Up @@ -133,7 +134,7 @@ func TestZapLoggerHealthDebug(t *testing.T) {

obs, logs := observer.New(zap.DebugLevel)
logger := zap.New(obs)
err := middleware.ZapLogger(logger, &config.Config{Debug: true})(h)(c)
err := middleware.ZapLogger(logger, &config.Config{LogLevel: "debug"})(h)(c)
assert.Nil(t, err)
assert.Equal(t, 1, len(logs.AllUntimed()))
}
2 changes: 1 addition & 1 deletion cmd/relayproxy/api/routes_monitoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func (s *Server) addMonitoringRoutes() {
s.monitoringEcho = echo.New()
s.monitoringEcho.HideBanner = true
s.monitoringEcho.HidePort = true
s.monitoringEcho.Debug = s.config.Debug
s.monitoringEcho.Debug = s.config.IsDebugEnabled()
s.monitoringEcho.Use(custommiddleware.ZapLogger(s.zapLog, s.config))
s.monitoringEcho.Use(middleware.CORSWithConfig(middleware.DefaultCORSConfig))
s.monitoringEcho.Use(custommiddleware.VersionHeader(s.config))
Expand Down
12 changes: 7 additions & 5 deletions cmd/relayproxy/api/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ package api
import (
"errors"
"fmt"
"net/http"
"strings"
"time"

"github.com/labstack/echo-contrib/echoprometheus"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
Expand All @@ -15,9 +19,6 @@ import (
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/service"
"go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho"
"go.uber.org/zap"
"net/http"
"strings"
"time"
)

// New is used to create a new instance of the API server
Expand Down Expand Up @@ -50,15 +51,16 @@ type Server struct {
func (s *Server) initRoutes() {
s.apiEcho.HideBanner = true
s.apiEcho.HidePort = true
s.apiEcho.Debug = s.config.Debug
s.apiEcho.Debug = s.config.IsDebugEnabled()
s.apiEcho.Use(custommiddleware.ZapLogger(s.zapLog, s.config))
if s.services.Metrics != (metric.Metrics{}) {
s.apiEcho.Use(echoprometheus.NewMiddlewareWithConfig(echoprometheus.MiddlewareConfig{
Subsystem: metric.GOFFSubSystem,
Registerer: s.services.Metrics.Registry,
}))
}

s.apiEcho.Use(otelecho.Middleware("go-feature-flag"))
s.apiEcho.Use(custommiddleware.ZapLogger(s.zapLog, s.config))
s.apiEcho.Use(middleware.CORSWithConfig(middleware.DefaultCORSConfig))
s.apiEcho.Use(custommiddleware.VersionHeader(s.config))
s.apiEcho.Use(middleware.Recover())
Expand Down
27 changes: 14 additions & 13 deletions cmd/relayproxy/api/server_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package api_test

import (
"net/http"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/api"
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/config"
Expand All @@ -9,9 +13,6 @@ import (
"github.com/thomaspoignant/go-feature-flag/cmd/relayproxy/service"
"github.com/thomaspoignant/go-feature-flag/notifier"
"go.uber.org/zap"
"net/http"
"testing"
"time"
)

func Test_Starting_RelayProxy_with_monitoring_on_same_port(t *testing.T) {
Expand All @@ -22,18 +23,18 @@ func Test_Starting_RelayProxy_with_monitoring_on_same_port(t *testing.T) {
},
ListenPort: 11024,
}
zapLog := log.InitLogger()
defer func() { _ = zapLog.Sync() }()
log := log.InitLogger()
defer func() { _ = log.ZapLogger.Sync() }()

metricsV2, err := metric.NewMetrics()
if err != nil {
zapLog.Error("impossible to initialize prometheus metrics", zap.Error(err))
log.ZapLogger.Error("impossible to initialize prometheus metrics", zap.Error(err))
}
wsService := service.NewWebsocketService()
defer wsService.Close() // close all the open connections
prometheusNotifier := metric.NewPrometheusNotifier(metricsV2)
proxyNotifier := service.NewNotifierWebsocket(wsService)
goff, err := service.NewGoFeatureFlagClient(proxyConf, zapLog, []notifier.Notifier{
goff, err := service.NewGoFeatureFlagClient(proxyConf, log.ZapLogger, []notifier.Notifier{
prometheusNotifier,
proxyNotifier,
})
Expand All @@ -48,7 +49,7 @@ func Test_Starting_RelayProxy_with_monitoring_on_same_port(t *testing.T) {
Metrics: metricsV2,
}

s := api.New(proxyConf, services, zapLog)
s := api.New(proxyConf, services, log.ZapLogger)
go func() { s.Start() }()
defer s.Stop()

Expand Down Expand Up @@ -76,18 +77,18 @@ func Test_Starting_RelayProxy_with_monitoring_on_different_port(t *testing.T) {
ListenPort: 11024,
MonitoringPort: 11025,
}
zapLog := log.InitLogger()
defer func() { _ = zapLog.Sync() }()
log := log.InitLogger()
defer func() { _ = log.ZapLogger.Sync() }()

metricsV2, err := metric.NewMetrics()
if err != nil {
zapLog.Error("impossible to initialize prometheus metrics", zap.Error(err))
log.ZapLogger.Error("impossible to initialize prometheus metrics", zap.Error(err))
}
wsService := service.NewWebsocketService()
defer wsService.Close() // close all the open connections
prometheusNotifier := metric.NewPrometheusNotifier(metricsV2)
proxyNotifier := service.NewNotifierWebsocket(wsService)
goff, err := service.NewGoFeatureFlagClient(proxyConf, zapLog, []notifier.Notifier{
goff, err := service.NewGoFeatureFlagClient(proxyConf, log.ZapLogger, []notifier.Notifier{
prometheusNotifier,
proxyNotifier,
})
Expand All @@ -102,7 +103,7 @@ func Test_Starting_RelayProxy_with_monitoring_on_different_port(t *testing.T) {
Metrics: metricsV2,
}

s := api.New(proxyConf, services, zapLog)
s := api.New(proxyConf, services, log.ZapLogger)
go func() { s.Start() }()
defer s.Stop()

Expand Down
65 changes: 57 additions & 8 deletions cmd/relayproxy/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ package config

import (
"fmt"
"net/http"
"os"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/knadh/koanf/parsers/json"
"github.com/knadh/koanf/parsers/toml"
"github.com/knadh/koanf/parsers/yaml"
Expand All @@ -13,12 +20,7 @@ import (
"github.com/spf13/pflag"
"github.com/xitongsys/parquet-go/parquet"
"go.uber.org/zap"
"net/http"
"os"
"path/filepath"
"strconv"
"strings"
"time"
"go.uber.org/zap/zapcore"
)

var k = koanf.New(".")
Expand All @@ -32,6 +34,8 @@ var DefaultRetriever = struct {
GitBranch: "main",
}

const DefaultLogLevel = "info"

var DefaultExporter = struct {
Format string
LogFormat string
Expand All @@ -40,6 +44,7 @@ var DefaultExporter = struct {
FlushInterval time.Duration
MaxEventInMemory int64
ParquetCompressionCodec string
LogLevel string
}{
Format: "JSON",
LogFormat: "[{{ .FormattedDate}}] user=\"{{ .UserKey}}\", flag=\"{{ .Key}}\", value=\"{{ .Value}}\"",
Expand All @@ -49,6 +54,7 @@ var DefaultExporter = struct {
FlushInterval: 60000 * time.Millisecond,
MaxEventInMemory: 100000,
ParquetCompressionCodec: parquet.CompressionCodec_SNAPPY.String(),
LogLevel: DefaultLogLevel,
}

// New is reading the configuration file
Expand All @@ -62,6 +68,7 @@ func New(flagSet *pflag.FlagSet, log *zap.Logger, version string) (*Config, erro
"fileFormat": "yaml",
"restApiTimeout": 5000,
"pollingInterval": 60000,
"logLevel": DefaultLogLevel,
}, "."), nil)

// mapping command line parameters to koanf
Expand Down Expand Up @@ -112,6 +119,14 @@ func New(flagSet *pflag.FlagSet, log *zap.Logger, version string) (*Config, erro
if errUnmarshal != nil {
return nil, errUnmarshal
}

if proxyConf.Debug {
log.Warn(
"Option Debug that you are using in your configuration file is deprecated" +
"and will be removed in future versions." +
"Please use logLevel: debug to continue to run the relay-proxy with debug logs.")
}

return proxyConf, nil
}

Expand All @@ -122,14 +137,20 @@ type Config struct {
// HideBanner (optional) if true, we don't display the go-feature-flag relay proxy banner
HideBanner bool `mapstructure:"hideBanner" koanf:"hidebanner"`

// Debug (optional) if true, go-feature-flag relay proxy will run on debug mode, with more logs and custom responses
Debug bool `mapstructure:"debug" koanf:"debug"`

// EnableSwagger (optional) to have access to the swagger
EnableSwagger bool `mapstructure:"enableSwagger" koanf:"enableswagger"`

// Host should be set if you are using swagger (default is localhost)
Host string `mapstructure:"host" koanf:"host"`

// Debug (optional) if true, go-feature-flag relay proxy will run on debug mode, with more logs and custom responses
Debug bool `mapstructure:"debug" koanf:"debug"`
fals marked this conversation as resolved.
Show resolved Hide resolved
// LogLevel (optional) sets the verbosity for logging,
// Possible values: debug, info, warn, error, dpanic, panic, fatal
// If level debug go-feature-flag relay proxy will run on debug mode, with more logs and custom responses
// Default: debug
LogLevel string `mapstructure:"logLevel" koanf:"loglevel"`

// PollingInterval (optional) Poll every X time
// The minimum possible is 1 second
Expand Down Expand Up @@ -294,6 +315,11 @@ func (c *Config) IsValid() error {
}
}
}
if c.LogLevel != "" {
if _, err := zapcore.ParseLevel(c.LogLevel); err != nil {
return err
}
}

return nil
}
Expand Down Expand Up @@ -387,3 +413,26 @@ func loadArrayEnv(s string, v string, configMap map[string]interface{}) error {
}
return nil
}

func (c *Config) IsDebugEnabled() bool {
if c == nil {
return false
}
return strings.ToLower(c.LogLevel) == "debug" || c.Debug
}

func (c *Config) ZapLogLevel() zapcore.Level {
if c == nil {
return zapcore.InvalidLevel
}
// Use debug flag for backward compatibility
if c.Debug {
return zapcore.DebugLevel
}

level, err := zapcore.ParseLevel(c.LogLevel)
if err != nil {
return zapcore.InvalidLevel
}
return level
}
Loading
Loading