diff --git a/conf/proxy.toml b/conf/proxy.toml index 8845212d..57c4b837 100644 --- a/conf/proxy.toml +++ b/conf/proxy.toml @@ -109,12 +109,6 @@ graceful-close-conn-timeout = 15 # require-backend-tls = false -[metrics] - -# WARNING: know what you are doing, these two are for debugging. -# metrics-addr = "" -# metrics-interval = 0 - [advance] # ignore-wrong-namespace = true diff --git a/lib/config/proxy.go b/lib/config/proxy.go index b7508e09..c9b1c39c 100644 --- a/lib/config/proxy.go +++ b/lib/config/proxy.go @@ -24,15 +24,9 @@ type Config struct { Advance Advance `yaml:"advance,omitempty" toml:"advance,omitempty" json:"advance,omitempty"` Workdir string `yaml:"workdir,omitempty" toml:"workdir,omitempty" json:"workdir,omitempty"` Security Security `yaml:"security,omitempty" toml:"security,omitempty" json:"security,omitempty"` - Metrics Metrics `yaml:"metrics,omitempty" toml:"metrics,omitempty" json:"metrics,omitempty"` Log Log `yaml:"log,omitempty" toml:"log,omitempty" json:"log,omitempty"` } -type Metrics struct { - MetricsAddr string `toml:"metrics-addr" json:"metrics-addr"` - MetricsInterval uint `toml:"metrics-interval" json:"metrics-interval"` -} - type KeepAlive struct { Enabled bool `yaml:"enabled,omitempty" toml:"enabled,omitempty" json:"enabled,omitempty"` // Idle, Cnt, and Intvl works only when the connection is idle. User packets will interrupt keep-alive. diff --git a/lib/config/proxy_test.go b/lib/config/proxy_test.go index c472bb47..7f49be50 100644 --- a/lib/config/proxy_test.go +++ b/lib/config/proxy_test.go @@ -31,10 +31,6 @@ var testProxyConfig = Config{ API: API{ Addr: "0.0.0.0:3080", }, - Metrics: Metrics{ - MetricsAddr: "127.0.0.1:9021", - MetricsInterval: 15, - }, Log: Log{ Encoder: "tidb", LogOnline: LogOnline{ diff --git a/pkg/manager/config/config_test.go b/pkg/manager/config/config_test.go index 093ff990..101fc6a3 100644 --- a/pkg/manager/config/config_test.go +++ b/pkg/manager/config/config_test.go @@ -69,24 +69,24 @@ func TestConfigReload(t *testing.T) { }, { name: "override empty fields", - precfg: `metrics.metrics-addr = ""`, + precfg: `api.addr = ""`, precheck: func(c *config.Config) bool { - return c.Metrics.MetricsAddr == "" + return c.API.Addr == "" }, - postcfg: `metrics.metrics-addr = "gg"`, + postcfg: `api.addr = "0.0.0.0:3080"`, postcheck: func(c *config.Config) bool { - return c.Metrics.MetricsAddr == "gg" + return c.API.Addr == "0.0.0.0:3080" }, }, { name: "override non-empty fields", - precfg: `metrics.metrics-addr = "ee"`, + precfg: `api.addr = "0.0.0.0:3080"`, precheck: func(c *config.Config) bool { - return c.Metrics.MetricsAddr == "ee" + return c.API.Addr == "0.0.0.0:3080" }, - postcfg: `metrics.metrics-addr = "gg"`, + postcfg: `api.addr = "0.0.0.0:3081"`, postcheck: func(c *config.Config) bool { - return c.Metrics.MetricsAddr == "gg" + return c.API.Addr == "0.0.0.0:3081" }, }, { diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 7ae0c4c1..da33252f 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -8,19 +8,14 @@ package metrics import ( "context" - "fmt" - "net" - "os" "runtime" "sync" "time" - "github.com/pingcap/tiproxy/lib/config" "github.com/pingcap/tiproxy/lib/util/systimemon" "github.com/pingcap/tiproxy/lib/util/waitgroup" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" - "github.com/prometheus/client_golang/prometheus/push" dto "github.com/prometheus/client_model/go" "go.uber.org/zap" ) @@ -53,12 +48,11 @@ func NewMetricsManager() *MetricsManager { var registerOnce = &sync.Once{} // Init registers metrics and pushes metrics to prometheus. -func (mm *MetricsManager) Init(ctx context.Context, logger *zap.Logger, proxyAddr string, cfg config.Metrics, cfgch <-chan *config.Config) { +func (mm *MetricsManager) Init(ctx context.Context, logger *zap.Logger) { mm.logger = logger registerOnce.Do(registerProxyMetrics) ctx, mm.cancel = context.WithCancel(ctx) mm.setupMonitor(ctx) - mm.pushMetric(ctx, proxyAddr, cfg, cfgch) } // Close stops all goroutines. @@ -89,65 +83,6 @@ func (mm *MetricsManager) setupMonitor(ctx context.Context) { }) } -// pushMetric pushes metrics in background. -func (mm *MetricsManager) pushMetric(ctx context.Context, proxyAddr string, cfg config.Metrics, cfgch <-chan *config.Config) { - mm.wg.Run(func() { - proxyInstance := instanceName(proxyAddr) - addr := cfg.MetricsAddr - interval := time.Duration(cfg.MetricsInterval) * time.Second - pusher := mm.buildPusher(addr, interval, proxyInstance) - - for ctx.Err() == nil { - select { - case newCfg := <-cfgch: - if newCfg == nil { - return - } - interval = time.Duration(newCfg.Metrics.MetricsInterval) * time.Second - if addr != newCfg.Metrics.MetricsAddr { - addr = newCfg.Metrics.MetricsAddr - pusher = mm.buildPusher(addr, interval, proxyInstance) - } - default: - } - - // Wait until the config is legal. - if interval == 0 || pusher == nil { - select { - case <-time.After(time.Second): - continue - case <-ctx.Done(): - return - } - } - - if err := pusher.Push(); err != nil { - mm.logger.Error("could not push metrics to prometheus pushgateway", zap.Error(err)) - } - select { - case <-time.After(interval): - case <-ctx.Done(): - return - } - } - }) -} - -func (mm *MetricsManager) buildPusher(addr string, interval time.Duration, proxyInstance string) *push.Pusher { - var pusher *push.Pusher - if len(addr) > 0 { - // Create a new pusher when the address changes. - mm.logger.Info("start prometheus push client", zap.String("server addr", addr), zap.Stringer("interval", interval)) - pusher = push.New(addr, "tiproxy") - pusher = pusher.Gatherer(prometheus.DefaultGatherer) - pusher = pusher.Grouping("instance", proxyInstance) - } else { - mm.logger.Info("disable prometheus push client") - pusher = nil - } - return pusher -} - // registerProxyMetrics registers metrics. func registerProxyMetrics() { prometheus.DefaultRegisterer.Unregister(collectors.NewGoCollector()) @@ -173,18 +108,6 @@ func registerProxyMetrics() { prometheus.MustRegister(MigrateDurationHistogram) } -func instanceName(proxyAddr string) string { - hostname, err := os.Hostname() - if err != nil { - return "unknown" - } - _, port, err := net.SplitHostPort(proxyAddr) - if err != nil { - return "unknown" - } - return fmt.Sprintf("%s_%s", hostname, port) -} - // ReadCounter reads the value from the counter. It is only used for testing. func ReadCounter(counter prometheus.Counter) (int, error) { var metric dto.Metric diff --git a/pkg/metrics/metrics_test.go b/pkg/metrics/metrics_test.go index 6e381caf..261900d6 100644 --- a/pkg/metrics/metrics_test.go +++ b/pkg/metrics/metrics_test.go @@ -5,97 +5,14 @@ package metrics import ( "context" - "fmt" - "io" - "net/http" - "net/http/httptest" - "os" "testing" - "time" - "github.com/pingcap/tiproxy/lib/config" "github.com/pingcap/tiproxy/lib/util/logger" - "github.com/stretchr/testify/require" ) -// Test that the metrics are pushed or not pushed with different configurations. -func TestPushMetrics(t *testing.T) { - proxyAddr := "0.0.0.0:6000" - labelName := fmt.Sprintf("%s_%s_maxprocs", ModuleProxy, LabelServer) - bodyCh1, bodyCh2 := make(chan string), make(chan string) - pgwOK1, pgwOK2 := setupServer(t, bodyCh1), setupServer(t, bodyCh2) +func TestStartMetricsManager(t *testing.T) { log, _ := logger.CreateLoggerForTest(t) - - tests := []struct { - metricsAddr string - metricsInterval uint - pushedCh chan string - }{ - { - metricsAddr: pgwOK1.URL, - metricsInterval: 1, - pushedCh: bodyCh1, - }, - { - metricsAddr: pgwOK1.URL, - metricsInterval: 0, - pushedCh: nil, - }, - { - metricsAddr: pgwOK2.URL, - metricsInterval: 1, - pushedCh: bodyCh2, - }, - { - metricsAddr: "", - metricsInterval: 1, - pushedCh: nil, - }, - } mm := NewMetricsManager() - cfgCh := make(chan *config.Config, 1) - mm.Init(context.Background(), log, proxyAddr, config.Metrics{}, cfgCh) - for _, tt := range tests { - cfgCh <- &config.Config{ - Metrics: config.Metrics{ - MetricsAddr: tt.metricsAddr, - MetricsInterval: tt.metricsInterval, - }, - } - if tt.pushedCh != nil { - select { - case body := <-tt.pushedCh: - require.Contains(t, body, labelName) - case <-time.After(2 * time.Second): - t.Fatal("not pushed") - } - } else { - select { - case <-bodyCh1: - t.Fatal("pushed 1") - case <-bodyCh2: - t.Fatal("pushed 2") - case <-time.After(2 * time.Second): - } - } - } + mm.Init(context.Background(), log) mm.Close() } - -func setupServer(t *testing.T, bodyCh chan string) *httptest.Server { - hostname, err := os.Hostname() - require.NoError(t, err) - expectedPath := fmt.Sprintf("/metrics/job/tiproxy/instance/%s_6000", hostname) - server := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - body, err := io.ReadAll(r.Body) - require.NoError(t, err) - bodyCh <- string(body) - require.Equal(t, expectedPath, r.URL.EscapedPath()) - w.Header().Set("Content-Type", `text/plain; charset=utf-8`) - w.WriteHeader(http.StatusOK) - }), - ) - t.Cleanup(server.Close) - return server -} diff --git a/pkg/server/api/config_test.go b/pkg/server/api/config_test.go index 11734726..3b19ba47 100644 --- a/pkg/server/api/config_test.go +++ b/pkg/server/api/config_test.go @@ -75,7 +75,7 @@ max-backups = 3 doHTTP(t, http.MethodGet, "/api/admin/config?format=json", nil, func(t *testing.T, r *http.Response) { all, err := io.ReadAll(r.Body) require.NoError(t, err) - require.Equal(t, `{"proxy":{"addr":"0.0.0.0:6000","pd-addrs":"127.0.0.1:2379","frontend-keepalive":{"enabled":true},"backend-healthy-keepalive":{"enabled":true,"idle":60000000000,"cnt":5,"intvl":3000000000,"timeout":15000000000},"backend-unhealthy-keepalive":{"enabled":true,"idle":10000000000,"cnt":5,"intvl":1000000000,"timeout":5000000000},"graceful-close-conn-timeout":15},"api":{"addr":"0.0.0.0:3080"},"advance":{"ignore-wrong-namespace":true},"security":{"server-tls":{"min-tls-version":"1.1"},"server-http-tls":{"min-tls-version":"1.1"},"cluster-tls":{"min-tls-version":"1.1"},"sql-tls":{"min-tls-version":"1.1"}},"metrics":{"metrics-addr":"","metrics-interval":0},"log":{"encoder":"tidb","level":"info","log-file":{"max-size":300,"max-days":3,"max-backups":3}}}`, + require.Equal(t, `{"proxy":{"addr":"0.0.0.0:6000","pd-addrs":"127.0.0.1:2379","frontend-keepalive":{"enabled":true},"backend-healthy-keepalive":{"enabled":true,"idle":60000000000,"cnt":5,"intvl":3000000000,"timeout":15000000000},"backend-unhealthy-keepalive":{"enabled":true,"idle":10000000000,"cnt":5,"intvl":1000000000,"timeout":5000000000},"graceful-close-conn-timeout":15},"api":{"addr":"0.0.0.0:3080"},"advance":{"ignore-wrong-namespace":true},"security":{"server-tls":{"min-tls-version":"1.1"},"server-http-tls":{"min-tls-version":"1.1"},"cluster-tls":{"min-tls-version":"1.1"},"sql-tls":{"min-tls-version":"1.1"}},"log":{"encoder":"tidb","level":"info","log-file":{"max-size":300,"max-days":3,"max-backups":3}}}`, string(regexp.MustCompile(`"workdir":"[^"]+",`).ReplaceAll(all, nil))) require.Equal(t, http.StatusOK, r.StatusCode) }) diff --git a/pkg/server/server.go b/pkg/server/server.go index 97530c83..63d1fef5 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -71,7 +71,7 @@ func NewServer(ctx context.Context, sctx *sctx.Context) (srv *Server, err error) cfg := srv.ConfigManager.GetConfig() // setup metrics - srv.MetricsManager.Init(ctx, lg.Named("metrics"), cfg.Proxy.Addr, cfg.Metrics, srv.ConfigManager.WatchConfig()) + srv.MetricsManager.Init(ctx, lg.Named("metrics")) metrics.ServerEventCounter.WithLabelValues(metrics.EventStart).Inc() // setup certs