From a520cf3ea7e4a93a55774a73c13c3a2f83a5ca8e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 17 Aug 2020 14:12:04 -0400 Subject: [PATCH] testing: disable global metrics sink in tests This might be better handled by allowing configuration for the InMemSink interval and retail, and disabling the global. For now this is a smaller change to remove the goroutine leak caused by tests because go-metrics does not provide any way of shutting down the global goroutine. --- agent/acl_test.go | 10 +++++++--- agent/agent.go | 4 ++-- agent/config/runtime_test.go | 1 + agent/setup.go | 11 ++++++++--- agent/testagent.go | 10 +++++++--- lib/telemetry.go | 7 +++++++ 6 files changed, 32 insertions(+), 11 deletions(-) diff --git a/agent/acl_test.go b/agent/acl_test.go index 63525f039..a88d89273 100644 --- a/agent/acl_test.go +++ b/agent/acl_test.go @@ -42,12 +42,16 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe dataDir := testutil.TempDir(t, "acl-agent") logBuffer := testutil.NewLogBuffer(t) - loader := func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error) { + loader := func(source config.Source) (*config.RuntimeConfig, []string, error) { dataDir := fmt.Sprintf(`data_dir = "%s"`, dataDir) opts := config.BuilderOpts{ HCL: []string{TestConfigHCL(NodeID()), hcl, dataDir}, } - return config.Load(opts, source) + cfg, warnings, err := config.Load(opts, source) + if cfg != nil { + cfg.Telemetry.Disable = true + } + return cfg, warnings, err } bd, err := NewBaseDeps(loader, logBuffer) require.NoError(t, err) @@ -58,7 +62,7 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe Output: logBuffer, TimeFormat: "04:05.000", }) - bd.TelemetrySink = metrics.NewInmemSink(1*time.Second, time.Minute) + bd.MetricsHandler = metrics.NewInmemSink(1*time.Second, time.Minute) agent, err := New(bd) require.NoError(t, err) diff --git a/agent/agent.go b/agent/agent.go index 7af0f5d15..bc5af6abc 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -171,7 +171,7 @@ type Agent struct { logger hclog.InterceptLogger // In-memory sink used for collecting metrics - MemSink *metrics.InmemSink + MemSink MetricsHandler // delegate is either a *consul.Server or *consul.Client // depending on the configuration @@ -349,7 +349,7 @@ func New(bd BaseDeps) (*Agent, error) { tlsConfigurator: bd.TLSConfigurator, config: bd.RuntimeConfig, cache: bd.Cache, - MemSink: bd.TelemetrySink, + MemSink: bd.MetricsHandler, connPool: bd.ConnPool, autoConf: bd.AutoConfig, } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index dee77376c..3f51c6d9a 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -7100,6 +7100,7 @@ func TestSanitize(t *testing.T) { "CirconusCheckTags": "", "CirconusSubmissionInterval": "", "CirconusSubmissionURL": "", + "Disable": false, "DisableHostname": false, "DogstatsdAddr": "", "DogstatsdTags": [], diff --git a/agent/setup.go b/agent/setup.go index 9feb9d6f8..9554ef7b5 100644 --- a/agent/setup.go +++ b/agent/setup.go @@ -5,9 +5,9 @@ import ( "fmt" "io" "net" + "net/http" "time" - "github.com/armon/go-metrics" autoconf "github.com/hashicorp/consul/agent/auto-config" "github.com/hashicorp/consul/agent/cache" certmon "github.com/hashicorp/consul/agent/cert-monitor" @@ -28,7 +28,7 @@ import ( type BaseDeps struct { Logger hclog.InterceptLogger TLSConfigurator *tlsutil.Configurator // TODO: use an interface - TelemetrySink *metrics.InmemSink // TODO: use an interface + MetricsHandler MetricsHandler RuntimeConfig *config.RuntimeConfig Tokens *token.Store Cache *cache.Cache @@ -36,6 +36,11 @@ type BaseDeps struct { ConnPool *pool.ConnPool // TODO: use an interface } +// MetricsHandler provides an http.Handler for displaying metrics. +type MetricsHandler interface { + DisplayMetrics(resp http.ResponseWriter, req *http.Request) (interface{}, error) +} + type ConfigLoader func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error) func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error) { @@ -71,7 +76,7 @@ func NewBaseDeps(configLoader ConfigLoader, logOut io.Writer) (BaseDeps, error) return d, fmt.Errorf("failed to setup node ID: %w", err) } - d.TelemetrySink, err = lib.InitTelemetry(cfg.Telemetry) + d.MetricsHandler, err = lib.InitTelemetry(cfg.Telemetry) if err != nil { return d, fmt.Errorf("failed to initialize telemetry: %w", err) } diff --git a/agent/testagent.go b/agent/testagent.go index 0cf69fb9e..8f05b6ed4 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -170,7 +170,7 @@ func (a *TestAgent) Start(t *testing.T) (err error) { // Create NodeID outside the closure, so that it does not change testHCLConfig := TestConfigHCL(NodeID()) - loader := func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error) { + loader := func(source config.Source) (*config.RuntimeConfig, []string, error) { opts := config.BuilderOpts{ HCL: []string{testHCLConfig, portsConfig, a.HCL, hclDataDir}, } @@ -182,13 +182,17 @@ func (a *TestAgent) Start(t *testing.T) (err error) { config.DefaultConsulSource(), config.DevConsulSource(), } - return config.Load(opts, source, overrides...) + cfg, warnings, err := config.Load(opts, source, overrides...) + if cfg != nil { + cfg.Telemetry.Disable = true + } + return cfg, warnings, err } bd, err := NewBaseDeps(loader, logOutput) require.NoError(t, err) bd.Logger = logger - bd.TelemetrySink = metrics.NewInmemSink(1*time.Second, time.Minute) + bd.MetricsHandler = metrics.NewInmemSink(1*time.Second, time.Minute) a.Config = bd.RuntimeConfig agent, err := New(bd) diff --git a/lib/telemetry.go b/lib/telemetry.go index 8933c0c30..595e6005e 100644 --- a/lib/telemetry.go +++ b/lib/telemetry.go @@ -19,6 +19,10 @@ import ( // the shared InitTelemetry functions below, but we can't import agent/config // due to a dependency cycle. type TelemetryConfig struct { + // Disable may be set to true to have InitTelemetry to skip initialization + // and return a nil MetricsSink. + Disable bool + // Circonus*: see https://github.com/circonus-labs/circonus-gometrics // for more details on the various configuration options. // Valid configuration combinations: @@ -326,6 +330,9 @@ func circonusSink(cfg TelemetryConfig, hostname string) (metrics.MetricSink, err // InitTelemetry configures go-metrics based on map of telemetry config // values as returned by Runtimecfg.Config(). func InitTelemetry(cfg TelemetryConfig) (*metrics.InmemSink, error) { + if cfg.Disable { + return nil, nil + } // Setup telemetry // Aggregate on 10 second intervals for 1 minute. Expose the // metrics over stderr when there is a SIGUSR1 received.