From 3c62ebaf10cd377feeebe30c0d48a5d8152a844a Mon Sep 17 00:00:00 2001 From: Michael Nikitochkin Date: Sat, 17 Sep 2022 14:24:22 +0200 Subject: [PATCH 1/5] Introduce App object with logger initialization --- app/app.go | 43 +++++++++++++++++++++++++++++++++++++ app/logger.go | 49 +++++++++++++++++++++++++++++++++++++++++++ app/metrics.go | 5 +++++ cmd/server/server.go | 50 +++++++------------------------------------- 4 files changed, 104 insertions(+), 43 deletions(-) create mode 100644 app/app.go create mode 100644 app/logger.go create mode 100644 app/metrics.go diff --git a/app/app.go b/app/app.go new file mode 100644 index 00000000..b57f9787 --- /dev/null +++ b/app/app.go @@ -0,0 +1,43 @@ +package app + +import ( + "fmt" + + "github.com/rs/zerolog" +) + +// App is used for keep central location of configuration and resources. +type App struct { + Logger zerolog.Logger +} + +// NewApp initialize App instance. +func NewApp() (*App, error) { + app := &App{} + + start([]unit{ + {"Logger", app.setLogger}, + {"Metrics", app.setMetrics}, + }) + + return app, nil +} + +// unit keeps initialization tasks. +// could be used later for graceful stop function per service if it is required. +type unit struct { + name string + start func() error +} + +// start run initialized step for resource. +// could be wrapped with debug information and resource usage. +func start(units []unit) error { + for _, unit := range units { + err := unit.start() + if err != nil { + return fmt.Errorf("initialization %s failed: %w", unit.name, err) + } + } + return nil +} diff --git a/app/logger.go b/app/logger.go new file mode 100644 index 00000000..1700feac --- /dev/null +++ b/app/logger.go @@ -0,0 +1,49 @@ +package app + +import ( + "os" + "strconv" + "time" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func (a *App) setLogger() error { + zerolog.TimestampFunc = func() time.Time { + return time.Now().UTC() + } + + zerolog.CallerMarshalFunc = func(pc uintptr, file string, line int) string { + short := file + for i := len(file) - 1; i > 0; i-- { + if file[i] == '/' { + short = file[i+1:] + break + } + } + file = short + return file + ":" + strconv.Itoa(line) + } + + logger := zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() + defer func(a *App, logger zerolog.Logger) { + a.Logger = logger + log.Logger = logger + }(a, logger) + + val, ok := os.LookupEnv("LOG_LEVEL") + if !ok { + return nil + } + + lvl, err := zerolog.ParseLevel(val) + if err == nil { + a.Logger = a.Logger.Level(lvl) + } else { + l := &logger + l.Err(err).Msgf("unknown LOG_LEVEL value: \"%s\"", val) + } + + return nil +} diff --git a/app/metrics.go b/app/metrics.go new file mode 100644 index 00000000..3f8e3a07 --- /dev/null +++ b/app/metrics.go @@ -0,0 +1,5 @@ +package app + +func (a *App) setMetrics() error { + return nil +} diff --git a/cmd/server/server.go b/cmd/server/server.go index d188adb1..75e57e6d 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -7,15 +7,13 @@ import ( "net" "os" "os/signal" - "strconv" "syscall" "time" "github.com/prometheus/client_golang/prometheus" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" "github.com/Shopify/toxiproxy/v2" + "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/collectors" ) @@ -61,7 +59,6 @@ func main() { func run() error { cli := parseArguments() - if cli.printVersion { fmt.Printf("toxiproxy-server version %s\n", toxiproxy.Version) return nil @@ -69,9 +66,11 @@ func run() error { rand.Seed(cli.seed) - logger := setupLogger() - log.Logger = logger - + app, err := app.NewApp() + if err != nil { + return err + } + logger := app.Logger logger. Info(). Str("version", toxiproxy.Version). @@ -102,44 +101,9 @@ func run() error { signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM) <-signals server.Logger.Info().Msg("Shutdown started") - err := server.Shutdown() + err = server.Shutdown() if err != nil { logger.Err(err).Msg("Shutdown finished with error") } return nil } - -func setupLogger() zerolog.Logger { - zerolog.TimestampFunc = func() time.Time { - return time.Now().UTC() - } - - zerolog.CallerMarshalFunc = func(pc uintptr, file string, line int) string { - short := file - for i := len(file) - 1; i > 0; i-- { - if file[i] == '/' { - short = file[i+1:] - break - } - } - file = short - return file + ":" + strconv.Itoa(line) - } - - logger := zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() - - val, ok := os.LookupEnv("LOG_LEVEL") - if !ok { - return logger - } - - lvl, err := zerolog.ParseLevel(val) - if err == nil { - logger = logger.Level(lvl) - } else { - l := &logger - l.Err(err).Msgf("unknown LOG_LEVEL value: \"%s\"", val) - } - - return logger -} From 051d40ecc90109089b1e9e29f1aeb77221114b04 Mon Sep 17 00:00:00 2001 From: Michael Nikitochkin Date: Sat, 17 Sep 2022 15:22:15 +0200 Subject: [PATCH 2/5] Initialize app with metrics and logger --- api.go | 9 +++--- api_test.go | 4 +-- app/app.go | 3 ++ app/metrics.go | 5 +++ cmd/server/server.go | 5 +-- metrics.go => collectors/metrics_container.go | 31 +++++++++---------- link.go | 4 +-- metrics_test.go | 7 ++--- toxics/toxic_test.go | 3 +- toxiproxy_test.go | 3 +- 10 files changed, 37 insertions(+), 37 deletions(-) rename metrics.go => collectors/metrics_container.go (50%) diff --git a/api.go b/api.go index d639749f..d2b28ba0 100644 --- a/api.go +++ b/api.go @@ -14,6 +14,7 @@ import ( "github.com/rs/zerolog/hlog" "github.com/Shopify/toxiproxy/v2/toxics" + "github.com/Shopify/toxiproxy/v2/collectors" ) func stopBrowsersMiddleware(next http.Handler) http.Handler { @@ -32,7 +33,7 @@ func timeoutMiddleware(next http.Handler) http.Handler { type ApiServer struct { Collection *ProxyCollection - Metrics *metricsContainer + Metrics *collectors.MetricsContainer Logger *zerolog.Logger http *http.Server } @@ -42,7 +43,7 @@ const ( read_timeout = 15 * time.Second ) -func NewServer(m *metricsContainer, logger zerolog.Logger) *ApiServer { +func NewServer(m *collectors.MetricsContainer, logger zerolog.Logger) *ApiServer { return &ApiServer{ Collection: NewProxyCollection(), Metrics: m, @@ -136,8 +137,8 @@ func (server *ApiServer) Routes() *mux.Router { r.HandleFunc("/version", server.Version).Methods("GET").Name("Version") - if server.Metrics.anyMetricsEnabled() { - r.Handle("/metrics", server.Metrics.handler()).Name("Metrics") + if server.Metrics.AnyMetricsEnabled() { + r.Handle("/metrics", server.Metrics.Handler()).Name("Metrics") } return r diff --git a/api_test.go b/api_test.go index 322cfe07..2b0d60c6 100644 --- a/api_test.go +++ b/api_test.go @@ -9,10 +9,10 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus" "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2" + "github.com/Shopify/toxiproxy/v2/collectors" tclient "github.com/Shopify/toxiproxy/v2/client" ) @@ -30,7 +30,7 @@ func WithServer(t *testing.T, f func(string)) { // way to shut it down between each test run. if testServer == nil { testServer = toxiproxy.NewServer( - toxiproxy.NewMetricsContainer(prometheus.NewRegistry()), + collectors.NewMetricsContainer(), log, ) diff --git a/app/app.go b/app/app.go index b57f9787..75f8a3b2 100644 --- a/app/app.go +++ b/app/app.go @@ -4,11 +4,14 @@ import ( "fmt" "github.com/rs/zerolog" + + "github.com/Shopify/toxiproxy/v2/collectors" ) // App is used for keep central location of configuration and resources. type App struct { Logger zerolog.Logger + Metrics *collectors.MetricsContainer } // NewApp initialize App instance. diff --git a/app/metrics.go b/app/metrics.go index 3f8e3a07..4f277eaa 100644 --- a/app/metrics.go +++ b/app/metrics.go @@ -1,5 +1,10 @@ package app +import ( + "github.com/Shopify/toxiproxy/v2/collectors" +) + func (a *App) setMetrics() error { + a.Metrics = collectors.NewMetricsContainer() return nil } diff --git a/cmd/server/server.go b/cmd/server/server.go index 75e57e6d..788c8241 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -10,8 +10,6 @@ import ( "syscall" "time" - "github.com/prometheus/client_golang/prometheus" - "github.com/Shopify/toxiproxy/v2" "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/collectors" @@ -76,8 +74,7 @@ func run() error { Str("version", toxiproxy.Version). Msg("Starting Toxiproxy") - metrics := toxiproxy.NewMetricsContainer(prometheus.NewRegistry()) - server := toxiproxy.NewServer(metrics, logger) + server := toxiproxy.NewServer(app.Metrics, app.Logger) if cli.proxyMetrics { server.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() } diff --git a/metrics.go b/collectors/metrics_container.go similarity index 50% rename from metrics.go rename to collectors/metrics_container.go index 8870ab10..77d536f6 100644 --- a/metrics.go +++ b/collectors/metrics_container.go @@ -1,50 +1,47 @@ -package toxiproxy +package collectors import ( "net/http" - "github.com/Shopify/toxiproxy/v2/collectors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" ) // NewMetricsContainer initializes a container for storing all prometheus metrics. -func NewMetricsContainer(registry *prometheus.Registry) *metricsContainer { - if registry == nil { - registry = prometheus.NewRegistry() - } - return &metricsContainer{ +func NewMetricsContainer() *MetricsContainer { + registry := prometheus.NewRegistry() + return &MetricsContainer{ registry: registry, } } -type metricsContainer struct { - RuntimeMetrics *collectors.RuntimeMetricCollectors - ProxyMetrics *collectors.ProxyMetricCollectors +type MetricsContainer struct { + RuntimeMetrics *RuntimeMetricCollectors + ProxyMetrics *ProxyMetricCollectors registry *prometheus.Registry } -func (m *metricsContainer) runtimeMetricsEnabled() bool { +func (m *MetricsContainer) runtimeMetricsEnabled() bool { return m.RuntimeMetrics != nil } -func (m *metricsContainer) proxyMetricsEnabled() bool { +func (m *MetricsContainer) ProxyMetricsEnabled() bool { return m.ProxyMetrics != nil } -// anyMetricsEnabled determines whether we have any prometheus metrics registered for exporting. -func (m *metricsContainer) anyMetricsEnabled() bool { - return m.runtimeMetricsEnabled() || m.proxyMetricsEnabled() +// AnyMetricsEnabled determines whether we have any prometheus metrics registered for exporting. +func (m *MetricsContainer) AnyMetricsEnabled() bool { + return m.runtimeMetricsEnabled() || m.ProxyMetricsEnabled() } // handler returns an HTTP handler with the necessary collectors registered // via a global prometheus registry. -func (m *metricsContainer) handler() http.Handler { +func (m *MetricsContainer) Handler() http.Handler { if m.runtimeMetricsEnabled() { m.registry.MustRegister(m.RuntimeMetrics.Collectors()...) } - if m.proxyMetricsEnabled() { + if m.ProxyMetricsEnabled() { m.registry.MustRegister(m.ProxyMetrics.Collectors()...) } return promhttp.HandlerFor( diff --git a/link.go b/link.go index 03ae9d56..56d96a21 100644 --- a/link.go +++ b/link.go @@ -126,7 +126,7 @@ func (link *ToxicLink) read( Err(err). Msg("Source terminated") } - if server.Metrics.proxyMetricsEnabled() { + if server.Metrics.ProxyMetricsEnabled() { server.Metrics.ProxyMetrics.ReceivedBytesTotal. WithLabelValues(metricLabels...).Add(float64(bytes)) } @@ -155,7 +155,7 @@ func (link *ToxicLink) write( Int64("bytes", bytes). Err(err). Msg("Could not write to destination") - } else if server.Metrics.proxyMetricsEnabled() { + } else if server.Metrics.ProxyMetricsEnabled() { server.Metrics.ProxyMetrics.SentBytesTotal. WithLabelValues(metricLabels...).Add(float64(bytes)) } diff --git a/metrics_test.go b/metrics_test.go index 7c84c932..e00449a6 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -10,7 +10,6 @@ import ( "strings" "testing" - "github.com/prometheus/client_golang/prometheus" "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2/collectors" @@ -18,7 +17,7 @@ import ( ) func TestProxyMetricsReceivedSentBytes(t *testing.T) { - srv := NewServer(NewMetricsContainer(prometheus.NewRegistry()), zerolog.Nop()) + srv := NewServer(collectors.NewMetricsContainer(), zerolog.Nop()) srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() proxy := NewProxy(srv, "test_proxy_metrics_received_sent_bytes", "localhost:0", "upstream") @@ -55,7 +54,7 @@ func TestProxyMetricsReceivedSentBytes(t *testing.T) { } func TestRuntimeMetricsBuildInfo(t *testing.T) { - srv := NewServer(NewMetricsContainer(prometheus.NewRegistry()), zerolog.Nop()) + srv := NewServer(collectors.NewMetricsContainer(), zerolog.Nop()) srv.Metrics.RuntimeMetrics = collectors.NewRuntimeMetricCollectors() expected := `go_build_info{checksum="[^"]*",path="[^"]*",version="[^"]*"} 1` @@ -90,7 +89,7 @@ func (t *testWriteCloser) Close() error { func prometheusOutput(t *testing.T, apiServer *ApiServer, prefix string) []string { t.Helper() - testServer := httptest.NewServer(apiServer.Metrics.handler()) + testServer := httptest.NewServer(apiServer.Metrics.Handler()) defer testServer.Close() resp, err := http.Get(testServer.URL) diff --git a/toxics/toxic_test.go b/toxics/toxic_test.go index b8a123a1..3a5525f5 100644 --- a/toxics/toxic_test.go +++ b/toxics/toxic_test.go @@ -14,7 +14,6 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus" "github.com/rs/zerolog" tomb "gopkg.in/tomb.v1" @@ -30,7 +29,7 @@ func NewTestProxy(name, upstream string) *toxiproxy.Proxy { log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() } srv := toxiproxy.NewServer( - toxiproxy.NewMetricsContainer(prometheus.NewRegistry()), + collectors.NewMetricsContainer(), log, ) srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() diff --git a/toxiproxy_test.go b/toxiproxy_test.go index 976279b5..49b61e47 100644 --- a/toxiproxy_test.go +++ b/toxiproxy_test.go @@ -6,7 +6,6 @@ import ( "os" "testing" - "github.com/prometheus/client_golang/prometheus" "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2" @@ -20,7 +19,7 @@ func NewTestProxy(name, upstream string) *toxiproxy.Proxy { log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() } srv := toxiproxy.NewServer( - toxiproxy.NewMetricsContainer(prometheus.NewRegistry()), + collectors.NewMetricsContainer(), log, ) srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() From f6b98cf41e8bb99978ded7dc884f937e9872ca4c Mon Sep 17 00:00:00 2001 From: Michael Nikitochkin Date: Sat, 17 Sep 2022 19:11:08 +0200 Subject: [PATCH 3/5] Tests bypass --- api.go | 29 ++++++++++++----- api_test.go | 25 ++++++++------- app/app.go | 31 ++++++++++++++++--- app/logger.go | 12 ++++--- app/metrics.go | 9 +++++- cmd/server/server.go | 74 ++++++++++++-------------------------------- metrics_test.go | 13 +++----- toxics/toxic_test.go | 19 ++++++------ toxiproxy_test.go | 16 +++++----- 9 files changed, 118 insertions(+), 110 deletions(-) diff --git a/api.go b/api.go index d2b28ba0..30a3cd4b 100644 --- a/api.go +++ b/api.go @@ -13,8 +13,9 @@ import ( "github.com/rs/zerolog" "github.com/rs/zerolog/hlog" - "github.com/Shopify/toxiproxy/v2/toxics" + "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/collectors" + "github.com/Shopify/toxiproxy/v2/toxics" ) func stopBrowsersMiddleware(next http.Handler) http.Handler { @@ -32,6 +33,7 @@ func timeoutMiddleware(next http.Handler) http.Handler { } type ApiServer struct { + app *app.App Collection *ProxyCollection Metrics *collectors.MetricsContainer Logger *zerolog.Logger @@ -43,22 +45,29 @@ const ( read_timeout = 15 * time.Second ) -func NewServer(m *collectors.MetricsContainer, logger zerolog.Logger) *ApiServer { - return &ApiServer{ +func NewServer(app *app.App) *ApiServer { + server := ApiServer{ + app: app, Collection: NewProxyCollection(), - Metrics: m, - Logger: &logger, + Metrics: app.Metrics, + Logger: app.Logger, + } + + if len(app.Config) > 0 { + server.PopulateConfig(app.Config) } + + return &server } -func (server *ApiServer) Listen(addr string) error { +func (server *ApiServer) Listen() error { server.Logger. Info(). - Str("address", addr). + Str("address", server.app.Addr). Msg("Starting Toxiproxy HTTP server") server.http = &http.Server{ - Addr: addr, + Addr: server.app.Addr, Handler: server.Routes(), WriteTimeout: wait_timeout, ReadTimeout: read_timeout, @@ -74,6 +83,10 @@ func (server *ApiServer) Listen(addr string) error { } func (server *ApiServer) Shutdown() error { + server.Logger. + Info(). + Msg("Shutdown started") + if server.http == nil { return nil } diff --git a/api_test.go b/api_test.go index 2b0d60c6..117993ee 100644 --- a/api_test.go +++ b/api_test.go @@ -12,8 +12,9 @@ import ( "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2" - "github.com/Shopify/toxiproxy/v2/collectors" + "github.com/Shopify/toxiproxy/v2/app" tclient "github.com/Shopify/toxiproxy/v2/client" + "github.com/Shopify/toxiproxy/v2/collectors" ) var testServer *toxiproxy.ApiServer @@ -21,20 +22,22 @@ var testServer *toxiproxy.ApiServer var client = tclient.NewClient("http://127.0.0.1:8475") func WithServer(t *testing.T, f func(string)) { - log := zerolog.Nop() - if flag.Lookup("test.v").DefValue == "true" { - log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() - } - // Make sure only one server is running at a time. Apparently there's no clean // way to shut it down between each test run. if testServer == nil { - testServer = toxiproxy.NewServer( - collectors.NewMetricsContainer(), - log, - ) + log := zerolog.Nop() + if flag.Lookup("test.v").DefValue == "true" { + log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() + } + + a := app.App{ + Addr: "localhost:8475", + Logger: &log, + Metrics: &collectors.MetricsContainer{}, + } + testServer = toxiproxy.NewServer(&a) - go testServer.Listen("localhost:8475") + go testServer.Listen() // Allow server to start. There's no clean way to know when it listens. time.Sleep(50 * time.Millisecond) diff --git a/app/app.go b/app/app.go index 75f8a3b2..edde20bc 100644 --- a/app/app.go +++ b/app/app.go @@ -2,21 +2,44 @@ package app import ( "fmt" + "math/rand" + "net" "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2/collectors" ) +type ServerOptions struct { + Host string + Port string + Config string + Seed int64 + PrintVersion bool + ProxyMetrics bool + RuntimeMetrics bool +} + // App is used for keep central location of configuration and resources. type App struct { - Logger zerolog.Logger - Metrics *collectors.MetricsContainer + Addr string + Logger *zerolog.Logger + Metrics *collectors.MetricsContainer + Config string + EnabledProxyMetrics bool + EnabledRuntimeMetrics bool } // NewApp initialize App instance. -func NewApp() (*App, error) { - app := &App{} +func NewApp(options ServerOptions) (*App, error) { + rand.Seed(options.Seed) + + app := &App{ + Addr: net.JoinHostPort(options.Host, options.Port), + Config: options.Config, + EnabledProxyMetrics: options.ProxyMetrics, + EnabledRuntimeMetrics: options.RuntimeMetrics, + } start([]unit{ {"Logger", app.setLogger}, diff --git a/app/logger.go b/app/logger.go index 1700feac..00cfbfba 100644 --- a/app/logger.go +++ b/app/logger.go @@ -6,10 +6,10 @@ import ( "time" "github.com/rs/zerolog" - "github.com/rs/zerolog/log" + // "github.com/rs/zerolog/log". ) -func (a *App) setLogger() error { +func init() { zerolog.TimestampFunc = func() time.Time { return time.Now().UTC() } @@ -25,11 +25,13 @@ func (a *App) setLogger() error { file = short return file + ":" + strconv.Itoa(line) } +} +func (a *App) setLogger() error { logger := zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() defer func(a *App, logger zerolog.Logger) { - a.Logger = logger - log.Logger = logger + a.Logger = &logger + // log.Logger = logger }(a, logger) val, ok := os.LookupEnv("LOG_LEVEL") @@ -39,7 +41,7 @@ func (a *App) setLogger() error { lvl, err := zerolog.ParseLevel(val) if err == nil { - a.Logger = a.Logger.Level(lvl) + logger = logger.Level(lvl) } else { l := &logger l.Err(err).Msgf("unknown LOG_LEVEL value: \"%s\"", val) diff --git a/app/metrics.go b/app/metrics.go index 4f277eaa..68b77f31 100644 --- a/app/metrics.go +++ b/app/metrics.go @@ -5,6 +5,13 @@ import ( ) func (a *App) setMetrics() error { - a.Metrics = collectors.NewMetricsContainer() + metrics := collectors.NewMetricsContainer() + if a.EnabledProxyMetrics { + metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() + } + if a.EnabledRuntimeMetrics { + metrics.RuntimeMetrics = collectors.NewRuntimeMetricCollectors() + } + a.Metrics = metrics return nil } diff --git a/cmd/server/server.go b/cmd/server/server.go index 788c8241..83bca29c 100644 --- a/cmd/server/server.go +++ b/cmd/server/server.go @@ -3,8 +3,6 @@ package main import ( "flag" "fmt" - "math/rand" - "net" "os" "os/signal" "syscall" @@ -12,34 +10,23 @@ import ( "github.com/Shopify/toxiproxy/v2" "github.com/Shopify/toxiproxy/v2/app" - "github.com/Shopify/toxiproxy/v2/collectors" ) -type cliArguments struct { - host string - port string - config string - seed int64 - printVersion bool - proxyMetrics bool - runtimeMetrics bool -} - -func parseArguments() cliArguments { - result := cliArguments{} - flag.StringVar(&result.host, "host", "localhost", +func parseArguments() app.ServerOptions { + result := app.ServerOptions{} + flag.StringVar(&result.Host, "host", "localhost", "Host for toxiproxy's API to listen on") - flag.StringVar(&result.port, "port", "8474", + flag.StringVar(&result.Port, "port", "8474", "Port for toxiproxy's API to listen on") - flag.StringVar(&result.config, "config", "", + flag.StringVar(&result.Config, "config", "", "JSON file containing proxies to create on startup") - flag.Int64Var(&result.seed, "seed", time.Now().UTC().UnixNano(), + flag.Int64Var(&result.Seed, "seed", time.Now().UTC().UnixNano(), "Seed for randomizing toxics with") - flag.BoolVar(&result.runtimeMetrics, "runtime-metrics", false, + flag.BoolVar(&result.RuntimeMetrics, "runtime-metrics", false, `enable runtime-related prometheus metrics (default "false")`) - flag.BoolVar(&result.proxyMetrics, "proxy-metrics", false, + flag.BoolVar(&result.ProxyMetrics, "proxy-metrics", false, `enable toxiproxy-specific prometheus metrics (default "false")`) - flag.BoolVar(&result.printVersion, "version", false, + flag.BoolVar(&result.PrintVersion, "version", false, `print the version (default "false")`) flag.Parse() @@ -57,50 +44,29 @@ func main() { func run() error { cli := parseArguments() - if cli.printVersion { + if cli.PrintVersion { fmt.Printf("toxiproxy-server version %s\n", toxiproxy.Version) return nil } - rand.Seed(cli.seed) - - app, err := app.NewApp() + app, err := app.NewApp(cli) if err != nil { return err } - logger := app.Logger - logger. - Info(). - Str("version", toxiproxy.Version). - Msg("Starting Toxiproxy") - server := toxiproxy.NewServer(app.Metrics, app.Logger) - if cli.proxyMetrics { - server.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() - } - if cli.runtimeMetrics { - server.Metrics.RuntimeMetrics = collectors.NewRuntimeMetricCollectors() - } + server := toxiproxy.NewServer(app) - if len(cli.config) > 0 { - server.PopulateConfig(cli.config) - } + stop := make(chan os.Signal, 1) + signal.Notify(stop, syscall.SIGINT, syscall.SIGTERM) - addr := net.JoinHostPort(cli.host, cli.port) - go func(server *toxiproxy.ApiServer, addr string) { - err := server.Listen(addr) + go func(server *toxiproxy.ApiServer, stop chan os.Signal) { + err := server.Listen() if err != nil { server.Logger.Err(err).Msg("Server finished with error") } - }(server, addr) + close(stop) + }(server, stop) - signals := make(chan os.Signal, 1) - signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM) - <-signals - server.Logger.Info().Msg("Shutdown started") - err = server.Shutdown() - if err != nil { - logger.Err(err).Msg("Shutdown finished with error") - } - return nil + <-stop + return server.Shutdown() } diff --git a/metrics_test.go b/metrics_test.go index e00449a6..3ba52f69 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -10,16 +10,13 @@ import ( "strings" "testing" - "github.com/rs/zerolog" - - "github.com/Shopify/toxiproxy/v2/collectors" + "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/stream" ) func TestProxyMetricsReceivedSentBytes(t *testing.T) { - srv := NewServer(collectors.NewMetricsContainer(), zerolog.Nop()) - srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() - + a, _ := app.NewApp(app.ServerOptions{ProxyMetrics: true}) + srv := NewServer(a) proxy := NewProxy(srv, "test_proxy_metrics_received_sent_bytes", "localhost:0", "upstream") r := bufio.NewReader(bytes.NewBufferString("hello")) @@ -54,8 +51,8 @@ func TestProxyMetricsReceivedSentBytes(t *testing.T) { } func TestRuntimeMetricsBuildInfo(t *testing.T) { - srv := NewServer(collectors.NewMetricsContainer(), zerolog.Nop()) - srv.Metrics.RuntimeMetrics = collectors.NewRuntimeMetricCollectors() + a, _ := app.NewApp(app.ServerOptions{RuntimeMetrics: true}) + srv := NewServer(a) expected := `go_build_info{checksum="[^"]*",path="[^"]*",version="[^"]*"} 1` diff --git a/toxics/toxic_test.go b/toxics/toxic_test.go index 3a5525f5..d7d104b5 100644 --- a/toxics/toxic_test.go +++ b/toxics/toxic_test.go @@ -6,33 +6,32 @@ import ( "context" "crypto/rand" "encoding/json" - "flag" "io" "net" - "os" "strings" "testing" "time" - "github.com/rs/zerolog" tomb "gopkg.in/tomb.v1" "github.com/Shopify/toxiproxy/v2" + "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/collectors" "github.com/Shopify/toxiproxy/v2/stream" "github.com/Shopify/toxiproxy/v2/toxics" + "github.com/rs/zerolog" ) func NewTestProxy(name, upstream string) *toxiproxy.Proxy { log := zerolog.Nop() - if flag.Lookup("test.v").DefValue == "true" { - log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() + a := app.App{ + Metrics: &collectors.MetricsContainer{ + ProxyMetrics: collectors.NewProxyMetricCollectors(), + }, + Logger: &log, } - srv := toxiproxy.NewServer( - collectors.NewMetricsContainer(), - log, - ) - srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() + + srv := toxiproxy.NewServer(&a) proxy := toxiproxy.NewProxy(srv, name, "localhost:0", upstream) return proxy diff --git a/toxiproxy_test.go b/toxiproxy_test.go index 49b61e47..2db1593f 100644 --- a/toxiproxy_test.go +++ b/toxiproxy_test.go @@ -1,28 +1,26 @@ package toxiproxy_test import ( - "flag" "net" - "os" "testing" "github.com/rs/zerolog" "github.com/Shopify/toxiproxy/v2" + "github.com/Shopify/toxiproxy/v2/app" "github.com/Shopify/toxiproxy/v2/collectors" "github.com/Shopify/toxiproxy/v2/testhelper" ) func NewTestProxy(name, upstream string) *toxiproxy.Proxy { log := zerolog.Nop() - if flag.Lookup("test.v").DefValue == "true" { - log = zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() + a := app.App{ + Metrics: &collectors.MetricsContainer{ + ProxyMetrics: collectors.NewProxyMetricCollectors(), + }, + Logger: &log, } - srv := toxiproxy.NewServer( - collectors.NewMetricsContainer(), - log, - ) - srv.Metrics.ProxyMetrics = collectors.NewProxyMetricCollectors() + srv := toxiproxy.NewServer(&a) proxy := toxiproxy.NewProxy(srv, name, "localhost:0", upstream) return proxy From 949aa3d0550ef06cfd1223efe52e1aa3e424b5fb Mon Sep 17 00:00:00 2001 From: Michael Nikitochkin Date: Sat, 17 Sep 2022 19:23:20 +0200 Subject: [PATCH 4/5] Add logger to toxicstub --- app/logger.go | 2 -- link.go | 16 +++++++++------- toxics/bandwidth.go | 4 +--- toxics/limit_data_test.go | 13 +++++++++---- toxics/slicer_test.go | 7 +++++-- toxics/toxic.go | 10 +++++++++- toxics/toxic_test.go | 3 ++- 7 files changed, 35 insertions(+), 20 deletions(-) diff --git a/app/logger.go b/app/logger.go index 00cfbfba..3bd5823a 100644 --- a/app/logger.go +++ b/app/logger.go @@ -6,7 +6,6 @@ import ( "time" "github.com/rs/zerolog" - // "github.com/rs/zerolog/log". ) func init() { @@ -31,7 +30,6 @@ func (a *App) setLogger() error { logger := zerolog.New(os.Stdout).With().Caller().Timestamp().Logger() defer func(a *App, logger zerolog.Logger) { a.Logger = &logger - // log.Logger = logger }(a, logger) val, ok := os.LookupEnv("LOG_LEVEL") diff --git a/link.go b/link.go index 56d96a21..e5eb3f6b 100644 --- a/link.go +++ b/link.go @@ -9,6 +9,7 @@ import ( "github.com/rs/zerolog" + "github.com/Shopify/toxiproxy/v2/collectors" "github.com/Shopify/toxiproxy/v2/stream" "github.com/Shopify/toxiproxy/v2/toxics" ) @@ -59,7 +60,7 @@ func NewToxicLink( next = make(chan *stream.StreamChunk) } - link.stubs[i] = toxics.NewToxicStub(last, next) + link.stubs[i] = toxics.NewToxicStub(last, next, &logger) last = next } link.output = stream.NewChanReader(last) @@ -109,7 +110,7 @@ func (link *ToxicLink) Start( go link.stubs[i].Run(toxic) } - go link.write(labels, name, server, dest) + go link.write(labels, name, server.Metrics, dest) } // read copies bytes from a source to the link's input channel. @@ -137,7 +138,7 @@ func (link *ToxicLink) read( func (link *ToxicLink) write( metricLabels []string, name string, - server *ApiServer, // TODO: Replace with AppConfig for Metrics and Logger + metrics *collectors.MetricsContainer, // TODO: Replace with AppConfig for Metrics and Logger dest io.WriteCloser, ) { logger := link.Logger. @@ -155,9 +156,10 @@ func (link *ToxicLink) write( Int64("bytes", bytes). Err(err). Msg("Could not write to destination") - } else if server.Metrics.ProxyMetricsEnabled() { - server.Metrics.ProxyMetrics.SentBytesTotal. - WithLabelValues(metricLabels...).Add(float64(bytes)) + } else if metrics.ProxyMetricsEnabled() { + metrics.ProxyMetrics.SentBytesTotal. + WithLabelValues(metricLabels...). + Add(float64(bytes)) } dest.Close() @@ -172,7 +174,7 @@ func (link *ToxicLink) AddToxic(toxic *toxics.ToxicWrapper) { i := len(link.stubs) newin := make(chan *stream.StreamChunk, toxic.BufferSize) - link.stubs = append(link.stubs, toxics.NewToxicStub(newin, link.stubs[i-1].Output)) + link.stubs = append(link.stubs, toxics.NewToxicStub(newin, link.stubs[i-1].Output, link.Logger)) // Interrupt the last toxic so that we don't have a race when moving channels if link.stubs[i-1].InterruptToxic() { diff --git a/toxics/bandwidth.go b/toxics/bandwidth.go index 2dd1e881..600de25f 100644 --- a/toxics/bandwidth.go +++ b/toxics/bandwidth.go @@ -4,8 +4,6 @@ import ( "fmt" "time" - "github.com/rs/zerolog/log" - "github.com/Shopify/toxiproxy/v2/stream" ) @@ -16,7 +14,7 @@ type BandwidthToxic struct { } func (t *BandwidthToxic) Pipe(stub *ToxicStub) { - logger := log.With(). + logger := stub.Logger.With(). Str("component", "BandwidthToxic"). Str("method", "Pipe"). Str("toxic_type", "bandwidth"). diff --git a/toxics/limit_data_test.go b/toxics/limit_data_test.go index 881d4ca4..517b477c 100644 --- a/toxics/limit_data_test.go +++ b/toxics/limit_data_test.go @@ -7,6 +7,7 @@ import ( "github.com/Shopify/toxiproxy/v2/stream" "github.com/Shopify/toxiproxy/v2/toxics" + "github.com/rs/zerolog" ) func buffer(size int) []byte { @@ -32,7 +33,8 @@ func checkRemainingChunks(t *testing.T, output chan *stream.StreamChunk) { func check(t *testing.T, toxic *toxics.LimitDataToxic, chunks [][]byte, expectedChunks [][]byte) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk, 100) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) stub.State = toxic.NewState() go toxic.Pipe(stub) @@ -53,7 +55,8 @@ func TestLimitDataToxicMayBeRestarted(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk, 100) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) stub.State = toxic.NewState() buf := buffer(90) @@ -84,7 +87,8 @@ func TestLimitDataToxicMayBeInterrupted(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) stub.State = toxic.NewState() go func() { @@ -99,7 +103,8 @@ func TestLimitDataToxicNilShouldClosePipe(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) stub.State = toxic.NewState() go func() { diff --git a/toxics/slicer_test.go b/toxics/slicer_test.go index 7b396893..d3d89a32 100644 --- a/toxics/slicer_test.go +++ b/toxics/slicer_test.go @@ -8,6 +8,7 @@ import ( "github.com/Shopify/toxiproxy/v2/stream" "github.com/Shopify/toxiproxy/v2/toxics" + "github.com/rs/zerolog" ) func TestSlicerToxic(t *testing.T) { @@ -16,7 +17,8 @@ func TestSlicerToxic(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) done := make(chan bool) go func() { @@ -64,7 +66,8 @@ func TestSlicerToxicZeroSizeVariation(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) done := make(chan bool) go func() { diff --git a/toxics/toxic.go b/toxics/toxic.go index 058c60d9..0cfe8f86 100644 --- a/toxics/toxic.go +++ b/toxics/toxic.go @@ -7,6 +7,8 @@ import ( "sync" "time" + "github.com/rs/zerolog" + "github.com/Shopify/toxiproxy/v2/stream" ) @@ -59,6 +61,7 @@ type ToxicWrapper struct { } type ToxicStub struct { + Logger *zerolog.Logger Input <-chan *stream.StreamChunk Output chan<- *stream.StreamChunk State interface{} @@ -67,8 +70,13 @@ type ToxicStub struct { closed chan struct{} } -func NewToxicStub(input <-chan *stream.StreamChunk, output chan<- *stream.StreamChunk) *ToxicStub { +func NewToxicStub( + input <-chan *stream.StreamChunk, + output chan<- *stream.StreamChunk, + logger *zerolog.Logger, +) *ToxicStub { return &ToxicStub{ + Logger: logger, Interrupt: make(chan struct{}), closed: make(chan struct{}), Input: input, diff --git a/toxics/toxic_test.go b/toxics/toxic_test.go index d7d104b5..2166cc53 100644 --- a/toxics/toxic_test.go +++ b/toxics/toxic_test.go @@ -361,7 +361,8 @@ func BenchmarkProxyBandwidth(b *testing.B) { func TestToxicStub_WriteOutput(t *testing.T) { input := make(chan *stream.StreamChunk) output := make(chan *stream.StreamChunk) - stub := toxics.NewToxicStub(input, output) + logger := zerolog.Nop() + stub := toxics.NewToxicStub(input, output, &logger) buf := make([]byte, 42) rand.Read(buf) From 98f8dab69b46bd13540cb9213ee3a40ec958e4f7 Mon Sep 17 00:00:00 2001 From: Michael Nikitochkin Date: Sun, 18 Sep 2022 18:42:24 +0200 Subject: [PATCH 5/5] [#419]: Update runtime metrics name --- collectors/metrics_container.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/collectors/metrics_container.go b/collectors/metrics_container.go index 77d536f6..934240e4 100644 --- a/collectors/metrics_container.go +++ b/collectors/metrics_container.go @@ -22,7 +22,7 @@ type MetricsContainer struct { registry *prometheus.Registry } -func (m *MetricsContainer) runtimeMetricsEnabled() bool { +func (m *MetricsContainer) RuntimeMetricsEnabled() bool { return m.RuntimeMetrics != nil } @@ -32,13 +32,13 @@ func (m *MetricsContainer) ProxyMetricsEnabled() bool { // AnyMetricsEnabled determines whether we have any prometheus metrics registered for exporting. func (m *MetricsContainer) AnyMetricsEnabled() bool { - return m.runtimeMetricsEnabled() || m.ProxyMetricsEnabled() + return m.RuntimeMetricsEnabled() || m.ProxyMetricsEnabled() } -// handler returns an HTTP handler with the necessary collectors registered +// Handler returns an HTTP handler with the necessary collectors registered // via a global prometheus registry. func (m *MetricsContainer) Handler() http.Handler { - if m.runtimeMetricsEnabled() { + if m.RuntimeMetricsEnabled() { m.registry.MustRegister(m.RuntimeMetrics.Collectors()...) } if m.ProxyMetricsEnabled() {